Support creating FileDescriptors dynamically from binary data.

Related to #658 and #5007.

6 files changed, 168 insertions, 40 deletions

diff --git a/csharp/src/Google.Protobuf.Test/Reflection/DescriptorsTest.cs b/csharp/src/Google.Protobuf.Test/Reflection/DescriptorsTest.cs
index 29a376e0..9abee951 100644
--- a/csharp/src/Google.Protobuf.Test/Reflection/DescriptorsTest.cs
+++ b/csharp/src/Google.Protobuf.Test/Reflection/DescriptorsTest.cs
@@ -30,10 +30,11 @@
 // OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 #endregion
 
-using System.Linq;
 using Google.Protobuf.TestProtos;
 using NUnit.Framework;
-using UnitTest.Issues.TestProtos;
+using System;
+using System.Collections.Generic;
+using System.Linq;
 
 namespace Google.Protobuf.Reflection
 {
@@ -44,9 +45,32 @@ namespace Google.Protobuf.Reflection
     public class DescriptorsTest
     {
         [Test]
-        public void FileDescriptor()
+        public void FileDescriptor_GeneratedCode()
+        {
+            TestFileDescriptor(
+                UnittestProto3Reflection.Descriptor,
+                UnittestImportProto3Reflection.Descriptor,
+                UnittestImportPublicProto3Reflection.Descriptor);
+        }
+
+        [Test]
+        public void FileDescriptor_BuildFromByteStrings()
+        {
+            // The descriptors have to be supplied in an order such that all the
+            // dependencies come before the descriptors depending on them.
+            var descriptorData = new List<ByteString>
+            {
+                UnittestImportPublicProto3Reflection.Descriptor.Proto.ToByteString(),
+                UnittestImportProto3Reflection.Descriptor.Proto.ToByteString(),
+                UnittestProto3Reflection.Descriptor.Proto.ToByteString()
+            };
+            var converted = FileDescriptor.BuildFromByteStrings(descriptorData);
+            Assert.AreEqual(3, converted.Count);
+            TestFileDescriptor(converted[2], converted[1], converted[0]);
+        }
+
+        private void TestFileDescriptor(FileDescriptor file, FileDescriptor importedFile, FileDescriptor importedPublicFile)
         {
-            FileDescriptor file = UnittestProto3Reflection.Descriptor;
 
             Assert.AreEqual("unittest_proto3.proto", file.Name);
             Assert.AreEqual("protobuf_unittest3", file.Package);
@@ -56,17 +80,12 @@ namespace Google.Protobuf.Reflection
 
             // unittest_proto3.proto doesn't have any public imports, but unittest_import_proto3.proto does.
             Assert.AreEqual(0, file.PublicDependencies.Count);
-            Assert.AreEqual(1, UnittestImportProto3Reflection.Descriptor.PublicDependencies.Count);
-            Assert.AreEqual(UnittestImportPublicProto3Reflection.Descriptor, UnittestImportProto3Reflection.Descriptor.PublicDependencies[0]);
+            Assert.AreEqual(1, importedFile.PublicDependencies.Count);
+            Assert.AreEqual(importedPublicFile, importedFile.PublicDependencies[0]);
 
             Assert.AreEqual(1, file.Dependencies.Count);
-            Assert.AreEqual(UnittestImportProto3Reflection.Descriptor, file.Dependencies[0]);
+            Assert.AreEqual(importedFile, file.Dependencies[0]);
 
-            MessageDescriptor messageType = TestAllTypes.Descriptor;
-            Assert.AreSame(typeof(TestAllTypes), messageType.ClrType);
-            Assert.AreSame(TestAllTypes.Parser, messageType.Parser);
-            Assert.AreEqual(messageType, file.MessageTypes[0]);
-            Assert.AreEqual(messageType, file.FindTypeByName<MessageDescriptor>("TestAllTypes"));
             Assert.Null(file.FindTypeByName<MessageDescriptor>("NoSuchType"));
             Assert.Null(file.FindTypeByName<MessageDescriptor>("protobuf_unittest3.TestAllTypes"));
             for (int i = 0; i < file.MessageTypes.Count; i++)
@@ -77,8 +96,8 @@ namespace Google.Protobuf.Reflection
             Assert.AreEqual(file.EnumTypes[0], file.FindTypeByName<EnumDescriptor>("ForeignEnum"));
             Assert.Null(file.FindTypeByName<EnumDescriptor>("NoSuchType"));
             Assert.Null(file.FindTypeByName<EnumDescriptor>("protobuf_unittest3.ForeignEnum"));
-            Assert.AreEqual(1, UnittestImportProto3Reflection.Descriptor.EnumTypes.Count);
-            Assert.AreEqual("ImportEnum", UnittestImportProto3Reflection.Descriptor.EnumTypes[0].Name);
+            Assert.AreEqual(1, importedFile.EnumTypes.Count);
+            Assert.AreEqual("ImportEnum", importedFile.EnumTypes[0].Name);
             for (int i = 0; i < file.EnumTypes.Count; i++)
             {
                 Assert.AreEqual(i, file.EnumTypes[i].Index);
@@ -98,6 +117,56 @@ namespace Google.Protobuf.Reflection
         }
 
         [Test]
+        public void FileDescriptor_BuildFromByteStrings_MissingDependency()
+        {
+            var descriptorData = new List<ByteString>
+            {
+                UnittestImportProto3Reflection.Descriptor.Proto.ToByteString(),
+                UnittestProto3Reflection.Descriptor.Proto.ToByteString(),
+            };
+            // This will fail, because we're missing UnittestImportPublicProto3Reflection
+            Assert.Throws<ArgumentException>(() => FileDescriptor.BuildFromByteStrings(descriptorData));
+        }
+
+        [Test]
+        public void FileDescriptor_BuildFromByteStrings_DuplicateNames()
+        {
+            var descriptorData = new List<ByteString>
+            {
+                UnittestImportPublicProto3Reflection.Descriptor.Proto.ToByteString(),
+                UnittestImportPublicProto3Reflection.Descriptor.Proto.ToByteString(),
+            };
+            // This will fail due to the same name being used twice
+            Assert.Throws<ArgumentException>(() => FileDescriptor.BuildFromByteStrings(descriptorData));
+        }
+
+        [Test]
+        public void FileDescriptor_BuildFromByteStrings_IncorrectOrder()
+        {
+            var descriptorData = new List<ByteString>
+            {
+                UnittestProto3Reflection.Descriptor.Proto.ToByteString(),
+                UnittestImportPublicProto3Reflection.Descriptor.Proto.ToByteString(),
+                UnittestImportProto3Reflection.Descriptor.Proto.ToByteString()
+            };
+            // This will fail, because the dependencies should come first
+            Assert.Throws<ArgumentException>(() => FileDescriptor.BuildFromByteStrings(descriptorData));
+
+        }
+
+        [Test]
+        public void MessageDescriptorFromGeneratedCodeFileDescriptor()
+        {
+            var file = UnittestProto3Reflection.Descriptor;
+
+            MessageDescriptor messageType = TestAllTypes.Descriptor;
+            Assert.AreSame(typeof(TestAllTypes), messageType.ClrType);
+            Assert.AreSame(TestAllTypes.Parser, messageType.Parser);
+            Assert.AreEqual(messageType, file.MessageTypes[0]);
+            Assert.AreEqual(messageType, file.FindTypeByName<MessageDescriptor>("TestAllTypes"));
+        }
+
+        [Test]
         public void MessageDescriptor()
         {
             MessageDescriptor messageType = TestAllTypes.Descriptor;
@@ -163,7 +232,7 @@ namespace Google.Protobuf.Reflection
             
             Assert.AreEqual("single_nested_enum", enumField.Name);
             Assert.AreEqual(FieldType.Enum, enumField.FieldType);
-            // Assert.AreEqual(TestAllTypes.Types.NestedEnum.DescriptorProtoFile, enumField.EnumType);
+            Assert.AreEqual(messageType.EnumTypes[0], enumField.EnumType);
 
             Assert.AreEqual("single_foreign_message", messageField.Name);
             Assert.AreEqual(FieldType.Message, messageField.FieldType);
diff --git a/csharp/src/Google.Protobuf/Reflection/DescriptorPool.cs b/csharp/src/Google.Protobuf/Reflection/DescriptorPool.cs
index 99ca4bf3..9c2160fe 100644
--- a/csharp/src/Google.Protobuf/Reflection/DescriptorPool.cs
+++ b/csharp/src/Google.Protobuf/Reflection/DescriptorPool.cs
@@ -53,13 +53,13 @@ namespace Google.Protobuf.Reflection
 
         private readonly HashSet<FileDescriptor> dependencies;
 
-        internal DescriptorPool(FileDescriptor[] dependencyFiles)
+        internal DescriptorPool(IEnumerable<FileDescriptor> dependencyFiles)
         {
             dependencies = new HashSet<FileDescriptor>();
-            for (int i = 0; i < dependencyFiles.Length; i++)
+            foreach (var dependencyFile in dependencyFiles)
             {
-                dependencies.Add(dependencyFiles[i]);
-                ImportPublicDependencies(dependencyFiles[i]);
+                dependencies.Add(dependencyFile);
+                ImportPublicDependencies(dependencyFile);
             }
 
             foreach (FileDescriptor dependency in dependencyFiles)
diff --git a/csharp/src/Google.Protobuf/Reflection/FieldDescriptor.cs b/csharp/src/Google.Protobuf/Reflection/FieldDescriptor.cs
index 2a3d5c7a..152467d8 100644
--- a/csharp/src/Google.Protobuf/Reflection/FieldDescriptor.cs
+++ b/csharp/src/Google.Protobuf/Reflection/FieldDescriptor.cs
@@ -116,13 +116,18 @@ namespace Google.Protobuf.Reflection
         /// that is the responsibility of the accessor.
         /// </para>
         /// <para>
-        /// The value returned by this property will be non-null for all regular fields. However,
-        /// if a message containing a map field is introspected, the list of nested messages will include
+        /// In descriptors for generated code, the value returned by this property will be non-null for all
+        /// regular fields. However, if a message containing a map field is introspected, the list of nested messages will include
         /// an auto-generated nested key/value pair message for the field. This is not represented in any
         /// generated type, and the value of the map field itself is represented by a dictionary in the
         /// reflection API. There are never instances of those "hidden" messages, so no accessor is provided
         /// and this property will return null.
         /// </para>
+        /// <para>
+        /// In dynamically loaded descriptors, the value returned by this property will current be null;
+        /// if and when dynamic messages are supported, it will return a suitable accessor to work with
+        /// them.
+        /// </para>
         /// </remarks>
         public IFieldAccessor Accessor => accessor;
         
@@ -330,7 +335,8 @@ namespace Google.Protobuf.Reflection
         private IFieldAccessor CreateAccessor()
         {
             // If we're given no property name, that's because we really don't want an accessor.
-            // (At the moment, that means it's a map entry message...)
+            // This could be because it's a map message, or it could be that we're loading a FileDescriptor dynamically.
+            // TODO: Support dynamic messages.
             if (propertyName == null)
             {
                 return null;
diff --git a/csharp/src/Google.Protobuf/Reflection/FileDescriptor.cs b/csharp/src/Google.Protobuf/Reflection/FileDescriptor.cs
index be94cb10..799f7291 100644
--- a/csharp/src/Google.Protobuf/Reflection/FileDescriptor.cs
+++ b/csharp/src/Google.Protobuf/Reflection/FileDescriptor.cs
@@ -34,6 +34,7 @@ using Google.Protobuf.WellKnownTypes;
 using System;
 using System.Collections.Generic;
 using System.Collections.ObjectModel;
+using System.Linq;
 
 namespace Google.Protobuf.Reflection
 {
@@ -54,12 +55,12 @@ namespace Google.Protobuf.Reflection
             ForceReflectionInitialization<Value.KindOneofCase>();
         }
 
-        private FileDescriptor(ByteString descriptorData, FileDescriptorProto proto, FileDescriptor[] dependencies, DescriptorPool pool, bool allowUnknownDependencies, GeneratedClrTypeInfo generatedCodeInfo)
+        private FileDescriptor(ByteString descriptorData, FileDescriptorProto proto, IEnumerable<FileDescriptor> dependencies, DescriptorPool pool, bool allowUnknownDependencies, GeneratedClrTypeInfo generatedCodeInfo)
         {
             SerializedData = descriptorData;
             DescriptorPool = pool;
             Proto = proto;
-            Dependencies = new ReadOnlyCollection<FileDescriptor>((FileDescriptor[]) dependencies.Clone());
+            Dependencies = new ReadOnlyCollection<FileDescriptor>(dependencies.ToList());
 
             PublicDependencies = DeterminePublicDependencies(this, proto, dependencies, allowUnknownDependencies);
 
@@ -67,11 +68,11 @@ namespace Google.Protobuf.Reflection
 
             MessageTypes = DescriptorUtil.ConvertAndMakeReadOnly(proto.MessageType,
                                                                  (message, index) =>
-                                                                 new MessageDescriptor(message, this, null, index, generatedCodeInfo.NestedTypes[index]));
+                                                                 new MessageDescriptor(message, this, null, index, generatedCodeInfo?.NestedTypes[index]));
 
             EnumTypes = DescriptorUtil.ConvertAndMakeReadOnly(proto.EnumType,
                                                               (enumType, index) =>
-                                                              new EnumDescriptor(enumType, this, null, index, generatedCodeInfo.NestedEnums[index]));
+                                                              new EnumDescriptor(enumType, this, null, index, generatedCodeInfo?.NestedEnums[index]));
 
             Services = DescriptorUtil.ConvertAndMakeReadOnly(proto.Service,
                                                              (service, index) =>
@@ -98,13 +99,9 @@ namespace Google.Protobuf.Reflection
         /// Extracts public dependencies from direct dependencies. This is a static method despite its
         /// first parameter, as the value we're in the middle of constructing is only used for exceptions.
         /// </summary>
-        private static IList<FileDescriptor> DeterminePublicDependencies(FileDescriptor @this, FileDescriptorProto proto, FileDescriptor[] dependencies, bool allowUnknownDependencies)
+        private static IList<FileDescriptor> DeterminePublicDependencies(FileDescriptor @this, FileDescriptorProto proto, IEnumerable<FileDescriptor> dependencies, bool allowUnknownDependencies)
         {
-            var nameToFileMap = new Dictionary<string, FileDescriptor>();
-            foreach (var file in dependencies)
-            {
-                nameToFileMap[file.Name] = file;
-            }
+            var nameToFileMap = dependencies.ToDictionary(file => file.Name);
             var publicDependencies = new List<FileDescriptor>();
             for (int i = 0; i < proto.PublicDependency.Count; i++)
             {
@@ -114,8 +111,7 @@ namespace Google.Protobuf.Reflection
                     throw new DescriptorValidationException(@this, "Invalid public dependency index.");
                 }
                 string name = proto.Dependency[index];
-                FileDescriptor file = nameToFileMap[name];
-                if (file == null)
+                if (!nameToFileMap.TryGetValue(name, out var file))
                 {
                     if (!allowUnknownDependencies)
                     {
@@ -316,6 +312,49 @@ namespace Google.Protobuf.Reflection
         }
 
         /// <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)
+        {
+            ProtoPreconditions.CheckNotNull(descriptorData, nameof(descriptorData));
+
+            // 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 dependencies = new List<FileDescriptor>();
+                foreach (var dependencyName in proto.Dependency)
+                {
+                    if (!descriptorsByName.TryGetValue(dependencyName, out var dependency))
+                    {
+                        throw new ArgumentException($"Dependency missing: {dependencyName}");
+                    }
+                    dependencies.Add(dependency);
+                }
+                var pool = new DescriptorPool(dependencies);
+                FileDescriptor descriptor = new FileDescriptor(
+                    data, proto, dependencies, pool,
+                    allowUnknownDependencies: false, generatedCodeInfo: null);
+                descriptors.Add(descriptor);
+                if (descriptorsByName.ContainsKey(descriptor.Name))
+                {
+                    throw new ArgumentException($"Duplicate descriptor name: {descriptor.Name}");
+                }
+                descriptorsByName.Add(descriptor.Name, descriptor);
+            }
+            return new ReadOnlyCollection<FileDescriptor>(descriptors);
+        }
+
+        /// <summary>
         /// Returns a <see cref="System.String" /> that represents this instance.
         /// </summary>
         /// <returns>
diff --git a/csharp/src/Google.Protobuf/Reflection/MessageDescriptor.cs b/csharp/src/Google.Protobuf/Reflection/MessageDescriptor.cs
index 86942acc..dbb6768b 100644
--- a/csharp/src/Google.Protobuf/Reflection/MessageDescriptor.cs
+++ b/csharp/src/Google.Protobuf/Reflection/MessageDescriptor.cs
@@ -72,23 +72,21 @@ namespace Google.Protobuf.Reflection
             ClrType = generatedCodeInfo?.ClrType;
             ContainingType = parent;
 
-            // Note use of generatedCodeInfo. rather than generatedCodeInfo?. here... we don't expect
-            // to see any nested oneofs, types or enums in "not actually generated" code... we do
-            // expect fields though (for map entry messages).
+            // If generatedCodeInfo is null, we just won't generate an accessor for any fields.
             Oneofs = DescriptorUtil.ConvertAndMakeReadOnly(
                 proto.OneofDecl,
                 (oneof, index) =>
-                new OneofDescriptor(oneof, file, this, index, generatedCodeInfo.OneofNames[index]));
+                new OneofDescriptor(oneof, file, this, index, generatedCodeInfo?.OneofNames[index]));
 
             NestedTypes = DescriptorUtil.ConvertAndMakeReadOnly(
                 proto.NestedType,
                 (type, index) =>
-                new MessageDescriptor(type, file, this, index, generatedCodeInfo.NestedTypes[index]));
+                new MessageDescriptor(type, file, this, index, generatedCodeInfo?.NestedTypes[index]));
 
             EnumTypes = DescriptorUtil.ConvertAndMakeReadOnly(
                 proto.EnumType,
                 (type, index) =>
-                new EnumDescriptor(type, file, this, index, generatedCodeInfo.NestedEnums[index]));
+                new EnumDescriptor(type, file, this, index, generatedCodeInfo?.NestedEnums[index]));
 
             fieldsInDeclarationOrder = DescriptorUtil.ConvertAndMakeReadOnly(
                 proto.Field,
diff --git a/csharp/src/Google.Protobuf/Reflection/OneofDescriptor.cs b/csharp/src/Google.Protobuf/Reflection/OneofDescriptor.cs
index 5906c2e3..c026bea6 100644
--- a/csharp/src/Google.Protobuf/Reflection/OneofDescriptor.cs
+++ b/csharp/src/Google.Protobuf/Reflection/OneofDescriptor.cs
@@ -85,6 +85,16 @@ namespace Google.Protobuf.Reflection
         /// Gets an accessor for reflective access to the values associated with the oneof
         /// in a particular message.
         /// </summary>
+        /// <remarks>
+        /// <para>
+        /// In descriptors for generated code, the value returned by this property will always be non-null.
+        /// </para>
+        /// <para>
+        /// In dynamically loaded descriptors, the value returned by this property will current be null;
+        /// if and when dynamic messages are supported, it will return a suitable accessor to work with
+        /// them.
+        /// </para>
+        /// </remarks>
         /// <value>
         /// The accessor used for reflective access.
         /// </value>
@@ -110,6 +120,12 @@ namespace Google.Protobuf.Reflection
 
         private OneofAccessor CreateAccessor(string clrName)
         {
+            // We won't have a CLR name if this is from a dynamically-loaded FileDescriptor.
+            // TODO: Support dynamic messages.
+            if (clrName == null)
+            {
+                return null;
+            }
             var caseProperty = containingType.ClrType.GetProperty(clrName + "Case");
             if (caseProperty == null)
             {