ReferenceHandler.Preserve does not work well with JsonConverters

[YeskaNova]

When I use a simple JsonConverter, with Reference handler equal to Preserve to serialize cyclic objects, the $id property added to each object is not unique, in this example "$id":1 exists two times :

System.Text.Json : 5.0.0

using System;
using System.Diagnostics;
using System.Text.Json;
using System.Text.Json.Serialization;

namespace ConsoleApp2
{
    public class ActionResultDto<T>
    {
        public T Result { get; set; }
    }
    public class Data
    {
    }
    class Program
    {
        static void Main(string[] args)
        {
            var data = new ActionResultDto<object>
            {
                Result = new Data() 
            };
            var options = new JsonSerializerOptions()
            {
                ReferenceHandler = ReferenceHandler.Preserve
            };
            options.Converters.Add(new EmptyJsonConverter());
            var str = JsonSerializer.Serialize(data, options);
            Debug.Assert(str.Split("\"$id\":\"1\"").Length == 2); // we must have only one occurence of "$id":"1"
        }
    }
    internal class EmptyJsonConverter
       : JsonConverter<object>
    {
        public override object Read(ref Utf8JsonReader reader,Type typeToConvert,JsonSerializerOptions options)
        {
            throw new InvalidOperationException("Should not get here.");
        }

        public override void Write(Utf8JsonWriter writer,object objectToWrite,JsonSerializerOptions options)
        {
            JsonSerializer.Serialize(writer, objectToWrite, options);
        }
    }
}

// Output =>    {"$id":"1","Result":{"$id":"1"}}

---

Document Details

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

• ID: 5de37d79-751f-e372-e866-efde28f4908c
• Version Independent ID: 678c4f23-764a-7ccd-0d6d-159a30957477
• Content: How to write custom converters for JSON serialization - .NET
• Content Source: docs/standard/serialization/system-text-json-converters-how-to.md
• Product: dotnet-fundamentals
• GitHub Login: @tdykstra
• Microsoft Alias: tdykstra

[layomia]

cc @jozkee to confirm the story with JsonConverters and the preserved-reference feature.

Since reference data is only cached per call to Serialize/Deserialize, I don't see a way for existing reference ID metadata to be passed along when calling back into the serializer from a custom converter. Maybe in the future when we expose ReadStack and other metadata types (dotnet/runtime#34456), this scenario will be possible.

---

On a side note, I'd expect normally a StackOverflow to be thrown given the way the custom converter calls back to the serializer. The serializer would ordinarily fetch the same converter instance to handle the (de)serialization, which would cause the overflow. This is not the case here. I haven't dug deep here, but maybe it has to do with the converter being for System.Object. See https://docs.microsoft.com/en-us/dotnet/standard/serialization/system-text-json-migrate-from-newtonsoft-how-to?pivots=dotnet-5-0#required-properties for more information on calling back into the serializer from a custom converter.

[jozkee]

You can root the ReferenceResolver instance in the call site of JsonSerializer.Serialize in order to make it persistent on nested calls to Serialize. However, be careful to not expand this pattern to a "global" JsonSerializerOptions, otherwise the reference data will never reset and the underlying dictionaries may grow forever.

class Program
{
    static void Main(string[] args)
    {
        var data = new ActionResultDto<object>
        {
            Result = new Data() 
        };
        var options = new JsonSerializerOptions()
        {
            ReferenceHandler = ReferenceHandler.Preserve
        };
        options.Converters.Add(new EmptyJsonConverter());
        options.ReferenceHandler = new MyReferenceHandler();

        string str = JsonSerializer.Serialize(data, options);

        // Always reset or null-out options.ReferenceHandler after you are done serializing 
        // in order to avoid out of bounds memory growth in the reference resolver.
        options.ReferenceHandler = null;

        System.Console.WriteLine(str); // Output =>    {"$id":"1","Result":{"$id":"2"}}
    }
}

class MyReferenceHandler : ReferenceHandler
{
    public MyReferenceHandler()
    {
        _rootedResolver = new MyReferenceResolver();
    }

    private ReferenceResolver _rootedResolver;
    public override ReferenceResolver CreateResolver() => _rootedResolver;

    class MyReferenceResolver : ReferenceResolver
    {
        private uint _referenceCount;
        private readonly Dictionary<string, object> _referenceIdToObjectMap = new Dictionary<string, object>();
        private readonly Dictionary<object, string> _objectToReferenceIdMap = new Dictionary<object, string>(ReferenceEqualityComparer.Instance);

        public override void AddReference(string referenceId, object value)
        {
            if (!_referenceIdToObjectMap.TryAdd(referenceId, value))
            {
                throw new JsonException();
            }
        }

        public override string GetReference(object value, out bool alreadyExists)
        {
            if (_objectToReferenceIdMap.TryGetValue(value, out string referenceId))
            {
                alreadyExists = true;
            }
            else
            {
                _referenceCount++;
                referenceId = _referenceCount.ToString();
                _objectToReferenceIdMap.Add(value, referenceId);
                alreadyExists = false;
            }

            return referenceId;
        }

        public override object ResolveReference(string referenceId)
        {
            if (!_referenceIdToObjectMap.TryGetValue(referenceId, out object value))
            {
                throw new JsonException();
            }

            return value;
        }
    }
}

[layomia]

Transferring this to dotnet/docs to consider adding a sample for

• preserving references + custom converters
• preserving references across multiple (de)serialization calls

cc @tdykstra