diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Object/ObjectDefaultConverter.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Object/ObjectDefaultConverter.cs index 9871d76f83d5b0..679c0bc5b1c4d4 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Object/ObjectDefaultConverter.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Object/ObjectDefaultConverter.cs @@ -200,7 +200,7 @@ internal override bool OnTryRead(ref Utf8JsonReader reader, Type typeToConvert, if (state.Current.PropertyState < StackFramePropertyState.ReadValue) { - if (!jsonPropertyInfo.CanDeserialize) + if (!jsonPropertyInfo.HasSetter) { if (!reader.TrySkip()) { @@ -289,7 +289,7 @@ internal sealed override bool OnTryWrite( for (int i = 0; i < properties.Count; i++) { JsonPropertyInfo jsonPropertyInfo = properties[i].Value; - if (jsonPropertyInfo.CanSerialize) + if (jsonPropertyInfo.HasGetter) { // Remember the current property for JsonPath support if an exception is thrown. state.Current.JsonPropertyInfo = jsonPropertyInfo; @@ -305,7 +305,7 @@ internal sealed override bool OnTryWrite( // Write extension data after the normal properties. JsonPropertyInfo? extensionDataProperty = jsonTypeInfo.ExtensionDataProperty; - if (extensionDataProperty?.CanSerialize == true) + if (extensionDataProperty?.HasGetter == true) { // Remember the current property for JsonPath support if an exception is thrown. state.Current.JsonPropertyInfo = extensionDataProperty; @@ -339,7 +339,7 @@ internal sealed override bool OnTryWrite( while (state.Current.EnumeratorIndex < propertyList.Count) { JsonPropertyInfo jsonPropertyInfo = propertyList[state.Current.EnumeratorIndex].Value; - if (jsonPropertyInfo.CanSerialize) + if (jsonPropertyInfo.HasGetter) { state.Current.JsonPropertyInfo = jsonPropertyInfo; state.Current.NumberHandling = jsonPropertyInfo.EffectiveNumberHandling; @@ -368,7 +368,7 @@ internal sealed override bool OnTryWrite( if (state.Current.EnumeratorIndex == propertyList.Count) { JsonPropertyInfo? extensionDataProperty = jsonTypeInfo.ExtensionDataProperty; - if (extensionDataProperty?.CanSerialize == true) + if (extensionDataProperty?.HasGetter == true) { // Remember the current property for JsonPath support if an exception is thrown. state.Current.JsonPropertyInfo = extensionDataProperty; @@ -415,7 +415,7 @@ protected static void ReadPropertyValue( bool useExtensionProperty) { // Skip the property if not found. - if (!jsonPropertyInfo.CanDeserialize) + if (!jsonPropertyInfo.HasSetter) { reader.Skip(); } diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Object/ObjectWithParameterizedConstructorConverter.Large.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Object/ObjectWithParameterizedConstructorConverter.Large.cs index ba190f1bf1c1b4..a369e8a349bcf6 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Object/ObjectWithParameterizedConstructorConverter.Large.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Object/ObjectWithParameterizedConstructorConverter.Large.cs @@ -21,7 +21,7 @@ protected sealed override bool ReadAndCacheConstructorArgument(ref ReadStack sta bool success = jsonParameterInfo.ConverterBase.TryReadAsObject(ref reader, jsonParameterInfo.Options!, ref state, out object? arg); - if (success && !(arg == null && jsonParameterInfo.IgnoreDefaultValuesOnRead)) + if (success && !(arg == null && jsonParameterInfo.IgnoreNullTokensOnRead)) { ((object[])state.Current.CtorArgumentState!.Arguments)[jsonParameterInfo.ClrInfo.Position] = arg!; } diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Object/ObjectWithParameterizedConstructorConverter.Small.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Object/ObjectWithParameterizedConstructorConverter.Small.cs index f206adba75f2d2..913198dd364199 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Object/ObjectWithParameterizedConstructorConverter.Small.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Object/ObjectWithParameterizedConstructorConverter.Small.cs @@ -67,7 +67,7 @@ private static bool TryRead( bool success = converter.TryRead(ref reader, info.PropertyType, info.Options!, ref state, out TArg? value); - arg = value == null && jsonParameterInfo.IgnoreDefaultValuesOnRead + arg = value == null && jsonParameterInfo.IgnoreNullTokensOnRead ? (TArg?)info.DefaultValue! // Use default value specified on parameter, if any. : value!; diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Object/ObjectWithParameterizedConstructorConverter.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Object/ObjectWithParameterizedConstructorConverter.cs index 4fa80afa6c8066..95809800844089 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Object/ObjectWithParameterizedConstructorConverter.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Object/ObjectWithParameterizedConstructorConverter.cs @@ -287,7 +287,7 @@ private void ReadConstructorArguments(ref ReadStack state, ref Utf8JsonReader re out _, createExtensionProperty: false); - if (jsonPropertyInfo.CanDeserialize) + if (jsonPropertyInfo.HasSetter) { ArgumentState argumentState = state.Current.CtorArgumentState!; @@ -452,7 +452,7 @@ private static bool HandlePropertyWithContinuation( { if (state.Current.PropertyState < StackFramePropertyState.ReadValue) { - if (!jsonPropertyInfo.CanDeserialize) + if (!jsonPropertyInfo.HasSetter) { if (!reader.TrySkip()) { @@ -486,7 +486,7 @@ private static bool HandlePropertyWithContinuation( } } - Debug.Assert(jsonPropertyInfo.CanDeserialize); + Debug.Assert(jsonPropertyInfo.HasSetter); // Ensure that the cache has enough capacity to add this property. diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonParameterInfo.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonParameterInfo.cs index 76866852d2bc5d..e5342b24c0791c 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonParameterInfo.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonParameterInfo.cs @@ -20,7 +20,7 @@ internal abstract class JsonParameterInfo // The default value of the parameter. This is `DefaultValue` of the `ParameterInfo`, if specified, or the CLR `default` for the `ParameterType`. public object? DefaultValue { get; private protected set; } - public bool IgnoreDefaultValuesOnRead { get; private set; } + public bool IgnoreNullTokensOnRead { get; private set; } // Options can be referenced here since all JsonPropertyInfos originate from a JsonTypeInfo that is cached on JsonSerializerOptions. public JsonSerializerOptions? Options { get; set; } // initialized in Init method @@ -62,7 +62,7 @@ public virtual void Initialize(JsonParameterInfoValues parameterInfo, JsonProper PropertyType = matchingProperty.PropertyType; NameAsUtf8Bytes = matchingProperty.NameAsUtf8Bytes!; ConverterBase = matchingProperty.EffectiveConverter; - IgnoreDefaultValuesOnRead = matchingProperty.IgnoreDefaultValuesOnRead; + IgnoreNullTokensOnRead = matchingProperty.IgnoreNullTokensOnRead; NumberHandling = matchingProperty.EffectiveNumberHandling; MatchingPropertyCanBeNull = matchingProperty.PropertyTypeCanBeNull; } diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonPropertyInfo.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonPropertyInfo.cs index 70ab27004493cc..04f78e1baffc80 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonPropertyInfo.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonPropertyInfo.cs @@ -53,6 +53,7 @@ public JsonConverter? CustomConverter set { VerifyMutable(); + DetermineEffectiveConverter(value); _customConverter = value; } } @@ -68,7 +69,11 @@ public JsonConverter? CustomConverter public Func? Get { get => _untypedGet; - set => SetGetter(value); + set + { + VerifyMutable(); + SetGetter(value); + } } /// @@ -80,11 +85,17 @@ public JsonConverter? CustomConverter public Action? Set { get => _untypedSet; - set => SetSetter(value); + set + { + VerifyMutable(); + SetSetter(value); + _isUserSpecifiedSetter = true; + } } private protected Func? _untypedGet; private protected Action? _untypedSet; + private bool _isUserSpecifiedSetter; private protected abstract void SetGetter(Delegate? getter); private protected abstract void SetSetter(Delegate? setter); @@ -106,32 +117,28 @@ public JsonConverter? CustomConverter set { VerifyMutable(); - _shouldSerialize = value; - // By default we will go through faster path (not using delegate) and use IgnoreCondition - // If user sets it explicitly we always go through delegate - _ignoreCondition = null; - _shouldSerializeIsExplicitlySet = true; + SetShouldSerialize(value); + // Invalidate any JsonIgnore configuration if delegate set manually by user + IgnoreDefaultValuesOnWrite = false; } } + private protected Func? _shouldSerialize; + private protected abstract void SetShouldSerialize(Delegate? predicate); + internal JsonIgnoreCondition? IgnoreCondition { get => _ignoreCondition; set { Debug.Assert(!_isConfigured); - + ConfigureIgnoreCondition(value); _ignoreCondition = value; - _shouldSerialize = value != null ? GetShouldSerializeForIgnoreCondition(value.Value) : null; - _shouldSerializeIsExplicitlySet = false; } } - private Func? _shouldSerialize; - private bool _shouldSerializeIsExplicitlySet; private JsonIgnoreCondition? _ignoreCondition; - - private protected abstract Func GetShouldSerializeForIgnoreCondition(JsonIgnoreCondition condition); + private protected abstract void ConfigureIgnoreCondition(JsonIgnoreCondition? ignoreCondition); /// /// Gets or sets a custom attribute provider for the current property. @@ -199,8 +206,8 @@ internal static JsonPropertyInfo GetPropertyPlaceholder() JsonPropertyInfo info = new JsonPropertyInfo(typeof(object), declaringTypeInfo: null, options: null!); Debug.Assert(!info.IsForTypeInfo); - Debug.Assert(!info.CanDeserialize); - Debug.Assert(!info.CanSerialize); + Debug.Assert(!info.HasSetter); + Debug.Assert(!info.HasGetter); info.Name = string.Empty; @@ -220,6 +227,7 @@ private protected void VerifyMutable() } } + internal bool IsConfigured => _isConfigured; private volatile bool _isConfigured; internal void EnsureConfigured() @@ -246,13 +254,12 @@ internal void Configure() CacheNameAsUtf8BytesAndEscapedNameSection(); } - if (IsIgnored) + if (EffectiveConverter is null) { - return; + Debug.Assert(CustomConverter is null); + DetermineEffectiveConverter(customConverter: null); } - DetermineEffectiveConverter(); - if (IsForTypeInfo) { DetermineNumberHandlingForTypeInfo(); @@ -260,17 +267,11 @@ internal void Configure() else { DetermineNumberHandlingForProperty(); - - if (!IsIgnored) - { - DetermineIgnoreCondition(IgnoreCondition); - } - - DetermineSerializationCapabilities(IgnoreCondition); + DetermineIgnoreCondition(); } } - private protected abstract void DetermineEffectiveConverter(); + private protected abstract void DetermineEffectiveConverter(JsonConverter? customConverter); private protected abstract void DetermineMemberAccessors(MemberInfo memberInfo); private void DeterminePoliciesFromMember(MemberInfo memberInfo) @@ -315,116 +316,36 @@ internal void CacheNameAsUtf8BytesAndEscapedNameSection() EscapedNameSection = JsonHelpers.GetEscapedPropertyNameSection(NameAsUtf8Bytes, Options.Encoder); } - internal void DetermineSerializationCapabilities(JsonIgnoreCondition? ignoreCondition) + internal void DetermineIgnoreCondition() { - if (IsIgnored) + if (_ignoreCondition != null) { - CanSerialize = false; - CanDeserialize = false; + // Do not apply global policy if already configured on the property level. return; } - Debug.Assert(MemberType == MemberTypes.Property || MemberType == MemberTypes.Field || MemberType == default); - - if ((ConverterStrategy & (ConverterStrategy.Enumerable | ConverterStrategy.Dictionary)) == 0) - { - Debug.Assert(ignoreCondition != JsonIgnoreCondition.Always); - - // Three possible values for ignoreCondition: - // null = JsonIgnore was not placed on this property, global IgnoreReadOnlyProperties/Fields wins - // WhenNull = only ignore when null, global IgnoreReadOnlyProperties/Fields loses - // Never = never ignore (always include), global IgnoreReadOnlyProperties/Fields loses - bool serializeReadOnlyProperty = ignoreCondition != null || (MemberType == MemberTypes.Property - ? !Options.IgnoreReadOnlyProperties - : !Options.IgnoreReadOnlyFields); - - // We serialize if there is a getter + not ignoring readonly properties. - CanSerialize = HasGetter && (HasSetter || serializeReadOnlyProperty || _shouldSerializeIsExplicitlySet); - - // We deserialize if there is a setter. - CanDeserialize = HasSetter; - } - else - { - if (HasGetter) - { - Debug.Assert(EffectiveConverter != null); - - CanSerialize = true; - - if (HasSetter) - { - CanDeserialize = true; - } - } - } - } - - internal void DetermineIgnoreCondition(JsonIgnoreCondition? ignoreCondition) - { - if (_shouldSerializeIsExplicitlySet) - { - Debug.Assert(ignoreCondition == null); #pragma warning disable SYSLIB0020 // JsonSerializerOptions.IgnoreNullValues is obsolete - if (Options.IgnoreNullValues) + if (Options.IgnoreNullValues) #pragma warning restore SYSLIB0020 - { - Debug.Assert(Options.DefaultIgnoreCondition == JsonIgnoreCondition.Never); - if (PropertyTypeCanBeNull) - { - IgnoreDefaultValuesOnRead = true; - } - } - - return; - } - - if (ignoreCondition != null) - { - // This is not true for CodeGen scenarios since we do not cache this as of yet. - // Debug.Assert(MemberInfo != null); - Debug.Assert(ignoreCondition != JsonIgnoreCondition.Always); - - if (ignoreCondition == JsonIgnoreCondition.WhenWritingDefault) - { - IgnoreDefaultValuesOnWrite = true; - } - else if (ignoreCondition == JsonIgnoreCondition.WhenWritingNull) - { - if (PropertyTypeCanBeNull) - { - IgnoreDefaultValuesOnWrite = true; - } - else - { - ThrowHelper.ThrowInvalidOperationException_IgnoreConditionOnValueTypeInvalid(MemberName!, DeclaringType); - } - } - } -#pragma warning disable SYSLIB0020 // JsonSerializerOptions.IgnoreNullValues is obsolete - else if (Options.IgnoreNullValues) { Debug.Assert(Options.DefaultIgnoreCondition == JsonIgnoreCondition.Never); if (PropertyTypeCanBeNull) { - IgnoreDefaultValuesOnRead = true; - IgnoreDefaultValuesOnWrite = true; + IgnoreNullTokensOnRead = !_isUserSpecifiedSetter; + IgnoreDefaultValuesOnWrite = ShouldSerialize is null; } } else if (Options.DefaultIgnoreCondition == JsonIgnoreCondition.WhenWritingNull) { - Debug.Assert(!Options.IgnoreNullValues); if (PropertyTypeCanBeNull) { - IgnoreDefaultValuesOnWrite = true; + IgnoreDefaultValuesOnWrite = ShouldSerialize is null; } } else if (Options.DefaultIgnoreCondition == JsonIgnoreCondition.WhenWritingDefault) { - Debug.Assert(!Options.IgnoreNullValues); - IgnoreDefaultValuesOnWrite = true; + IgnoreDefaultValuesOnWrite = ShouldSerialize is null; } -#pragma warning restore SYSLIB0020 } internal void DetermineNumberHandlingForTypeInfo() @@ -524,8 +445,8 @@ internal string GetDebugInfo(int indent = 0) sb.AppendLine($"{ind} NameAsUtf8.Length: {(NameAsUtf8Bytes?.Length ?? -1)},"); sb.AppendLine($"{ind} IsConfigured: {_isConfigured},"); sb.AppendLine($"{ind} IsIgnored: {IsIgnored},"); - sb.AppendLine($"{ind} CanSerialize: {CanSerialize},"); - sb.AppendLine($"{ind} CanDeserialize: {CanDeserialize},"); + sb.AppendLine($"{ind} HasGetter: {HasGetter},"); + sb.AppendLine($"{ind} HasSetter: {HasSetter},"); sb.AppendLine($"{ind}}}"); return sb.ToString(); @@ -535,7 +456,7 @@ internal string GetDebugInfo(int indent = 0) internal bool HasGetter => _untypedGet is not null; internal bool HasSetter => _untypedSet is not null; - internal void InitializeUsingMemberReflection(MemberInfo memberInfo) + internal void InitializeUsingMemberReflection(MemberInfo memberInfo, JsonConverter? customConverter, JsonIgnoreCondition? ignoreCondition) { Debug.Assert(AttributeProvider == null); @@ -559,19 +480,37 @@ internal void InitializeUsingMemberReflection(MemberInfo memberInfo) break; } + CustomConverter = customConverter; DeterminePoliciesFromMember(memberInfo); DeterminePropertyNameFromMember(memberInfo); - if (!IsIgnored) + if (ignoreCondition != JsonIgnoreCondition.Always) { DetermineMemberAccessors(memberInfo); } + // NB setting the ignore condition must follow converter & getter/setter + // configuration in order for access policies to be applied correctly. + IgnoreCondition = ignoreCondition; IsExtensionData = memberInfo.GetCustomAttribute(inherit: false) != null; } - internal bool IgnoreDefaultValuesOnRead { get; private set; } - internal bool IgnoreDefaultValuesOnWrite { get; private set; } + internal bool IgnoreNullTokensOnRead { get; private protected set; } + internal bool IgnoreDefaultValuesOnWrite { get; private protected set; } + + internal bool IgnoreReadOnlyMember + { + get + { + Debug.Assert(MemberType == MemberTypes.Property || MemberType == MemberTypes.Field || MemberType == default); + return MemberType switch + { + MemberTypes.Property => Options.IgnoreReadOnlyProperties, + MemberTypes.Field => Options.IgnoreReadOnlyFields, + _ => false, + }; + } + } /// /// True if the corresponding cref="JsonTypeInfo.PropertyInfoForTypeInfo"/> is this instance. @@ -773,10 +712,6 @@ internal JsonTypeInfo JsonTypeInfo internal abstract void SetExtensionDictionaryAsObject(object obj, object? extensionDict); - internal bool CanSerialize { get; private set; } - - internal bool CanDeserialize { get; private set; } - internal bool IsIgnored => _ignoreCondition == JsonIgnoreCondition.Always; /// diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonPropertyInfoOfT.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonPropertyInfoOfT.cs index faa58bd81fc1e0..3f9497b8f56535 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonPropertyInfoOfT.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonPropertyInfoOfT.cs @@ -37,8 +37,7 @@ internal JsonPropertyInfo(Type declaringType, JsonTypeInfo? declaringTypeInfo, J private protected override void SetGetter(Delegate? getter) { Debug.Assert(getter is null or Func or Func); - - VerifyMutable(); + Debug.Assert(!IsConfigured); if (getter is null) { @@ -61,8 +60,7 @@ private protected override void SetGetter(Delegate? getter) private protected override void SetSetter(Delegate? setter) { Debug.Assert(setter is null or Action or Action); - - VerifyMutable(); + Debug.Assert(!IsConfigured); if (setter is null) { @@ -82,6 +80,37 @@ private protected override void SetSetter(Delegate? setter) } } + internal new Func? ShouldSerialize + { + get => _shouldSerializeTyped; + set => SetShouldSerialize(value); + } + + private Func? _shouldSerializeTyped; + + private protected override void SetShouldSerialize(Delegate? predicate) + { + Debug.Assert(predicate is null or Func or Func); + Debug.Assert(!IsConfigured); + + if (predicate is null) + { + _shouldSerializeTyped = null; + _shouldSerialize = null; + } + else if (predicate is Func typedPredicate) + { + _shouldSerializeTyped = typedPredicate; + _shouldSerialize = typedPredicate is Func untypedPredicate ? untypedPredicate : (obj, value) => typedPredicate(obj, (T?)value); + } + else + { + Func untypedPredicate = (Func)predicate; + _shouldSerializeTyped = (obj, value) => untypedPredicate(obj, value); + _shouldSerialize = untypedPredicate; + } + } + internal override object? DefaultValue => default(T); internal override bool PropertyTypeCanBeNull => default(T) is null; @@ -94,43 +123,37 @@ private protected override void DetermineMemberAccessors(MemberInfo memberInfo) switch (memberInfo) { case PropertyInfo propertyInfo: - { - bool useNonPublicAccessors = propertyInfo.GetCustomAttribute(inherit: false) != null; - - MethodInfo? getMethod = propertyInfo.GetMethod; - if (getMethod != null && (getMethod.IsPublic || useNonPublicAccessors)) - { - Get = Options.MemberAccessorStrategy.CreatePropertyGetter(propertyInfo); - } - - MethodInfo? setMethod = propertyInfo.SetMethod; - if (setMethod != null && (setMethod.IsPublic || useNonPublicAccessors)) - { - Set = Options.MemberAccessorStrategy.CreatePropertySetter(propertyInfo); - } + bool useNonPublicAccessors = propertyInfo.GetCustomAttribute(inherit: false) != null; - break; + MethodInfo? getMethod = propertyInfo.GetMethod; + if (getMethod != null && (getMethod.IsPublic || useNonPublicAccessors)) + { + Get = Options.MemberAccessorStrategy.CreatePropertyGetter(propertyInfo); } - case FieldInfo fieldInfo: + MethodInfo? setMethod = propertyInfo.SetMethod; + if (setMethod != null && (setMethod.IsPublic || useNonPublicAccessors)) { - Debug.Assert(fieldInfo.IsPublic); + Set = Options.MemberAccessorStrategy.CreatePropertySetter(propertyInfo); + } - Get = Options.MemberAccessorStrategy.CreateFieldGetter(fieldInfo); + break; - if (!fieldInfo.IsInitOnly) - { - Set = Options.MemberAccessorStrategy.CreateFieldSetter(fieldInfo); - } + case FieldInfo fieldInfo: + Debug.Assert(fieldInfo.IsPublic); - break; - } + Get = Options.MemberAccessorStrategy.CreateFieldGetter(fieldInfo); - default: + if (!fieldInfo.IsInitOnly) { - Debug.Fail($"Invalid MemberInfo type: {memberInfo.MemberType}"); - break; + Set = Options.MemberAccessorStrategy.CreateFieldSetter(fieldInfo); } + + break; + + default: + Debug.Fail($"Invalid MemberInfo type: {memberInfo.MemberType}"); + break; } } @@ -168,20 +191,21 @@ internal JsonPropertyInfo(JsonPropertyInfoValues propertyInfo, JsonSerializer DefaultConverterForType = propertyInfo.PropertyTypeInfo?.Converter as JsonConverter; CustomConverter = propertyInfo.Converter; - IgnoreCondition = propertyInfo.IgnoreCondition; - if (IgnoreCondition != JsonIgnoreCondition.Always) + if (propertyInfo.IgnoreCondition != JsonIgnoreCondition.Always) { Get = propertyInfo.Getter!; Set = propertyInfo.Setter; } + // NB setting the ignore condition must follow converter & getter/setter + // configuration in order for access policies to be applied correctly. + IgnoreCondition = propertyInfo.IgnoreCondition; JsonTypeInfo = propertyInfo.PropertyTypeInfo; NumberHandling = propertyInfo.NumberHandling; } - private protected override void DetermineEffectiveConverter() + private protected override void DetermineEffectiveConverter(JsonConverter? customConverter) { - JsonConverter? customConverter = CustomConverter; if (customConverter != null) { customConverter = Options.ExpandConverterFactory(customConverter, PropertyType); @@ -229,23 +253,13 @@ value is not null && if (IgnoreDefaultValuesOnWrite) { - if (default(T) is null) + // Fast path `ShouldSerialize` check when using JsonIgnoreCondition.WhenWritingNull/Default configuration + if (IsDefaultValue(value)) { - if (value is null) - { - return true; - } - } - else - { - if (EqualityComparer.Default.Equals(default, value)) - { - return true; - } + return true; } } - - if (ShouldSerialize?.Invoke(obj, value) == false) + else if (ShouldSerialize?.Invoke(obj, value) == false) { // We return true here. // False means that there is not enough data. @@ -326,7 +340,7 @@ internal override bool ReadJsonAndSetMember(object obj, ref ReadStack state, ref ThrowHelper.ThrowJsonException_DeserializeUnableToConvertValue(TypedEffectiveConverter.TypeToConvert); } - if (!IgnoreDefaultValuesOnRead) + if (!IgnoreNullTokensOnRead) { T? value = default; Set!(obj, value!); @@ -339,7 +353,7 @@ internal override bool ReadJsonAndSetMember(object obj, ref ReadStack state, ref // CanUseDirectReadOrWrite == false when using streams Debug.Assert(!state.IsContinuation); - if (!isNullToken || !IgnoreDefaultValuesOnRead || default(T) is not null) + if (!isNullToken || !IgnoreNullTokensOnRead || default(T) is not null) { // Optimize for internal converters by avoiding the extra call to TryRead. T? fastValue = TypedEffectiveConverter.Read(ref reader, PropertyType, Options); @@ -351,7 +365,7 @@ internal override bool ReadJsonAndSetMember(object obj, ref ReadStack state, ref else { success = true; - if (!isNullToken || !IgnoreDefaultValuesOnRead || default(T) is not null || state.IsContinuation) + if (!isNullToken || !IgnoreNullTokensOnRead || default(T) is not null || state.IsContinuation) { success = TypedEffectiveConverter.TryRead(ref reader, PropertyType, Options, ref state, out T? value); if (success) @@ -406,40 +420,91 @@ internal override void SetExtensionDictionaryAsObject(object obj, object? extens Set!(obj, typedValue); } - private protected override Func GetShouldSerializeForIgnoreCondition(JsonIgnoreCondition ignoreCondition) + private protected override void ConfigureIgnoreCondition(JsonIgnoreCondition? ignoreCondition) { + DetermineSerializationCapabilities(ignoreCondition); + switch (ignoreCondition) { - case JsonIgnoreCondition.Always: return ShouldSerializeIgnoreConditionAlways; - case JsonIgnoreCondition.Never: return ShouldSerializeIgnoreConditionNever; + case null: + break; + + case JsonIgnoreCondition.Never: + ShouldSerialize = ShouldSerializeIgnoreConditionNever; + break; + + case JsonIgnoreCondition.Always: + ShouldSerialize = ShouldSerializeIgnoreConditionAlways; + break; + case JsonIgnoreCondition.WhenWritingNull: - if (!PropertyTypeCanBeNull) + if (PropertyTypeCanBeNull) { - return ShouldSerializeIgnoreConditionNever; + ShouldSerialize = ShouldSerializeIgnoreWhenWritingDefault; + IgnoreDefaultValuesOnWrite = true; } - - goto case JsonIgnoreCondition.WhenWritingDefault; - case JsonIgnoreCondition.WhenWritingDefault: + else { - return ShouldSerializeIgnoreConditionWhenWritingDefaultPropertyTypeEqualsTypeToConvert; + ThrowHelper.ThrowInvalidOperationException_IgnoreConditionOnValueTypeInvalid(MemberName!, DeclaringType); } + break; + + case JsonIgnoreCondition.WhenWritingDefault: + ShouldSerialize = ShouldSerializeIgnoreWhenWritingDefault; + IgnoreDefaultValuesOnWrite = true; + break; + default: Debug.Fail($"Unknown value of JsonIgnoreCondition '{ignoreCondition}'"); - return null!; + break; + } + + static bool ShouldSerializeIgnoreConditionNever(object _, T? value) => true; + static bool ShouldSerializeIgnoreConditionAlways(object _, T? value) => false; + static bool ShouldSerializeIgnoreWhenWritingDefault(object _, T? value) + { + return default(T) is null ? value is not null : !EqualityComparer.Default.Equals(default, value); } } - private static bool ShouldSerializeIgnoreConditionAlways(object obj, object? value) => false; - private static bool ShouldSerializeIgnoreConditionNever(object obj, object? value) => true; - private static bool ShouldSerializeIgnoreConditionWhenWritingDefaultPropertyTypeEqualsTypeToConvert(object obj, object? value) + private static bool IsDefaultValue(T? value) { - if (value == null) + return default(T) is null ? value is null : EqualityComparer.Default.Equals(default, value); + } + + private void DetermineSerializationCapabilities(JsonIgnoreCondition? ignoreCondition) + { + if (ignoreCondition == JsonIgnoreCondition.Always) { - return false; + Debug.Assert(Get == null && Set == null, "Getters or Setters should not be set when JsonIgnoreCondition.Always is specified."); + return; } - T typedValue = (T)value; - return !EqualityComparer.Default.Equals(default, typedValue); + Debug.Assert(TypedEffectiveConverter != null, "Must have calculated the effective converter strategy."); + if ((ConverterStrategy & (ConverterStrategy.Enumerable | ConverterStrategy.Dictionary)) == 0) + { + // We serialize if there is a getter + not ignoring readonly properties. + if (Get != null && Set == null) + { + // Three possible values for ignoreCondition: + // null = JsonIgnore was not placed on this property, global IgnoreReadOnlyProperties/Fields wins + // WhenNull = only ignore when null, global IgnoreReadOnlyProperties/Fields loses + // Never = never ignore (always include), global IgnoreReadOnlyProperties/Fields loses + bool serializeReadOnlyProperty = ignoreCondition != null || !IgnoreReadOnlyMember; + if (!serializeReadOnlyProperty) + { + Get = null; + } + } + } + else + { + if (Get == null && Set != null) + { + // Collection properties with setters only are not supported. + Set = null; + } + } } } } diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/ReflectionJsonTypeInfoOfT.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/ReflectionJsonTypeInfoOfT.cs index a0efc0629a26d4..f7b91f3127fc15 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/ReflectionJsonTypeInfoOfT.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/ReflectionJsonTypeInfoOfT.cs @@ -209,9 +209,7 @@ private void CacheMember( } JsonPropertyInfo jsonPropertyInfo = CreatePropertyUsingReflection(typeToConvert, converter); - jsonPropertyInfo.IgnoreCondition = ignoreCondition; - jsonPropertyInfo.CustomConverter = customConverter; - jsonPropertyInfo.InitializeUsingMemberReflection(memberInfo); + jsonPropertyInfo.InitializeUsingMemberReflection(memberInfo, customConverter, ignoreCondition); return jsonPropertyInfo; } diff --git a/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/MetadataTests/DefaultJsonTypeInfoResolverTests.JsonPropertyInfo.cs b/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/MetadataTests/DefaultJsonTypeInfoResolverTests.JsonPropertyInfo.cs index 3642e9459f6571..c45b2a2e217dd0 100644 --- a/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/MetadataTests/DefaultJsonTypeInfoResolverTests.JsonPropertyInfo.cs +++ b/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/MetadataTests/DefaultJsonTypeInfoResolverTests.JsonPropertyInfo.cs @@ -1019,17 +1019,19 @@ public static void JsonIgnoreConditionIsCorrectlyTranslatedToShouldSerializeDele static void TestJsonIgnoreConditionDelegate(JsonIgnoreCondition defaultIgnoreCondition, JsonIgnoreCondition? ignoreConditionOnProperty, JsonPropertyInfo property, ModifyJsonIgnore modify) { - // defaultIgnoreCondition is not taken into accound, we might expect null if defaultIgnoreCondition == ignoreConditionOnProperty + // defaultIgnoreCondition is not taken into account, we might expect null if defaultIgnoreCondition == ignoreConditionOnProperty switch (ignoreConditionOnProperty) { case null: Assert.Null(property.ShouldSerialize); break; + case JsonIgnoreCondition.Always: Assert.NotNull(property.ShouldSerialize); Assert.False(property.ShouldSerialize(null, null)); Assert.False(property.ShouldSerialize(null, "")); Assert.False(property.ShouldSerialize(null, "asd")); + Assert.Throws(() => property.ShouldSerialize(null, 0)); Assert.Null(property.Get); Assert.Null(property.Set); @@ -1039,18 +1041,22 @@ static void TestJsonIgnoreConditionDelegate(JsonIgnoreCondition defaultIgnoreCon Assert.False(property.ShouldSerialize(null, 0)); Assert.True(property.ShouldSerialize(null, 1)); Assert.True(property.ShouldSerialize(null, -1)); + Assert.Throws(() => property.ShouldSerialize(null, null)); + Assert.Throws(() => property.ShouldSerialize(null, "string")); break; case JsonIgnoreCondition.WhenWritingNull: Assert.NotNull(property.ShouldSerialize); Assert.False(property.ShouldSerialize(null, null)); Assert.True(property.ShouldSerialize(null, "")); Assert.True(property.ShouldSerialize(null, "asd")); + Assert.Throws(() => property.ShouldSerialize(null, 0)); break; case JsonIgnoreCondition.Never: Assert.NotNull(property.ShouldSerialize); Assert.True(property.ShouldSerialize(null, null)); Assert.True(property.ShouldSerialize(null, "")); Assert.True(property.ShouldSerialize(null, "asd")); + Assert.Throws(() => property.ShouldSerialize(null, 0)); break; } @@ -1444,5 +1450,248 @@ public class ClassWithFieldsAndProperties public string Field; public int Property { get; set; } } + + [Fact] + public static void CanEnableDeserializationOnAlwaysIgnoreProperty() + { + var options = new JsonSerializerOptions + { + TypeInfoResolver = new DefaultJsonTypeInfoResolver + { + Modifiers = + { + jsonTypeInfo => + { + if (jsonTypeInfo.Type != typeof(ClassWithJsonIgnoreAlwaysProperty)) + { + return; + } + + Assert.Equal(1, jsonTypeInfo.Properties.Count); + JsonPropertyInfo jsonPropertyInfo = jsonTypeInfo.Properties[0]; + + Assert.NotNull(jsonPropertyInfo.ShouldSerialize); + Assert.Null(jsonPropertyInfo.Get); + Assert.Null(jsonPropertyInfo.Set); + + jsonPropertyInfo.Set = (obj, value) => ((ClassWithJsonIgnoreAlwaysProperty)obj).Value = (int)value; + } + } + } + }; + + ClassWithJsonIgnoreAlwaysProperty value = JsonSerializer.Deserialize("""{"Value":42}""", options); + Assert.Equal(42, value.Value); + } + + public class ClassWithJsonIgnoreAlwaysProperty + { + [JsonIgnore] + public int Value { get; set; } + } + + [Theory] + [MemberData(nameof(ClassWithSetterOnlyCollectionProperty_SupportedWhenSetManually_MemberData))] + public static void ClassWithSetterOnlyCollectionProperty_SupportedWhenSetManually(T value, string json) + { + _ = value; // parameter not used + + string fullJson = $@"{{""Value"":{json}}}"; + + // sanity check: deserialization is not supported using the default contract + ClassWithSetterOnlyProperty result = JsonSerializer.Deserialize>(fullJson); + Assert.Null(result.GetValue()); + + var options = new JsonSerializerOptions + { + TypeInfoResolver = new DefaultJsonTypeInfoResolver + { + Modifiers = + { + jsonTypeInfo => + { + if (jsonTypeInfo.Type != typeof(ClassWithSetterOnlyProperty)) + { + return; + } + + Assert.Equal(1, jsonTypeInfo.Properties.Count); + + JsonPropertyInfo jpi = jsonTypeInfo.Properties[0]; + Assert.Null(jpi.Set); + jpi.Set = (obj, value) => ((ClassWithSetterOnlyProperty)obj)._value = (T)value; + } + } + } + }; + + result = JsonSerializer.Deserialize>(fullJson, options); + Assert.NotNull(result.GetValue()); + } + + public static IEnumerable ClassWithSetterOnlyCollectionProperty_SupportedWhenSetManually_MemberData() + { + yield return Wrap(new List(), "[1,2,3]"); + yield return Wrap(new Dictionary(), """{"key":"value"}"""); + + static object[] Wrap(T value, string json) => new object[] { value, json }; + } + + public class ClassWithSetterOnlyProperty + { + internal T _value; + public T Value { set => _value = value; } + public T GetValue() => _value; + } + + [Fact] + public static void ClassWithReadOnlyMembers_SupportedWhenSetManually() + { + var value = new ClassWithReadOnlyMembers(); + + // Sanity check -- default contract does not support serialization + var options = new JsonSerializerOptions { IgnoreReadOnlyFields = true, IgnoreReadOnlyProperties = true }; + string json = JsonSerializer.Serialize(value, options); + Assert.Equal("{}", json); + + // Use a constract resolver that updates getters manually + options = new JsonSerializerOptions + { + IgnoreReadOnlyFields = true, + IgnoreReadOnlyProperties = true, + + TypeInfoResolver = new DefaultJsonTypeInfoResolver + { + Modifiers = + { + jsonTypeInfo => + { + if (jsonTypeInfo.Type != typeof(ClassWithReadOnlyMembers)) + { + return; + } + + Assert.Equal(2, jsonTypeInfo.Properties.Count); + + JsonPropertyInfo jpi = jsonTypeInfo.Properties[0]; + Assert.Equal("Field", jpi.Name); // JsonPropertyOrder should ensure correct ordering of properties. + Assert.Null(jpi.Get); + jpi.Get = obj => ((ClassWithReadOnlyMembers)obj).Field; + + jpi = jsonTypeInfo.Properties[1]; + Assert.Equal("Property", jpi.Name); // JsonPropertyOrder should ensure correct ordering of properties. + Assert.Null(jpi.Get); + jpi.Get = obj => ((ClassWithReadOnlyMembers)obj).Property; + } + } + } + }; + + json = JsonSerializer.Serialize(value, options); + Assert.Equal("""{"Field":1,"Property":2}""", json); + } + + public class ClassWithReadOnlyMembers + { + [JsonInclude, JsonPropertyOrder(0)] + public readonly int Field = 1; + [JsonPropertyOrder(1)] + public int Property => 2; + } + + [Fact] + public static void IgnoreNullValues_Serialization_CanBeInvalidatedByCustomContracts() + { + var value = new ClassWithNullableProperty(); + + // Sanity check -- property is ignored using default contract + var options = new JsonSerializerOptions { IgnoreNullValues = true }; + string json = JsonSerializer.Serialize(value, options); + Assert.Equal("{}", json); + + // Property can be re-enabled using a custom contract + options = new JsonSerializerOptions + { + IgnoreNullValues = true, + TypeInfoResolver = new DefaultJsonTypeInfoResolver + { + Modifiers = + { + jsonTypeInfo => + { + if (jsonTypeInfo.Type != typeof(ClassWithNullableProperty)) + { + return; + } + + Assert.Equal(1, jsonTypeInfo.Properties.Count); + + JsonPropertyInfo jpi = jsonTypeInfo.Properties[0]; + Assert.Null(jpi.ShouldSerialize); + Assert.NotNull(jpi.Get); + + // Because null `ShouldSerialize` gets overridden by + // `DefaultIgnoreCondition`/`IgnoreNullValues` post-configuration, + // we need to explicitly set a delegate that always returns true. + // This is a design quirk we might want to consider changing. + jpi.ShouldSerialize = (_,value) => true; + } + } + } + }; + + json = JsonSerializer.Serialize(value, options); + Assert.Equal("""{"Value":null}""", json); + } + + [Fact] + public static void IgnoreNullValues_Deserialization_CanBeInvalidatedByCustomContracts() + { + string json = """{"Value":null}"""; + + // Sanity check -- property is ignored using default contract + var options = new JsonSerializerOptions { IgnoreNullValues = true }; + ClassWithNullableProperty value = JsonSerializer.Deserialize(json, options); + Assert.Null(value.Value); + + // Property can be re-enabled using a custom contract + options = new JsonSerializerOptions + { + IgnoreNullValues = true, + TypeInfoResolver = new DefaultJsonTypeInfoResolver + { + Modifiers = + { + jsonTypeInfo => + { + if (jsonTypeInfo.Type != typeof(ClassWithNullableProperty)) + { + return; + } + + Assert.Equal(1, jsonTypeInfo.Properties.Count); + + JsonPropertyInfo jpi = jsonTypeInfo.Properties[0]; + Assert.NotNull(jpi.Set); + jpi.Set = (obj, value) => ((ClassWithNullableProperty)obj).Value = (string)value; + } + } + } + }; + + value = JsonSerializer.Deserialize(json, options); + Assert.Equal("NULL", value.Value); + } + + public class ClassWithNullableProperty + { + private string? _value; + + public string? Value + { + get => _value; + set => _value = value ?? "NULL"; + } + } } }