Browse Source

deduplicate ExtensionSet.TryMergeFieldFrom implementation, add test

Jan Tattermusch 5 years ago
parent
commit
dd97af88db

+ 16 - 0
csharp/src/Google.Protobuf.Test/ExtensionSetTest.cs

@@ -61,6 +61,22 @@ namespace Google.Protobuf
             Assert.AreEqual(message.CalculateSize(), other.CalculateSize());
         }
 
+        [Test]
+        public void TryMergeFieldFrom_CodedInputStream()
+        {
+            var message = new TestAllExtensions();
+            message.SetExtension(OptionalStringExtension, "abcd");
+
+            var input = new CodedInputStream(message.ToByteArray());
+            input.ExtensionRegistry = new ExtensionRegistry() { OptionalStringExtension };
+            input.ReadTag(); // TryMergeFieldFrom expects that a tag was just read and will inspect the LastTag value
+
+            ExtensionSet<TestAllExtensions> extensionSet = null;
+            // test the legacy overload of TryMergeFieldFrom that takes a CodedInputStream
+            Assert.IsTrue(ExtensionSet.TryMergeFieldFrom(ref extensionSet, input));
+            Assert.AreEqual("abcd", ExtensionSet.Get(ref extensionSet, OptionalStringExtension));
+        }
+
         [Test]
         public void TestEquals()
         {

+ 5 - 18
csharp/src/Google.Protobuf/ExtensionSet.cs

@@ -183,26 +183,14 @@ namespace Google.Protobuf
         /// </summary>
         public static bool TryMergeFieldFrom<TTarget>(ref ExtensionSet<TTarget> set, CodedInputStream stream) where TTarget : IExtendableMessage<TTarget>
         {
-            Extension extension;
-            int lastFieldNumber = WireFormat.GetTagFieldNumber(stream.LastTag);
-            
-            IExtensionValue extensionValue;
-            if (set != null && set.ValuesByNumber.TryGetValue(lastFieldNumber, out extensionValue))
+            ParseContext.Initialize(stream, out ParseContext ctx);
+            try
             {
-                extensionValue.MergeFrom(stream);
-                return true;
+                return TryMergeFieldFrom<TTarget>(ref set, ref ctx);
             }
-            else if (stream.ExtensionRegistry != null && stream.ExtensionRegistry.ContainsInputField(stream.LastTag, typeof(TTarget), out extension))
+            finally
             {
-                IExtensionValue value = extension.CreateValue();
-                value.MergeFrom(stream);
-                set = (set ?? new ExtensionSet<TTarget>());
-                set.ValuesByNumber.Add(extension.FieldNumber, value);
-                return true;
-            }
-            else
-            {
-                return false;
+                ctx.CopyStateTo(stream);
             }
         }
 
@@ -212,7 +200,6 @@ namespace Google.Protobuf
         /// </summary>
         public static bool TryMergeFieldFrom<TTarget>(ref ExtensionSet<TTarget> set, ref ParseContext ctx) where TTarget : IExtendableMessage<TTarget>
         {
-            // TODO(jtattermusch): deduplicate code
             Extension extension;
             int lastFieldNumber = WireFormat.GetTagFieldNumber(ctx.LastTag);
 

+ 0 - 15
csharp/src/Google.Protobuf/ExtensionValue.cs

@@ -38,8 +38,6 @@ namespace Google.Protobuf
 {
     internal interface IExtensionValue : IEquatable<IExtensionValue>, IDeepCloneable<IExtensionValue>
     {
-        void MergeFrom(CodedInputStream input);
-
         void MergeFrom(ref ParseContext ctx);
 
         void MergeFrom(IExtensionValue value);
@@ -94,19 +92,6 @@ namespace Google.Protobuf
             }
         }
 
-        public void MergeFrom(CodedInputStream input)
-        {
-            ParseContext.Initialize(input, out ParseContext ctx);
-            try
-            {
-                codec.ValueMerger(ref ctx, ref field);
-            }
-            finally
-            {
-                ctx.CopyStateTo(input);
-            }
-        }
-
         public void MergeFrom(ref ParseContext ctx)
         {
             codec.ValueMerger(ref ctx, ref field);