refactor(advanced-search): reuse thread-safe Server.Types for general property parsing
Server.Types is now thread-safe (ConcurrentDictionary for the IsParsable and Parse-method caches; per-call args array instead of a shared static one), so it can be used off the main thread. AdvancedSearch's CompareValues now delegates its general/reference/IParsable parsing branch to Types instead of comparing against a raw string — so reference types (Poison, entity-by-serial) and unlisted IParsable value types (Guid, ...) now parse into their real type and compare by value. The hot typed paths (numeric/string/bool/TimeSpan/DateTime/ enum) stay span-based and zero-alloc; only this uncommon branch allocates. Fixes: 'Poison = Lethal' (and any reference-type/IParsable property search) previously always returned no-match. Adds Poison + Guid tests. Full suite 534/534.
This commit is contained in:
parent
9836218b85
commit
1adf282c6b
4 changed files with 65 additions and 30 deletions
|
|
@ -0,0 +1,29 @@
|
|||
using Server;
|
||||
using Server.Engines.AdvancedSearch;
|
||||
using Xunit;
|
||||
|
||||
namespace UOContent.Tests;
|
||||
|
||||
// Fixture-backed: parsing a reference type through Server.Types (Poison) needs the poison registry
|
||||
// populated, which requires Core.Expansion (AOS+, set by the fixture) and PoisonKinds.Configure().
|
||||
[Collection("Sequential UOContent Tests")]
|
||||
public class AdvancedSearchTypesTests
|
||||
{
|
||||
[Fact]
|
||||
public void CompareValues_Poison_ReferenceTypeParsedViaTypes()
|
||||
{
|
||||
PoisonKinds.Configure(); // idempotent; registers Lesser..Lethal now that Core.Expansion is set
|
||||
|
||||
// Poison is a reference type implementing ISpanParsable; it can't use the compile-time span
|
||||
// path and routes through the shared Server.Types converter. Poison.Parse returns the
|
||||
// registered singleton, so "= Lethal" is a reference-equality match — this is the case that
|
||||
// previously compared a Poison against the raw string and always failed.
|
||||
var prop = Poison.Lethal;
|
||||
Assert.True(AdvancedSearchUtilities.CompareValues(typeof(Poison), prop, "Lethal", "="));
|
||||
Assert.False(AdvancedSearchUtilities.CompareValues(typeof(Poison), prop, "Lesser", "="));
|
||||
|
||||
var ex = Record.Exception(() =>
|
||||
Assert.False(AdvancedSearchUtilities.CompareValues(typeof(Poison), prop, "notapoison", "=")));
|
||||
Assert.Null(ex);
|
||||
}
|
||||
}
|
||||
|
|
@ -86,4 +86,14 @@ public class AdvancedSearchUtilitiesTests
|
|||
Assert.False(AdvancedSearchUtilities.CompareValues(typeof(TimeSpan), TimeSpan.Zero, "notaspan", "=")));
|
||||
Assert.Null(ex);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void CompareValues_Guid_ValueTypeParsedViaTypes()
|
||||
{
|
||||
// Guid is a value type not named by the hot paths; it's parsed via Types (IParsable) and
|
||||
// compared by value.
|
||||
var g = Guid.Parse("00000000-0000-0000-0000-000000000001");
|
||||
Assert.True(AdvancedSearchUtilities.CompareValues(typeof(Guid), g, "00000000-0000-0000-0000-000000000001", "="));
|
||||
Assert.False(AdvancedSearchUtilities.CompareValues(typeof(Guid), g, "00000000-0000-0000-0000-000000000002", "="));
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -127,9 +127,14 @@ public static class AdvancedSearchUtilities
|
|||
_ => false
|
||||
};
|
||||
}
|
||||
if (!propertyType.IsValueType)
|
||||
// Anything the hot typed paths above didn't handle — reference types (Poison, Map, entity
|
||||
// properties resolved by serial, ...) and value types exposing IParsable (Guid,
|
||||
// DateTimeOffset, ...). Delegate parsing to the shared, thread-safe Types converter so the
|
||||
// target is parsed into the property's real type, then compare by value. A string is
|
||||
// allocated here, but this is the uncommon path; the common types never reach it.
|
||||
if (!propertyType.IsValueType || Types.IsParsable(propertyType))
|
||||
{
|
||||
return TryParseValue<object>(valuePart, out var parsedValue) &&
|
||||
return Types.TryParse(propertyType, valuePart.ToString(), out var parsedValue) == null &&
|
||||
CompareReference(propertyValue!, parsedValue, operatorSpan);
|
||||
}
|
||||
|
||||
|
|
@ -345,9 +350,8 @@ public static class AdvancedSearchUtilities
|
|||
return TryParseNumericValue<double, T>(valuePart, out value);
|
||||
}
|
||||
|
||||
// string/object need no parsing — the span itself is the value (object is the reference-type
|
||||
// comparison path, which compares against the raw text).
|
||||
if (typeof(T) == typeof(string) || typeof(T) == typeof(object))
|
||||
// string needs no parsing — the span itself is the value.
|
||||
if (typeof(T) == typeof(string))
|
||||
{
|
||||
value = (T)(object)valuePart.ToString();
|
||||
return true;
|
||||
|
|
|
|||
|
|
@ -1,4 +1,5 @@
|
|||
using System;
|
||||
using System.Collections.Concurrent;
|
||||
using System.Collections.Generic;
|
||||
using System.Globalization;
|
||||
using System.Reflection;
|
||||
|
|
@ -10,7 +11,6 @@ namespace Server
|
|||
{
|
||||
public static readonly Type[] ParseStringParamTypes = { typeof(string), typeof(IFormatProvider) };
|
||||
public static readonly Type[] ParseStringNumericParamTypes = { typeof(string), typeof(NumberStyles) };
|
||||
private static object[] _parseParams = { null, null };
|
||||
|
||||
public static readonly Type OfByte = typeof(byte);
|
||||
public static readonly Type OfSByte = typeof(sbyte);
|
||||
|
|
@ -75,7 +75,9 @@ namespace Server
|
|||
OfULong
|
||||
};
|
||||
|
||||
private static Dictionary<Type, bool> _isParsable;
|
||||
// Thread-safe: parse metadata is read from parallel callers (e.g. the Advanced Search workers),
|
||||
// not just the single-threaded command path.
|
||||
private static readonly ConcurrentDictionary<Type, bool> _isParsable = new();
|
||||
|
||||
[MethodImpl(MethodImplOptions.AggressiveInlining)]
|
||||
public static bool IsType(Type type, Type check) => check.IsAssignableFrom(type);
|
||||
|
|
@ -89,25 +91,19 @@ namespace Server
|
|||
[MethodImpl(MethodImplOptions.AggressiveInlining)]
|
||||
public static bool IsText(Type t) => IsType(t, OfText);
|
||||
|
||||
public static bool IsParsable(Type t)
|
||||
{
|
||||
_isParsable ??= new();
|
||||
if (_isParsable.TryGetValue(t, out var isParsable))
|
||||
public static bool IsParsable(Type t) =>
|
||||
_isParsable.GetOrAdd(t, static type =>
|
||||
{
|
||||
return isParsable;
|
||||
}
|
||||
|
||||
foreach (var x in t.GetInterfaces())
|
||||
{
|
||||
if (x.IsGenericType && x.GetGenericTypeDefinition() == typeof(IParsable<>))
|
||||
foreach (var x in type.GetInterfaces())
|
||||
{
|
||||
isParsable = true;
|
||||
break;
|
||||
if (x.IsGenericType && x.GetGenericTypeDefinition() == typeof(IParsable<>))
|
||||
{
|
||||
return true;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
return _isParsable[t] = isParsable;
|
||||
}
|
||||
return false;
|
||||
});
|
||||
|
||||
[MethodImpl(MethodImplOptions.AggressiveInlining)]
|
||||
public static bool IsDecimal(Type t) => Array.IndexOf(DecimalTypes, t) >= 0;
|
||||
|
|
@ -118,18 +114,14 @@ namespace Server
|
|||
[MethodImpl(MethodImplOptions.AggressiveInlining)]
|
||||
public static bool IsEntity(Type t) => OfEntity.IsAssignableFrom(t);
|
||||
|
||||
private static Dictionary<Type, MethodInfo> _parseMethods;
|
||||
private static readonly ConcurrentDictionary<Type, MethodInfo> _parseMethods = new();
|
||||
|
||||
public static object Parse(Type t, string value)
|
||||
{
|
||||
_parseMethods ??= new();
|
||||
if (!_parseMethods.TryGetValue(t, out var method))
|
||||
{
|
||||
_parseMethods[t] = method = t.GetMethod("Parse", ParseStringParamTypes);
|
||||
}
|
||||
var method = _parseMethods.GetOrAdd(t, static type => type.GetMethod("Parse", ParseStringParamTypes));
|
||||
|
||||
_parseParams[0] = value;
|
||||
return method?.Invoke(null, _parseParams);
|
||||
// Fresh args array per call — a shared static array would race across concurrent callers.
|
||||
return method?.Invoke(null, new object[] { value, null });
|
||||
}
|
||||
|
||||
// Do not use this in "Parse" methods, it may cause a stack overflow
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue