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

Implement JsonTypeInfoResolver.WithAddedModifier #88255

Merged
merged 1 commit into from
Jul 5, 2023
Merged

Implement JsonTypeInfoResolver.WithAddedModifier #88255

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
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 @@ -1315,6 +1315,7 @@ public enum JsonTypeInfoKind
public static partial class JsonTypeInfoResolver
{
public static System.Text.Json.Serialization.Metadata.IJsonTypeInfoResolver Combine(params System.Text.Json.Serialization.Metadata.IJsonTypeInfoResolver?[] resolvers) { throw null; }
public static System.Text.Json.Serialization.Metadata.IJsonTypeInfoResolver WithAddedModifier(this System.Text.Json.Serialization.Metadata.IJsonTypeInfoResolver resolver, System.Action<System.Text.Json.Serialization.Metadata.JsonTypeInfo> modifier) { throw null; }
}
public sealed partial class JsonTypeInfo<T> : System.Text.Json.Serialization.Metadata.JsonTypeInfo
{
Expand Down
1 change: 1 addition & 0 deletions src/libraries/System.Text.Json/src/System.Text.Json.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,7 @@ The System.Text.Json library is built-in as part of the shared framework in .NET
<Compile Include="System\Text\Json\Serialization\Metadata\JsonTypeInfoResolver.cs" />
<Compile Include="System\Text\Json\Serialization\Metadata\JsonTypeInfoKind.cs" />
<Compile Include="System\Text\Json\Serialization\Metadata\JsonTypeInfoResolverChain.cs" />
<Compile Include="System\Text\Json\Serialization\Metadata\JsonTypeInfoResolverWithAddedModifiers.cs" />
<Compile Include="System\Text\Json\Serialization\PolymorphicSerializationState.cs" />
<Compile Include="System\Text\Json\Writer\Utf8JsonWriterCache.cs" />
<Compile Include="System\Text\Json\Serialization\ReferenceEqualsWrapper.cs" />
Expand Down
2 changes: 1 addition & 1 deletion 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 @@ -899,7 +899,7 @@ private static JsonSerializerOptions GetOrCreateDefaultOptionsInstance()

TypeInfoResolver = JsonSerializer.IsReflectionEnabledByDefault
? DefaultJsonTypeInfoResolver.RootDefaultInstance()
: new JsonTypeInfoResolverChain(),
: JsonTypeInfoResolver.Empty,

_isReadOnly = true,
};
Expand Down
2 changes: 1 addition & 1 deletion ...son/src/System/Text/Json/Serialization/Metadata/DefaultJsonTypeInfoResolver.Converters.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ internal static bool TryGetDefaultSimpleConverter(Type typeToConvert, [NotNullWh

[RequiresUnreferencedCode(JsonSerializer.SerializationUnreferencedCodeMessage)]
[RequiresDynamicCode(JsonSerializer.SerializationRequiresDynamicCodeMessage)]
internal static JsonConverter? GetCustomConverterForMember(Type typeToConvert, MemberInfo memberInfo, JsonSerializerOptions options)
private static JsonConverter? GetCustomConverterForMember(Type typeToConvert, MemberInfo memberInfo, JsonSerializerOptions options)
{
Debug.Assert(memberInfo is FieldInfo or PropertyInfo);
Debug.Assert(typeToConvert != null);
Expand Down
33 changes: 32 additions & 1 deletion ...ries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfoResolver.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ public static IJsonTypeInfoResolver Combine(params IJsonTypeInfoResolver?[] reso
{
if (resolvers is null)
{
throw new ArgumentNullException(nameof(resolvers));
ThrowHelper.ThrowArgumentNullException(nameof(resolvers));
}

var resolverChain = new JsonTypeInfoResolverChain();
Expand All @@ -39,6 +39,37 @@ public static IJsonTypeInfoResolver Combine(params IJsonTypeInfoResolver?[] reso
return resolverChain.Count == 1 ? resolverChain[0] : resolverChain;
}

/// <summary>
/// Creates a resolver applies modifications to the metadata generated by the source <paramref name="resolver"/>.
/// </summary>
/// <param name="resolver">The source resolver generating <see cref="JsonTypeInfo"/> metadata.</param>
/// <param name="modifier">The delegate modifying non-null <see cref="JsonTypeInfo"/> results.</param>
/// <returns>A new <see cref="IJsonTypeInfoResolver"/> instance applying the modifications.</returns>
/// <remarks>
/// This method is closely related to <see cref="DefaultJsonTypeInfoResolver.Modifiers"/> property
/// extended to arbitrary <see cref="IJsonTypeInfoResolver"/> instances.
/// </remarks>
public static IJsonTypeInfoResolver WithAddedModifier(this IJsonTypeInfoResolver resolver, Action<JsonTypeInfo> modifier)
{
if (resolver is null)
{
ThrowHelper.ThrowArgumentNullException(nameof(resolver));
}
if (modifier is null)
{
ThrowHelper.ThrowArgumentNullException(nameof(modifier));
}

return resolver is JsonTypeInfoResolverWithAddedModifiers resolverWithModifiers
? resolverWithModifiers.WithAddedModifier(modifier)
: new JsonTypeInfoResolverWithAddedModifiers(resolver, new[] { modifier });
}

/// <summary>
/// Gets a resolver that returns null <see cref="JsonTypeInfo"/> for every type.
/// </summary>
internal static IJsonTypeInfoResolver Empty { get; } = new JsonTypeInfoResolverChain();

/// <summary>
/// Indicates whether the metadata generated by the current resolver
/// are compatible with the run time specified <see cref="JsonSerializerOptions"/>.
Expand Down
44 changes: 44 additions & 0 deletions ...son/src/System/Text/Json/Serialization/Metadata/JsonTypeInfoResolverWithAddedModifiers.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.Diagnostics;

namespace System.Text.Json.Serialization.Metadata
{
internal sealed class JsonTypeInfoResolverWithAddedModifiers : IJsonTypeInfoResolver
{
private readonly IJsonTypeInfoResolver _source;
private readonly Action<JsonTypeInfo>[] _modifiers;
Copy link
Member

@krwq krwq Jul 5, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I'm curious if you could support it by List and count (you always append so technically you could reuse same instance) or ImmutableArray/List - not a big deal - this is not in a hot path

Copy link
Member Author

Choose a reason for hiding this comment

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

We want the resolver instances to be immutable -- if we reused the same List<T> then calling var bar = foo.WithAddedModifiers(...); then foo would be mutated.


public JsonTypeInfoResolverWithAddedModifiers(IJsonTypeInfoResolver source, Action<JsonTypeInfo>[] modifiers)
{
Debug.Assert(modifiers.Length > 0);
krwq marked this conversation as resolved.
Show resolved Hide resolved
_source = source;
_modifiers = modifiers;
}

public JsonTypeInfoResolverWithAddedModifiers WithAddedModifier(Action<JsonTypeInfo> modifier)
krwq marked this conversation as resolved.
Show resolved Hide resolved
{
var newModifiers = new Action<JsonTypeInfo>[_modifiers.Length + 1];
_modifiers.CopyTo(newModifiers, 0);
newModifiers[_modifiers.Length] = modifier;

return new JsonTypeInfoResolverWithAddedModifiers(_source, newModifiers);
}

public JsonTypeInfo? GetTypeInfo(Type type, JsonSerializerOptions options)
{
JsonTypeInfo? typeInfo = _source.GetTypeInfo(type, options);

if (typeInfo != null)
{
foreach (Action<JsonTypeInfo> modifier in _modifiers)
{
modifier(typeInfo);
}
}

return typeInfo;
}
}
}
34 changes: 2 additions & 32 deletions src/libraries/System.Text.Json/tests/Common/JsonSerializerWrapper.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ public JsonSerializerOptions GetDefaultOptionsWithMetadataModifier(Action<JsonTy
JsonSerializerOptions defaultOptions = DefaultOptions;
return new JsonSerializerOptions(defaultOptions)
{
TypeInfoResolver = defaultOptions.TypeInfoResolver.WithModifier(modifier)
TypeInfoResolver = defaultOptions.TypeInfoResolver.WithAddedModifier(modifier)
};
}

Expand All @@ -79,7 +79,7 @@ public JsonSerializerOptions CreateOptions(

if (modifier != null && options.TypeInfoResolver != null)
{
options.TypeInfoResolver = DefaultOptions.TypeInfoResolver.WithModifier(modifier);
options.TypeInfoResolver = DefaultOptions.TypeInfoResolver.WithAddedModifier(modifier);
}

if (customConverters != null)
Expand All @@ -100,34 +100,4 @@ public JsonSerializerOptions CreateOptions(
return options;
}
}

public static class JsonTypeInfoResolverExtensions
{
public static IJsonTypeInfoResolver WithModifier(this IJsonTypeInfoResolver resolver, Action<JsonTypeInfo> modifier)
=> new JsonTypeInfoResolverWithModifier(resolver, modifier);

private class JsonTypeInfoResolverWithModifier : IJsonTypeInfoResolver
{
private readonly IJsonTypeInfoResolver _source;
private readonly Action<JsonTypeInfo> _modifier;

public JsonTypeInfoResolverWithModifier(IJsonTypeInfoResolver source, Action<JsonTypeInfo> modifier)
{
_source = source;
_modifier = modifier;
}

public JsonTypeInfo? GetTypeInfo(Type type, JsonSerializerOptions options)
{
JsonTypeInfo? typeInfo = _source.GetTypeInfo(type, options);

if (typeInfo != null)
{
_modifier(typeInfo);
}

return typeInfo;
}
}
}
}
95 changes: 14 additions & 81 deletions ....Json/tests/System.Text.Json.Tests/Serialization/MetadataTests/DefaultJsonPropertyInfo.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,79 +9,30 @@ namespace System.Text.Json.Serialization.Tests
{
public class DefaultJsonPropertyInfoTests_DefaultJsonTypeInfoResolver : DefaultJsonPropertyInfoTests
{
protected override IJsonTypeInfoResolver CreateResolverWithModifiers(params Action<JsonTypeInfo>[] modifiers)
{
var resolver = new DefaultJsonTypeInfoResolver();

foreach (var modifier in modifiers)
{
resolver.Modifiers.Add(modifier);
}

return resolver;
}
protected override IJsonTypeInfoResolver Resolver { get; } = new DefaultJsonTypeInfoResolver();
}

public class DefaultJsonPropertyInfoTests_SerializerContextNoWrapping : DefaultJsonPropertyInfoTests
{
protected override bool ModifiersNotSupported => true;

protected override IJsonTypeInfoResolver CreateResolverWithModifiers(params Action<JsonTypeInfo>[] modifiers)
{
if (modifiers.Length != 0)
{
Assert.Fail($"Testing non wrapped JsonSerializerContext but modifier is provided. Make sure to check {nameof(ModifiersNotSupported)}.");
}

return Context.Default;
}
protected override IJsonTypeInfoResolver Resolver { get; } = Context.Default;
}

public class DefaultJsonPropertyInfoTests_SerializerContextWrapped : DefaultJsonPropertyInfoTests
{
protected override IJsonTypeInfoResolver CreateResolverWithModifiers(params Action<JsonTypeInfo>[] modifiers)
=> new ContextWithModifiers(Context.Default, modifiers);

private class ContextWithModifiers : IJsonTypeInfoResolver
{
private IJsonTypeInfoResolver _context;
private Action<JsonTypeInfo>[] _modifiers;

public ContextWithModifiers(JsonSerializerContext context, Action<JsonTypeInfo>[] modifiers)
{
_context = context;
_modifiers = modifiers;
}

public JsonTypeInfo? GetTypeInfo(Type type, JsonSerializerOptions options)
{
JsonTypeInfo? typeInfo = _context.GetTypeInfo(type, options);

if (typeInfo != null)
{
foreach (var modifier in _modifiers)
{
modifier(typeInfo);
}
}

return typeInfo;
}
}
protected override IJsonTypeInfoResolver Resolver { get; } = Context.Default;
}

public abstract partial class DefaultJsonPropertyInfoTests
{
protected virtual bool ModifiersNotSupported => false;
protected abstract IJsonTypeInfoResolver CreateResolverWithModifiers(params Action<JsonTypeInfo>[] modifiers);
protected abstract IJsonTypeInfoResolver Resolver { get; }

private JsonSerializerOptions CreateOptionsWithModifiers(params Action<JsonTypeInfo>[] modifiers)
=> new JsonSerializerOptions()
private JsonSerializerOptions CreateOptionsWithModifier(Action<JsonTypeInfo> modifier)
=> new JsonSerializerOptions
{
TypeInfoResolver = CreateResolverWithModifiers(modifiers)
TypeInfoResolver = Resolver.WithAddedModifier(modifier)
};

private JsonSerializerOptions CreateOptions() => CreateOptionsWithModifiers();
private JsonSerializerOptions CreateOptions() => new JsonSerializerOptions { TypeInfoResolver = Resolver };

[Fact]
public void RequiredAttributesGetDetectedAndFailDeserializationWhenValuesNotPresent()
Expand Down Expand Up @@ -136,10 +87,7 @@ public void RequiredAttributesGetDetectedAndFailDeserializationWhenValuesNotPres
[Fact]
public void RequiredMemberCanBeModifiedToNonRequired()
{
if (ModifiersNotSupported)
return;

JsonSerializerOptions options = CreateOptionsWithModifiers(ti =>
JsonSerializerOptions options = CreateOptionsWithModifier(ti =>
{
if (ti.Type == typeof(ClassWithRequiredCustomAttributes))
{
Expand Down Expand Up @@ -184,10 +132,7 @@ public void RequiredMemberCanBeModifiedToNonRequired()
[Fact]
public void NonRequiredMemberCanBeModifiedToRequired()
{
if (ModifiersNotSupported)
return;

JsonSerializerOptions options = CreateOptionsWithModifiers(ti =>
JsonSerializerOptions options = CreateOptionsWithModifier(ti =>
{
if (ti.Type == typeof(ClassWithRequiredCustomAttributes))
{
Expand Down Expand Up @@ -241,10 +186,7 @@ public void RequiredExtensionDataPropertyThrows()
[Fact]
public void RequiredExtensionDataPropertyCanBeFixedToNotBeRequiredWithResolver()
{
if (ModifiersNotSupported)
return;

JsonSerializerOptions options = CreateOptionsWithModifiers(ti =>
JsonSerializerOptions options = CreateOptionsWithModifier(ti =>
{
if (ti.Type == typeof(ClassWithRequiredCustomAttributeAndDataExtensionProperty))
{
Expand Down Expand Up @@ -275,10 +217,7 @@ public void RequiredExtensionDataPropertyCanBeFixedToNotBeRequiredWithResolver()
[Fact]
public void RequiredExtensionDataPropertyCanBeFixedToNotBeExtensionDataWithResolver()
{
if (ModifiersNotSupported)
return;

JsonSerializerOptions options = CreateOptionsWithModifiers(ti =>
JsonSerializerOptions options = CreateOptionsWithModifier(ti =>
{
if (ti.Type == typeof(ClassWithRequiredCustomAttributeAndDataExtensionProperty))
{
Expand Down Expand Up @@ -319,10 +258,7 @@ public void RequiredReadOnlyPropertyThrows()
[Fact]
public void RequiredReadOnlyPropertyCanBeFixedToNotBeRequiredWithResolver()
{
if (ModifiersNotSupported)
return;

JsonSerializerOptions options = CreateOptionsWithModifiers(ti =>
JsonSerializerOptions options = CreateOptionsWithModifier(ti =>
{
if (ti.Type == typeof(ClassWithRequiredCustomAttributeAndReadOnlyProperty))
{
Expand Down Expand Up @@ -354,10 +290,7 @@ public void RequiredReadOnlyPropertyCanBeFixedToNotBeRequiredWithResolver()
[Fact]
public void RequiredReadOnlyPropertyCanBeFixedToBeWritableWithResolver()
{
if (ModifiersNotSupported)
return;

JsonSerializerOptions options = CreateOptionsWithModifiers(ti =>
JsonSerializerOptions options = CreateOptionsWithModifier(ti =>
{
if (ti.Type == typeof(ClassWithRequiredCustomAttributeAndReadOnlyProperty))
{
Expand Down
Loading