From ef3464dff648362683a75ddf593c9d47ec3c33ce Mon Sep 17 00:00:00 2001 From: Jon Skeet Date: Fri, 10 Jul 2015 14:04:53 +0100 Subject: [PATCH] Oneof reflection support. (Generated code changes in next commit.) --- .../GeneratedMessageTest.cs | 20 +++++++ .../FieldAccess/FieldAccessorTable.cs | 29 +++++++++- .../FieldAccess/OneofAccessor.cs | 57 +++++++++---------- .../FieldAccess/ReflectionUtil.cs | 15 ++++- .../compiler/csharp/csharp_message.cc | 1 + 5 files changed, 87 insertions(+), 35 deletions(-) diff --git a/csharp/src/ProtocolBuffers.Test/GeneratedMessageTest.cs b/csharp/src/ProtocolBuffers.Test/GeneratedMessageTest.cs index 8cee98207a..28c2195f9f 100644 --- a/csharp/src/ProtocolBuffers.Test/GeneratedMessageTest.cs +++ b/csharp/src/ProtocolBuffers.Test/GeneratedMessageTest.cs @@ -734,5 +734,25 @@ namespace Google.Protobuf var message = SampleMessages.CreateFullTestAllTypes(); Assert.Throws(() => message.Fields[TestAllTypes.SingleBoolFieldNumber].GetValue(new TestMap())); } + + [Test] + public void Reflection_Oneof() + { + var message = new TestAllTypes(); + var fields = message.Fields; + Assert.AreEqual(1, fields.Oneofs.Count); + var oneof = fields.Oneofs[0]; + Assert.AreEqual("oneof_field", oneof.Descriptor.Name); + Assert.IsNull(oneof.GetCaseFieldDescriptor(message)); + + message.OneofString = "foo"; + Assert.AreSame(fields[TestAllTypes.OneofStringFieldNumber].Descriptor, oneof.GetCaseFieldDescriptor(message)); + + message.OneofUint32 = 10; + Assert.AreSame(fields[TestAllTypes.OneofUint32FieldNumber].Descriptor, oneof.GetCaseFieldDescriptor(message)); + + oneof.Clear(message); + Assert.AreEqual(TestAllTypes.OneofFieldOneofCase.None, message.OneofFieldCase); + } } } diff --git a/csharp/src/ProtocolBuffers/FieldAccess/FieldAccessorTable.cs b/csharp/src/ProtocolBuffers/FieldAccess/FieldAccessorTable.cs index 57ea9c8768..c69612fb63 100644 --- a/csharp/src/ProtocolBuffers/FieldAccess/FieldAccessorTable.cs +++ b/csharp/src/ProtocolBuffers/FieldAccess/FieldAccessorTable.cs @@ -42,6 +42,7 @@ namespace Google.Protobuf.FieldAccess public sealed class FieldAccessorTable { private readonly ReadOnlyCollection accessors; + private readonly ReadOnlyCollection oneofs; private readonly MessageDescriptor descriptor; /// @@ -51,7 +52,7 @@ namespace Google.Protobuf.FieldAccess /// The CLR type for the message. /// The type's descriptor /// The Pascal-case names of all the field-based properties in the message. - public FieldAccessorTable(Type type, MessageDescriptor descriptor, string[] propertyNames) + public FieldAccessorTable(Type type, MessageDescriptor descriptor, string[] propertyNames, string[] oneofPropertyNames) { this.descriptor = descriptor; var accessorsArray = new IFieldAccessor[descriptor.Fields.Count]; @@ -65,7 +66,13 @@ namespace Google.Protobuf.FieldAccess : (IFieldAccessor) new SingleFieldAccessor(type, name, field); } accessors = new ReadOnlyCollection(accessorsArray); - // TODO(jonskeet): Oneof support + var oneofsArray = new OneofAccessor[descriptor.Oneofs.Count]; + for (int i = 0; i < oneofsArray.Length; i++) + { + var oneof = descriptor.Oneofs[i]; + oneofsArray[i] = new OneofAccessor(type, oneofPropertyNames[i], oneof); + } + oneofs = new ReadOnlyCollection(oneofsArray); } // TODO: Validate the name here... should possibly make this type a more "general reflection access" type, @@ -75,6 +82,10 @@ namespace Google.Protobuf.FieldAccess /// public ReadOnlyCollection Accessors { get { return accessors; } } + public ReadOnlyCollection Oneofs { get { return oneofs; } } + + // TODO: Review the API for the indexers. Now that we have fields and oneofs, it's not as clear... + public IFieldAccessor this[int fieldNumber] { get @@ -84,7 +95,7 @@ namespace Google.Protobuf.FieldAccess } } - internal IFieldAccessor this[FieldDescriptor field] + public IFieldAccessor this[FieldDescriptor field] { get { @@ -95,5 +106,17 @@ namespace Google.Protobuf.FieldAccess return accessors[field.Index]; } } + + public OneofAccessor this[OneofDescriptor oneof] + { + get + { + if (oneof.ContainingType != descriptor) + { + throw new ArgumentException("OneofDescriptor does not match message type."); + } + return oneofs[oneof.Index]; + } + } } } \ No newline at end of file diff --git a/csharp/src/ProtocolBuffers/FieldAccess/OneofAccessor.cs b/csharp/src/ProtocolBuffers/FieldAccess/OneofAccessor.cs index feaa6232d1..590b63097c 100644 --- a/csharp/src/ProtocolBuffers/FieldAccess/OneofAccessor.cs +++ b/csharp/src/ProtocolBuffers/FieldAccess/OneofAccessor.cs @@ -30,62 +30,57 @@ // OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. #endregion +using Google.Protobuf.Descriptors; +using System; +using System.Reflection; + namespace Google.Protobuf.FieldAccess { - // TODO(jonskeet): Add "new" oneof API support - /// - /// Access for an oneof + /// Reflection access for a oneof, allowing clear and "get case" actions. /// - internal class OneofAccessor where TMessage : IMessage + public sealed class OneofAccessor { - /* - private readonly Func caseDelegate; - private readonly Func clearDelegate; - private MessageDescriptor descriptor; + private readonly Func caseDelegate; + private readonly Action clearDelegate; + private OneofDescriptor descriptor; - internal OneofAccessor(MessageDescriptor descriptor, string name) + internal OneofAccessor(Type type, string propertyName, OneofDescriptor descriptor) { - this.descriptor = descriptor; - MethodInfo clearMethod = typeof(TBuilder).GetMethod("Clear" + name); - PropertyInfo caseProperty = typeof(TMessage).GetProperty(name + "Case"); - if (clearMethod == null || caseProperty == null) + PropertyInfo property = type.GetProperty(propertyName + "Case"); + if (property == null || !property.CanRead) { - throw new ArgumentException("Not all required properties/methods available for oneof"); + throw new ArgumentException("Not all required properties/methods available"); } - + this.descriptor = descriptor; + caseDelegate = ReflectionUtil.CreateFuncObjectT(property.GetGetMethod()); - clearDelegate = ReflectionUtil.CreateDelegateFunc(clearMethod); - caseDelegate = ReflectionUtil.CreateUpcastDelegate(caseProperty.GetGetMethod()); + this.descriptor = descriptor; + MethodInfo clearMethod = type.GetMethod("Clear" + propertyName); + clearDelegate = ReflectionUtil.CreateActionObject(clearMethod); } - /// - /// Indicates whether the specified message has set any field in the oneof. - /// - public bool Has(TMessage message) - { - return ((int) caseDelegate(message) != 0); - } + public OneofDescriptor Descriptor { get { return descriptor; } } /// - /// Clears the oneof in the specified builder. + /// Clears the oneof in the specified message. /// - public void Clear(TBuilder builder) + public void Clear(object message) { - clearDelegate(builder); + clearDelegate(message); } /// /// Indicates which field in the oneof is set for specified message /// - public virtual FieldDescriptor GetOneofFieldDescriptor(TMessage message) + public FieldDescriptor GetCaseFieldDescriptor(object message) { - int fieldNumber = (int) caseDelegate(message); + int fieldNumber = caseDelegate(message); if (fieldNumber > 0) { - return descriptor.FindFieldByNumber(fieldNumber); + return descriptor.ContainingType.FindFieldByNumber(fieldNumber); } return null; - }*/ + } } } diff --git a/csharp/src/ProtocolBuffers/FieldAccess/ReflectionUtil.cs b/csharp/src/ProtocolBuffers/FieldAccess/ReflectionUtil.cs index d305392089..08ef6c0c01 100644 --- a/csharp/src/ProtocolBuffers/FieldAccess/ReflectionUtil.cs +++ b/csharp/src/ProtocolBuffers/FieldAccess/ReflectionUtil.cs @@ -63,7 +63,20 @@ namespace Google.Protobuf.FieldAccess Expression upcast = Expression.Convert(call, typeof(object)); return Expression.Lambda>(upcast, parameter).Compile(); } - + + /// + /// Creates a delegate which will cast the argument to the appropriate method target type, + /// call the method on it, then convert the result to the specified type. + /// + internal static Func CreateFuncObjectT(MethodInfo method) + { + ParameterExpression parameter = Expression.Parameter(typeof(object), "p"); + Expression downcast = Expression.Convert(parameter, method.DeclaringType); + Expression call = Expression.Call(downcast, method); + Expression upcast = Expression.Convert(call, typeof(T)); + return Expression.Lambda>(upcast, parameter).Compile(); + } + /// /// Creates a delegate which will execute the given method after casting the first argument to /// the target type of the method, and the second argument to the first parameter type of the method. diff --git a/src/google/protobuf/compiler/csharp/csharp_message.cc b/src/google/protobuf/compiler/csharp/csharp_message.cc index ac13595178..3fbec2b5aa 100644 --- a/src/google/protobuf/compiler/csharp/csharp_message.cc +++ b/src/google/protobuf/compiler/csharp/csharp_message.cc @@ -151,6 +151,7 @@ void MessageGenerator::GenerateStaticVariableInitializers(io::Printer* printer) printer->Print("\"$property_name$\", ", "property_name", GetPropertyName(descriptor_->field(i))); } + printer->Print("}, new string[] { "); for (int i = 0; i < descriptor_->oneof_decl_count(); i++) { printer->Print("\"$oneof_name$\", ", "oneof_name",