dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
15.29k stars 4.74k forks source link

JsonNode property names are not renamed according to JsonNamingPolicy #101068

Closed Nefcanto closed 7 months ago

Nefcanto commented 7 months ago

Edit: Reduced repro is below.

When a JsonNode with PascalCase property names is serialized, those property names are not renamed to reflect JsonNamingPolicy.CamelCase.

Is there an existing issue for this?

Describe the bug

I have created a custom JSON converter for my ASP.NET Core application. This is the code:

    public override void Write(Utf8JsonWriter writer, object value, JsonSerializerOptions options)
    {
        var stopwatch = new Stopwatch();
        stopwatch.Start();
        var jsonString = JsonSerializer.Serialize(value);
        var node = JsonNode.Parse(jsonString);
        FlattenRelatedItems(node);
        node.WriteTo(writer, JsonHelper.Options);
        stopwatch.Stop();
    }

    public static void FlattenRelatedItems(JsonNode node)
    {
        if (node is JsonArray jsonArray)
        {
            foreach (var item in jsonArray)
            {
                FlattenRelatedItems(item);
            }
        }
        else if (node is JsonObject jsonObject)
        {
            if (jsonObject.ContainsKey("RelatedItems"))
            {
                jsonObject.TryGetPropertyValue("RelatedItems", out JsonNode relatedItems);
                foreach (var property in relatedItems.AsObject())
                {
                    var valueJson = property.Value.ToString();
                    if (property.Value.GetValueKind() == JsonValueKind.String)
                    {
                        valueJson = $"\"{valueJson}\"";
                    }
                    var value = JsonSerializer.Deserialize<JsonNode>(valueJson);
                    if (jsonObject.ContainsKey(property.Key))
                    {
                        Logger.LogWarning(property.ToString());
                    }
                    else
                    {
                        jsonObject.Add(property.Key, value);
                    }
                }
                jsonObject.Remove("RelatedItems");

            }
            foreach (var property in jsonObject)
            {
                FlattenRelatedItems(property.Value);
            }
        }
    }

And this is my JsonHelper class:

namespace Infra;

public static class JsonHelper
{
    private static object lockToken = new Object();

    public static void ConfigureJsonSerializerOptions(JsonSerializerOptions options)
    {
        options.PropertyNameCaseInsensitive = true;
        options.PropertyNamingPolicy = JsonNamingPolicy.CamelCase;
        options.DictionaryKeyPolicy = JsonNamingPolicy.CamelCase;
        options.Encoder = JavaScriptEncoder.Create(UnicodeRanges.All);
        options.NumberHandling = JsonNumberHandling.AllowReadingFromString;
        options.ReadCommentHandling = JsonCommentHandling.Skip;
        options.UnknownTypeHandling = JsonUnknownTypeHandling.JsonNode;
        options.DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingNull;
        options.AllowTrailingCommas = true;
        options.WriteIndented = true;
        if (!options.Converters.Any(c => c.GetType() == typeof(RelatedItemsFlattenerJsonConverter)))
        {
            if (InfraConfig.GetBooleanSetting("FlattenRelatedItems"))
            {
                options.Converters.Add(new RelatedItemsFlattenerJsonConverter());
            }
        }
        if (!options.Converters.Any(c => c.GetType() == typeof(JsonStringEnumConverter)))
        {
            options.Converters.Add(new JsonStringEnumConverter());
        }
    }

    private static JsonSerializerOptions options;

    public static JsonSerializerOptions Options
    {
        get
        {
            if (options == null)
            {
                lock (lockToken)
                {
                    options = new JsonSerializerOptions();
                    ConfigureJsonSerializerOptions(options);
                }
            }
            return options;
        }
    }

    public static JsonElement? Get(this JsonElement element, string name) =>
        element.ValueKind != JsonValueKind.Null && element.ValueKind != JsonValueKind.Undefined && element.TryGetProperty(name, out var value)
            ? value : (JsonElement?)null;

    public static JsonElement? Get(this JsonElement element, int index)
    {
        if (element.ValueKind == JsonValueKind.Null || element.ValueKind == JsonValueKind.Undefined)
            return null;
        var value = element.EnumerateArray().ElementAtOrDefault(index);
        return value.ValueKind != JsonValueKind.Undefined ? value : (JsonElement?)null;
    }
}

But the output JSON does not respect the options passed to it.

Expected Behavior

The Utf8JsonWriter and JsonNode should respect the options passed to them.

Steps To Reproduce

  1. Create a custom converter
  2. Create custom options (for example, DictionaryKeyPolicy)
  3. Register that converter
  4. Run through the converter

Exceptions (if any)

No response

.NET Version

8.0.201

Anything else?

.NET SDK: Version: 8.0.201 Commit: 4c2d78f037 Workload version: 8.0.200-manifests.3097af8b

Runtime Environment: OS Name: debian OS Version: 12 OS Platform: Linux RID: linux-x64 Base Path: /usr/share/dotnet/sdk/8.0.201/

.NET workloads installed: There are no installed workloads to display.

Host: Version: 8.0.2 Architecture: x64 Commit: 1381d5ebd2

.NET SDKs installed: 8.0.201 [/usr/share/dotnet/sdk]

.NET runtimes installed: Microsoft.AspNetCore.App 8.0.2 [/usr/share/dotnet/shared/Microsoft.AspNetCore.App] Microsoft.NETCore.App 8.0.2 [/usr/share/dotnet/shared/Microsoft.NETCore.App]

Other architectures found: None

Environment variables: Not set

global.json file: Not found

Learn more: https://aka.ms/dotnet/info

Download .NET: https://aka.ms/dotnet/download

martincostello commented 7 months ago

How are you calling/wiring-up ConfigureJsonSerializerOptions()? Are you using MVC or Minimal APIs?

Nefcanto commented 7 months ago

@martincostello, this is part of my Startup.cs:

        var mvcBuilder = services.AddControllers(options =>
        {
            options.ModelBinderProviders.Insert(0, new ListParametersModelBinderProvider());
            options.EnableEndpointRouting = false;
            options.Conventions.Add(new ReferenceTypeBodyJsonBindingConvention());
            options.Filters.Add(new ModelChecker());
        }).AddJsonOptions(options =>
        {
            JsonHelper.ConfigureJsonSerializerOptions(options.JsonSerializerOptions);
        });

The point is, when I remove this custom converter from the circuit, the options work as expected. But when I plug it in, those options are not respected.

amcasey commented 7 months ago

I may just be missing something, but is this specific to aspnetcore? If you just put that code in a console app, would you see the same honoring/ignoring of the options?

Nefcanto commented 7 months ago

@amcasey, I don't know how to configure a console app to use that indirectly. Can I register that custom mapper in a console app?

amcasey commented 7 months ago

@Nefcanto I suppose my question was around whether a custom mapper is an important part of the repro. From your description above, it sounds like serializing an object to Utf8JsonWriter is sufficient?

Nefcanto commented 7 months ago

@amcasey, if serializing does not respect the parameters/options passed, then that's our current problem. We deliver API, and we want every property in our API JSON to be camelCased. This is a sample of our C# object:

public class Post
{
    public long Id { get; set; }

    public string Title { get; set; }

    public dynamic RelatedItems { get; set; }
}

We can create a Post object, set some dynamic properties on it, and return it:

var post = new Post()
post.Id = 1;
post.Title = "something"
post.RelatedItems = new ExpandoObject();
post.RelatedItems.Slug = "amazing-slug"

This object will be converted into this JSON (based on our options) if we do not register a custom mapper:

{
    "id": 1,
    "title": "something",
    "relatedItems": {
        "slug": "amazing-slug"
    }
}

Now we would like to remove that relatedItems in the output JSON, but we want to keep it in our C# code, because it gives us dynamic capabilities. We want the output JSON to look like this:

{
    "id": 1,
    "title": "something",
    "slug": "amazing-slug"
}

But when we register our custom mapper for this purpose, we get this JSON:

{
    "Id": 1,
    "Title": "something",
    "Slug": "amazing-slug"
}

This means that the mapper should have respected our options. We have specified in our options that:

        options.PropertyNamingPolicy = JsonNamingPolicy.CamelCase;
        options.DictionaryKeyPolicy = JsonNamingPolicy.CamelCase;

I hope this explanation makes it more transparent. Please let me know if you need more data.

amcasey commented 7 months ago

On my box, this code prints {"Id":1,"Title":"something","Slug":"amazing-slug"}. I understand you would like it to print {"id":1,"title":"something","slug":"amazing-slug"}. If you agree that this is fair representation of what you're trying to do, I'll move this issue to dotnet/runtime since it happens without MVC.

using System.Dynamic;
using System.Text;
using System.Text.Encodings.Web;
using System.Text.Json;
using System.Text.Json.Nodes;
using System.Text.Json.Serialization;
using System.Text.Unicode;

var post = new Post
{
    Id = 1,
    Title = "something",
    RelatedItems = new ExpandoObject()
};
post.RelatedItems.Slug = "amazing-slug";

var options = new JsonSerializerOptions
{
    PropertyNameCaseInsensitive = true,
    PropertyNamingPolicy = JsonNamingPolicy.CamelCase,
    DictionaryKeyPolicy = JsonNamingPolicy.CamelCase,
    Encoder = JavaScriptEncoder.Create(UnicodeRanges.All),
    NumberHandling = JsonNumberHandling.AllowReadingFromString,
    ReadCommentHandling = JsonCommentHandling.Skip,
    UnknownTypeHandling = JsonUnknownTypeHandling.JsonNode,
    DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingNull,
    AllowTrailingCommas = true,
    WriteIndented = true,
};

using var stream = new MemoryStream();
using var writer = new Utf8JsonWriter(stream);

var jsonString = JsonSerializer.Serialize(post);
var node = JsonNode.Parse(jsonString);
FlattenRelatedItems(node);
node.WriteTo(writer, options);
writer.Flush();

string json = Encoding.UTF8.GetString(stream.ToArray());
Console.WriteLine(json);

static void FlattenRelatedItems(JsonNode node)
{
    if (node is JsonArray jsonArray)
    {
        foreach (var item in jsonArray)
        {
            FlattenRelatedItems(item);
        }
    }
    else if (node is JsonObject jsonObject)
    {
        if (jsonObject.ContainsKey("RelatedItems"))
        {
            jsonObject.TryGetPropertyValue("RelatedItems", out JsonNode relatedItems);
            foreach (var property in relatedItems.AsObject())
            {
                var valueJson = property.Value.ToString();
                if (property.Value.GetValueKind() == JsonValueKind.String)
                {
                    valueJson = $"\"{valueJson}\"";
                }
                var value = JsonSerializer.Deserialize<JsonNode>(valueJson);
                if (jsonObject.ContainsKey(property.Key))
                {
                    Console.WriteLine(property.ToString());
                }
                else
                {
                    jsonObject.Add(property.Key, value);
                }
            }
            jsonObject.Remove("RelatedItems");

        }
        foreach (var property in jsonObject)
        {
            FlattenRelatedItems(property.Value);
        }
    }
}

class Post
{
    public long Id { get; set; }
    public string Title { get; set; }
    public dynamic RelatedItems { get; set; }
}
amcasey commented 7 months ago

I'm not familiar with JsonNode, but I wouldn't be surprised to learn that its name isn't covered by serialization options because they're considered to have come from JSON and thus already be correct.

Note, for example, that if you pass the same options object to this call, var jsonString = JsonSerializer.Serialize(value);, you get camel-cased names out. One option might be to do that and then update FlattenRelatedItems to operate on "relatedItems" (possibly based on a flag if the code is used in other places).

amcasey commented 7 months ago

I think this succinctly illustrates the behavior you are describing:

using System.Text.Json;
using System.Text.Json.Nodes;

var post = new Post
{
    SomeProperty = "value"
};

var options = new JsonSerializerOptions
{
    PropertyNamingPolicy = JsonNamingPolicy.CamelCase,
};

Console.WriteLine(JsonSerializer.Serialize(post, options));
Console.WriteLine(JsonNode.Parse(JsonSerializer.Serialize(post))!.ToJsonString(options)); // NB: options not passed to Serialize

class Post
{
    public required string SomeProperty { get; init; }
}

It prints

{"someProperty":"value"}
{"SomeProperty":"value"}
Nefcanto commented 7 months ago

@amcasey, yeah, please move it to dotnet/runtime. I didn't know that it happens without MVC. And regarding your last example, I didn't understand it to be honset. However, the MRE you provided for a console app is enough for the team to understand why it does not follow the options.

amcasey commented 7 months ago

@Nefcanto Will do. Just to set expectations, they will likely close the issue as by-design (hopefully, with an explanation or a link to documentation). If that is the case, the mitigation for you is probably to pass your serializer options to JsonSerializer.Serialize when you compute jsonString.

dotnet-policy-service[bot] commented 7 months ago

Tagging subscribers to this area: @dotnet/area-system-text-json, @gregsdennis See info in area-owners.md if you want to be subscribed.

eiriktsarpalis commented 7 months ago

@amcasey is correct to point out that PropertyNamingPolicy is a JsonSerializer feature that is not in scope for JsonNode which simply defines a JSON DOM. Duplicate of https://github.com/dotnet/runtime/issues/66661

Nefcanto commented 7 months ago

@eiriktsarpalis, thank you for the explanation and linking to another issue. I understood. JsonNode represents JSON precisely as it is by design. But what should I do now? I need a custom mapper to change the JSON my API returns. And I want to keep those policies. Any guidance for this part?

amcasey commented 7 months ago

@Nefcanto As I mentioned above, you can pass your serialization options when you create the json string that you eventually parse into a JsonNode. Then the JsonNode will have the camelCase names. You'll need to update FlattenRelatedItems to use the new names.

Nefcanto commented 7 months ago

@amcasey, I updated my code as you mentioned:

    public override void Write(Utf8JsonWriter writer, object value, JsonSerializerOptions options)
    {
        var stopwatch = new Stopwatch();
        stopwatch.Start();
        var jsonString = JsonSerializer.Serialize(value, JsonHelper.Options);
        var node = JsonNode.Parse(jsonString);
        FlattenRelatedItems(node);
        node.WriteTo(writer);
        stopwatch.Stop();
    }

But now I get this recursive error:

   at System.Text.Json.Serialization.JsonConverter`1[[System.__Canon, System.Private.CoreLib, Version=8.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e]].WriteCore(System.Text.Json.Utf8JsonWriter, System.__Canon ByRef, System.Text.Json.JsonSerializerOptions, System.Text.Json.WriteStack ByRef)
   at System.Text.Json.Serialization.Metadata.JsonTypeInfo`1[[System.__Canon, System.Private.CoreLib, Version=8.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e]].Serialize(System.Text.Json.Utf8JsonWriter, System.__Canon ByRef, System.Object)
   at System.Text.Json.JsonSerializer.WriteString[[System.__Canon, System.Private.CoreLib, Version=8.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e]](System.__Canon ByRef, System.Text.Json.Serialization.Metadata.JsonTypeInfo`1<System.__Canon>)
   at System.Text.Json.JsonSerializer.Serialize[[System.__Canon, System.Private.CoreLib, Version=8.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e]](System.__Canon, System.Text.Json.JsonSerializerOptions)
   at Infra.RelatedItemsFlattenerJsonConverter.Write(System.Text.Json.Utf8JsonWriter, System.Object, System.Text.Json.JsonSerializerOptions)
   at System.Text.Json.Serialization.JsonConverter`1[[System.__Canon, System.Private.CoreLib, Version=8.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e]].TryWrite(System.Text.Json.Utf8JsonWriter, System.__Canon ByRef, System.Text.Json.JsonSerializerOptions, System.Text.Json.WriteStack ByRef)
   at System.Text.Json.Serialization.JsonConverter`1[[System.__Canon, System.Private.CoreLib, Version=8.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e]].WriteCore(System.Text.Json.Utf8JsonWriter, System.__Canon ByRef, System.Text.Json.JsonSerializerOptions, System.Text.Json.WriteStack ByRef)
   at System.Text.Json.Serialization.Metadata.JsonTypeInfo`1[[System.__Canon, System.Private.CoreLib, Version=8.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e]].Serialize(System.Text.Json.Utf8JsonWriter, System.__Canon ByRef, System.Object)
   at System.Text.Json.JsonSerializer.WriteString[[System.__Canon, System.Private.CoreLib, Version=8.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e]](System.__Canon ByRef, System.Text.Json.Serialization.Metadata.JsonTypeInfo`1<System.__Canon>)
   at System.Text.Json.JsonSerializer.Serialize[[System.__Canon, System.Private.CoreLib, Version=8.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e]](System.__Canon, System.Text.Json.JsonSerializerOptions)
   at Infra.RelatedItemsFlattenerJsonConverter.Write(System.Text.Json.Utf8JsonWriter, System.Object, System.Text.Json.JsonSerializerOptions)
   at System.Text.Json.Serialization.JsonConverter`1[[System.__Canon, System.Private.CoreLib, Version=8.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e]].TryWrite(System.Text.Json.Utf8JsonWriter, System.__Canon ByRef, System.Text.Json.JsonSerializerOptions, System.Text.Json.WriteStack ByRef)

And It's so much that I can't even see the first lines in the terminal.

Is that code correct?

It seems to me that by calling JsonSerializer.Serialize(value, JsonHelper.Options); we enter a recursion in which JsonSerializer wants to serialize the given JSON, yet it is configured to use RelatedItemsFlattenerJsonConverter, then in the Write method we're using JsonSerializer and it goes into a loop. So what is the solution here?

eiriktsarpalis commented 7 months ago

@Nefcanto have you tried using one of the JsonSerializer.SerializeToNode methods?

amcasey commented 7 months ago

@Nefcanto my best guess would be that you need to make a copy of your options without

        if (!options.Converters.Any(c => c.GetType() == typeof(RelatedItemsFlattenerJsonConverter)))
        {
            if (InfraConfig.GetBooleanSetting("FlattenRelatedItems"))
            {
                options.Converters.Add(new RelatedItemsFlattenerJsonConverter());
            }
        }

But I'd investigate Eirik's suggestion first.