ch-robinson / dotnet-avro

An Avro implementation for .NET
https://engineering.chrobinson.com/dotnet-avro/
MIT License
135 stars 51 forks source link

Support dictionary keys of `Enum` type #153

Closed magierska closed 2 years ago

magierska commented 3 years ago

Our model contains dictionary property with enum JobEventType type as a key.

public IDictionary<JobEventType, DateTimeOffset> DictionaryProperty { get; set; }

I tried serializing the model using AsyncSchemaRegistrySerializer with ReflectionResolver and received an exception

No coercion operator is defined between types 'Chr.Vision.Shared.Models.Enums.JobEventType' and 'System.String'.

with call stack:

at System.Linq.Expressions.Expression.GetUserDefinedCoercionOrThrow(ExpressionType coercionType, Expression expression, Type convertToType) at System.Linq.Expressions.Expression.ConvertChecked(Expression expression, Type type, MethodInfo method) at System.Linq.Expressions.Expression.ConvertChecked(Expression expression, Type type) at Chr.Avro.Serialization.BinarySerializerBuilderCase.GenerateConversion(Expression input, Type intermediate)

I've also tried

but unfortunately it throws the same exception.

I suppose the issue lies within Avro's map type requiring key of type string. And the library only supports converting c# enum type to Avro's enum type or underlying number types. Is my assumption correct?

Is there an easy way to alter the library behavior to convert c# enum to Avro's string? I suppose one could create new ResolverCase implementing conversion from enum to string and alter the collection returned here to use the new ResolverCase instead of the default one. Or does the interface provides other solution for altering the ResolverCase collection?

I'm wondering if that's something, that should be handled on the client side, or could that be an improvement to the library to handle enums as dictionary keys?

dstelljes commented 3 years ago

I suppose the issue lies within Avro's map type requiring key of type string. And the library only supports converting c# enum type to Avro's enum type or underlying number types. Is my assumption correct?

Yes, you nailed it. I’m going to tag this as a possible inclusion for 8.x, hopefully prioritized for Q4.

In the meantime, it’d be possible to implement custom cases to accomplish the conversion yourself (see the Extending and overriding built-in features page). A custom serializer builder case for this would look something like:

public class StringEnumSerializerBuilderCase : BinarySerializerBuilderCase
{
    public IBinaryCodec Codec { get; }

    public StringEnumSerializerBuilderCase(IBinaryCodec codec)
    {
        Codec = codec ?? throw new ArgumentNullException(nameof(codec), "Binary codec cannot be null.");
    }

    public override IBinarySerializerBuildResult BuildExpression(Expression value, TypeResolution resolution, Schema schema, IBinarySerializerBuilderContext context)
    {
        var result = new BinarySerializerBuildResult();

        if (schema is StringSchema stringSchema)
        {
            if (resolution is EnumResolution enumResolution)
            {
                var cases = enumResolution.Symbols.Select(symbol =>
                {
                    var bytes = Encoding.UTF8.GetBytes(symbol.Name);

                    return Expression.SwitchCase(
                        Expression.Block(
                            Codec.WriteInteger(Expression.Constant(bytes.LongLength), context.Stream),
                            Codec.Write(Expression.Constant(bytes), context.Stream)),
                        Expression.Constant(symbol.Value));
                });

                var exceptionConstructor = typeof(ArgumentOutOfRangeException)
                    .GetConstructor(new[] { typeof(string) });

                var exception = Expression.New(exceptionConstructor, Expression.Constant("Enum value out of range."));

                result.Expression = Expression.Switch(value, Expression.Throw(exception), cases.ToArray());
            }
            else
            {
                result.Exceptions.Add(new UnsupportedTypeException(resolution.Type));
            }
        }
        else
        {
            result.Exceptions.Add(new UnsupportedSchemaException(schema));
        }

        return result;
    }
}
var codec = new BinaryCodec();
var resolver = new ReflectionResolver();

return new BinarySerializerBuilder(BinarySerializerBuilder.CreateBinarySerializerCaseBuilders(codec)
    .Prepend(builder => new StringEnumSerializerBuilderCase(resolver, codec, builder)))
    .BuildSerializer<T>(schema);

You could do something similar for the deserializing side (and optionally the schema builder if you wanted to generate "string" schemas for enum types.

magierska commented 3 years ago

Thank you for the quick response. We will sort it out in a different way for now, but I'm glad this can be taken into account in the future version.

dstelljes commented 2 years ago

Released in 8.0.0-rc.0.