orleans icon indicating copy to clipboard operation
orleans copied to clipboard

Collection Expression syntax not compatible with orleans serializer

Open icanhasjonas opened this issue 1 year ago • 12 comments

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 avatar Apr 04 '24 04:04 icanhasjonas

Adding the expected error here just in case :-)

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

icanhasjonas avatar Apr 04 '24 04:04 icanhasjonas

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.

ReubenBond avatar Apr 04 '24 14:04 ReubenBond

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();

cbgrasshopper avatar Apr 12 '24 21:04 cbgrasshopper

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

ReubenBond avatar Apr 12 '24 21:04 ReubenBond

@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 avatar Apr 12 '24 21:04 cbgrasshopper

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

cbgrasshopper avatar Apr 12 '24 21:04 cbgrasshopper

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

cbgrasshopper avatar Apr 12 '24 21:04 cbgrasshopper

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.

ReubenBond avatar Apr 12 '24 21:04 ReubenBond

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.

cbgrasshopper avatar Apr 12 '24 21:04 cbgrasshopper

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();
}

icanhasjonas avatar Apr 27 '24 05:04 icanhasjonas

Drop in Workaround

... add codec to your service collection...

.AddSingleton<ISpecializableCodec, GeneratedArrayExpressionCodec>()
public class GeneratedArrayExpressionCodec(IServiceProvider services, ICodecProvider codecProvider) : ISpecializableCodec
{
	private sealed class InnerCodec<T> : IFieldCodec<IReadOnlyList<T>>
	{
		private readonly IFieldCodec<T> _fieldCodec;
		private readonly Type _codecElementType = typeof(T);

		public InnerCodec(IFieldCodec<T> fieldCodec) {
			_fieldCodec = OrleansGeneratedCodeHelper.UnwrapService(this, fieldCodec);
		}

		public void WriteField<TBufferWriter>(ref Writer<TBufferWriter> writer, uint fieldIdDelta, Type expectedType, IReadOnlyList<T> value) where TBufferWriter : IBufferWriter<byte> {
			if( ReferenceCodec.TryWriteReferenceField(ref writer, fieldIdDelta, expectedType, value) ) {
				return;
			}

			writer.WriteFieldHeader(fieldIdDelta, expectedType, _codecElementType.MakeArrayType(), WireType.TagDelimited);

			if( value.Count > 0 ) {
				UInt32Codec.WriteField(ref writer, 0, (uint)value.Count);
				uint innerFieldIdDelta = 1;
				foreach( var element in value ) {
					_fieldCodec.WriteField(ref writer, innerFieldIdDelta, _codecElementType, element);
					innerFieldIdDelta = 0;
				}
			}

			writer.WriteEndObject();
		}

		public IReadOnlyList<T> ReadValue<TInput>(ref Reader<TInput> reader, Field field) {
			// No need, the ArrayCodec will take care of reading :-)
			throw new NotImplementedException();
		}
	}


	public bool IsSupportedType(Type type) =>
		type.Name.StartsWith("<>z", StringComparison.Ordinal) &&
		type.GetCustomAttribute<CompilerGeneratedAttribute>() is not null &&
		type.IsGenericType &&
		type.GetGenericArguments() is [var elementType] &&
		type.IsAssignableTo(typeof(IReadOnlyList<>).MakeGenericType(elementType));


	public IFieldCodec GetSpecializedCodec(Type type) {
		if( type.GetGenericArguments() is [var elementType] ) {
			var codecType = typeof(InnerCodec<>).MakeGenericType(elementType);
			var codec = Activator.CreateInstance(codecType, codecProvider.GetCodec(elementType));
			return (IFieldCodec)codec;
		}

		throw new NotSupportedException();
	}
}

icanhasjonas avatar Nov 18 '24 14:11 icanhasjonas