Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve stringify for JObject #1009

Merged
merged 2 commits into from
Nov 17, 2021
Merged

Improve stringify for JObject #1009

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
32e9d78
Improve stringify for JObject
lahma Nov 16, 2021
61b7f78
add IConvertible support
lahma Nov 17, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Jint.Tests/Runtime/Domain/ArrayConverterTestClass.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ public int ToInt32(IFormatProvider provider)
#region NotImplemented
public TypeCode GetTypeCode()
{
throw new NotImplementedException();
return TypeCode.Object;
}

public bool ToBoolean(IFormatProvider provider)
Expand Down
18 changes: 18 additions & 0 deletions Jint.Tests/Runtime/InteropTests.NewtonsoftJson.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,5 +90,23 @@ public void EngineShouldStringifyAnJObjectListWithValuesCorrectly()

Assert.Equal("{\"Current\":null}", result);
}

[Fact]
public void EngineShouldStringifyJObjectFromObjectListWithValuesCorrectly()
{
var engine = new Engine();

var source = new dynamic[]
{
new { Text = "Text1", Value = 1 },
new { Text = "Text2", Value = 2 }
};

engine.SetValue("testSubject", source.Select(x => JObject.FromObject(x)).ToList());
var fromEngine = engine.Evaluate("return JSON.stringify(testSubject);");
var result = fromEngine.ToString();

Assert.Equal("[{\"Text\":\"Text1\",\"Value\":1},{\"Text\":\"Text2\",\"Value\":2}]", result);
}
}
}
114 changes: 86 additions & 28 deletions Jint/Runtime/Interop/DefaultObjectConverter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ internal static class DefaultObjectConverter

public static bool TryConvert(Engine engine, object value, out JsValue result)
{
result = null;
var valueType = value.GetType();

var typeMappers = _typeMappers;
Expand All @@ -57,57 +58,114 @@ public static bool TryConvert(Engine engine, object value, out JsValue result)
}, typeMappers);

result = ConvertArray(engine, a);
return result is not null;
}

if (value is IConvertible convertible)
{
result = ConvertConvertible(engine, convertible);
if (result is not null)
{
return true;
}
}

if (value is Delegate d)
{
result = new DelegateWrapper(engine, d);
}
else
{
if (value is Delegate d)
var t = value.GetType();

if (!engine.Options.Interop.AllowSystemReflection
&& t.Namespace?.StartsWith("System.Reflection") == true)
{
result = new DelegateWrapper(engine, d);
const string message = "Cannot access System.Reflection namespace, check Engine's interop options";
ExceptionHelper.ThrowInvalidOperationException(message);
}
else

if (t.IsEnum)
{
var t = value.GetType();
var ut = Enum.GetUnderlyingType(t);

if (!engine.Options.Interop.AllowSystemReflection
&& t.Namespace?.StartsWith("System.Reflection") == true)
if (ut == typeof(ulong))
{
const string message = "Cannot access System.Reflection namespace, check Engine's interop options";
ExceptionHelper.ThrowInvalidOperationException(message);
result = JsNumber.Create(Convert.ToDouble(value));
}

if (t.IsEnum)
else
{
var ut = Enum.GetUnderlyingType(t);

if (ut == typeof(ulong))
if (ut == typeof(uint) || ut == typeof(long))
{
result = JsNumber.Create(Convert.ToDouble(value));
result = JsNumber.Create(Convert.ToInt64(value));
}
else
{
if (ut == typeof(uint) || ut == typeof(long))
{
result = JsNumber.Create(Convert.ToInt64(value));
}
else
{
result = JsNumber.Create(Convert.ToInt32(value));
}
result = JsNumber.Create(Convert.ToInt32(value));
}
}
else
{
result = engine.Options.Interop.WrapObjectHandler.Invoke(engine, value);
}

// if no known type could be guessed, use the default of wrapping using using ObjectWrapper.
}
else
{
result = engine.Options.Interop.WrapObjectHandler.Invoke(engine, value);
}

// if no known type could be guessed, use the default of wrapping using using ObjectWrapper.
}
}

return result is not null;
}

private static JsValue ConvertConvertible(Engine engine, IConvertible convertible)
{
JsValue result = null;
switch (convertible.GetTypeCode())
{
case TypeCode.Boolean:
result = convertible.ToBoolean(engine.Options.Culture) ? JsBoolean.True : JsBoolean.False;
break;
case TypeCode.Byte:
result = JsNumber.Create(convertible.ToByte(engine.Options.Culture));
break;
case TypeCode.Char:
result = JsString.Create(convertible.ToChar(engine.Options.Culture));
break;
case TypeCode.Double:
result = JsNumber.Create(convertible.ToDouble(engine.Options.Culture));
break;
case TypeCode.SByte:
result = JsNumber.Create(convertible.ToSByte(engine.Options.Culture));
break;
case TypeCode.Int16:
result = JsNumber.Create(convertible.ToInt16(engine.Options.Culture));
break;
case TypeCode.Int32:
result = JsNumber.Create(convertible.ToInt32(engine.Options.Culture));
break;
case TypeCode.UInt16:
result = JsNumber.Create(convertible.ToUInt16(engine.Options.Culture));
break;
case TypeCode.Int64:
result = JsNumber.Create(convertible.ToInt64(engine.Options.Culture));
break;
case TypeCode.Single:
result = JsNumber.Create(convertible.ToSingle(engine.Options.Culture));
break;
case TypeCode.String:
result = JsString.Create(convertible.ToString(engine.Options.Culture));
break;
case TypeCode.UInt32:
result = JsNumber.Create(convertible.ToUInt32(engine.Options.Culture));
break;
case TypeCode.UInt64:
result = JsNumber.Create(convertible.ToUInt64(engine.Options.Culture));
break;
}

return result;
}

private static JsValue ConvertArray(Engine e, object v)
{
var array = (Array)v;
Expand Down
49 changes: 15 additions & 34 deletions Jint/Runtime/Interop/ObjectWrapper.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,37 +98,6 @@ public override JsValue Get(JsValue property, JsValue receiver)
return Undefined;
}

if (property is JsString stringKey)
{
var member = stringKey.ToString();

// expando object for instance
if (_typeDescriptor.IsStringKeyedGenericDictionary)
{
if (_typeDescriptor.TryGetValue(Target, member, out var value))
{
return FromObject(_engine, value);
}
}

var result = Engine.Options.Interop.MemberAccessor?.Invoke(Engine, Target, member);
if (result is not null)
{
return result;
}

if (_properties is null || !_properties.ContainsKey(member))
{
// can try utilize fast path
var accessor = _engine.Options.Interop.TypeResolver.GetAccessor(_engine, Target.GetType(), member);
var value = accessor.GetValue(_engine, Target);
if (value is not null)
{
return FromObject(_engine, value);
}
}
}

return base.Get(property, receiver);
}

Expand All @@ -152,9 +121,10 @@ private IEnumerable<JsValue> EnumerateOwnPropertyKeys(Types types)
var processed = basePropertyKeys.Count > 0 ? new HashSet<JsValue>() : null;

var includeStrings = (types & Types.String) != 0;
if (includeStrings && Target is IDictionary<string, object> stringKeyedDictionary) // expando object for instance
if (includeStrings && _typeDescriptor.IsStringKeyedGenericDictionary) // expando object for instance
{
foreach (var key in stringKeyedDictionary.Keys)
var keys = _typeDescriptor.GetKeys(Target);
foreach (var key in keys)
{
var jsString = JsString.Create(key);
processed?.Add(jsString);
Expand All @@ -166,7 +136,9 @@ private IEnumerable<JsValue> EnumerateOwnPropertyKeys(Types types)
// we take values exposed as dictionary keys only
foreach (var key in dictionary.Keys)
{
if (_engine.ClrTypeConverter.TryConvert(key, typeof(string), CultureInfo.InvariantCulture, out var stringKey))
object stringKey = key as string;
if (stringKey is not null
|| _engine.ClrTypeConverter.TryConvert(key, typeof(string), CultureInfo.InvariantCulture, out stringKey))
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would probably work without the custom IObjectConverter by checking for IConvertible and ultimately for IFormattable if we know we want a string.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's how Fluid can handle JToken without configuration.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here I just wanted to create a fast path when we already have a string, but might be worth some PR to improve string coercion

{
var jsString = JsString.Create((string) stringKey);
processed?.Add(jsString);
Expand Down Expand Up @@ -234,6 +206,15 @@ public override PropertyDescriptor GetOwnProperty(JsValue property)
}

var member = property.ToString();

if (_typeDescriptor.IsStringKeyedGenericDictionary)
{
if (_typeDescriptor.TryGetValue(Target, member, out var value))
{
return new PropertyDescriptor(FromObject(_engine, value), PropertyFlag.OnlyEnumerable);
}
}

var result = Engine.Options.Interop.MemberAccessor(Engine, Target, member);
if (result is not null)
{
Expand Down
12 changes: 12 additions & 0 deletions Jint/Runtime/Interop/TypeDescriptor.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ internal sealed class TypeDescriptor
private static readonly Type _stringType = typeof(string);

private readonly PropertyInfo _stringIndexer;
private readonly PropertyInfo _keysAccessor;

private TypeDescriptor(Type type)
{
Expand All @@ -24,6 +25,7 @@ private TypeDescriptor(Type type)
&& i.GenericTypeArguments[0] == _stringType)
{
_stringIndexer = i.GetProperty("Item");
_keysAccessor = i.GetProperty("Keys");
break;
}
}
Expand Down Expand Up @@ -99,5 +101,15 @@ public bool TryGetValue(object target, string member, out object o)
return false;
}
}

public ICollection<string> GetKeys(object target)
{
if (!IsStringKeyedGenericDictionary)
{
ExceptionHelper.ThrowInvalidOperationException("Not a string-keyed dictionary");
}

return (ICollection<string>)_keysAccessor.GetValue(target);
}
}
}