Merge pull request #8220 from jskeet/bytestrings-with-extensions

Allow FileDescriptors to be parsed with extension registries

csharp/src/Google.Protobuf.Test/ExtensionSetTest.cs

@@ -116,7 +116,22 @@ public void TestClone()
             var other = message.Clone();
 
             Assert.AreEqual(message, other);
-            Assert.AreEqual(message.CalculateSize(), message.CalculateSize());
+            Assert.AreEqual(message.CalculateSize(), other.CalculateSize());
+        }
+
+        [Test]
+        public void TestDefaultValueRoundTrip()
+        {
+            var message = new TestAllExtensions();
+            message.SetExtension(OptionalBoolExtension, false);
+            Assert.IsFalse(message.GetExtension(OptionalBoolExtension));
+            Assert.IsTrue(message.HasExtension(OptionalBoolExtension));
+
+            var bytes = message.ToByteArray();
+            var registry = new ExtensionRegistry { OptionalBoolExtension };
+            var parsed = TestAllExtensions.Parser.WithExtensionRegistry(registry).ParseFrom(bytes);
+            Assert.IsFalse(parsed.GetExtension(OptionalBoolExtension));
+            Assert.IsTrue(parsed.HasExtension(OptionalBoolExtension));
         }
     }
 }

csharp/src/Google.Protobuf.Test/Reflection/DescriptorsTest.cs

@@ -35,7 +35,9 @@
 using ProtobufUnittest;
 using System;
 using System.Collections.Generic;
+using System.IO;
 using System.Linq;
+using UnitTest.Issues.TestProtos;
 
 namespace Google.Protobuf.Reflection
 {
@@ -70,6 +72,24 @@ public void FileDescriptor_BuildFromByteStrings()
             TestFileDescriptor(converted[2], converted[1], converted[0]);
         }
 
+        [Test]
+        public void FileDescriptor_BuildFromByteStrings_WithExtensionRegistry()
+        {
+            var extension = UnittestCustomOptionsProto3Extensions.MessageOpt1;
+
+            var byteStrings = new[]
+            {
+                DescriptorReflection.Descriptor.Proto.ToByteString(),
+                UnittestCustomOptionsProto3Reflection.Descriptor.Proto.ToByteString()
+            };
+            var registry = new ExtensionRegistry { extension };
+
+            var descriptor = FileDescriptor.BuildFromByteStrings(byteStrings, registry).Last();
+            var message = descriptor.MessageTypes.Single(t => t.Name == nameof(TestMessageWithCustomOptions));
+            var extensionValue = message.GetOptions().GetExtension(extension);
+            Assert.AreEqual(-56, extensionValue);
+        }
+
         private void TestFileDescriptor(FileDescriptor file, FileDescriptor importedFile, FileDescriptor importedPublicFile)
         {
             Assert.AreEqual("unittest_proto3.proto", file.Name);

csharp/src/Google.Protobuf/ExtensionValue.cs

@@ -59,7 +59,7 @@ internal ExtensionValue(FieldCodec<T> codec)
 
         public int CalculateSize()
         {
-            return codec.CalculateSizeWithTag(field);
+            return codec.CalculateUnconditionalSizeWithTag(field);
         }
 
         public IExtensionValue Clone()

csharp/src/Google.Protobuf/FieldCodec.cs

@@ -876,6 +876,12 @@ public T Read(ref ParseContext ctx)
         /// </summary>
         public int CalculateSizeWithTag(T value) => IsDefault(value) ? 0 : ValueSizeCalculator(value) + tagSize;
 
+        /// <summary>
+        /// Calculates the size required to write the given value, with a tag, even
+        /// if the value is the default.
+        /// </summary>
+        internal int CalculateUnconditionalSizeWithTag(T value) => ValueSizeCalculator(value) + tagSize;
+
         private bool IsDefault(T value) => EqualityComparer.Equals(value, DefaultValue);
     }
 }

csharp/src/Google.Protobuf/Reflection/FileDescriptor.cs

@@ -481,18 +481,21 @@ private static IEnumerable<Extension> GetAllDependedExtensionsFromMessage(Messag
         /// dependencies must come before the descriptor which depends on them. (If A depends on B, and B
         /// depends on C, then the descriptors must be presented in the order C, B, A.) This is compatible
         /// with the order in which protoc provides descriptors to plugins.</param>
+        /// <param name="registry">The extension registry to use when parsing, or null if no extensions are required.</param>
         /// <returns>The file descriptors corresponding to <paramref name="descriptorData"/>.</returns>
-        public static IReadOnlyList<FileDescriptor> BuildFromByteStrings(IEnumerable<ByteString> descriptorData)
+        public static IReadOnlyList<FileDescriptor> BuildFromByteStrings(IEnumerable<ByteString> descriptorData, ExtensionRegistry registry)
         {
             ProtoPreconditions.CheckNotNull(descriptorData, nameof(descriptorData));
 
+            var parser = FileDescriptorProto.Parser.WithExtensionRegistry(registry);
+
             // TODO: See if we can build a single DescriptorPool instead of building lots of them.
             // This will all behave correctly, but it's less efficient than we'd like.
             var descriptors = new List<FileDescriptor>();
             var descriptorsByName = new Dictionary<string, FileDescriptor>();
             foreach (var data in descriptorData)
             {
-                var proto = FileDescriptorProto.Parser.ParseFrom(data);
+                var proto = parser.ParseFrom(data);
                 var dependencies = new List<FileDescriptor>();
                 foreach (var dependencyName in proto.Dependency)
                 {
@@ -518,6 +521,18 @@ public static IReadOnlyList<FileDescriptor> BuildFromByteStrings(IEnumerable<Byt
             return new ReadOnlyCollection<FileDescriptor>(descriptors);
         }
 
+        /// <summary>
+        /// Converts the given descriptor binary data into FileDescriptor objects.
+        /// Note: reflection using the returned FileDescriptors is not currently supported.
+        /// </summary>
+        /// <param name="descriptorData">The binary file descriptor proto data. Must not be null, and any
+        /// dependencies must come before the descriptor which depends on them. (If A depends on B, and B
+        /// depends on C, then the descriptors must be presented in the order C, B, A.) This is compatible
+        /// with the order in which protoc provides descriptors to plugins.</param>
+        /// <returns>The file descriptors corresponding to <paramref name="descriptorData"/>.</returns>
+        public static IReadOnlyList<FileDescriptor> BuildFromByteStrings(IEnumerable<ByteString> descriptorData) =>
+            BuildFromByteStrings(descriptorData, null);
+
         /// <summary>
         /// Returns a <see cref="System.String" /> that represents this instance.
         /// </summary>