Skip to content

Commit

Permalink
[Issue 9200] Implement IBaseCodec<T> on CollectionCodec<T> (#9209)
Browse files Browse the repository at this point in the history
* Implement IBaseCodec in CollectionCodec

* use Deserialize() in WriteField

* Add tests and fix base copier

---------

Co-authored-by: Reuben Bond <[email protected]>
  • Loading branch information
Chris-Eckhardt and ReubenBond authored Nov 2, 2024
1 parent 86f0ca5 commit 909769c
Show file tree
Hide file tree
Showing 2 changed files with 105 additions and 11 deletions.
66 changes: 55 additions & 11 deletions src/Orleans.Serialization/Codecs/CollectionCodec.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
using Orleans.Serialization.Cloning;
using Orleans.Serialization.GeneratedCodeHelpers;
using Orleans.Serialization.WireProtocol;
using Orleans.Serialization.Serializers;

namespace Orleans.Serialization.Codecs;

Expand All @@ -15,7 +16,7 @@ namespace Orleans.Serialization.Codecs;
/// </summary>
/// <typeparam name="T">The element type.</typeparam>
[RegisterSerializer]
public sealed class CollectionCodec<T> : IFieldCodec<Collection<T>>
public sealed class CollectionCodec<T> : IFieldCodec<Collection<T>>, IBaseCodec<Collection<T>>
{
private readonly Type CodecElementType = typeof(T);

Expand All @@ -41,16 +42,7 @@ public void WriteField<TBufferWriter>(ref Writer<TBufferWriter> writer, uint fie

writer.WriteFieldHeader(fieldIdDelta, expectedType, value.GetType(), 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;
}
}
Serialize(ref writer, value);

writer.WriteEndObject();
}
Expand Down Expand Up @@ -116,6 +108,56 @@ public Collection<T> ReadValue<TInput>(ref Reader<TInput> reader, Field field)
$"Declared length of {typeof(Collection<T>)}, {length}, is greater than total length of input.");

private void ThrowLengthFieldMissing() => throw new RequiredFieldMissingException("Serialized array is missing its length field.");

public void Serialize<TBufferWriter>(ref Writer<TBufferWriter> writer, Collection<T> value) where TBufferWriter : IBufferWriter<byte>
{
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;
}
}
}

public void Deserialize<TInput>(ref Reader<TInput> reader, Collection<T> value)
{
// If the value has some values added by the constructor, clear them.
// If those values are in the serialized payload, they will be added below.
value.Clear();

uint fieldId = 0;
while (true)
{
var header = reader.ReadFieldHeader();
if (header.IsEndBaseOrEndObject)
{
break;
}

fieldId += header.FieldIdDelta;
switch (fieldId)
{
case 0:
var length = (int)UInt32Codec.ReadValue(ref reader, header);
if (length > 10240 && length > reader.Length)
{
ThrowInvalidSizeException(length);
}

break;
case 1:
value.Add(_fieldCodec.ReadValue(ref reader, header));
break;
default:
reader.ConsumeUnknownField(header);
break;
}
}
}
}

/// <summary>
Expand Down Expand Up @@ -162,6 +204,8 @@ public Collection<T> DeepCopy(Collection<T> input, CopyContext context)
/// <inheritdoc/>
public void DeepCopy(Collection<T> input, Collection<T> output, CopyContext context)
{
output.Clear();

foreach (var item in input)
{
output.Add(_copier.DeepCopy(item, context));
Expand Down
50 changes: 50 additions & 0 deletions test/Orleans.Serialization.UnitTests/BuiltInCodecTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2215,6 +2215,56 @@ protected override Collection<int> CreateValue()
protected override Collection<int>[] TestValues => [null, new Collection<int>(), CreateValue(), CreateValue(), CreateValue()];
}

[GenerateSerializer]
public class TypeWithCollectionBase : Collection<int>
{
public TypeWithCollectionBase() : this(true) { }
public TypeWithCollectionBase(bool addDefaultValue)
{
if (addDefaultValue)
{
this.Add(42);
}
}

[Id(0)]
public int OtherProperty { get; set; }

public override string ToString() => $"[OtherProperty: {OtherProperty}, Values: [{string.Join(", ", this)}]]";
}

public class CollectionBaseCodecTests(ITestOutputHelper output) : FieldCodecTester<TypeWithCollectionBase, IFieldCodec<TypeWithCollectionBase>>(output)
{
private static TypeWithCollectionBase AddValues(TypeWithCollectionBase value)
{
value.Add(1);
value.Add(2);
value.Add(3);
return value;
}

protected override TypeWithCollectionBase[] TestValues => [null, new(), new(addDefaultValue: false), new() { 15 }, AddValues(new() { OtherProperty = 123 })];

protected override TypeWithCollectionBase CreateValue() => AddValues(new() { OtherProperty = Random.Next() });
protected override bool Equals(TypeWithCollectionBase left, TypeWithCollectionBase right) => ReferenceEquals(left, right) || left.SequenceEqual(right) && left.OtherProperty == right.OtherProperty;
}

public class CollectionBaseCopierTests(ITestOutputHelper output) : CopierTester<TypeWithCollectionBase, IDeepCopier<TypeWithCollectionBase>>(output)
{
private static TypeWithCollectionBase AddValues(TypeWithCollectionBase value)
{
value.Add(1);
value.Add(2);
value.Add(3);
return value;
}

protected override TypeWithCollectionBase[] TestValues => [null, new(), new(addDefaultValue: false), new() { 15 }, AddValues(new() { OtherProperty = 123 })];

protected override TypeWithCollectionBase CreateValue() => AddValues(new() { OtherProperty = Random.Next() });
protected override bool Equals(TypeWithCollectionBase left, TypeWithCollectionBase right) => ReferenceEquals(left, right) || left.SequenceEqual(right) && left.OtherProperty == right.OtherProperty;
}

public class ReadOnlyCollectionCodecTests(ITestOutputHelper output) : FieldCodecTester<ReadOnlyCollection<int>, ReadOnlyCollectionCodec<int>>(output)
{
protected override ReadOnlyCollection<int> CreateValue()
Expand Down

0 comments on commit 909769c

Please sign in to comment.