Skip to content

Ignore overridden properties in JSON (de)serialization #52036

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

Closed
devsko wants to merge 4 commits into from
Closed

Ignore overridden properties in JSON (de)serialization #52036

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
d6d7e77
fields cannot be virtual
devsko Apr 26, 2021
ad23d17
introduce dictionary overrideProperties
devsko Apr 28, 2021
a6f8943
check overridden properties
devsko Apr 29, 2021
5b26a2d
merge main
devsko Apr 29, 2021
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
Prev Previous commit
Next Next commit
introduce dictionary overrideProperties
  • Loading branch information
@devsko
devsko committed Apr 28, 2021
commit ad23d170f20691e89c8c5d4eff3514f4c14556c2
51 changes: 45 additions & 6 deletions src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfo.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,7 @@ internal JsonTypeInfo(Type type, JsonConverter converter, Type runtimeType, Json
: StringComparer.Ordinal);

Dictionary<string, MemberInfo>? ignoredMembers = null;
Dictionary<string, PropertyInfo>? overrideProperties = null;

// We start from the most derived type.
for (Type? currentType = type; currentType != null; currentType = currentType.BaseType)
Expand All @@ -216,7 +217,7 @@ internal JsonTypeInfo(Type type, JsonConverter converter, Type runtimeType, Json
if (propertyInfo.GetMethod?.IsPublic == true ||
propertyInfo.SetMethod?.IsPublic == true)
{
CacheMember(currentType, propertyInfo.PropertyType, propertyInfo, typeNumberHandling, cache, ref ignoredMembers);
CacheMember(currentType, propertyInfo.PropertyType, propertyInfo, typeNumberHandling, cache, ref ignoredMembers, ref overrideProperties);
}
else
{
Expand All @@ -237,7 +238,7 @@ internal JsonTypeInfo(Type type, JsonConverter converter, Type runtimeType, Json
{
if (hasJsonInclude || Options.IncludeFields)
{
CacheMember(currentType, fieldInfo.FieldType, fieldInfo, typeNumberHandling, cache, ref ignoredMembers);
CacheMember(currentType, fieldInfo.FieldType, fieldInfo, typeNumberHandling, cache, ref ignoredMembers, ref overrideProperties);
}
}
else
Expand Down Expand Up @@ -319,12 +320,14 @@ private void CacheMember(
MemberInfo memberInfo,
JsonNumberHandling? typeNumberHandling,
Dictionary<string, JsonPropertyInfo> cache,
ref Dictionary<string, MemberInfo>? ignoredMembers)
ref Dictionary<string, MemberInfo>? ignoredMembers,
ref Dictionary<string, PropertyInfo>? overrideProperties)
{
JsonPropertyInfo jsonPropertyInfo = AddProperty(memberInfo, memberType, declaringType, typeNumberHandling, Options);
Debug.Assert(jsonPropertyInfo.NameAsString != null);

string memberName = memberInfo.Name;
PropertyInfo? property = memberInfo as PropertyInfo;

// The JsonPropertyNameAttribute or naming policy resulted in a collision.
if (!JsonHelpers.TryAdd(cache, jsonPropertyInfo.NameAsString, jsonPropertyInfo, out JsonPropertyInfo? other))
Expand All @@ -340,7 +343,7 @@ private void CacheMember(
// Is the current property hidden by the previously cached property
// (with `new` keyword, or by overriding)?
other.MemberInfo!.Name != memberName &&
// Was a property with the same CLR name was ignored? That property hid the current property,
// Was a property with the same CLR name ignored? That property hid the current property,
// thus, if it was ignored, the current property should be ignored too.
ignoredMembers?.ContainsKey(memberName) != true)
{
Expand All @@ -349,11 +352,48 @@ private void CacheMember(
}
// Ignore the current property.
}
else if (overrideProperties != null &&
overrideProperties.TryGetValue(memberName, out PropertyInfo? overrideProperty) &&
property != null &&
IsOverrideOf(property, overrideProperty))
{
// This property is overridden in a derived class. Implicitly ignore it
// by removing from the cache, regardless of an applied JsonPropertyName attribute.
cache.Remove(jsonPropertyInfo.NameAsString);
}

if (jsonPropertyInfo.IsIgnored)
{
(ignoredMembers ??= new Dictionary<string, MemberInfo>()).Add(memberName, memberInfo);
}
else if (property != null && IsOverride(property))
{
(overrideProperties ??= new Dictionary<string, PropertyInfo>())[memberName] = property;
}
}

private static bool IsOverrideOf(PropertyInfo property, PropertyInfo other)
{
MethodInfo? getMethod = property.GetMethod;
if (getMethod != null)
{
return getMethod.GetBaseDefinition() == other.GetMethod?.GetBaseDefinition();
}

MethodInfo? setMethod = property.SetMethod;
return setMethod != null && setMethod.GetBaseDefinition() == other.SetMethod?.GetBaseDefinition();
}

private static bool IsOverride(PropertyInfo property)
{
MethodInfo? getMethod = property.GetMethod;
if (getMethod != null)
{
return getMethod.GetBaseDefinition() != getMethod;
}

MethodInfo? setMethod = property.SetMethod;
return setMethod != null && setMethod.GetBaseDefinition() == setMethod;
}

private sealed class ParameterLookupKey
Expand Down Expand Up @@ -473,8 +513,7 @@ private static bool PropertyIsOverridenAndIgnored(PropertyInfo currentProperty,
return false;
}

return currentProperty.GetMethod?.GetBaseDefinition() == ignoredProperty.GetMethod?.GetBaseDefinition() ||
currentProperty.SetMethod?.GetBaseDefinition() == ignoredProperty.SetMethod?.GetBaseDefinition();
return IsOverrideOf(currentProperty, ignoredProperty);
}

private bool DetermineExtensionDataProperty(Dictionary<string, JsonPropertyInfo> cache)
Expand Down
11 changes: 10 additions & 1 deletion ...es/System.Text.Json/tests/System.Text.Json.Tests/Serialization/PropertyVisibilityTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -637,7 +637,11 @@ public static void OverriddenPropertiesIgnored_WhenRenamed()
Assert.Equal(@"{""Renamed"":true}", serialized);

serialized = JsonSerializer.Serialize(new FurtherDerivedClass_WithRenamedOverride());
Assert.Equal(@"{""MyProp"":true}", serialized);
Assert.Equal(@"{""MyProp"":false}", serialized);

// Don't ignore virtual properties with same name and type when they are in a new slot.
serialized = JsonSerializer.Serialize(new FurtherDerivedClass_WithRenamedNewSlotVirtual());
Assert.Equal(@"{""MyProp"":false,""Renamed"":true}", serialized);
}

public class ClassWithInternalField
Expand Down Expand Up @@ -983,6 +987,11 @@ private class FurtherDerivedClass_WithRenamedOverride : DerivedClass_WithRenamed
public override bool MyProp { get; set; }
}

private class FurtherDerivedClass_WithRenamedNewSlotVirtual : DerivedClass_WithRenamedOverride
{
public new virtual bool MyProp { get; set; }
}

[Fact]
public static void IgnoreReadOnlyProperties()
{
Expand Down