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

DictionaryTKeyEnumTValueConverter does not play nicely with Xamarin iOS/Android and AOT in general #16922

Closed
softlion opened this issue Feb 1, 2020 — with docs.microsoft.com · 4 comments
Assignees
Labels
dotnet/svc needs-more-info Needs more info from OP. Auto-closed after 2 weeks if no response. [org][resolution]

Comments

Copy link

softlion commented Feb 1, 2020

AOT does not like reflection and factories building objects from reflection, as it needs to detect all types used in the app from the static analysis, to be able to strip unused classes and methods, and to precompile used classes and methods to machine code for performance.

The example you provide DictionaryTKeyEnumTValueConverter fall typically in this case.

The way to go is to use the inner class as the converter. So make i made it public, without requiring the json options, as it is unavailable there.

public class DictionaryEnumConverter<TKey, TValue> : JsonConverter<Dictionary<TKey, TValue>> where TKey : struct, Enum
    {
        private JsonConverter<TValue> _valueConverter;
        private bool valueConverterSearched = false;
        private Type _valueType = typeof(TValue);

        public DictionaryEnumConverter()
        {
        }

        public override Dictionary<TKey, TValue> Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
        {
            // For performance, use the existing converter if available.
            if (!valueConverterSearched)
            {
                _valueConverter = (JsonConverter<TValue>)options.GetConverter(typeof(TValue));
                valueConverterSearched = true;
            }

            if (reader.TokenType != JsonTokenType.StartObject)
                throw new JsonException();

            var dictionary = new Dictionary<TKey, TValue>();

            while (reader.Read())
            {
                if (reader.TokenType == JsonTokenType.EndObject)
                    return dictionary;

                // Get the key.
                if (reader.TokenType != JsonTokenType.PropertyName)
                    throw new JsonException();

                var propertyName = reader.GetString();

                // For performance, parse with ignoreCase:false first.
                if (!Enum.TryParse(propertyName, ignoreCase: false, out TKey key) &&
                    !Enum.TryParse(propertyName, ignoreCase: true, out key))
                {
                    throw new JsonException($"Unable to convert \"{propertyName}\" to Enum \"{typeof(TKey)}\".");
                }

                // Get the value.
                TValue v;
                if (_valueConverter != null)
                {
                    reader.Read();
                    v = _valueConverter.Read(ref reader, _valueType, options);
                }
                else
                {
                    v = JsonSerializer.Deserialize<TValue>(ref reader, options);
                }

                // Add to dictionary.
                dictionary.Add(key, v);
            }

            throw new JsonException("Missing EndObject on dictionary");
        }

        public override void Write(Utf8JsonWriter writer, Dictionary<TKey, TValue> dictionary, JsonSerializerOptions options)
        {
            // For performance, use the existing converter if available.
            if (!valueConverterSearched)
            {
                _valueConverter = (JsonConverter<TValue>)options.GetConverter(typeof(TValue));
                valueConverterSearched = true;
            }

            writer.WriteStartObject();

            foreach (var kvp in dictionary)
            {
                writer.WritePropertyName(kvp.Key.ToString());

                if (_valueConverter != null)
                    _valueConverter.Write(writer, kvp.Value, options);
                else
                    JsonSerializer.Serialize(writer, kvp.Value, options);
            }

            writer.WriteEndObject();
        }
    }

Document Details

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

@tdykstra tdykstra added waiting-on-feedback Waiting for feedback from SMEs before they can be merged and removed ⌚ Not Triaged Not triaged labels Feb 3, 2020
@tdykstra
Copy link
Contributor

tdykstra commented Feb 3, 2020

cc @steveharter

@tdykstra tdykstra added the doc-bug Problem with the content; needs to be fixed [org][type][category] label Feb 3, 2020
@tdykstra tdykstra self-assigned this Feb 3, 2020
@tdykstra tdykstra added the P1 label Feb 4, 2020
@steveharter
Copy link
Member

Yes the published sample may not work in all AOT scenarios, but does support all Enum types globally.

The provided example above works only when closing the generic ahead of time with either applying the [JsonConverter] to each such property or adding all possibilities to the options instance:

    class Program
    {
        static void Main(string[] args)
        {
            POCO poco = JsonSerializer.Deserialize<POCO>(@"{""D"":{""A"" : ""hello""}}");
            Debug.Assert(poco.D[MyEnum.A] == "hello");

            // or

            JsonSerializerOptions options = new JsonSerializerOptions();
            options.Converters.Add(new DictionaryEnumConverter<MyEnum, string>());
            POCO2 poco2 = JsonSerializer.Deserialize<POCO2>(@"{""D"":{""A"" : ""world""}}", options);
            Debug.Assert(poco2.D[MyEnum.A] == "hello");
        }
    }

    public enum MyEnum
    {
        A = 1,
        B = 2
    }

    public class POCO
    {
        [JsonConverter(typeof(DictionaryEnumConverter<MyEnum, string>))]
        public Dictionary<MyEnum, string> D { get; set; }
    }

    public class POCO2
    {
        public Dictionary<MyEnum, string> D { get; set; }
    }

Also note for 5.0, Enums and other number types can be used as keys for dictionaries, so no work-around is needed.

@steveharter
Copy link
Member

@tdykstra we could remove this sample, since it is no longer needed in the 5.0 timeframe since we added support for any TKey.

@softlion can you clarify whether Xamarin or other AOT platform actually breaks with that sample. I'm aware of a Xamarin issue in 3.0\3.01 where the serializer, due to usage of CreateGenericType, had issues but that usage was much more complicated than in this example.

@tdykstra tdykstra removed the waiting-on-feedback Waiting for feedback from SMEs before they can be merged label Sep 22, 2020
@tdykstra tdykstra added needs-more-info Needs more info from OP. Auto-closed after 2 weeks if no response. [org][resolution] and removed P1 doc-bug Problem with the content; needs to be fixed [org][type][category] labels Nov 3, 2020
@no-response
Copy link

no-response bot commented Nov 17, 2020

This issue has been automatically closed due to no response from the original author. Please feel free to reopen it if you have more information that can help us investigate the issue further.

@no-response no-response bot closed this as completed Nov 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dotnet/svc needs-more-info Needs more info from OP. Auto-closed after 2 weeks if no response. [org][resolution]
Projects
None yet
Development

No branches or pull requests

5 participants