Skip to content

Commit

Permalink
Fix behavior of DictionaryBaseCodec when values are added from constr…
Browse files Browse the repository at this point in the history 
…uctor (dotnet#8993)

(cherry picked from commit f89629b)

# Conflicts:
#	test/Orleans.Serialization.UnitTests/BuiltInCodecTests.cs
  • Loading branch information
@ReubenBond
ReubenBond committed May 10, 2024
1 parent 59861c5 commit ce800f9
Show file tree
Hide file tree
Showing 2 changed files with 62 additions and 13 deletions.
41 changes: 28 additions & 13 deletions src/Orleans.Serialization/Codecs/DictionaryCodec.cs
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
#nullable enable
using System;
using System.Buffers;
using System.Collections.Generic;
using System.Linq;
using System.Reflection;
using Orleans.Serialization.Buffers;
using Orleans.Serialization.Cloning;
Expand All @@ -17,7 +19,7 @@ namespace Orleans.Serialization.Codecs
/// <typeparam name="TKey">The key type.</typeparam>
/// <typeparam name="TValue">The value type.</typeparam>
[RegisterSerializer]
public sealed class DictionaryCodec<TKey, TValue> : IFieldCodec<Dictionary<TKey, TValue>>
public sealed class DictionaryCodec<TKey, TValue> : IFieldCodec<Dictionary<TKey, TValue>> where TKey : notnull
{
private readonly Type _keyFieldType = typeof(TKey);
private readonly Type _valueFieldType = typeof(TValue);
Expand Down Expand Up @@ -83,10 +85,10 @@ public Dictionary<TKey, TValue> ReadValue<TInput>(ref Reader<TInput> reader, Fie
field.EnsureWireTypeTagDelimited();

var placeholderReferenceId = ReferenceCodec.CreateRecordPlaceholder(reader.Session);
TKey key = default;
TKey? key = default;
var valueExpected = false;
Dictionary<TKey, TValue> result = null;
IEqualityComparer<TKey> comparer = null;
Dictionary<TKey, TValue>? result = null;
IEqualityComparer<TKey>? comparer = null;
uint fieldId = 0;
while (true)
{
Expand Down Expand Up @@ -122,7 +124,7 @@ public Dictionary<TKey, TValue> ReadValue<TInput>(ref Reader<TInput> reader, Fie
}
else
{
result.Add(key, _valueCodec.ReadValue(ref reader, header));
result!.Add(key!, _valueCodec.ReadValue(ref reader, header));
valueExpected = false;
}
break;
Expand All @@ -136,7 +138,7 @@ public Dictionary<TKey, TValue> ReadValue<TInput>(ref Reader<TInput> reader, Fie
return result;
}

private Dictionary<TKey, TValue> CreateInstance(int length, IEqualityComparer<TKey> comparer, SerializerSession session, uint placeholderReferenceId)
private Dictionary<TKey, TValue> CreateInstance(int length, IEqualityComparer<TKey>? comparer, SerializerSession session, uint placeholderReferenceId)
{
var result = new Dictionary<TKey, TValue>(length, comparer);
ReferenceCodec.RecordObject(session, result, placeholderReferenceId);
Expand All @@ -155,10 +157,11 @@ private Dictionary<TKey, TValue> CreateInstance(int length, IEqualityComparer<TK
/// <typeparam name="TKey">The type of the t key.</typeparam>
/// <typeparam name="TValue">The type of the t value.</typeparam>
[RegisterCopier]
public sealed class DictionaryCopier<TKey, TValue> : IDeepCopier<Dictionary<TKey, TValue>>, IBaseCopier<Dictionary<TKey, TValue>>
public sealed class DictionaryCopier<TKey, TValue> : IDeepCopier<Dictionary<TKey, TValue>>, IBaseCopier<Dictionary<TKey, TValue>> where TKey : notnull
{
private readonly IDeepCopier<TKey> _keyCopier;
private readonly IDeepCopier<TValue> _valueCopier;
private readonly ConstructorInfo _baseConstructor;

/// <summary>
/// Initializes a new instance of the <see cref="DictionaryCopier{TKey, TValue}"/> class.
Expand All @@ -169,6 +172,7 @@ public DictionaryCopier(IDeepCopier<TKey> keyCopier, IDeepCopier<TValue> valueCo
{
_keyCopier = keyCopier;
_valueCopier = valueCopier;
_baseConstructor = typeof(Dictionary<TKey, TValue>).GetConstructor([typeof(int), typeof(IEqualityComparer<TKey>)])!;
}

/// <inheritdoc/>
Expand Down Expand Up @@ -197,6 +201,12 @@ public Dictionary<TKey, TValue> DeepCopy(Dictionary<TKey, TValue> input, CopyCon
/// <inheritdoc/>
public void DeepCopy(Dictionary<TKey, TValue> input, Dictionary<TKey, TValue> output, CopyContext context)
{
output.Clear();
if (input.Comparer is { } comparer)
{
_baseConstructor.Invoke(output, [input.Count, comparer]);
}

foreach (var pair in input)
{
output[_keyCopier.DeepCopy(pair.Key, context)] = _valueCopier.DeepCopy(pair.Value, context);
Expand All @@ -210,7 +220,7 @@ public void DeepCopy(Dictionary<TKey, TValue> input, Dictionary<TKey, TValue> ou
/// <typeparam name="TKey">The key type.</typeparam>
/// <typeparam name="TValue">The value type.</typeparam>
[RegisterSerializer]
public sealed class DictionaryBaseCodec<TKey, TValue> : IBaseCodec<Dictionary<TKey, TValue>>
public sealed class DictionaryBaseCodec<TKey, TValue> : IBaseCodec<Dictionary<TKey, TValue>> where TKey : notnull
{
private readonly Type _keyFieldType = typeof(TKey);
private readonly Type _valueFieldType = typeof(TValue);
Expand All @@ -234,7 +244,7 @@ public DictionaryBaseCodec(
_keyCodec = OrleansGeneratedCodeHelper.UnwrapService(this, keyCodec);
_valueCodec = OrleansGeneratedCodeHelper.UnwrapService(this, valueCodec);
_comparerCodec = OrleansGeneratedCodeHelper.UnwrapService(this, comparerCodec);
_baseConstructor = typeof(Dictionary<TKey, TValue>).GetConstructor(new Type[] { typeof(int), typeof(IEqualityComparer<TKey>) });
_baseConstructor = typeof(Dictionary<TKey, TValue>).GetConstructor([typeof(int), typeof(IEqualityComparer<TKey>)])!;
}

void IBaseCodec<Dictionary<TKey, TValue>>.Serialize<TBufferWriter>(ref Writer<TBufferWriter> writer, Dictionary<TKey, TValue> value)
Expand All @@ -259,9 +269,13 @@ void IBaseCodec<Dictionary<TKey, TValue>>.Serialize<TBufferWriter>(ref Writer<TB

void IBaseCodec<Dictionary<TKey, TValue>>.Deserialize<TInput>(ref Reader<TInput> reader, Dictionary<TKey, TValue> value)
{
TKey key = default;
// If the dictionary has some values added by the default constructor, clear them.
// If those values are in the serialized payload, they will be added below.
value.Clear();

TKey? key = default;
var valueExpected = false;
IEqualityComparer<TKey> comparer = null;
IEqualityComparer<TKey>? comparer = null;
uint fieldId = 0;
bool hasLengthField = false;
while (true)
Expand All @@ -286,7 +300,8 @@ void IBaseCodec<Dictionary<TKey, TValue>>.Deserialize<TInput>(ref Reader<TInput>
}

hasLengthField = true;
_baseConstructor.Invoke(value, new object[] { length, comparer });
_baseConstructor.Invoke(value, [length, comparer]);

break;
case 2:
if (!hasLengthField)
Expand All @@ -301,7 +316,7 @@ void IBaseCodec<Dictionary<TKey, TValue>>.Deserialize<TInput>(ref Reader<TInput>
}
else
{
value.Add(key, _valueCodec.ReadValue(ref reader, header));
value.Add(key!, _valueCodec.ReadValue(ref reader, header));
valueExpected = false;
}
break;
Expand Down
34 changes: 34 additions & 0 deletions test/Orleans.Serialization.UnitTests/BuiltInCodecTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2154,6 +2154,40 @@ protected override Dictionary<string, int> CreateValue()
protected override bool Equals(Dictionary<string, int> left, Dictionary<string, int> right) => object.ReferenceEquals(left, right) || left.SequenceEqual(right);
}

[GenerateSerializer]
public class TypeWithDictionaryBase : Dictionary<string, int>
{
public TypeWithDictionaryBase() : this(true) { }
public TypeWithDictionaryBase(bool addDefaultValue)
{
if (addDefaultValue)
{
this["key"] = 1;
}
}

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

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

public class DictionaryBaseCodecTests(ITestOutputHelper output) : FieldCodecTester<TypeWithDictionaryBase, IFieldCodec<TypeWithDictionaryBase>>(output)
{
protected override TypeWithDictionaryBase[] TestValues => [null, new(), new(addDefaultValue: false), new() { ["foo"] = 15 }, new() { ["foo"] = 15, OtherProperty = 123 }];

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

public class DictionaryBaseCopierTests(ITestOutputHelper output) : CopierTester<TypeWithDictionaryBase, IDeepCopier<TypeWithDictionaryBase>>(output)
{
protected override TypeWithDictionaryBase[] TestValues => [null, new(), new(addDefaultValue: false), new() { ["foo"] = 15 }, new() { ["foo"] = 15, OtherProperty = 123 }];

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

public class DictionaryWithComparerCodecTests : FieldCodecTester<Dictionary<string, int>, DictionaryCodec<string, int>>
{
protected override int[] MaxSegmentSizes => new[] { 1024 };
Expand Down

0 comments on commit ce800f9

Please sign in to comment.