From b926a7d2097405fd3be4cc4009d61710562852d7 Mon Sep 17 00:00:00 2001 From: Jon Skeet Date: Thu, 20 Jan 2022 08:39:38 +0000 Subject: [PATCH] Add ToProto() method to all C# descriptor classes Fixes #9425. --- .../Reflection/DescriptorsTest.cs | 24 +++++++++++++++++++ .../Reflection/EnumDescriptor.cs | 8 +++++++ .../Reflection/EnumValueDescriptor.cs | 8 +++++++ .../Reflection/FieldDescriptor.cs | 8 +++++++ .../Reflection/FileDescriptor.cs | 8 +++++++ .../Reflection/MessageDescriptor.cs | 8 +++++++ .../Reflection/MethodDescriptor.cs | 8 +++++++ .../Reflection/OneofDescriptor.cs | 24 +++++++++++++------ .../Reflection/ServiceDescriptor.cs | 8 +++++++ 9 files changed, 97 insertions(+), 7 deletions(-) diff --git a/csharp/src/Google.Protobuf.Test/Reflection/DescriptorsTest.cs b/csharp/src/Google.Protobuf.Test/Reflection/DescriptorsTest.cs index fab983d463517..65c8b8267fbd5 100644 --- a/csharp/src/Google.Protobuf.Test/Reflection/DescriptorsTest.cs +++ b/csharp/src/Google.Protobuf.Test/Reflection/DescriptorsTest.cs @@ -124,6 +124,7 @@ private void TestFileDescriptor(FileDescriptor file, FileDescriptor importedFile } Assert.AreEqual(10, file.SerializedData[0]); + TestDescriptorToProto(file.ToProto, file.Proto); } [Test] @@ -231,6 +232,7 @@ public void MessageDescriptor() { Assert.AreEqual(i, messageType.EnumTypes[i].Index); } + TestDescriptorToProto(messageType.ToProto, messageType.Proto); } [Test] @@ -294,6 +296,11 @@ public void TestFieldDescriptor( // For a field in a regular onoef, ContainingOneof and RealContainingOneof should be the same. Assert.AreEqual("oneof_field", fieldInOneof.ContainingOneof.Name); Assert.AreSame(fieldInOneof.ContainingOneof, fieldInOneof.RealContainingOneof); + + TestDescriptorToProto(primitiveField.ToProto, primitiveField.Proto); + TestDescriptorToProto(enumField.ToProto, enumField.Proto); + TestDescriptorToProto(foreignMessageField.ToProto, foreignMessageField.Proto); + TestDescriptorToProto(fieldInOneof.ToProto, fieldInOneof.Proto); } [Test] @@ -338,6 +345,8 @@ public void EnumDescriptor() { Assert.AreEqual(i, enumType.Values[i].Index); } + TestDescriptorToProto(enumType.ToProto, enumType.Proto); + TestDescriptorToProto(nestedType.ToProto, nestedType.Proto); } [Test] @@ -361,6 +370,7 @@ public void OneofDescriptor() } CollectionAssert.AreEquivalent(expectedFields, descriptor.Fields); + TestDescriptorToProto(descriptor.ToProto, descriptor.Proto); } [Test] @@ -370,6 +380,7 @@ public void MapEntryMessageDescriptor() Assert.IsNull(descriptor.Parser); Assert.IsNull(descriptor.ClrType); Assert.IsNull(descriptor.Fields[1].Accessor); + TestDescriptorToProto(descriptor.ToProto, descriptor.Proto); } // From TestFieldOrdering: @@ -391,6 +402,7 @@ public void DescriptorProtoFileDescriptor() { var descriptor = Google.Protobuf.Reflection.FileDescriptor.DescriptorProtoFileDescriptor; Assert.AreEqual("google/protobuf/descriptor.proto", descriptor.Name); + TestDescriptorToProto(descriptor.ToProto, descriptor.Proto); } [Test] @@ -453,5 +465,17 @@ public void SyntheticOneofReflection() } } } + + private static void TestDescriptorToProto(Func toProtoFunction, IMessage expectedProto) + { + var clone1 = toProtoFunction(); + var clone2 = toProtoFunction(); + Assert.AreNotSame(clone1, clone2); + Assert.AreNotSame(clone1, expectedProto); + Assert.AreNotSame(clone2, expectedProto); + + Assert.AreEqual(clone1, clone2); + Assert.AreEqual(clone1, expectedProto); + } } } diff --git a/csharp/src/Google.Protobuf/Reflection/EnumDescriptor.cs b/csharp/src/Google.Protobuf/Reflection/EnumDescriptor.cs index f7e8b5b5f28db..3f2e1c41f5754 100644 --- a/csharp/src/Google.Protobuf/Reflection/EnumDescriptor.cs +++ b/csharp/src/Google.Protobuf/Reflection/EnumDescriptor.cs @@ -68,6 +68,14 @@ internal EnumDescriptor(EnumDescriptorProto proto, FileDescriptor file, MessageD internal EnumDescriptorProto Proto { get { return proto; } } + /// + /// Returns a clone of the underlying describing this enum. + /// Note that a copy is taken every time this method is called, so clients using it frequently + /// (and not modifying it) may want to cache the returned value. + /// + /// A protobuf representation of this enum descriptor. + public EnumDescriptorProto ToProto() => Proto.Clone(); + /// /// The brief name of the descriptor's target. /// diff --git a/csharp/src/Google.Protobuf/Reflection/EnumValueDescriptor.cs b/csharp/src/Google.Protobuf/Reflection/EnumValueDescriptor.cs index 05097bd1da3d7..50b26a46bb4d7 100644 --- a/csharp/src/Google.Protobuf/Reflection/EnumValueDescriptor.cs +++ b/csharp/src/Google.Protobuf/Reflection/EnumValueDescriptor.cs @@ -55,6 +55,14 @@ internal EnumValueDescriptor(EnumValueDescriptorProto proto, FileDescriptor file internal EnumValueDescriptorProto Proto { get { return proto; } } + /// + /// Returns a clone of the underlying describing this enum value. + /// Note that a copy is taken every time this method is called, so clients using it frequently + /// (and not modifying it) may want to cache the returned value. + /// + /// A protobuf representation of this enum value descriptor. + public EnumValueDescriptorProto ToProto() => Proto.Clone(); + /// /// Returns the name of the enum value described by this object. /// diff --git a/csharp/src/Google.Protobuf/Reflection/FieldDescriptor.cs b/csharp/src/Google.Protobuf/Reflection/FieldDescriptor.cs index f9b90619adb54..aeab6860801b1 100644 --- a/csharp/src/Google.Protobuf/Reflection/FieldDescriptor.cs +++ b/csharp/src/Google.Protobuf/Reflection/FieldDescriptor.cs @@ -91,6 +91,14 @@ public sealed class FieldDescriptor : DescriptorBase, IComparable + /// Returns a clone of the underlying describing this field. + /// Note that a copy is taken every time this method is called, so clients using it frequently + /// (and not modifying it) may want to cache the returned value. + /// + /// A protobuf representation of this field descriptor. + public FieldDescriptorProto ToProto() => Proto.Clone(); + /// /// An extension identifier for this field, or null if this field isn't an extension. /// diff --git a/csharp/src/Google.Protobuf/Reflection/FileDescriptor.cs b/csharp/src/Google.Protobuf/Reflection/FileDescriptor.cs index 724bb3a04dad3..d7701da92e9c1 100644 --- a/csharp/src/Google.Protobuf/Reflection/FileDescriptor.cs +++ b/csharp/src/Google.Protobuf/Reflection/FileDescriptor.cs @@ -249,6 +249,14 @@ private static IList DeterminePublicDependencies(FileDescriptor /// internal FileDescriptorProto Proto { get; } + /// + /// Returns a clone of the underlying describing this file. + /// Note that a copy is taken every time this method is called, so clients using it frequently + /// (and not modifying it) may want to cache the returned value. + /// + /// A protobuf representation of this file descriptor. + public FileDescriptorProto ToProto() => Proto.Clone(); + /// /// The syntax of the file /// diff --git a/csharp/src/Google.Protobuf/Reflection/MessageDescriptor.cs b/csharp/src/Google.Protobuf/Reflection/MessageDescriptor.cs index f56dd893b8b2e..40a6ff832b1ae 100644 --- a/csharp/src/Google.Protobuf/Reflection/MessageDescriptor.cs +++ b/csharp/src/Google.Protobuf/Reflection/MessageDescriptor.cs @@ -151,6 +151,14 @@ internal override IReadOnlyList GetNestedDescriptorListForField( internal DescriptorProto Proto { get; } + /// + /// Returns a clone of the underlying describing this message. + /// Note that a copy is taken every time this method is called, so clients using it frequently + /// (and not modifying it) may want to cache the returned value. + /// + /// A protobuf representation of this message descriptor. + public DescriptorProto ToProto() => Proto.Clone(); + internal bool IsExtensionsInitialized(IMessage message) { if (Proto.ExtensionRange.Count == 0) diff --git a/csharp/src/Google.Protobuf/Reflection/MethodDescriptor.cs b/csharp/src/Google.Protobuf/Reflection/MethodDescriptor.cs index 8e1503767bd76..f5ecf2ea83cfd 100644 --- a/csharp/src/Google.Protobuf/Reflection/MethodDescriptor.cs +++ b/csharp/src/Google.Protobuf/Reflection/MethodDescriptor.cs @@ -114,6 +114,14 @@ internal MethodDescriptor(MethodDescriptorProto proto, FileDescriptor file, internal MethodDescriptorProto Proto { get { return proto; } } + /// + /// Returns a clone of the underlying describing this method. + /// Note that a copy is taken every time this method is called, so clients using it frequently + /// (and not modifying it) may want to cache the returned value. + /// + /// A protobuf representation of this method descriptor. + public MethodDescriptorProto ToProto() => Proto.Clone(); + /// /// The brief name of the descriptor's target. /// diff --git a/csharp/src/Google.Protobuf/Reflection/OneofDescriptor.cs b/csharp/src/Google.Protobuf/Reflection/OneofDescriptor.cs index b41d5205158c8..ae29ded6473bf 100644 --- a/csharp/src/Google.Protobuf/Reflection/OneofDescriptor.cs +++ b/csharp/src/Google.Protobuf/Reflection/OneofDescriptor.cs @@ -45,7 +45,6 @@ namespace Google.Protobuf.Reflection /// public sealed class OneofDescriptor : DescriptorBase { - private readonly OneofDescriptorProto proto; private MessageDescriptor containingType; private IList fields; private readonly OneofAccessor accessor; @@ -53,7 +52,7 @@ public sealed class OneofDescriptor : DescriptorBase internal OneofDescriptor(OneofDescriptorProto proto, FileDescriptor file, MessageDescriptor parent, int index, string clrName) : base(file, file.ComputeFullName(parent, proto.Name), index) { - this.proto = proto; + this.Proto = proto; containingType = parent; file.DescriptorPool.AddSymbol(this); @@ -68,7 +67,18 @@ internal OneofDescriptor(OneofDescriptorProto proto, FileDescriptor file, Messag /// /// The brief name of the descriptor's target. /// - public override string Name { get { return proto.Name; } } + public override string Name => Proto.Name; + + // Visible for testing + internal OneofDescriptorProto Proto { get; } + + /// + /// Returns a clone of the underlying describing this oneof. + /// Note that a copy is taken every time this method is called, so clients using it frequently + /// (and not modifying it) may want to cache the returned value. + /// + /// A protobuf representation of this oneof descriptor. + public OneofDescriptorProto ToProto() => Proto.Clone(); /// /// Gets the message type containing this oneof. @@ -118,7 +128,7 @@ public MessageDescriptor ContainingType /// The (possibly empty) set of custom options for this oneof. /// [Obsolete("CustomOptions are obsolete. Use the GetOptions method.")] - public CustomOptions CustomOptions => new CustomOptions(proto.Options?._extensions?.ValuesByNumber); + public CustomOptions CustomOptions => new CustomOptions(Proto.Options?._extensions?.ValuesByNumber); /// /// The OneofOptions, defined in descriptor.proto. @@ -126,7 +136,7 @@ public MessageDescriptor ContainingType /// Custom options can be retrieved as extensions of the returned message. /// NOTE: A defensive copy is created each time this property is retrieved. /// - public OneofOptions GetOptions() => proto.Options?.Clone(); + public OneofOptions GetOptions() => Proto.Options?.Clone(); /// /// Gets a single value oneof option for this descriptor @@ -134,7 +144,7 @@ public MessageDescriptor ContainingType [Obsolete("GetOption is obsolete. Use the GetOptions() method.")] public T GetOption(Extension extension) { - var value = proto.Options.GetExtension(extension); + var value = Proto.Options.GetExtension(extension); return value is IDeepCloneable ? (value as IDeepCloneable).Clone() : value; } @@ -144,7 +154,7 @@ public T GetOption(Extension extension) [Obsolete("GetOption is obsolete. Use the GetOptions() method.")] public RepeatedField GetOption(RepeatedExtension extension) { - return proto.Options.GetExtension(extension).Clone(); + return Proto.Options.GetExtension(extension).Clone(); } internal void CrossLink() diff --git a/csharp/src/Google.Protobuf/Reflection/ServiceDescriptor.cs b/csharp/src/Google.Protobuf/Reflection/ServiceDescriptor.cs index dab348b6f8ec6..944ea11de14b5 100644 --- a/csharp/src/Google.Protobuf/Reflection/ServiceDescriptor.cs +++ b/csharp/src/Google.Protobuf/Reflection/ServiceDescriptor.cs @@ -73,6 +73,14 @@ internal override IReadOnlyList GetNestedDescriptorListForField( internal ServiceDescriptorProto Proto { get { return proto; } } + /// + /// Returns a clone of the underlying describing this service. + /// Note that a copy is taken every time this method is called, so clients using it frequently + /// (and not modifying it) may want to cache the returned value. + /// + /// A protobuf representation of this service descriptor. + public ServiceDescriptorProto ToProto() => Proto.Clone(); + /// /// An unmodifiable list of methods in this service. ///