Skip to content
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.

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

How to write custom converters for JSON serialization (marshalling) in .NET should explain how to handle complex types #35020

Closed
konrad-jamrozik opened this issue Apr 15, 2023 · 4 comments
Labels
dotnet-fundamentals/svc Pri1 High priority, do before Pri2 and Pri3 ⌚ Not Triaged Not triaged

Comments

@konrad-jamrozik
Copy link
Contributor

konrad-jamrozik commented Apr 15, 2023

The How to write custom converters for JSON serialization (marshalling) in .NET page explains how to use default system converter here.

What is not clear is that these converters are only for primitive types. I was hoping this article will explain how I can do this with my own custom built-in types, but it doesn't.

Following example is given for using a default converter in a custom converter:

public class MyCustomConverter : JsonConverter<int>
{
    private readonly static JsonConverter<int> s_defaultConverter = 
        (JsonConverter<int>)JsonSerializerOptions.Default.GetConverter(typeof(int));

    // Custom serialization logic
    public override void Write(
        Utf8JsonWriter writer, int value, JsonSerializerOptions options)
    {
        writer.WriteStringValue(value.ToString());
    }

    // Fall back to default deserialization logic
    public override int Read(
        ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
    {
        return s_defaultConverter.Read(ref reader, typeToConvert, options);
    }
}

but this doesn't work if value is not int but a custom type. The custom converter will be null and just doing value.ToString() won't produce the right value. Instead, for serialization one has to do:

public override void Write(Utf8JsonWriter writer, MyType value, JsonSerializerOptions options)
{
    writer.WriteRawValue(JsonSerializer.Serialize(value));
}

Similarly, for deserialization:

public override MyType? Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
{
    return JsonSerializer.Deserialize<MyType>(ref reader);
}

Additional context

You might ask: why I am trying to write a custom type converter for a complex type in the first place? Performance will take a hit.

This is an excellent question. It is because of this limitation:

Quote:

Even though not all reference preservation scenaria contain cycles, it arguably is the raison d'etre of the feature. While we could try to make the feature smarter and only fail if cycles are detected, this would require substantially more state to track correctly and it would impact performance. Consequently, explicitly not supporting the feature at all for constructor deserialization is the right apprioach in my view. As you mention, the workaround is to simply refactor constructor parameters to be init or required properties.

In addition, the documentation on ReferenceHandler.Preserve says:

This feature can't be used to preserve value types or immutable types. On deserialization, the instance of an immutable type is created after the entire payload is read. So it would be impossible to deserialize the same instance if a reference to it appears within the JSON payload.

I am trying to implement my own serialization that will preserve all references and also use constructors, without using required or init properties. This is because I want to forbid using an object initializer because I want to ensure constructor is called, as I am going to put precondition checks in the constructor which would be circumvented otherwise.

Hence, if I could write a custom converter for my complex type I would make it know when to serialize its members fully and when just by reference, to avoid cycles and duplication. It would also have matching deserialization logic which knows when to deserialize given object and when to deserialize just a reference to it and call proper constructors in the right order.

@eiriktsarpalis if you perhaps have some insight how to best deal with my scenario I would be grateful. Maybe there is better approach than what I am thinking.

Related:


Document Details

Do not edit this section. It is required for learn.microsoft.com ➟ GitHub issue linking.

@eiriktsarpalis
Copy link
Member

The custom converter will be null and just doing value.ToString() won't produce the right value.

I believe JsonSerializerOptions.Default will always produce a converter for the type, unless the parameter is something wholy unsupported like typeof(void) or typeof(ReadOnlySpan<byte>). The value.ToString() form is appropriate for the purposes of the particular example, however it should be possible to compose converters in the general case as follows:

s_defaultConverter.Write(writer, value, options);

I am trying to implement my own serialization...

I'm not sure I understand the use case or how it is related to the issue. Given that this repo is for tracking documentation issues, I would recommend asking a question in the "Discussions" section of dotnet/runtime rather than here.

konrad-jamrozik pushed a commit to konrad-jamrozik/dotnet-lib that referenced this issue Apr 20, 2023
@konrad-jamrozik
Copy link
Contributor Author

konrad-jamrozik commented Apr 20, 2023

@eiriktsarpalis it doesn't work on custom types. It will fail with System.NullReferenceException on write attempt. Here is a test showing this: https://github.com/konrad-jamrozik/dotnet-lib/blob/ddd8a2e1277d8cc6aab2b7aa562f0d01e6f41482/default_json_converter_repro/MyCustomConverterTests.cs

I took the example from the doc, but I replaced MyCustomConverter : JsonConverter<int> with MyCustomConverter : JsonConverter<Employee> and applied the Write method you proposed.

To reproduce:

git clone https://github.com/konrad-jamrozik/dotnet-lib.git
cd dotnet-lib
git checkout ddd8a2e1277d8cc6aab2b7aa562f0d01e6f41482
cd default_json_converter_repro
dotnet test

Stack trace snippet:

A total of 1 test files matched the specified pattern.
  Failed Test [46 ms]
  Error Message:
   System.NullReferenceException : Object reference not set to an instance of an object.
  Stack Trace:
     at System.Text.Json.Serialization.Converters.ObjectDefaultConverter`1.OnTryWrite(Utf8JsonWriter writer, T value, JsonSerializerOptions options, WriteStack& state)

I confirmed the above-given code works if I modify it to be MyCustomConverter : JsonConverter<int> and use it convert int instead of Employee.

Hence my ask is to apply following changes to the page:

  • explain how to handle such scenarios as shown above (I provided some suggestions in the original post);
  • modify the example to use the s_defaultConverter.Write(writer, value, options); instead of the not-always-applicable special-case of writer.WriteStringValue(value.ToString()); and maybe mention that methods like writer.WriteStringValue are available, if applicable. Note that current example is asymmetrical in the sense that s_defaultConverter is used only to deserialize, but not serialize.

@eiriktsarpalis
Copy link
Member

Ah yes, that issue is tracked here dotnet/runtime#50205. But it is not the case that a null converter could ever be returned. One workaround might be to pass JsonSerializerOptions.Default as a parameter instead of options but depending on your use case this might not be ideal.

@eiriktsarpalis
Copy link
Member

Also related to dotnet/runtime#63791.

Given that this issue is not related to API documentation per se I'd be inclined to close it. Feel free to reopen or file a new issue if you were looking for a specific fix in the docs you cited.

@dotnet-bot dotnet-bot added ⌚ Not Triaged Not triaged and removed ⌚ Not Triaged Not triaged labels Apr 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dotnet-fundamentals/svc Pri1 High priority, do before Pri2 and Pri3 ⌚ Not Triaged Not triaged
Projects
None yet
Development

No branches or pull requests

3 participants