C#•2mo ago

Duplicate enum values serialization problems

So, for the context, we have a public enum in which we had to introduce/rename one value. The problem is, that it would be a breaking change, therefore we had to mark the old value as obsolete and introduce a new one. To avoid any migrations, we did a bit controversial act, and we set the two values as equal. We have yet another value which is almost identical and we quite often map them onto each other. The problem is, that one enum has even and the other odd amount of values. Therefore, the other enum has reversed order. This way methods such as ToString, TryParse, Enum.GetName and so on always return the new value. An example:

public enum MyEnum
{
    ValueA = 0,
    [Obsolete($"Use {nameof(ValueC)} instead."), ObsoleteSince(69, 42)]
    ValueB = ValueC,
    ValueC = 1,
    ValueD = 2,
}

public enum MyEnum2
{
    ValueA = 0,
    ValueC = 1,
    [Obsolete($"Use {nameof(ValueC)} instead."), ObsoleteSince(69, 42)]
    ValueB = ValueC,
    ValueD = 2,
    ValueD = 3,
}

public enum MyEnum
{
    ValueA = 0,
    [Obsolete($"Use {nameof(ValueC)} instead."), ObsoleteSince(69, 42)]
    ValueB = ValueC,
    ValueC = 1,
    ValueD = 2,
}

public enum MyEnum2
{
    ValueA = 0,
    ValueC = 1,
    [Obsolete($"Use {nameof(ValueC)} instead."), ObsoleteSince(69, 42)]
    ValueB = ValueC,
    ValueD = 2,
    ValueD = 3,
}

Everything seemed alright with this approach, until serialization came into play. Both Rider debugger and JsonStringEnumConverter seem to serialize into the now obsolete enum value for the first enum, where obsolete value is the first. After some investigation, I've found a reason to be inside an EnumCoverter<> class from .net source (https://github.com/dotnet/runtime/blob/main/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/EnumConverter.cs) There, in the constructor you get all the names, in order as they are in the file, and one by one add them into the dict/cache. Once obsolete value is added, the new one can not due to the key uniqueness constraint. Once you read from cache in the write method, the new name is simply not there and an old value will be always used. If the cache weren't used/filled in, it would work correctly, due to the usage of ToString method which retrieves the correct string value.

GitHub

runtime/src/libraries/System.Text.Json/src/System/Text/Json/Seriali...

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps. - dotnet/runtime

5 Replies

Rišo•2mo ago

Mentioned methods:

public EnumConverter(EnumConverterOptions converterOptions, JsonNamingPolicy? namingPolicy, JsonSerializerOptions serializerOptions)
{
    ...

#if NET
   string[] names = Enum.GetNames<T>();
   T[] values = Enum.GetValues<T>();
#else
   string[] names = Enum.GetNames(Type);
   Array values = Enum.GetValues(Type);
#endif
   Debug.Assert(names.Length == values.Length);

   JavaScriptEncoder? encoder = serializerOptions.Encoder;

   for (int i = 0; i < names.Length; i++)
   {
#if NET
        T value = values[i];
#else
        T value = (T)values.GetValue(i)!;
#endif
        ulong key = ConvertToUInt64(value);
        string name = names[i];

        string jsonName = FormatJsonName(name, namingPolicy);
        _nameCacheForWriting.TryAdd(key, JsonEncodedText.Encode(jsonName, encoder));
        _nameCacheForReading?.TryAdd(jsonName, value);

        ...
    }
}

public EnumConverter(EnumConverterOptions converterOptions, JsonNamingPolicy? namingPolicy, JsonSerializerOptions serializerOptions)
{
    ...

#if NET
   string[] names = Enum.GetNames<T>();
   T[] values = Enum.GetValues<T>();
#else
   string[] names = Enum.GetNames(Type);
   Array values = Enum.GetValues(Type);
#endif
   Debug.Assert(names.Length == values.Length);

   JavaScriptEncoder? encoder = serializerOptions.Encoder;

   for (int i = 0; i < names.Length; i++)
   {
#if NET
        T value = values[i];
#else
        T value = (T)values.GetValue(i)!;
#endif
        ulong key = ConvertToUInt64(value);
        string name = names[i];

        string jsonName = FormatJsonName(name, namingPolicy);
        _nameCacheForWriting.TryAdd(key, JsonEncodedText.Encode(jsonName, encoder));
        _nameCacheForReading?.TryAdd(jsonName, value);

        ...
    }
}

public override void Write(Utf8JsonWriter writer, T value, JsonSerializerOptions options)
{
    // If strings are allowed, attempt to write it out as a string value
    if ((_converterOptions & EnumConverterOptions.AllowStrings) != 0)
    {
        ulong key = ConvertToUInt64(value);

        if (_nameCacheForWriting.TryGetValue(key, out JsonEncodedText formatted))
        {
            writer.WriteStringValue(formatted);
            return;
        }

        string original = value.ToString();

        if (IsValidIdentifier(original))
        {
            // We are dealing with a combination of flag constants since
            // all constant values were cached during warm-up.
            Debug.Assert(original.Contains(ValueSeparator));

            original = FormatJsonName(original, _namingPolicy);

            if (_nameCacheForWriting.Count < NameCacheSizeSoftLimit)
            {
                        formatted = JsonEncodedText.Encode(original, options.Encoder);
                        writer.WriteStringValue(formatted);
                        _nameCacheForWriting.TryAdd(key, formatted);
            }
            else
            {
                // We also do not create a JsonEncodedText instance here because passing the string
                // directly to the writer is cheaper than creating one and not caching it for reuse.
                writer.WriteStringValue(original);
            }

            return;
        }
    }
    //The rest of the code
    ...
}

public override void Write(Utf8JsonWriter writer, T value, JsonSerializerOptions options)
{
    // If strings are allowed, attempt to write it out as a string value
    if ((_converterOptions & EnumConverterOptions.AllowStrings) != 0)
    {
        ulong key = ConvertToUInt64(value);

        if (_nameCacheForWriting.TryGetValue(key, out JsonEncodedText formatted))
        {
            writer.WriteStringValue(formatted);
            return;
        }

        string original = value.ToString();

        if (IsValidIdentifier(original))
        {
            // We are dealing with a combination of flag constants since
            // all constant values were cached during warm-up.
            Debug.Assert(original.Contains(ValueSeparator));

            original = FormatJsonName(original, _namingPolicy);

            if (_nameCacheForWriting.Count < NameCacheSizeSoftLimit)
            {
                        formatted = JsonEncodedText.Encode(original, options.Encoder);
                        writer.WriteStringValue(formatted);
                        _nameCacheForWriting.TryAdd(key, formatted);
            }
            else
            {
                // We also do not create a JsonEncodedText instance here because passing the string
                // directly to the writer is cheaper than creating one and not caching it for reuse.
                writer.WriteStringValue(original);
            }

            return;
        }
    }
    //The rest of the code
    ...
}

Angius•2mo ago

Seems like you might want a custom converter for this enum

Rišo•2mo ago

Yes, that's exactly what I was considering. However, I still would like to hear your opinions on the matter.
Also I understand that this is a bit of an edge case, but wouldn't opening an issue on github be an appropriate next step?

Angius•2mo ago

It very much is an edge case, yes You can open an issue, sure, nothing wrong with that Chances are you'll be told "it's an edge case", but it's worth knowing that an edge case like this even exists

Rišo•2mo ago

Alright, I will look into it further and open an issue later today, thanks!

Gaming

Programming

Duplicate enum values serialization problems