Runtime: JsonExtensionData should not use the dictionary name when serializing

[pranavkm]

Consider:

```C#
public class MyPoco
{
public int Id { get; set;}

[JsonExtensionData]
public Dictionary<string, object> Extensions { get; set;} = new Dictionary<string, object>();

}

Deserializing this works perfectly:

```C#
var json = "{\"Id\": 10, \"Hello\": \"world\" }";

var poco = JsonSerializer.Parse<MyPoco>(json);

However, if I attempt to serialize the type, it includes the dictionary name as the property key:

```C#
var serialized = JsonSerializer.ToString(poco);
Console.WriteLine(serialized); // Prints {"Id":10,"Extensions":{"Hello":"world"}}

While this does round-trip, the expectation is keys in `JsonExtensionData` should alongside other properties of the Poco object. Here's the same routine performed using Json.NET:

```C#
var json = "{\"Id\": 10, \"Hello\": \"world\" }";

var poco = JsonConvert.DeserializeObject<MyPoco>(json);
Console.WriteLine(poco.Extensions["Hello"]);

var serialized = JsonConvert.SerializeObject(poco);
Console.WriteLine(serialized); // Prints {"Id":10,"Hello":"world"}

More interestingly, Json.NET does not allow using the dictionary name for deserialization:

```C#
var json = "{\"Id\":10,\"Extensions\":{\"Hello\":\"world\"}}";

var poco = JsonConvert.DeserializeObject(json);
Console.WriteLine(poco.Extensions.ContainsKey("Hello")); // Returns false
```

This makes the serialized output particularly problematic since it's not consumable by Json.Net.

[pranavkm]

cc @JamesNK

[JamesNK]

This should be changed.

More interestingly, Json.NET does not allow using the dictionary name for deserialization:

My point of view is the extension property itself is ignored. Only the content is serialized and deserialized.

[ahsonkhan]

Can you make the Extensions dictionary property private?
Edit: Nevermind, that doesn't work

I am curious to see what would happen if you added JsonIgnore attribute on the Extensions dictionary along with the JsonExtensionData?
Edit:
We throw an exception:

System.InvalidOperationException : The data extension property 'System.Text.Json.Serialization.Tests.ValueTests+MyPoco.Extensions' does not match the required signature of IDictionary<string, JsonElement> or IDictionary<string, object>.

Similarly, what happens if a custom camel casing policy (or JsonPropertyName attribute) caused a property to collide with the overflow capturing dictionary that has JsonExtensionData on it.
Edit:
As expected, this throws the right error message:

 System.InvalidOperationException : The JSON property name for 'System.Text.Json.Serialization.Tests.ValueTests+MyPoco.MyType' collides with another property.

[ahsonkhan]

cc @steveharter

[ahsonkhan]

Looks like this regression occurred because of a change to resolve the other issue:
https://github.com/dotnet/corefx/pull/38306/files#diff-cf3c98b9ccd0b5614511b8adf262f8e9L41

https://github.com/dotnet/corefx/issues/38238

cc @MarcoRossignoli

[MarcoRossignoli]

Yep we discussed this update here
My first idea was emit properties with Dictionary<string,JsonElement> and standard dictionary for Dictionary<string,object> https://github.com/dotnet/corefx/pull/38306#issue-285800992 but at the end we decided to emit simple dictionary for every pairs.
Seem that alg should be more clever and "reflect" on how serialize object value.
Just to know @JamesNK your serialized has got some limit on value type?Or it serialize/deserialize also complex struct as property like dic of dic etc...?

[ahsonkhan]

This issue is primarily talking about serialization (not deserialization). On serialization, we don't want to emit the name of the dictionary property that has [JsonExtensionData] on it since that dictionary is only meant for capturing overflow and round-tripping.

[MarcoRossignoli]

@ahsonkhan some more infos, I found time to do some tests https://github.com/dotnet/corefx/compare/master...MarcoRossignoli:json_extensions and if I'm not mistaken code always emitted dic name also before that PR, I reverted the code(with WriteExtensionData inline) and tried to serialize and get same result {"Id":0,"Extensions":{"test":"world"}}

[ahsonkhan]

That's good info. Thanks for sharing @MarcoRossignoli

@steveharter, looks like our previous assumption was incorrect.

[MarcoRossignoli]

I try to help more with my thought, hope help...the problem seem that the check for state.Current.JsonClassInfo.DataExtensionProperty was done(before we removed it) inside HandleDictionary too late, after we write start(object or array)
Maybe we should check for extension and handle every item of dic like a value before outside HandleDictionary or push items inside it but it's more complex because we need to skip open/close object.
Seem not trivial.

[JamesNK]

I'll pre-empt someone bringing this up as an issue: it is valid to have duplicate names in a JSON object.

var t = new TypeWithExtensionData();
t.Name = "A name!";
t.ExtensionData["Name"] = "Extension data name!";

Would result in:

{
    "Name": "A name!",
    "Name": "Extension data name!"
}

This is valid JSON.

Note that if you deserialize it, it won't exactly roundtrip. The extension data dictionary will be empty and the Name property will be "Extension data name!" (because it is last, and the deserializer doesn't track that a property has already been set). IMO this is fine. This whole situation is an edge-case.

[kkatoch]

Is this issue resolved? I still get the extension data nested under the name of the dictionary, this doesn’t happen when using newtonsoft in .net Core 2.0