Skip to content

Commit

Permalink
Fix test failures and proper fix this time (#18)
Browse files Browse the repository at this point in the history
* Fix test failures and proper fix this time

* reinstate ActiveIssueAttribute

* PR feedback and adjust couple of tests which don't set TypeInfoResolver

* fix IAsyncEnumerable tests

* Lock JsonSerializerOptions in JsonTypeInfo.EnsureConfigured()

Co-authored-by: Eirik Tsarpalis <eirik.tsarpalis@gmail.com>
  • Loading branch information
krwq and eiriktsarpalis committed Jun 22, 2022
1 parent bb32820 commit 07aede0
Show file tree
Hide file tree
Showing 8 changed files with 91 additions and 87 deletions.
4 changes: 2 additions & 2 deletions src/libraries/System.Text.Json/src/Resources/Strings.resx
Original file line number Diff line number Diff line change
Expand Up @@ -663,6 +663,6 @@
<value>JsonPropertyInfo with name '{0}' for type '{1}' is already bound to different JsonTypeInfo.</value>
</data>
<data name="JsonTypeInfoUsedButTypeInfoResolverNotSet" xml:space="preserve">
<value>Using JsonTypeInfo for serialization is not possible when TypeInfoResolver has not been set.</value>
<value>JsonTypeInfo metadata references a JsonSerializerOptions instance that doesn't specify a TypeInfoResolver.</value>
</data>
</root>
</root>
Original file line number Diff line number Diff line change
Expand Up @@ -652,15 +652,22 @@ internal void InitializeForReflectionSerializer()
IsInitializedForReflectionSerializer = true;
}

private JsonTypeInfo? GetTypeInfoInternal(Type type)
internal bool IsInitializedForMetadataGeneration { get; private set; }
internal void InitializeForMetadataGeneration()
{
IJsonTypeInfoResolver? resolver = _effectiveJsonTypeInfoResolver ?? _typeInfoResolver;

if (resolver == null)
{
ThrowHelper.ThrowInvalidOperationException_JsonTypeInfoUsedButTypeInfoResolverNotSet();
}

_isLockedInstance = true;
IsInitializedForMetadataGeneration = true;
}

private JsonTypeInfo? GetTypeInfoInternal(Type type)
{
IJsonTypeInfoResolver? resolver = _effectiveJsonTypeInfoResolver ?? _typeInfoResolver;
JsonTypeInfo? info = resolver?.GetTypeInfo(type, this);

if (info != null)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -301,6 +301,11 @@ internal virtual void Configure()
{
Debug.Assert(Monitor.IsEntered(_configureLock), "Configure called directly, use EnsureConfigured which locks this method");

if (!Options.IsInitializedForMetadataGeneration)
{
Options.InitializeForMetadataGeneration();
}

PropertyInfoForTypeInfo.EnsureChildOf(this);
PropertyInfoForTypeInfo.EnsureConfigured();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ void RunTest<TConverterReturn>()
}

[ActiveIssue("https://github.com/dotnet/runtime/issues/66232", TargetFrameworkMonikers.NetFramework)]
[ActiveIssue("https://github.com/dotnet/runtime/issues/66371", typeof(PlatformDetection), nameof(PlatformDetection.IsMonoInterpreter))]
[ActiveIssue("https://github.com/dotnet/runtime/issues/66371", typeof(PlatformDetection), nameof(PlatformDetection.IsMonoInterpreter))]
[ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))]
public static void GetConverter_Poco_WriteThrowsNotSupportedException()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,67 @@ public async Task JsonSerializerSerializeWithTypeInfoOfT<T>(T testObj, string ex
Assert.Equal(expectedJson, json);
}

[Fact]
public async Task SerializationWithJsonTypeInfoWithoutSettingTypeInfoResolverThrows()
{
JsonSerializerOptions o = new();
DefaultJsonTypeInfoResolver r = new();
// note: TypeInfoResolver not set
JsonTypeInfo<SomeClass> ti = (JsonTypeInfo<SomeClass>)r.GetTypeInfo(typeof(SomeClass), o);
SomeClass obj = new()
{
ObjProp = "test",
IntProp = 42,
};

// TODO: reassess if this is expected behavior
await Assert.ThrowsAsync<InvalidOperationException>(() => Serializer.SerializeWrapper(obj, ti));
}

[Fact]
public async Task DeserializationWithJsonTypeInfoWithoutSettingTypeInfoResolverThrows()
{
JsonSerializerOptions o = new();
DefaultJsonTypeInfoResolver r = new();
// note: TypeInfoResolver not set
JsonTypeInfo<SomeClass> ti = (JsonTypeInfo<SomeClass>)r.GetTypeInfo(typeof(SomeClass), o);

// TODO: reassess if this is expected behavior
string json = """{"ObjProp":"test","IntProp":42}""";
await Assert.ThrowsAsync<InvalidOperationException>(() => Serializer.DeserializeWrapper(json, ti));
}

[Fact]
public async Task SerializationWithJsonTypeInfoWhenTypeInfoResolverSetIsPossible()
{
JsonSerializerOptions o = new();
DefaultJsonTypeInfoResolver r = new();
o.TypeInfoResolver = r;
JsonTypeInfo<SomeClass> ti = (JsonTypeInfo<SomeClass>)r.GetTypeInfo(typeof(SomeClass), o);
SomeClass obj = new()
{
ObjProp = "test",
IntProp = 42,
};

string json = await Serializer.SerializeWrapper(obj, ti);
Assert.Equal("""{"ObjProp":"test","IntProp":42}""", json);
}

[Fact]
public async Task DeserializationWithJsonTypeInfoWhenTypeInfoResolverSetIsPossible()
{
JsonSerializerOptions o = new();
DefaultJsonTypeInfoResolver r = new();
o.TypeInfoResolver = r;
JsonTypeInfo<SomeClass> ti = (JsonTypeInfo<SomeClass>)r.GetTypeInfo(typeof(SomeClass), o);
string json = """{"ObjProp":"test","IntProp":42}""";
SomeClass deserialized = await Serializer.DeserializeWrapper(json, ti);
Assert.IsType<JsonElement>(deserialized.ObjProp);
Assert.Equal("test", ((JsonElement)deserialized.ObjProp).GetString());
Assert.Equal(42, deserialized.IntProp);
}

public static IEnumerable<object[]> JsonSerializerSerializeWithTypeInfoOfT_TestData()
{
yield return new object[] { "value", @"""value""" };
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -605,10 +605,10 @@ private static void CreateJsonTypeInfo_Generic<T>()

static void TestCreateJsonTypeInfo(Func<JsonSerializerOptions, JsonTypeInfo<T>> getTypeInfo)
{
JsonSerializerOptions o = new();
JsonSerializerOptions o = new() { TypeInfoResolver = new DefaultJsonTypeInfoResolver() };
TestCreateJsonTypeInfoInstance(o, getTypeInfo(o));

o = new JsonSerializerOptions();
o = new JsonSerializerOptions() { TypeInfoResolver = new DefaultJsonTypeInfoResolver() };
var conv = new DummyConverter<T>();
o.Converters.Add(conv);
JsonTypeInfo<T> ti = getTypeInfo(o);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
using System.Text;
using System.Text.Json.Serialization.Metadata;
using System.Threading.Tasks;
using Microsoft.DotNet.RemoteExecutor;
using Xunit;

namespace System.Text.Json.Serialization.Tests
Expand Down Expand Up @@ -179,79 +178,6 @@ public static void ModifiersAreCalledAndModifyTypeInfos()
Assert.True(secondModifierCalled);
}

[ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))]
public static void StaticInitialization_SerializationWithJsonTypeInfoWithoutSettingTypeInfoResolverThrows()
{
RemoteExecutor.Invoke(static () =>
{
JsonSerializerOptions o = new();
DefaultJsonTypeInfoResolver r = new();
// note: TypeInfoResolver not set
JsonTypeInfo<SomeClass> ti = (JsonTypeInfo<SomeClass>)r.GetTypeInfo(typeof(SomeClass), o);
SomeClass obj = new()
{
ObjProp = "test",
IntProp = 42,
};
// TODO: reasses if this is expected behavior
Assert.Throws<InvalidOperationException>(() => JsonSerializer.Serialize(obj, ti));
}).Dispose();
}

[ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))]
public static void StaticInitialization_DeserializationWithJsonTypeInfoWithoutSettingTypeInfoResolverThrows()
{
RemoteExecutor.Invoke(static () =>
{
JsonSerializerOptions o = new();
DefaultJsonTypeInfoResolver r = new();
// note: TypeInfoResolver not set
JsonTypeInfo<SomeClass> ti = (JsonTypeInfo<SomeClass>)r.GetTypeInfo(typeof(SomeClass), o);
// TODO: reasses if this is expected behavior
string json = """{"ObjProp":"test","IntProp":42}""";
Assert.Throws<InvalidOperationException>(() => JsonSerializer.Deserialize(json, ti));
}).Dispose();
}

[ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))]
public static void StaticInitialization_SerializationWithJsonTypeInfoWhenTypeInfoResolverSetIsPossible()
{
RemoteExecutor.Invoke(static () =>
{
JsonSerializerOptions o = new();
DefaultJsonTypeInfoResolver r = new();
o.TypeInfoResolver = r;
JsonTypeInfo<SomeClass> ti = (JsonTypeInfo<SomeClass>)r.GetTypeInfo(typeof(SomeClass), o);
SomeClass obj = new()
{
ObjProp = "test",
IntProp = 42,
};
string json = JsonSerializer.Serialize(obj, ti);
Assert.Equal("""{"ObjProp":"test","IntProp":42}""", json);
}).Dispose();
}

[ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))]
public static void StaticInitialization_DeserializationWithJsonTypeInfoWhenTypeInfoResolverSetIsPossible()
{
RemoteExecutor.Invoke(static () =>
{
JsonSerializerOptions o = new();
DefaultJsonTypeInfoResolver r = new();
o.TypeInfoResolver = r;
JsonTypeInfo<SomeClass> ti = (JsonTypeInfo<SomeClass>)r.GetTypeInfo(typeof(SomeClass), o);
string json = """{"ObjProp":"test","IntProp":42}""";
SomeClass deserialized = JsonSerializer.Deserialize(json, ti);
Assert.IsType<JsonElement>(deserialized.ObjProp);
Assert.Equal("test", ((JsonElement)deserialized.ObjProp).GetString());
Assert.Equal(42, deserialized.IntProp);
}).Dispose();
}

private static void InvokeGeneric(Type type, string methodName, params object[] args)
{
typeof(DefaultJsonTypeInfoResolverTests)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,8 @@ public static async Task DeserializeAsyncEnumerable_ReadSourceAsync<TElement>(IE
{
JsonSerializerOptions options = new JsonSerializerOptions
{
DefaultBufferSize = bufferSize
DefaultBufferSize = bufferSize,
TypeInfoResolver = new DefaultJsonTypeInfoResolver()
};

byte[] data = JsonSerializer.SerializeToUtf8Bytes(source);
Expand All @@ -82,7 +83,12 @@ public static async Task DeserializeAsyncEnumerable_ShouldStreamPartialData(bool
string json = JsonSerializer.Serialize(Enumerable.Range(0, 100));

using var stream = new Utf8MemoryStream(json);
IAsyncEnumerable<int> asyncEnumerable = DeserializeAsyncEnumerableWrapper<int>(stream, new JsonSerializerOptions { DefaultBufferSize = 1 }, useJsonTypeInfoOverload: useJsonTypeInfoOverload);
JsonSerializerOptions options = new JsonSerializerOptions
{
DefaultBufferSize = 1
};

IAsyncEnumerable<int> asyncEnumerable = DeserializeAsyncEnumerableWrapper<int>(stream, options, useJsonTypeInfoOverload: useJsonTypeInfoOverload);
await using IAsyncEnumerator<int> asyncEnumerator = asyncEnumerable.GetAsyncEnumerator();

for (int i = 0; i < 20; i++)
Expand Down Expand Up @@ -181,7 +187,7 @@ public static async Task DeserializeAsyncEnumerable_CancellationToken_ThrowsOnCa
{
JsonSerializerOptions options = new JsonSerializerOptions
{
DefaultBufferSize = 1
DefaultBufferSize = 1,
};

byte[] data = JsonSerializer.SerializeToUtf8Bytes(Enumerable.Range(1, 100));
Expand Down Expand Up @@ -246,10 +252,9 @@ private static IAsyncEnumerable<T> DeserializeAsyncEnumerableWrapper<T>(Stream s

private static JsonTypeInfo<T> ResolveJsonTypeInfo<T>(JsonSerializerOptions? options = null)
{
// TODO replace with contract resolver once implemented -- only works with value converters.
options ??= JsonSerializerOptions.Default;
JsonConverter<T> converter = (JsonConverter<T>)options.GetConverter(typeof(T));
return JsonMetadataServices.CreateValueInfo<T>(options, converter);
JsonSerializer.Serialize(42, options); // Lock the options instance before initializing metadata
return (JsonTypeInfo<T>)options.TypeInfoResolver.GetTypeInfo(typeof(T), options);
}

private static async Task<List<T>> ToListAsync<T>(this IAsyncEnumerable<T> source)
Expand Down

0 comments on commit 07aede0

Please sign in to comment.