Skip to content
This repository has been archived by the owner on Jan 23, 2023. It is now read-only.
/ corefx Public archive

Cache polymorphic properties #41753

Merged
merged 6 commits into from
Oct 21, 2019
Merged
Show file tree
Hide file tree
Changes from 3 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
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,11 @@
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

using System.Collections.Generic;
using System.Collections.Concurrent;
using System.Diagnostics;
using System.Reflection;
using System.Text.Json.Serialization;
using System.Threading;

namespace System.Text.Json
{
Expand Down Expand Up @@ -209,18 +210,27 @@ internal JsonPropertyInfo CreateRootObject(JsonSerializerOptions options)
options: options);
}

internal JsonPropertyInfo CreatePolymorphicProperty(JsonPropertyInfo property, Type runtimePropertyType, JsonSerializerOptions options)
internal JsonPropertyInfo GetOrAddPolymorphicProperty(JsonPropertyInfo property, Type runtimePropertyType, JsonSerializerOptions options)
{
JsonPropertyInfo runtimeProperty = CreateProperty(
property.DeclaredPropertyType, runtimePropertyType,
property.ImplementedPropertyType,
property.PropertyInfo,
parentClassType: Type,
converter: null,
options: options);
property.CopyRuntimeSettingsTo(runtimeProperty);
static JsonPropertyInfo CreateRuntimeProperty((JsonPropertyInfo property, Type runtimePropertyType) key, (JsonSerializerOptions options, Type classType) arg)
{
JsonPropertyInfo runtimeProperty = CreateProperty(
key.property.DeclaredPropertyType, key.runtimePropertyType,
key.property.ImplementedPropertyType,
key.property.PropertyInfo,
parentClassType: arg.classType,
converter: null,
options: arg.options);

key.property.CopyRuntimeSettingsTo(runtimeProperty);

return runtimeProperty;
}

ConcurrentDictionary<(JsonPropertyInfo, Type), JsonPropertyInfo> cache =
LazyInitializer.EnsureInitialized(ref RuntimePropertyCache, () => new ConcurrentDictionary<(JsonPropertyInfo, Type), JsonPropertyInfo>());

return runtimeProperty;
return cache.GetOrAdd((property, runtimePropertyType), (key, arg) => CreateRuntimeProperty(key, arg), (options, Type));
Copy link
Member

@ahsonkhan ahsonkhan Oct 17, 2019

Choose a reason for hiding this comment

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

Specify the generic explicitly?

Suggested change
return cache.GetOrAdd((property, runtimePropertyType), (key, arg) => CreateRuntimeProperty(key, arg), (options, Type));
return cache.GetOrAdd<(JsonSerializerOptions, Type)>((property, runtimePropertyType), (key, arg) => CreateRuntimeProperty(key, arg), (options, Type));

Edit: Nevermind

Copy link
Member

Choose a reason for hiding this comment

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

This API isn't available on netstandard 2.0, which is why the build is failing.
Can you #if/def the code using BUILDING_INBOX_LIBRARY and in the else block, only use netstandard2.0 compatible APIs?

}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
// See the LICENSE file in the project root for more information.

using System.Collections;
using System.Collections.Concurrent;
using System.Collections.Generic;
using System.Diagnostics;
using System.Reflection;
Expand All @@ -27,6 +28,9 @@ internal sealed partial class JsonClassInfo
// All of the serializable properties on a POCO (except the optional extension property) keyed on property name.
public volatile Dictionary<string, JsonPropertyInfo> PropertyCache;

// Serializable runtime/polymorphic proerties, keyed on property and runtime type.
svick marked this conversation as resolved.
Show resolved Hide resolved
public ConcurrentDictionary<(JsonPropertyInfo, Type), JsonPropertyInfo> RuntimePropertyCache;

// All of the serializable properties on a POCO including the optional extension property.
// Used for performance during serialization instead of 'PropertyCache' above.
public volatile JsonPropertyInfo[] PropertyCacheArray;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ public static partial class JsonSerializer
}
else if (state.Current.JsonClassInfo.ClassType == ClassType.Unknown)
{
jsonPropertyInfo = state.Current.JsonClassInfo.CreatePolymorphicProperty(jsonPropertyInfo, typeof(object), options);
jsonPropertyInfo = state.Current.JsonClassInfo.GetOrAddPolymorphicProperty(jsonPropertyInfo, typeof(object), options);
}

// Verify that we have a valid enumerable.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ private static void HandleValue(JsonTokenType tokenType, JsonSerializerOptions o
}
else if (state.Current.JsonClassInfo.ClassType == ClassType.Unknown)
{
jsonPropertyInfo = state.Current.JsonClassInfo.CreatePolymorphicProperty(jsonPropertyInfo, typeof(object), options);
jsonPropertyInfo = state.Current.JsonClassInfo.GetOrAddPolymorphicProperty(jsonPropertyInfo, typeof(object), options);
}

jsonPropertyInfo.Read(tokenType, ref state, ref reader);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ private static void GetRuntimePropertyInfo(object value, JsonClassInfo jsonClass
// Nothing to do for typeof(object)
if (runtimeType != typeof(object))
{
jsonPropertyInfo = jsonClassInfo.CreatePolymorphicProperty(jsonPropertyInfo, runtimeType, options);
jsonPropertyInfo = jsonClassInfo.GetOrAddPolymorphicProperty(jsonPropertyInfo, runtimeType, options);
}
}
}
Expand Down
25 changes: 25 additions & 0 deletions src/System.Text.Json/tests/Serialization/Object.WriteTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -75,5 +75,30 @@ private class Indexer

public string NonIndexerProp { get; set; }
}

[Fact]
public static void WritePolymorhicSimple()
{
string json = JsonSerializer.Serialize(new { Prop = (object)new[] { 0 } });
Assert.Equal(@"{""Prop"":[0]}", json);
}

[Fact]
public static void WritePolymorphicDifferentAttributes()
{
string json = JsonSerializer.Serialize(new Polymorphic());
Assert.Equal(@"{""P1"":"""",""p_3"":""""}", json);
}

private class Polymorphic
{
public object P1 => "";

[JsonIgnore]
public object P2 => "";

[JsonPropertyName("p_3")]
public object P3 => "";
}
}
}
Copy link
Member

Choose a reason for hiding this comment

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

Please add tests with different properties with the same Type but with different attributes (e.g. [JsonIgnore]) to ensure the cache's key works as expected.