ch-robinson / dotnet-avro

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

Support for Nullable Uri #277

Closed kevin-mcmanus closed 9 months ago

kevin-mcmanus commented 9 months ago

I'm trying to serialize/deserialize a class containing a nullable Uri property (Uri?). Everything works as expected when the Uri is populated, but when it is null I get a NullReferenceException.

Looking at StringSerializerBuilderCase, Uri is the only special case that's a reference type, where as the others (DateTime, Guid, etc.) are all structs and behave well.

What is the best way to serialize a null Uri correctly?

This code reproduces the issue:

    public class UnitTest1
    {
        [Fact]
        public void Test1()
        {
            var schemaBuilder = new SchemaBuilder(nullableReferenceTypeBehavior: NullableReferenceTypeBehavior.All);
            var schema = schemaBuilder.BuildSchema<Class1>();
            var serdes = new Chr.Avro.Serialization.BinarySerializerBuilder().BuildDelegate<Class1>(schema);

            using (var stream = new MemoryStream())
            {
                using (var writer = new Chr.Avro.Serialization.BinaryWriter(stream))
                {
                    serdes.Invoke(new Class1(), writer);
                }
            }

            // ...
        }
    }

    public class Class1
    {
        public Uri? Uri { get; set; }
    }

It throws:

System.NullReferenceException
Object reference not set to an instance of an object.
   at Class1 to Class1 serializer(Closure, Class1, BinaryWriter)
   at ChrAvroNullUri.UnitTest1.Test1() in C:\...\UnitTest1.cs:line 22
   at System.RuntimeMethodHandle.InvokeMethod(Object target, Void** arguments, Signature sig, Boolean isConstructor)
   at System.Reflection.MethodInvoker.Invoke(Object obj, IntPtr* args, BindingFlags invokeAttr)
dstelljes commented 9 months ago

Thanks for the detailed reproduction! This turned out to be an issue with the schema builder; a "string" schema was generated for Uri? instead of ["null", "string"]. The fix will ship with the next release.