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

Add a JsonTypeInfoResolver.Combine test for JsonSerializerContext #4

Merged
merged 2 commits into from
Jun 6, 2022
Merged

Add a JsonTypeInfoResolver.Combine test for JsonSerializerContext #4

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
1dcbd57
Fix JsonTypeInfoResolver.Combine for JsonSerializerContext
eiriktsarpalis Jun 6, 2022
17d134d
Break up failing test
eiriktsarpalis Jun 6, 2022
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
36 changes: 30 additions & 6 deletions src/libraries/System.Text.Json/gen/JsonSourceGenerator.Emitter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ private sealed partial class Emitter
private const string TypeTypeRef = "global::System.Type";
private const string UnsafeTypeRef = "global::System.Runtime.CompilerServices.Unsafe";
private const string NullableTypeRef = "global::System.Nullable";
private const string ConditionalWeakTableTypeRef = "global::System.Runtime.CompilerServices.ConditionalWeakTable";
private const string EqualityComparerTypeRef = "global::System.Collections.Generic.EqualityComparer";
private const string IListTypeRef = "global::System.Collections.Generic.IList";
private const string KeyValuePairTypeRef = "global::System.Collections.Generic.KeyValuePair";
Expand All @@ -70,6 +71,7 @@ private sealed partial class Emitter
private const string JsonPropertyInfoTypeRef = "global::System.Text.Json.Serialization.Metadata.JsonPropertyInfo";
private const string JsonPropertyInfoValuesTypeRef = "global::System.Text.Json.Serialization.Metadata.JsonPropertyInfoValues";
private const string JsonTypeInfoTypeRef = "global::System.Text.Json.Serialization.Metadata.JsonTypeInfo";
private const string JsonTypeInfoResolverTypeRef = "global::System.Text.Json.Serialization.Metadata.IJsonTypeInfoResolver";

private static DiagnosticDescriptor TypeNotSupported { get; } = new DiagnosticDescriptor(
id: "SYSLIB1030",
Expand Down Expand Up @@ -131,14 +133,14 @@ public void Emit()
isRootContextDef: true);

// Add GetJsonTypeInfo override implementation.
AddSource($"{contextName}.GetJsonTypeInfo.g.cs", GetGetTypeInfoImplementation());
AddSource($"{contextName}.GetJsonTypeInfo.g.cs", GetGetTypeInfoImplementation(), interfaceImplementation: JsonTypeInfoResolverTypeRef);

// Add property name initialization.
AddSource($"{contextName}.PropertyNames.g.cs", GetPropertyNameInitialization());
}
}

private void AddSource(string fileName, string source, bool isRootContextDef = false)
private void AddSource(string fileName, string source, bool isRootContextDef = false, string? interfaceImplementation = null)
{
string? generatedCodeAttributeSource = isRootContextDef ? s_generatedCodeAttributeSource : null;

Expand Down Expand Up @@ -175,7 +177,7 @@ namespace {@namespace}

// Add the core implementation for the derived context class.
string partialContextImplementation = $@"
{generatedCodeAttributeSource}{declarationList[0]}
{generatedCodeAttributeSource}{declarationList[0]}{(interfaceImplementation is null ? "" : ": " + interfaceImplementation)}
{{
{IndentSource(source, Math.Max(1, declarationCount - 1))}
}}";
Expand Down Expand Up @@ -338,7 +340,7 @@ private static string GenerateForTypeWithUnknownConverter(TypeGenerationSpec typ
{{
// Allow nullable handling to forward to the underlying type's converter.
converter = {JsonMetadataServicesTypeRef}.GetNullableConverter<{typeCompilableName}>(this.{typeFriendlyName})!;
converter = (({ JsonConverterFactoryTypeRef })converter).CreateConverter(typeToConvert, { OptionsInstanceVariableName })!;
converter = (({JsonConverterFactoryTypeRef})converter).CreateConverter(typeToConvert, {OptionsInstanceVariableName})!;
}}
else
{{
Expand All @@ -356,7 +358,7 @@ private static string GenerateForTypeWithUnknownConverter(TypeGenerationSpec typ
}

metadataInitSource.Append($@"
_{typeFriendlyName} = { JsonMetadataServicesTypeRef }.{ GetCreateValueInfoMethodRef(typeCompilableName)} ({ OptionsInstanceVariableName}, converter); ");
_{typeFriendlyName} = {JsonMetadataServicesTypeRef}.{GetCreateValueInfoMethodRef(typeCompilableName)} ({OptionsInstanceVariableName}, converter); ");

return GenerateForType(typeMetadata, metadataInitSource.ToString());
}
Expand Down Expand Up @@ -1176,6 +1178,10 @@ private string GetRootJsonContextImplementation()
{{
}}

private {contextTypeName}({JsonSerializerOptionsTypeRef} options, bool bindOptionsToContext) : base(options, bindOptionsToContext)
{{
}}

{GetFetchLogicForRuntimeSpecifiedCustomConverter()}");

if (_generateGetConverterMethodForProperties)
Expand Down Expand Up @@ -1291,10 +1297,28 @@ private string GetGetTypeInfoImplementation()
}
}

sb.Append(@"
sb.AppendLine(@"
return null!;
}");

// Explicit IJsonTypeInfoResolver implementation
string contextTypeName = _currentContext.ContextType.Name;

sb.AppendLine();
sb.Append(@$"{JsonTypeInfoTypeRef}? {JsonTypeInfoResolverTypeRef}.GetTypeInfo({TypeTypeRef} type, {JsonSerializerOptionsTypeRef} options)
{{
{contextTypeName} context = this;

if (options != null && {OptionsInstanceVariableName} != options)
{{
context = (s_resolverCache ??= new()).GetValue(options, static options => new {contextTypeName}(options, bindOptionsToContext: false));
}}

return context.GetTypeInfo(type);
}}

private {ConditionalWeakTableTypeRef}<{JsonSerializerOptionsTypeRef},{contextTypeName}>? s_resolverCache;");

return sb.ToString();
}

Expand Down
1 change: 1 addition & 0 deletions src/libraries/System.Text.Json/ref/System.Text.Json.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -960,6 +960,7 @@ public JsonSerializableAttribute(System.Type type) { }
public abstract partial class JsonSerializerContext : System.Text.Json.Serialization.Metadata.IJsonTypeInfoResolver
{
protected JsonSerializerContext(System.Text.Json.JsonSerializerOptions? options) { }
protected JsonSerializerContext(System.Text.Json.JsonSerializerOptions? options, bool bindOptionsToContext) { }
protected abstract System.Text.Json.JsonSerializerOptions? GeneratedSerializerOptions { get; }
public System.Text.Json.JsonSerializerOptions Options { get { throw null; } }
public abstract System.Text.Json.Serialization.Metadata.JsonTypeInfo? GetTypeInfo(System.Type type);
Expand Down
23 changes: 22 additions & 1 deletion src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerContext.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,10 +81,31 @@ internal bool CanUseSerializationLogic
/// or until <see cref="Options"/> is called, where a new options instance is created and bound.
/// </remarks>
protected JsonSerializerContext(JsonSerializerOptions? options)
: this(options, bindOptionsToContext: true)
{
}

/// <summary>
/// Creates an instance of <see cref="JsonSerializerContext"/> and optionally binds it with the indicated <see cref="JsonSerializerOptions"/>.
/// </summary>
/// <param name="options">The run time provided options for the context instance.</param>
/// <param name="bindOptionsToContext">Specify whether the options <paramref name="options"/> instance should be bound to the new context.</param>
/// <remarks>
/// If no instance options are passed, then no options are set until the context is bound using <see cref="JsonSerializerOptions.AddContext{TContext}"/>,
/// or until <see cref="Options"/> is called, where a new options instance is created and bound.
/// </remarks>
protected JsonSerializerContext(JsonSerializerOptions? options, bool bindOptionsToContext)
{
if (options != null)
{
options.TypeInfoResolver = this;
if (bindOptionsToContext)
{
options.TypeInfoResolver = this;
}
else
{
_options = options;
Copy link
Owner

Choose a reason for hiding this comment

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

I think you should always be setting this regardless of the flag

}
}
}

Expand Down
43 changes: 43 additions & 0 deletions ...tem.Text.Json/tests/System.Text.Json.SourceGeneration.Tests/JsonSerializerContextTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,49 @@ public static void SupportsPositionalRecords()
Assert.Equal("Doe", person.LastName);
}

[Fact]
public static void CombiningContexts_ResolveJsonTypeInfo()
{
// Basic smoke test establishing combination of JsonSerializerContext classes.
IJsonTypeInfoResolver combined = JsonTypeInfoResolver.Combine(NestedContext.Default, PersonJsonContext.Default);
var options = new JsonSerializerOptions { TypeInfoResolver = combined };

JsonTypeInfo messageInfo = combined.GetTypeInfo(typeof(JsonMessage), options);
Assert.IsAssignableFrom<JsonTypeInfo<JsonMessage>>(messageInfo);
Assert.Same(options, messageInfo.Options);

JsonTypeInfo personInfo = combined.GetTypeInfo(typeof(Person), options);
Assert.IsAssignableFrom<JsonTypeInfo<Person>>(personInfo);
Assert.Same(options, personInfo.Options);
}

[Theory]
[MemberData(nameof(GetCombiningContextsData))]
public static void CombiningContexts_Serialization<T>(T value, string expectedJson)
{
// Basic smoke test establishing combination of JsonSerializerContext classes.
IJsonTypeInfoResolver combined = JsonTypeInfoResolver.Combine(NestedContext.Default, PersonJsonContext.Default);
var options = new JsonSerializerOptions { TypeInfoResolver = combined };

JsonTypeInfo<T> typeInfo = (JsonTypeInfo<T>)combined.GetTypeInfo(typeof(T), options)!;

string json = JsonSerializer.Serialize(value, typeInfo);
JsonTestHelper.AssertJsonEqual(expectedJson, json);

json = JsonSerializer.Serialize(value, options);
JsonTestHelper.AssertJsonEqual(expectedJson, json);

JsonSerializer.Deserialize<T>(json, typeInfo);
JsonSerializer.Deserialize<T>(json, options);
}

public static IEnumerable<object[]> GetCombiningContextsData()
{
yield return WrapArgs(new JsonMessage { Message = "Hi" }, """{ "Message" : { "Hi" } }""");
yield return WrapArgs(new Person("John", "Doe"), """{ "FirstName" : "John", "LastName" : "Doe" }""");
static object[] WrapArgs<T>(T value, string expectedJson) => new object[] { value, expectedJson };
}

[JsonSerializable(typeof(JsonMessage))]
internal partial class NestedContext : JsonSerializerContext { }

Expand Down