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.

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

Minor refactorings #1

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -1215,6 +1215,7 @@ public abstract partial class JsonTypeInfo<T> : System.Text.Json.Serialization.M
{
internal JsonTypeInfo() { }
public new System.Func<T>? CreateObject { get { throw null; } set { } }
[System.ComponentModel.EditorBrowsableAttribute(System.ComponentModel.EditorBrowsableState.Never)]
public System.Action<System.Text.Json.Utf8JsonWriter, T>? SerializeHandler { get { throw null; } }
}
public enum JsonTypeInfoKind
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,29 +4,25 @@
using System.Collections;
using System.Collections.Generic;
using System.Diagnostics;
using System.Globalization;

namespace System.Text.Json.Serialization
{
/// <summary>
/// A list of configuration items that respects the options class being immutable once (de)serialization occurs.
/// A list of configuration items that can be locked for modification
/// </summary>
internal sealed class ConfigurationList<TItem> : IList<TItem>
internal abstract class ConfigurationList<TItem> : IList<TItem>
{
private readonly List<TItem> _list;

public Action<TItem>? OnElementAdded { get; set; }
public Func<bool>? IsReadOnlyFunc { get; set; }
public Action? ThrowImmutableFunc { get; set; }

public ConfigurationList()
public ConfigurationList(IList<TItem>? source = null)
{
_list = new List<TItem>();
_list = source is null ? new List<TItem>() : new List<TItem>(source);
}

public ConfigurationList(IList<TItem> source)
{
_list = new List<TItem>(source is ConfigurationList<TItem> cl ? cl._list : source);
}
protected abstract bool IsLockedInstance { get; }
protected abstract void VerifyMutable();
protected virtual void OnItemAdded(TItem item) { }

public TItem this[int index]
{
Expand All @@ -43,24 +39,13 @@ public TItem this[int index]

VerifyMutable();
_list[index] = value;
OnElementAdded?.Invoke(value);
OnItemAdded(value);
}
}

public int Count => _list.Count;

public bool IsReadOnly => IsReadOnlyFunc != null ? IsReadOnlyFunc() : false;

private void VerifyMutable()
{
Debug.Assert((IsReadOnlyFunc == null) == (ThrowImmutableFunc == null), "IsReadOnlyFunc and ThrowImmutableFunc should be either both set or both unset");

if (IsReadOnlyFunc != null && IsReadOnlyFunc())
{
Debug.Assert(ThrowImmutableFunc != null);
ThrowImmutableFunc();
}
}
public bool IsReadOnly => IsLockedInstance;

public void Add(TItem item)
{
Expand All @@ -71,7 +56,7 @@ public void Add(TItem item)

VerifyMutable();
_list.Add(item);
OnElementAdded?.Invoke(item);
OnItemAdded(item);
}

public void Clear()
Expand Down Expand Up @@ -109,7 +94,7 @@ public void Insert(int index, TItem item)

VerifyMutable();
_list.Insert(index, item);
OnElementAdded?.Invoke(item);
OnItemAdded(item);
}

public bool Remove(TItem item)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ internal override bool OnTryWrite(Utf8JsonWriter writer, T value, JsonSerializer
jsonTypeInfo is JsonTypeInfo<T> info &&
info.SerializeHandler != null &&
!state.CurrentContainsMetadata && // Do not use the fast path if state needs to write metadata.
info.Options.JsonSerializerContext?.CanUseSerializationLogic == true)
info.Options.SerializerContext?.CanUseSerializationLogic == true)
{
info.SerializeHandler(writer, value);
return true;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,10 @@ private static JsonTypeInfo GetTypeInfo(JsonSerializerOptions? options, Type run
Debug.Assert(runtimeType != null);

options ??= JsonSerializerOptions.Default;
options.EnsureInitializedForReflectionSerializer();
if (!options.IsInitializedForReflectionSerializer)
{
options.InitializeForReflectionSerializer();
}

return options.GetOrAddJsonTypeInfoForRootType(runtimeType);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -369,7 +369,10 @@ public static partial class JsonSerializer
}

options ??= JsonSerializerOptions.Default;
options.EnsureInitializedForReflectionSerializer();
if (!options.IsInitializedForReflectionSerializer)
{
options.InitializeForReflectionSerializer();
}

JsonTypeInfo jsonTypeInfo = options.GetOrAddJsonTypeInfoForRootType(typeof(TValue));
return CreateAsyncEnumerableDeserializer<TValue>(utf8Json, jsonTypeInfo, cancellationToken);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ private static void WriteUsingGeneratedSerializer<TValue>(Utf8JsonWriter writer,

if (jsonTypeInfo.HasSerialize &&
jsonTypeInfo is JsonTypeInfo<TValue> typedInfo &&
typedInfo.Options.JsonSerializerContext?.CanUseSerializationLogic == true)
typedInfo.Options.SerializerContext?.CanUseSerializationLogic == true)
{
Debug.Assert(typedInfo.SerializeHandler != null);
typedInfo.SerializeHandler(writer, value);
Expand All @@ -59,8 +59,8 @@ private static void WriteUsingSerializer<TValue>(Utf8JsonWriter writer, in TValu

Debug.Assert(!jsonTypeInfo.HasSerialize ||
jsonTypeInfo is not JsonTypeInfo<TValue> ||
jsonTypeInfo.Options.JsonSerializerContext == null ||
!jsonTypeInfo.Options.JsonSerializerContext.CanUseSerializationLogic,
jsonTypeInfo.Options.SerializerContext == null ||
!jsonTypeInfo.Options.SerializerContext.CanUseSerializationLogic,
"Incorrect method called. WriteUsingGeneratedSerializer() should have been called instead.");

WriteStack state = default;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,9 @@ public abstract partial class JsonSerializerContext : IJsonTypeInfoResolver
/// when instanciating the context, then a new instance is bound and returned.
/// </summary>
/// <remarks>
/// The instance cannot be mutated once it is bound with the context instance.
/// The instance cannot be mutated once it is bound to the context instance.
/// </remarks>
public JsonSerializerOptions Options => _options ??= new JsonSerializerOptions { JsonSerializerContext = this };

// Currently JsonSerializerContext starts falling back to DefaultJsonTypeInfoResolver when some methods are called.
internal IJsonTypeInfoResolver? FallbackResolver { get; set; }
public JsonSerializerOptions Options => _options ??= new JsonSerializerOptions { TypeInfoResolver = this };

/// <summary>
/// Indicates whether pre-generated serialization logic for types in the context
Expand Down Expand Up @@ -87,8 +84,7 @@ protected JsonSerializerContext(JsonSerializerOptions? options)
{
if (options != null)
{
options.JsonSerializerContext = this;
_options = options;
options.TypeInfoResolver = this;
eiriktsarpalis marked this conversation as resolved.
Show resolved Hide resolved
}
}

Expand All @@ -101,15 +97,13 @@ protected JsonSerializerContext(JsonSerializerOptions? options)

JsonTypeInfo? IJsonTypeInfoResolver.GetTypeInfo(Type type, JsonSerializerOptions options)
{
if (_options != options)
if (options != null && _options != options)
{
Debug.Assert(_options != null, "_options are null");
// TODO is this the appropriate exception message to throw?
ThrowHelper.ThrowInvalidOperationException_SerializerContextOptionsImmutable();
}

JsonTypeInfo? typeInfo = GetTypeInfo(type);
typeInfo ??= FallbackResolver?.GetTypeInfo(type, options);
return typeInfo;
return GetTypeInfo(type);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ internal JsonTypeInfo GetOrAddJsonTypeInfo(Type type)
if (_cachingContext == null)
{
InitializeCachingContext();
Debug.Assert(_cachingContext != null);
}

return _cachingContext.GetOrAddJsonTypeInfo(type);
Expand Down Expand Up @@ -71,15 +70,11 @@ internal void ClearCaches()
_lastTypeInfo = null;
}

[MemberNotNull(nameof(_cachingContext))]
private void InitializeCachingContext()
{
_isLockedInstance = true;
_cachingContext = TrackedCachingContexts.GetOrCreate(this);
if (_typeInfoResolver != null)
{
Debug.Assert(_cachingContext.Options._typeInfoResolver != null || _typeInfoResolver == s_defaultTypeInfoResolver,
"EqualityComparer should guarantee that null is equivalent to s_defaultTypeInfoResolver");
_cachingContext.Options._typeInfoResolver ??= s_defaultTypeInfoResolver;
}
}

/// <summary>
Expand Down Expand Up @@ -131,6 +126,7 @@ internal static class TrackedCachingContexts

public static CachingContext GetOrCreate(JsonSerializerOptions options)
{
Debug.Assert(options._isLockedInstance, "Cannot create caching contexts for mutable JsonSerializerOptions instances");
ConcurrentDictionary<JsonSerializerOptions, WeakReference<CachingContext>> cache = s_cache;

if (cache.TryGetValue(options, out WeakReference<CachingContext>? wr) && wr.TryGetTarget(out CachingContext? ctx))
Expand Down Expand Up @@ -169,7 +165,6 @@ public static CachingContext GetOrCreate(JsonSerializerOptions options)
{
// Copy fields ignored by the copy constructor
// but are necessary to determine equivalence.
_serializerContext = options._serializerContext,
_typeInfoResolver = options._typeInfoResolver,
};
Debug.Assert(key._cachingContext == null);
Expand Down Expand Up @@ -273,25 +268,6 @@ public bool Equals(JsonSerializerOptions? left, JsonSerializerOptions? right)
{
Debug.Assert(left != null && right != null);

// We cache these values in this specific order enforcing default resolver to be read last
// this is done in case _typeInfoResolver or s_defaultTypeInfoResolver is not initialized yet
// the behavior will still be correct
IJsonTypeInfoResolver? leftResolver = left._typeInfoResolver;
IJsonTypeInfoResolver? rightResolver = right._typeInfoResolver;
IJsonTypeInfoResolver? defaultResolver = Volatile.Read(ref s_defaultTypeInfoResolver);

// we ensure null and s_defaultTypeInfoResolver mean the same thing
// we normalize to null so that calculating hash code is consistent
if (leftResolver == defaultResolver)
{
leftResolver = null;
}

if (rightResolver == defaultResolver)
{
rightResolver = null;
}

return
left._dictionaryKeyPolicy == right._dictionaryKeyPolicy &&
left._jsonPropertyNamingPolicy == right._jsonPropertyNamingPolicy &&
Expand All @@ -310,8 +286,7 @@ public bool Equals(JsonSerializerOptions? left, JsonSerializerOptions? right)
left._includeFields == right._includeFields &&
left._propertyNameCaseInsensitive == right._propertyNameCaseInsensitive &&
left._writeIndented == right._writeIndented &&
leftResolver == rightResolver &&
left._serializerContext == right._serializerContext &&
left._typeInfoResolver == right._typeInfoResolver &&
CompareLists(left._converters, right._converters) &&
#pragma warning disable CA2252 // This API requires opting into preview features
CompareLists(left._polymorphicTypeConfigurations, right._polymorphicTypeConfigurations);
Expand Down Expand Up @@ -339,16 +314,6 @@ static bool CompareLists<TValue>(ConfigurationList<TValue> left, ConfigurationLi

public int GetHashCode(JsonSerializerOptions options)
{
IJsonTypeInfoResolver? typeInfoResolver = options._typeInfoResolver;
IJsonTypeInfoResolver? defaultResolver = Volatile.Read(ref s_defaultTypeInfoResolver);

// we normalize s_defaultTypeInfoResolver to null so that hash code is the same in case s_defaultTypeInfoResolver
// is not initialized yet
if (typeInfoResolver == defaultResolver)
{
typeInfoResolver = null;
}

HashCode hc = default;

hc.Add(options._dictionaryKeyPolicy);
Expand All @@ -368,8 +333,7 @@ public int GetHashCode(JsonSerializerOptions options)
hc.Add(options._includeFields);
hc.Add(options._propertyNameCaseInsensitive);
hc.Add(options._writeIndented);
hc.Add(typeInfoResolver);
hc.Add(options._serializerContext);
hc.Add(options._typeInfoResolver);
GetHashCode(ref hc, options._converters);
#pragma warning disable CA2252 // This API requires opting into preview features
GetHashCode(ref hc, options._polymorphicTypeConfigurations);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,30 +24,8 @@ public sealed partial class JsonSerializerOptions
// The global list of built-in converters that override CanConvert().
private static JsonConverter[]? s_defaultFactoryConverters;

// Stores the JsonTypeInfo factory, which requires unreferenced code and must be rooted by the reflection-based serializer.
private static IJsonTypeInfoResolver? s_defaultTypeInfoResolver;

[RequiresUnreferencedCode(JsonSerializer.SerializationUnreferencedCodeMessage)]
[RequiresDynamicCode(JsonSerializer.SerializationRequiresDynamicCodeMessage)]
[MemberNotNull(nameof(s_defaultTypeInfoResolver))]
private static void RootReflectionSerializerDependencies()
{
// We need to ensure that all threads use same instance of s_defaultTypeInfoResolver or
// otherwise EqualityComparer for CachingContext may fail even though
// two options might assign _typeInfoResolver to s_defaultTypeInfoResolver
Interlocked.CompareExchange(ref s_defaultTypeInfoResolver, new DefaultJsonTypeInfoResolver(mutable: false), null);

// constructor for DefaultJsonTypeInfoResolver calls RootConverters()
// and therefore both fields are initialized now
Debug.Assert(s_defaultSimpleConverters != null);
Debug.Assert(s_defaultFactoryConverters != null);
}

[RequiresUnreferencedCode(JsonSerializer.SerializationUnreferencedCodeMessage)]
[RequiresDynamicCode(JsonSerializer.SerializationRequiresDynamicCodeMessage)]
// This method should be called only within DefaultJsonTypeInfoResolver constructor
// We need to do this in case DefaultJsonTypeInfoResolver is called standalone without
// serialization happening first
internal static void RootConverters()
{
// s_defaultFactoryConverters is the last field assigned.
Expand Down Expand Up @@ -236,7 +214,7 @@ public JsonConverter GetConverter(Type typeToConvert)
ThrowHelper.ThrowArgumentNullException(nameof(typeToConvert));
}

RootReflectionSerializerDependencies();
RootConverters();
return GetConverterInternal(typeToConvert);
}

Expand All @@ -256,7 +234,7 @@ internal JsonConverter GetConverterFromType(Type typeToConvert)
Debug.Assert(typeToConvert != null);

// Priority 1: If there is a JsonSerializerContext, fetch the converter from there.
JsonConverter? converter = _serializerContext?.GetTypeInfo(typeToConvert)?.Converter;
JsonConverter? converter = SerializerContext?.GetTypeInfo(typeToConvert)?.Converter;

// Priority 2: Attempt to get custom converter added at runtime.
// Currently there is not a way at runtime to override the [JsonConverter] when applied to a property.
Expand Down Expand Up @@ -386,9 +364,9 @@ private JsonConverter GetConverterFromAttribute(JsonConverterAttribute converter

internal bool TryGetDefaultSimpleConverter(Type typeToConvert, [NotNullWhen(true)] out JsonConverter? converter)
{
if (_serializerContext == null && // For consistency do not return any default converters for
// options instances linked to a JsonSerializerContext,
// even if the default converters might have been rooted.
if (SerializerContext is null && // For consistency do not return any default converters for
// options instances linked to a JsonSerializerContext,
// even if the default converters might have been rooted.
s_defaultSimpleConverters != null &&
s_defaultSimpleConverters.TryGetValue(typeToConvert, out converter))
{
Expand Down
Loading