From cbe250591fca9d2e776776be065a72c5550a5556 Mon Sep 17 00:00:00 2001 From: Jon Skeet Date: Tue, 24 Oct 2017 14:14:15 +0100 Subject: Fix merging with message-valued oneof If messages A and B have the same oneof case, which is a message type, and we merge B into A, those sub-messages should be merged. Fixes #3200. Note that I haven't regenerated all the code, as some of the protos have been changed, breaking generation. --- csharp/protos/unittest_issues.proto | 15 + csharp/src/Google.Protobuf.Test/IssuesTest.cs | 14 +- .../TestProtos/UnittestIssues.cs | 357 ++++++++++++++++++++- .../src/Google.Protobuf/WellKnownTypes/Struct.cs | 10 +- 4 files changed, 387 insertions(+), 9 deletions(-) (limited to 'csharp') diff --git a/csharp/protos/unittest_issues.proto b/csharp/protos/unittest_issues.proto index 6c9f7634..332c81dc 100644 --- a/csharp/protos/unittest_issues.proto +++ b/csharp/protos/unittest_issues.proto @@ -124,3 +124,18 @@ message TestJsonName { string description = 2 [json_name = "desc"]; string guid = 3 [json_name = "exid"]; } + +// Issue 3200: When merging two messages which use the same +// oneof case, which is itself a message type, the submessages should +// be merged. +message OneofMerging { + message Nested { + int32 x = 1; + int32 y = 2; + } + + oneof value { + string text = 1; + Nested nested = 2; + } +} \ No newline at end of file diff --git a/csharp/src/Google.Protobuf.Test/IssuesTest.cs b/csharp/src/Google.Protobuf.Test/IssuesTest.cs index a38d6b08..2caf80a9 100644 --- a/csharp/src/Google.Protobuf.Test/IssuesTest.cs +++ b/csharp/src/Google.Protobuf.Test/IssuesTest.cs @@ -33,7 +33,7 @@ using Google.Protobuf.Reflection; using UnitTest.Issues.TestProtos; using NUnit.Framework; - +using static UnitTest.Issues.TestProtos.OneofMerging.Types; namespace Google.Protobuf { @@ -78,5 +78,17 @@ namespace Google.Protobuf Assert.AreEqual("{ \"name\": \"test\", \"desc\": \"test2\", \"exid\": \"test3\" }", JsonFormatter.Default.Format(message)); } + + [Test] + public void OneofMerging() + { + var message1 = new OneofMerging { Nested = new Nested { X = 10 } }; + var message2 = new OneofMerging { Nested = new Nested { Y = 20 } }; + var expected = new OneofMerging { Nested = new Nested { X = 10, Y = 20 } }; + + var merged = message1.Clone(); + merged.MergeFrom(message2); + Assert.AreEqual(expected, merged); + } } } diff --git a/csharp/src/Google.Protobuf.Test/TestProtos/UnittestIssues.cs b/csharp/src/Google.Protobuf.Test/TestProtos/UnittestIssues.cs index 7c0ba8a6..cbf7909c 100644 --- a/csharp/src/Google.Protobuf.Test/TestProtos/UnittestIssues.cs +++ b/csharp/src/Google.Protobuf.Test/TestProtos/UnittestIssues.cs @@ -42,11 +42,14 @@ namespace UnitTest.Issues.TestProtos { "MV9pbnQzMhgFIAEoBUgAEhQKDHBsYWluX3N0cmluZxgBIAEoCRISCghvMl9p", "bnQzMhgGIAEoBUgBEhMKCW8yX3N0cmluZxgDIAEoCUgBQgQKAm8xQgQKAm8y", "IksKDFRlc3RKc29uTmFtZRIMCgRuYW1lGAEgASgJEhkKC2Rlc2NyaXB0aW9u", - "GAIgASgJUgRkZXNjEhIKBGd1aWQYAyABKAlSBGV4aWQqVQoMTmVnYXRpdmVF", - "bnVtEhYKEk5FR0FUSVZFX0VOVU1fWkVSTxAAEhYKCUZpdmVCZWxvdxD7////", - "//////8BEhUKCE1pbnVzT25lEP///////////wEqLgoORGVwcmVjYXRlZEVu", - "dW0SEwoPREVQUkVDQVRFRF9aRVJPEAASBwoDb25lEAFCH0gBqgIaVW5pdFRl", - "c3QuSXNzdWVzLlRlc3RQcm90b3NiBnByb3RvMw==")); + "GAIgASgJUgRkZXNjEhIKBGd1aWQYAyABKAlSBGV4aWQifwoMT25lb2ZNZXJn", + "aW5nEg4KBHRleHQYASABKAlIABI2CgZuZXN0ZWQYAiABKAsyJC51bml0dGVz", + "dF9pc3N1ZXMuT25lb2ZNZXJnaW5nLk5lc3RlZEgAGh4KBk5lc3RlZBIJCgF4", + "GAEgASgFEgkKAXkYAiABKAVCBwoFdmFsdWUqVQoMTmVnYXRpdmVFbnVtEhYK", + "Ek5FR0FUSVZFX0VOVU1fWkVSTxAAEhYKCUZpdmVCZWxvdxD7//////////8B", + "EhUKCE1pbnVzT25lEP///////////wEqLgoORGVwcmVjYXRlZEVudW0SEwoP", + "REVQUkVDQVRFRF9aRVJPEAASBwoDb25lEAFCH0gBqgIaVW5pdFRlc3QuSXNz", + "dWVzLlRlc3RQcm90b3NiBnByb3RvMw==")); descriptor = pbr::FileDescriptor.FromGeneratedCode(descriptorData, new pbr::FileDescriptor[] { }, new pbr::GeneratedClrTypeInfo(new[] {typeof(global::UnitTest.Issues.TestProtos.NegativeEnum), typeof(global::UnitTest.Issues.TestProtos.DeprecatedEnum), }, new pbr::GeneratedClrTypeInfo[] { @@ -57,7 +60,8 @@ namespace UnitTest.Issues.TestProtos { new pbr::GeneratedClrTypeInfo(typeof(global::UnitTest.Issues.TestProtos.ItemField), global::UnitTest.Issues.TestProtos.ItemField.Parser, new[]{ "Item" }, null, null, null), new pbr::GeneratedClrTypeInfo(typeof(global::UnitTest.Issues.TestProtos.ReservedNames), global::UnitTest.Issues.TestProtos.ReservedNames.Parser, new[]{ "Types_", "Descriptor_" }, null, null, new pbr::GeneratedClrTypeInfo[] { new pbr::GeneratedClrTypeInfo(typeof(global::UnitTest.Issues.TestProtos.ReservedNames.Types.SomeNestedType), global::UnitTest.Issues.TestProtos.ReservedNames.Types.SomeNestedType.Parser, null, null, null, null)}), new pbr::GeneratedClrTypeInfo(typeof(global::UnitTest.Issues.TestProtos.TestJsonFieldOrdering), global::UnitTest.Issues.TestProtos.TestJsonFieldOrdering.Parser, new[]{ "PlainInt32", "O1String", "O1Int32", "PlainString", "O2Int32", "O2String" }, new[]{ "O1", "O2" }, null, null), - new pbr::GeneratedClrTypeInfo(typeof(global::UnitTest.Issues.TestProtos.TestJsonName), global::UnitTest.Issues.TestProtos.TestJsonName.Parser, new[]{ "Name", "Description", "Guid" }, null, null, null) + new pbr::GeneratedClrTypeInfo(typeof(global::UnitTest.Issues.TestProtos.TestJsonName), global::UnitTest.Issues.TestProtos.TestJsonName.Parser, new[]{ "Name", "Description", "Guid" }, null, null, null), + new pbr::GeneratedClrTypeInfo(typeof(global::UnitTest.Issues.TestProtos.OneofMerging), global::UnitTest.Issues.TestProtos.OneofMerging.Parser, new[]{ "Text", "Nested" }, new[]{ "Value" }, null, new pbr::GeneratedClrTypeInfo[] { new pbr::GeneratedClrTypeInfo(typeof(global::UnitTest.Issues.TestProtos.OneofMerging.Types.Nested), global::UnitTest.Issues.TestProtos.OneofMerging.Types.Nested.Parser, new[]{ "X", "Y" }, null, null, null)}) })); } #endregion @@ -1729,6 +1733,347 @@ namespace UnitTest.Issues.TestProtos { } + /// + /// Issue 3200: When merging two messages which use the same + /// oneof case, which is itself a message type, the submessages should + /// be merged. + /// + public sealed partial class OneofMerging : pb::IMessage { + private static readonly pb::MessageParser _parser = new pb::MessageParser(() => new OneofMerging()); + [global::System.Diagnostics.DebuggerNonUserCodeAttribute] + public static pb::MessageParser Parser { get { return _parser; } } + + [global::System.Diagnostics.DebuggerNonUserCodeAttribute] + public static pbr::MessageDescriptor Descriptor { + get { return global::UnitTest.Issues.TestProtos.UnittestIssuesReflection.Descriptor.MessageTypes[8]; } + } + + [global::System.Diagnostics.DebuggerNonUserCodeAttribute] + pbr::MessageDescriptor pb::IMessage.Descriptor { + get { return Descriptor; } + } + + [global::System.Diagnostics.DebuggerNonUserCodeAttribute] + public OneofMerging() { + OnConstruction(); + } + + partial void OnConstruction(); + + [global::System.Diagnostics.DebuggerNonUserCodeAttribute] + public OneofMerging(OneofMerging other) : this() { + switch (other.ValueCase) { + case ValueOneofCase.Text: + Text = other.Text; + break; + case ValueOneofCase.Nested: + Nested = other.Nested.Clone(); + break; + } + + } + + [global::System.Diagnostics.DebuggerNonUserCodeAttribute] + public OneofMerging Clone() { + return new OneofMerging(this); + } + + /// Field number for the "text" field. + public const int TextFieldNumber = 1; + [global::System.Diagnostics.DebuggerNonUserCodeAttribute] + public string Text { + get { return valueCase_ == ValueOneofCase.Text ? (string) value_ : ""; } + set { + value_ = pb::ProtoPreconditions.CheckNotNull(value, "value"); + valueCase_ = ValueOneofCase.Text; + } + } + + /// Field number for the "nested" field. + public const int NestedFieldNumber = 2; + [global::System.Diagnostics.DebuggerNonUserCodeAttribute] + public global::UnitTest.Issues.TestProtos.OneofMerging.Types.Nested Nested { + get { return valueCase_ == ValueOneofCase.Nested ? (global::UnitTest.Issues.TestProtos.OneofMerging.Types.Nested) value_ : null; } + set { + value_ = value; + valueCase_ = value == null ? ValueOneofCase.None : ValueOneofCase.Nested; + } + } + + private object value_; + /// Enum of possible cases for the "value" oneof. + public enum ValueOneofCase { + None = 0, + Text = 1, + Nested = 2, + } + private ValueOneofCase valueCase_ = ValueOneofCase.None; + [global::System.Diagnostics.DebuggerNonUserCodeAttribute] + public ValueOneofCase ValueCase { + get { return valueCase_; } + } + + [global::System.Diagnostics.DebuggerNonUserCodeAttribute] + public void ClearValue() { + valueCase_ = ValueOneofCase.None; + value_ = null; + } + + [global::System.Diagnostics.DebuggerNonUserCodeAttribute] + public override bool Equals(object other) { + return Equals(other as OneofMerging); + } + + [global::System.Diagnostics.DebuggerNonUserCodeAttribute] + public bool Equals(OneofMerging other) { + if (ReferenceEquals(other, null)) { + return false; + } + if (ReferenceEquals(other, this)) { + return true; + } + if (Text != other.Text) return false; + if (!object.Equals(Nested, other.Nested)) return false; + if (ValueCase != other.ValueCase) return false; + return true; + } + + [global::System.Diagnostics.DebuggerNonUserCodeAttribute] + public override int GetHashCode() { + int hash = 1; + if (valueCase_ == ValueOneofCase.Text) hash ^= Text.GetHashCode(); + if (valueCase_ == ValueOneofCase.Nested) hash ^= Nested.GetHashCode(); + hash ^= (int) valueCase_; + return hash; + } + + [global::System.Diagnostics.DebuggerNonUserCodeAttribute] + public override string ToString() { + return pb::JsonFormatter.ToDiagnosticString(this); + } + + [global::System.Diagnostics.DebuggerNonUserCodeAttribute] + public void WriteTo(pb::CodedOutputStream output) { + if (valueCase_ == ValueOneofCase.Text) { + output.WriteRawTag(10); + output.WriteString(Text); + } + if (valueCase_ == ValueOneofCase.Nested) { + output.WriteRawTag(18); + output.WriteMessage(Nested); + } + } + + [global::System.Diagnostics.DebuggerNonUserCodeAttribute] + public int CalculateSize() { + int size = 0; + if (valueCase_ == ValueOneofCase.Text) { + size += 1 + pb::CodedOutputStream.ComputeStringSize(Text); + } + if (valueCase_ == ValueOneofCase.Nested) { + size += 1 + pb::CodedOutputStream.ComputeMessageSize(Nested); + } + return size; + } + + [global::System.Diagnostics.DebuggerNonUserCodeAttribute] + public void MergeFrom(OneofMerging other) { + if (other == null) { + return; + } + switch (other.ValueCase) { + case ValueOneofCase.Text: + Text = other.Text; + break; + case ValueOneofCase.Nested: + if (Nested == null) { + Nested = new global::UnitTest.Issues.TestProtos.OneofMerging.Types.Nested(); + } + Nested.MergeFrom(other.Nested); + break; + } + + } + + [global::System.Diagnostics.DebuggerNonUserCodeAttribute] + public void MergeFrom(pb::CodedInputStream input) { + uint tag; + while ((tag = input.ReadTag()) != 0) { + switch(tag) { + default: + input.SkipLastField(); + break; + case 10: { + Text = input.ReadString(); + break; + } + case 18: { + global::UnitTest.Issues.TestProtos.OneofMerging.Types.Nested subBuilder = new global::UnitTest.Issues.TestProtos.OneofMerging.Types.Nested(); + if (valueCase_ == ValueOneofCase.Nested) { + subBuilder.MergeFrom(Nested); + } + input.ReadMessage(subBuilder); + Nested = subBuilder; + break; + } + } + } + } + + #region Nested types + /// Container for nested types declared in the OneofMerging message type. + [global::System.Diagnostics.DebuggerNonUserCodeAttribute] + public static partial class Types { + public sealed partial class Nested : pb::IMessage { + private static readonly pb::MessageParser _parser = new pb::MessageParser(() => new Nested()); + [global::System.Diagnostics.DebuggerNonUserCodeAttribute] + public static pb::MessageParser Parser { get { return _parser; } } + + [global::System.Diagnostics.DebuggerNonUserCodeAttribute] + public static pbr::MessageDescriptor Descriptor { + get { return global::UnitTest.Issues.TestProtos.OneofMerging.Descriptor.NestedTypes[0]; } + } + + [global::System.Diagnostics.DebuggerNonUserCodeAttribute] + pbr::MessageDescriptor pb::IMessage.Descriptor { + get { return Descriptor; } + } + + [global::System.Diagnostics.DebuggerNonUserCodeAttribute] + public Nested() { + OnConstruction(); + } + + partial void OnConstruction(); + + [global::System.Diagnostics.DebuggerNonUserCodeAttribute] + public Nested(Nested other) : this() { + x_ = other.x_; + y_ = other.y_; + } + + [global::System.Diagnostics.DebuggerNonUserCodeAttribute] + public Nested Clone() { + return new Nested(this); + } + + /// Field number for the "x" field. + public const int XFieldNumber = 1; + private int x_; + [global::System.Diagnostics.DebuggerNonUserCodeAttribute] + public int X { + get { return x_; } + set { + x_ = value; + } + } + + /// Field number for the "y" field. + public const int YFieldNumber = 2; + private int y_; + [global::System.Diagnostics.DebuggerNonUserCodeAttribute] + public int Y { + get { return y_; } + set { + y_ = value; + } + } + + [global::System.Diagnostics.DebuggerNonUserCodeAttribute] + public override bool Equals(object other) { + return Equals(other as Nested); + } + + [global::System.Diagnostics.DebuggerNonUserCodeAttribute] + public bool Equals(Nested other) { + if (ReferenceEquals(other, null)) { + return false; + } + if (ReferenceEquals(other, this)) { + return true; + } + if (X != other.X) return false; + if (Y != other.Y) return false; + return true; + } + + [global::System.Diagnostics.DebuggerNonUserCodeAttribute] + public override int GetHashCode() { + int hash = 1; + if (X != 0) hash ^= X.GetHashCode(); + if (Y != 0) hash ^= Y.GetHashCode(); + return hash; + } + + [global::System.Diagnostics.DebuggerNonUserCodeAttribute] + public override string ToString() { + return pb::JsonFormatter.ToDiagnosticString(this); + } + + [global::System.Diagnostics.DebuggerNonUserCodeAttribute] + public void WriteTo(pb::CodedOutputStream output) { + if (X != 0) { + output.WriteRawTag(8); + output.WriteInt32(X); + } + if (Y != 0) { + output.WriteRawTag(16); + output.WriteInt32(Y); + } + } + + [global::System.Diagnostics.DebuggerNonUserCodeAttribute] + public int CalculateSize() { + int size = 0; + if (X != 0) { + size += 1 + pb::CodedOutputStream.ComputeInt32Size(X); + } + if (Y != 0) { + size += 1 + pb::CodedOutputStream.ComputeInt32Size(Y); + } + return size; + } + + [global::System.Diagnostics.DebuggerNonUserCodeAttribute] + public void MergeFrom(Nested other) { + if (other == null) { + return; + } + if (other.X != 0) { + X = other.X; + } + if (other.Y != 0) { + Y = other.Y; + } + } + + [global::System.Diagnostics.DebuggerNonUserCodeAttribute] + public void MergeFrom(pb::CodedInputStream input) { + uint tag; + while ((tag = input.ReadTag()) != 0) { + switch(tag) { + default: + input.SkipLastField(); + break; + case 8: { + X = input.ReadInt32(); + break; + } + case 16: { + Y = input.ReadInt32(); + break; + } + } + } + } + + } + + } + #endregion + + } + #endregion } diff --git a/csharp/src/Google.Protobuf/WellKnownTypes/Struct.cs b/csharp/src/Google.Protobuf/WellKnownTypes/Struct.cs index 1fa35521..65119894 100644 --- a/csharp/src/Google.Protobuf/WellKnownTypes/Struct.cs +++ b/csharp/src/Google.Protobuf/WellKnownTypes/Struct.cs @@ -466,10 +466,16 @@ namespace Google.Protobuf.WellKnownTypes { BoolValue = other.BoolValue; break; case KindOneofCase.StructValue: - StructValue = other.StructValue; + if (StructValue == null) { + StructValue = new global::Google.Protobuf.WellKnownTypes.Struct(); + } + StructValue.MergeFrom(other.StructValue); break; case KindOneofCase.ListValue: - ListValue = other.ListValue; + if (ListValue == null) { + ListValue = new global::Google.Protobuf.WellKnownTypes.ListValue(); + } + ListValue.MergeFrom(other.ListValue); break; } -- cgit v1.2.3