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

Cannot serialize type inheriting from Dictionary #8989

Closed
defilerc opened this issue May 9, 2024 · 3 comments · Fixed by #8993
Closed

Cannot serialize type inheriting from Dictionary #8989

defilerc opened this issue May 9, 2024 · 3 comments · Fixed by #8993
Assignees

Comments

@defilerc
Copy link

defilerc commented May 9, 2024

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:

  • When the class doesn't inherit from Dictionary<IncidentType, int[]>, but it uses a property of this type, the serialization works as expected. Currently, this change is not trivial for us, though, as our codebase is huge and this is a core object.
  • The problem also persists if the Dictionary has an int type for keys, instead of enum and we set this[1] = new[] { 0, 0 } in the ctor. An additional entry (this[0]) is created, as well.

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

@ReubenBond
Copy link
Member

ReubenBond commented May 10, 2024

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
Copy link
Member

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

@defilerc
Copy link
Author

@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.

@github-actions github-actions bot locked and limited conversation to collaborators Jun 11, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
2 participants