JamesNK / Newtonsoft.Json

Json.NET is a popular high-performance JSON framework for .NET
https://www.newtonsoft.com/json
MIT License
10.64k stars 3.24k forks source link

Multiple JsonSerializerSettings are sharing same CamelCasePropertyNamesContractResolver cache #2927

Closed DanStory closed 6 months ago

DanStory commented 6 months ago

Source types

var data = new Dictionary<string, object>(){
    { "camelCase", "value"},
    { "PascalCase", "Value" },
    { "snake_case", "v_lue" },
    {"YELLING", "VALUE"}
};

destination JSON

{
  "camelCase": "value",
  "pascalCase": "Value",
  "snake_case": "v_lue",
  "yelling": "VALUE"
}
// or
 {
  "camelCase": "value",
  "PascalCase": "Value",
  "snake_case": "v_lue",
  "YELLING": "VALUE"
}

Expected behavior

Contract of type is isolated between two different JsonSerializerSettings

Actual behavior

Contract of type is being shared (with a static cache) across JsonSerializerSettings and serialization

Steps to reproduce

using System.Collections;
using System.Reflection;
using Newtonsoft.Json;
using Newtonsoft.Json.Serialization;

var data = new Dictionary<string, object>(){
    { "camelCase", "value"},
    { "PascalCase", "Value" },
    { "snake_case", "v_lue" },
    {"YELLING", "VALUE"}
};

var camelSettings = new JsonSerializerSettings()
{
    Formatting = Formatting.Indented,
    ContractResolver = new CamelCasePropertyNamesContractResolver()
        { NamingStrategy = new CamelCaseNamingStrategy() { ProcessDictionaryKeys = true } }
};
var unprocessedSettings = new JsonSerializerSettings()
{
    Formatting = Formatting.Indented,
    ContractResolver = new CamelCasePropertyNamesContractResolver()
        { NamingStrategy = new CamelCaseNamingStrategy() { ProcessDictionaryKeys = false } }
};

Console.WriteLine($"Camel Settings: {JsonConvert.SerializeObject(data, camelSettings)}\n");
Console.WriteLine($"Unprocessed Settings: {JsonConvert.SerializeObject(data, unprocessedSettings)}");

((IDictionary)typeof(CamelCasePropertyNamesContractResolver).GetField("_contractCache", BindingFlags.Static | BindingFlags.NonPublic)?.GetValue(null)!).Clear();
Console.WriteLine("\n-- Forced Clearing of CamelCasePropertyNamesContractResolver internal cache --\n");

Console.WriteLine($"Unprocessed Settings: {JsonConvert.SerializeObject(data, unprocessedSettings)}\n");
Console.WriteLine($"Camel Settings: {JsonConvert.SerializeObject(data, camelSettings)}");
Camel Settings: {
  "camelCase": "value",
  "pascalCase": "Value",
  "snake_case": "v_lue",
  "yelling": "VALUE"
}

Unprocessed Settings: {
  "camelCase": "value",
  "pascalCase": "Value",
  "snake_case": "v_lue",
  "yelling": "VALUE"
}

-- Forced Clearing of CamelCasePropertyNamesContractResolver internal cache --

Unprocessed Settings: {
  "camelCase": "value",
  "PascalCase": "Value",
  "snake_case": "v_lue",
  "YELLING": "VALUE"
}

Camel Settings: {
  "camelCase": "value",
  "PascalCase": "Value",
  "snake_case": "v_lue",
  "YELLING": "VALUE"
}

We have an application that interact with multiple different HTTP APIs, each having their own set of serialization rules, and depending on which endpoint is used first (who ever serializes data for a request) ends up causing invalid data for the others due to a shared cache of contracts.

DanStory commented 6 months ago

Digging into the issues here more closely, it seems this is expected behavior do to worry of performance degradation of legacy/existing applications..

Personally, I would suggest the CamelCasePropertyNamesContractResolver be marked with a deprecated message for the implementer to manage the lifetime of DefaultContractResolver instead or at the very least in its documentation describe its shared caching behavior.