dotnet / orleans

Cloud Native application framework for .NET
https://docs.microsoft.com/dotnet/orleans
MIT License
9.89k stars 2.01k forks source link

Cannot serialize type inheriting from Dictionary #8989

Closed defilerc closed 3 weeks ago

defilerc commented 3 weeks ago

We have a type that inherits from a Dictionary<IncidentType, int[]> (simplified version follows):

[GenerateSerializer]
public class MatchStatistics : Dictionary<IncidentType, int[]>
{
    public MatchStatistics()
    {
        this[IncidentType.Score] = new[] { 0, 0 };
    }
}

public enum IncidentType { Unknown = 0, Score = 1 }

When trying to serialize/deserialize it and assert the two objects are equal, the test fails, as one additional Dictionary entry (with a key value of 0) seems to be created by the serializer:

Unhandled exception. NUnit.Framework.AssertionException:   Assert.That(actual, Is.EquivalentTo(expected))
  Expected: equivalent to < [Score, < 0, 0 >] >
  But was:  < [Unknown, null], [Score, < 0, 0 >] >
  Extra (1): < [Unknown, null] >

We are currently trying to migrate to Orleans v7.2.6 (from v3.5.0), but the issue is also present in v8.1.0. I have also created a small repro project to assist you: orleans-serialization.zip

SDK versions used: v7.0.408 (but it's also reproducible with v8.0.204)

Additional Info:

cc: @ReubenBond does this seem like a bug to you or are we missing something?

ReubenBond commented 3 weeks ago

This does seem like a bug to me. Thanks for reporting. I'm investigating. For now, here are some workarounds:

  1. Make the constructor private and add a static method to create instances. This hides the ctor from the serializer.
using Orleans;
using Orleans.Serialization;
using Microsoft.Extensions.DependencyInjection;
using System.Collections.Generic;
using NUnit.Framework.Legacy;

var serviceProvider = new ServiceCollection()
    .AddSerializer()
    .BuildServiceProvider();

var serializer = serviceProvider.GetRequiredService<Serializer>();

var instance = MatchStatistics.Create();
var bytes = serializer.SerializeToArray(instance);
var deserialized = serializer.Deserialize<MatchStatistics>(bytes);

CollectionAssert.AreEquivalent(instance, deserialized);

[GenerateSerializer]
public class MatchStatistics : Dictionary<IncidentType, int[]>
{
    public static MatchStatistics Create() => new();

    private MatchStatistics()
    {
        this[IncidentType.Score] = new[] { 0, 0 };
    }
}

[GenerateSerializer]
public enum IncidentType { Unknown = 0, Score = 1 }
  1. Define a custom activator for the MatchStatistics class which avoids the dictionary mutation and add the [UseActivator] attribute to the MatchStatistics class
using Orleans;
using Orleans.Serialization;
using Microsoft.Extensions.DependencyInjection;
using System.Collections.Generic;
using NUnit.Framework.Legacy;
using Orleans.Serialization.Activators;

var serviceProvider = new ServiceCollection()
    .AddSerializer()
    .BuildServiceProvider();

var serializer = serviceProvider.GetRequiredService<Serializer>();

var instance = new MatchStatistics();
var bytes = serializer.SerializeToArray(instance);
var deserialized = serializer.Deserialize<MatchStatistics>(bytes);

CollectionAssert.AreEquivalent(instance, deserialized);

[GenerateSerializer]
[UseActivator]
public class MatchStatistics : Dictionary<IncidentType, int[]>
{
    public MatchStatistics() : this(true)
    {
    }

    internal MatchStatistics(bool addDefaults)
    {
        if (addDefaults)
        {
            this[IncidentType.Score] = new[] { 0, 0 };
        }
    }
}

[RegisterActivator]
public sealed class MatchStatisticsActivator : IActivator<MatchStatistics>
{
    public MatchStatistics Create() => new MatchStatistics(addDefaults: false);
}

[GenerateSerializer]
public enum IncidentType { Unknown = 0, Score = 1 }

Option 1 is less code. Option 2 does not require you to change call sites or break binary compatibility.

ReubenBond commented 3 weeks ago

@defilerc I've opened #8993 with a fix. I assume you will need this backported to v7.x

defilerc commented 3 weeks ago

@ReubenBond thx a lot for your prompt reply and fix! If that's possible, that would be great, as we're migrating to Orleans v7.x at the moment and will migrate to .NET8/Orleans 8.X at a later stage.