Fix failing FieldMask.Merge for well-known wrapper field types

pull/9977/head
Vassil Kovatchev 3 years ago committed by Jon Skeet
parent 6da9b69efe
commit 9d7a449e5c
  1. 84
      csharp/src/Google.Protobuf.Test/FieldMaskTreeTest.cs
  2. 19
      csharp/src/Google.Protobuf/FieldMaskTree.cs

@ -432,5 +432,89 @@ namespace Google.Protobuf
Assert.IsNotNull(destination.Payload);
}
[Test]
public void MergeWrapperFieldsWithNonNullFieldsInSource()
{
// Instantiate a destination with wrapper-based field types.
var destination = new TestWellKnownTypes()
{
StringField = "Hello",
Int32Field = 12,
Int64Field = 24,
BoolField = true,
};
// Set up a targeted update.
var source = new TestWellKnownTypes()
{
StringField = "Hi",
Int64Field = 240
};
Merge(new FieldMaskTree().AddFieldPath("string_field").AddFieldPath("int64_field"),
source,
destination,
new FieldMask.MergeOptions(),
false);
// Make sure the targeted fields changed.
Assert.AreEqual("Hi", destination.StringField);
Assert.AreEqual(240, destination.Int64Field);
// Prove that non-targeted fields stay intact...
Assert.AreEqual(12, destination.Int32Field);
Assert.IsTrue(destination.BoolField);
// ...including default values which were not explicitly set in the destination object.
Assert.IsNull(destination.FloatField);
}
[Test]
[TestCase(false, "Hello", 24)]
[TestCase(true, null, null)]
public void MergeWrapperFieldsWithNullFieldsInSource(
bool replaceMessageFields,
string expectedStringValue,
long? expectedInt64Value)
{
// Instantiate a destination with wrapper-based field types.
var destination = new TestWellKnownTypes()
{
StringField = "Hello",
Int32Field = 12,
Int64Field = 24,
BoolField = true,
};
// Set up a targeted update with null valued fields.
var source = new TestWellKnownTypes()
{
StringField = null,
Int64Field = null
};
Merge(new FieldMaskTree().AddFieldPath("string_field").AddFieldPath("int64_field"),
source,
destination,
new FieldMask.MergeOptions()
{
ReplaceMessageFields = replaceMessageFields
},
false);
// Make sure the targeted fields changed according to our expectations, depending on the value of ReplaceMessageFields.
// When ReplaceMessageFields is false, the null values are not applied to the destination, because, although wrapped types
// are semantically primitives, FieldMaskTree.Merge still treats them as message types in order to maintain consistency with other Protobuf
// libraries such as Java and C++.
Assert.AreEqual(expectedStringValue, destination.StringField);
Assert.AreEqual(expectedInt64Value, destination.Int64Field);
// Prove that non-targeted fields stay intact...
Assert.AreEqual(12, destination.Int32Field);
Assert.IsTrue(destination.BoolField);
// ...including default values which were not explicitly set in the destination object.
Assert.IsNull(destination.FloatField);
}
}
}

@ -333,15 +333,24 @@ namespace Google.Protobuf
{
if (sourceField != null)
{
var sourceByteString = ((IMessage)sourceField).ToByteString();
var destinationValue = (IMessage)field.Accessor.GetValue(destination);
if (destinationValue != null)
// Well-known wrapper types are represented as nullable primitive types, so we do not "merge" them.
// Instead, any non-null value just overwrites the previous value directly.
if (field.MessageType.IsWrapperType)
{
destinationValue.MergeFrom(sourceByteString);
field.Accessor.SetValue(destination, sourceField);
}
else
{
field.Accessor.SetValue(destination, field.MessageType.Parser.ParseFrom(sourceByteString));
var sourceByteString = ((IMessage)sourceField).ToByteString();
var destinationValue = (IMessage)field.Accessor.GetValue(destination);
if (destinationValue != null)
{
destinationValue.MergeFrom(sourceByteString);
}
else
{
field.Accessor.SetValue(destination, field.MessageType.Parser.ParseFrom(sourceByteString));
}
}
}
}

Loading…
Cancel
Save