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

Collection Expression syntax not compatible with orleans serializer #8934

Open
icanhasjonas opened this issue Apr 4, 2024 · 10 comments
Open
Assignees

Comments

@icanhasjonas
Copy link
Contributor

icanhasjonas commented Apr 4, 2024

The new fancy .NET 8 collection initialization expression uses types that are not compatible with Orleans 8.

I'm posting the issue here first, and a test to illustrate. I'll try to work on a PR tomorrow.

Example

using System.Buffers;
using System.Collections.Generic;
using Microsoft.Extensions.DependencyInjection;
using Orleans;
using Orleans.Serialization;
using Xunit;

public class SerializeCollectionSyntaxTests
{
	[GenerateSerializer]
	[Alias("SerializeCollectionSyntaxTests.A")]
	public class A
	{
		[Id(0)]
		public IReadOnlyList<string> Values { get; set; }
	}

	[Fact]
	public void CanSerializeCollectionExpressions() {
		var a = new A {
			Values = ["a", "b"] // Collection expression initializer syntax
		};

		var services = new ServiceCollection()
			.AddSerializer();
		var serviceProvider = services.BuildServiceProvider();
		var serializer = serviceProvider.GetRequiredService<Serializer>();

		var writer = new ArrayBufferWriter<byte>();
		serializer.Serialize(a, writer);
		var data = writer.WrittenMemory;

		var b = serializer.Deserialize<A>(new ReadOnlySequence<byte>(data));
		Assert.Collection(b.Values, x => Assert.Equal("a", x), x => Assert.Equal("b", x));
	}
}
@icanhasjonas
Copy link
Contributor Author

Adding the expected error here just in case :-)

Orleans.Serialization.CodecNotFoundException: Could not find a codec for type <>z__ReadOnlyArray`1[System.String].

@ReubenBond ReubenBond self-assigned this Apr 4, 2024
@ReubenBond
Copy link
Member

Good find, thank you for reporting. It might be time for us to add serializers for unknown collection types. eg, if a type implements IEnumerable<T> but otherwise has no known serializer, enumerate it to a list and serialize it as such. It would solve the issue with returning LINQ queries, too.

@cbgrasshopper
Copy link

cbgrasshopper commented Apr 12, 2024

We encountered a related issue with code like this:

private readonly List<Problem> _problems = [];

public List<Problem> Problems
{
  get
  {
    return _problems;
  }
  set
  {
    _problems.AddRange(value);
  }
}

public Problem Problem
{
  set => _problems.Add(value);
}

When the setter for Problems is called, _problems is null, so AddRange fails. The issue does not occur if we change the private field declaration to this (the remaining code is unchanged):

private readonly List<Problem> _problems = new();

@ReubenBond
Copy link
Member

That is surprising behavior, @cbgrasshopper. I don't see any serialization attributes in that code, but I assume this error occurs post-deserialization?

@cbgrasshopper
Copy link

@ReubenBond Yes, this is post-deserialization, and yes, this is surprising behavior 🤣 . I didn't include the serialization attributes in my snippet, but they are there as follows:

[GenerateSerializer]
[Alias("CommandOutcome")]
public record CommandOutcome
{
    private readonly List<Problem> _problems = [];

...

    [Id(0)]
    public List<Problem> Problems
    {
        get => _problems;
        set => _problems.AddRange(value);
    }

@cbgrasshopper
Copy link

And if we substitute new() for [], everything works as expected.

@cbgrasshopper
Copy link

Just discovered that we can also resolve it as follows:

[GenerateSerializer]
[Alias("CommandOutcome")]
public record CommandOutcome
{
    [Id(1)]
    private readonly List<Problem> _problems = [];

...

    [Id(0)]
    public List<Problem> Problems
    {
        get => _problems;
        set => _problems.AddRange(value);
    }

In this case, the collection expression works. Very interesting...

@ReubenBond
Copy link
Member

Oh, make sure you only give the field an [Id(...)], and not the property. That is important.
I'm still a little surprised this didn't work, since it looks like your type has a default constructor.

@cbgrasshopper
Copy link

It does have a default constructor.

Oh, make sure you only give the field an [Id(...)], and not the property. That is important.

This seems like a worthwhile addition to the Serialization in Orleans documentation.

@icanhasjonas
Copy link
Contributor Author

This is related - but also pretty much all Frozen collections also are failing on copy/serialization

public Task<IReadOnlySet<string>> ThisWillBreakSerialization() {
  return new HashSet<string> { "Hello World" }.ToFrozenSet();
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants