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

Jump to bottom

Test static initialization with JsonTypeInfo #17

Merged
merged 1 commit into from
Jun 22, 2022
Merged

Test static initialization with JsonTypeInfo #17

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
5 changes: 4 additions & 1 deletion src/libraries/System.Text.Json/src/Resources/Strings.resx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -244,7 +244,7 @@
<value>The requested operation requires an element of type '{0}', but the target element has type '{1}'.</value>
</data>
<data name="TypeInfoResolverImmutable" xml:space="preserve">
<value>Default TypeInfoResolver and custom TypeInfoResolver after first usage cannot be changed.</value>
<value>Default TypeInfoResolver and custom TypeInfoResolver cannot be changed after first usage.</value>
</data>
<data name="TypeInfoImmutable" xml:space="preserve">
<value>JsonTypeInfo cannot be changed after first usage.</value>
Expand Down Expand Up @@ -662,4 +662,7 @@
<data name="JsonPropertyInfoBoundToDifferentParent" xml:space="preserve">
<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>
</data>
</root>
6 changes: 6 additions & 0 deletions src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -655,6 +655,12 @@ internal void InitializeForReflectionSerializer()
private JsonTypeInfo? GetTypeInfoInternal(Type type)
{
IJsonTypeInfoResolver? resolver = _effectiveJsonTypeInfoResolver ?? _typeInfoResolver;

if (resolver == null)
{
ThrowHelper.ThrowInvalidOperationException_JsonTypeInfoUsedButTypeInfoResolverNotSet();
Copy link
Collaborator

@eiriktsarpalis eiriktsarpalis Jun 22, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

That doesn't look like the appropriate location to throw this error message. I think this method can be invoked from other contexts as well where no JsonTypeInfo is handled by the user explicitly, e.g. when doing options.GetConverter().Write(...)

Copy link
Owner Author

@krwq krwq Jun 22, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

I just noticed that and it broke some tests, will throw that from GetOrAddJsonTypeInfo

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think it should be moved to the root-level serialization methods that accept JsonTypeInfo

}

JsonTypeInfo? info = resolver?.GetTypeInfo(type, this);

if (info != null)
Expand Down
6 changes: 6 additions & 0 deletions src/libraries/System.Text.Json/src/System/Text/Json/ThrowHelper.Serialization.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,12 @@ public static void ThrowInvalidOperationException_ResolverTypeInfoOptionsNotComp
throw new InvalidOperationException(SR.ResolverTypeInfoOptionsNotCompatible);
}

[DoesNotReturn]
public static void ThrowInvalidOperationException_JsonTypeInfoUsedButTypeInfoResolverNotSet()
{
throw new InvalidOperationException(SR.JsonTypeInfoUsedButTypeInfoResolverNotSet);
}

[DoesNotReturn]
public static void ThrowInvalidOperationException_SerializationConverterOnAttributeInvalid(Type classType, MemberInfo? memberInfo)
{
Expand Down
74 changes: 74 additions & 0 deletions ...ts/System.Text.Json.Tests/Serialization/MetadataTests/DefaultJsonTypeInfoResolverTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
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 @@ -178,6 +179,79 @@ 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