From 78ea98f56f7a0a028e378aee6394549707e199bc Mon Sep 17 00:00:00 2001 From: Jon Skeet Date: Wed, 1 Jul 2015 14:47:03 +0100 Subject: Implement reflection properly for fields. - FieldAccessorTable is now non-generic - We don't have a static field per message type in the umbrella class. (Message descriptors are accessed via the file descriptor.) - Removed the "descriptor assigner" complication from the descriptor fixup; without extensions, we don't need it - MapField implements IDictionary (more tests would be good...) - RepeatedField implements IList (more tests would be good) - Use expression trees to build accessors. (Will need to test this on various platforms... probably need a fallback strategy just using reflection directly.) - Added FieldDescriptor.IsMap - Added tests for reflection with generated messages Changes to generated code coming in next commit. --- .../ProtocolBuffers.Test/GeneratedMessageTest.cs | 141 ++++++++++++++++++++- csharp/src/ProtocolBuffers/Collections/MapField.cs | 69 +++++++++- .../ProtocolBuffers/Collections/RepeatedField.cs | 63 ++++++++- .../ProtocolBuffers/Descriptors/FieldDescriptor.cs | 5 + .../ProtocolBuffers/Descriptors/FileDescriptor.cs | 33 +---- .../FieldAccess/FieldAccessorBase.cs | 23 ++-- .../FieldAccess/FieldAccessorTable.cs | 32 +++-- .../ProtocolBuffers/FieldAccess/IFieldAccessor.cs | 32 ++--- .../FieldAccess/MapFieldAccessor.cs | 59 +++++++++ .../ProtocolBuffers/FieldAccess/ReflectionUtil.cs | 110 ++++------------ .../FieldAccess/RepeatedFieldAccessor.cs | 17 +-- .../FieldAccess/SingleFieldAccessor.cs | 64 +++------- csharp/src/ProtocolBuffers/IMessage.cs | 7 +- csharp/src/ProtocolBuffers/ProtocolBuffers.csproj | 1 + 14 files changed, 436 insertions(+), 220 deletions(-) create mode 100644 csharp/src/ProtocolBuffers/FieldAccess/MapFieldAccessor.cs (limited to 'csharp') diff --git a/csharp/src/ProtocolBuffers.Test/GeneratedMessageTest.cs b/csharp/src/ProtocolBuffers.Test/GeneratedMessageTest.cs index e98ffabc..8c9e0514 100644 --- a/csharp/src/ProtocolBuffers.Test/GeneratedMessageTest.cs +++ b/csharp/src/ProtocolBuffers.Test/GeneratedMessageTest.cs @@ -29,11 +29,13 @@ // (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE // OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. #endregion - + using System; using System.IO; using Google.Protobuf.TestProtos; using NUnit.Framework; +using System.Collections; +using System.Collections.Generic; namespace Google.Protobuf { @@ -595,5 +597,142 @@ namespace Google.Protobuf Assert.AreEqual(message, message2); Assert.AreEqual(TestAllTypes.OneofFieldOneofCase.OneofUint32, message2.OneofFieldCase); } + + // TODO: Consider moving these tests to a separate reflection test - although they do require generated messages. + + [Test] + public void Reflection_GetValue() + { + var message = GetSampleMessage(); + var fields = message.Fields; + Assert.AreEqual(message.SingleBool, fields[TestAllTypes.SingleBoolFieldNumber].GetValue(message)); + Assert.AreEqual(message.SingleBytes, fields[TestAllTypes.SingleBytesFieldNumber].GetValue(message)); + Assert.AreEqual(message.SingleDouble, fields[TestAllTypes.SingleDoubleFieldNumber].GetValue(message)); + Assert.AreEqual(message.SingleFixed32, fields[TestAllTypes.SingleFixed32FieldNumber].GetValue(message)); + Assert.AreEqual(message.SingleFixed64, fields[TestAllTypes.SingleFixed64FieldNumber].GetValue(message)); + Assert.AreEqual(message.SingleFloat, fields[TestAllTypes.SingleFloatFieldNumber].GetValue(message)); + Assert.AreEqual(message.SingleForeignEnum, fields[TestAllTypes.SingleForeignEnumFieldNumber].GetValue(message)); + Assert.AreEqual(message.SingleForeignMessage, fields[TestAllTypes.SingleForeignMessageFieldNumber].GetValue(message)); + Assert.AreEqual(message.SingleImportEnum, fields[TestAllTypes.SingleImportEnumFieldNumber].GetValue(message)); + Assert.AreEqual(message.SingleImportMessage, fields[TestAllTypes.SingleImportMessageFieldNumber].GetValue(message)); + Assert.AreEqual(message.SingleInt32, fields[TestAllTypes.SingleInt32FieldNumber].GetValue(message)); + Assert.AreEqual(message.SingleInt64, fields[TestAllTypes.SingleInt64FieldNumber].GetValue(message)); + Assert.AreEqual(message.SingleNestedEnum, fields[TestAllTypes.SingleNestedEnumFieldNumber].GetValue(message)); + Assert.AreEqual(message.SingleNestedMessage, fields[TestAllTypes.SingleNestedMessageFieldNumber].GetValue(message)); + Assert.AreEqual(message.SinglePublicImportMessage, fields[TestAllTypes.SinglePublicImportMessageFieldNumber].GetValue(message)); + Assert.AreEqual(message.SingleSint32, fields[TestAllTypes.SingleSint32FieldNumber].GetValue(message)); + Assert.AreEqual(message.SingleSint64, fields[TestAllTypes.SingleSint64FieldNumber].GetValue(message)); + Assert.AreEqual(message.SingleString, fields[TestAllTypes.SingleStringFieldNumber].GetValue(message)); + Assert.AreEqual(message.SingleSfixed32, fields[TestAllTypes.SingleSfixed32FieldNumber].GetValue(message)); + Assert.AreEqual(message.SingleSfixed64, fields[TestAllTypes.SingleSfixed64FieldNumber].GetValue(message)); + Assert.AreEqual(message.SingleUint32, fields[TestAllTypes.SingleUint32FieldNumber].GetValue(message)); + Assert.AreEqual(message.SingleUint64, fields[TestAllTypes.SingleUint64FieldNumber].GetValue(message)); + Assert.AreEqual(message.OneofBytes, fields[TestAllTypes.OneofBytesFieldNumber].GetValue(message)); + Assert.AreEqual(message.OneofString, fields[TestAllTypes.OneofStringFieldNumber].GetValue(message)); + Assert.AreEqual(message.OneofNestedMessage, fields[TestAllTypes.OneofNestedMessageFieldNumber].GetValue(message)); + Assert.AreEqual(message.OneofUint32, fields[TestAllTypes.OneofUint32FieldNumber].GetValue(message)); + + // Just one example for repeated fields - they're all just returning the list + var list = (IList)fields[TestAllTypes.RepeatedInt32FieldNumber].GetValue(message); + Assert.AreEqual(message.RepeatedInt32, list); + Assert.AreEqual(message.RepeatedInt32[0], list[0]); // Just in case there was any doubt... + + // Just a single map field, for the same reason + var mapMessage = new TestMap { MapStringString = { { "key1", "value1" }, { "key2", "value2" } } }; + var dictionary = (IDictionary)mapMessage.Fields[TestMap.MapStringStringFieldNumber].GetValue(mapMessage); + Assert.AreEqual(mapMessage.MapStringString, dictionary); + Assert.AreEqual("value1", dictionary["key1"]); + } + + [Test] + public void Reflection_Clear() + { + var message = GetSampleMessage(); + var fields = message.Fields; + fields[TestAllTypes.SingleBoolFieldNumber].Clear(message); + fields[TestAllTypes.SingleInt32FieldNumber].Clear(message); + fields[TestAllTypes.SingleStringFieldNumber].Clear(message); + fields[TestAllTypes.SingleBytesFieldNumber].Clear(message); + fields[TestAllTypes.SingleForeignEnumFieldNumber].Clear(message); + fields[TestAllTypes.SingleForeignMessageFieldNumber].Clear(message); + fields[TestAllTypes.RepeatedDoubleFieldNumber].Clear(message); + + var expected = new TestAllTypes(GetSampleMessage()) + { + SingleBool = false, + SingleInt32 = 0, + SingleString = "", + SingleBytes = ByteString.Empty, + SingleForeignEnum = 0, + SingleForeignMessage = null, + }; + expected.RepeatedDouble.Clear(); + + Assert.AreEqual(expected, message); + + // Separately, maps. + var mapMessage = new TestMap { MapStringString = { { "key1", "value1" }, { "key2", "value2" } } }; + mapMessage.Fields[TestMap.MapStringStringFieldNumber].Clear(mapMessage); + Assert.AreEqual(0, mapMessage.MapStringString.Count); + } + + [Test] + public void Reflection_SetValue_SingleFields() + { + // Just a sample (primitives, messages, enums, strings, byte strings) + var message = GetSampleMessage(); + var fields = message.Fields; + fields[TestAllTypes.SingleBoolFieldNumber].SetValue(message, false); + fields[TestAllTypes.SingleInt32FieldNumber].SetValue(message, 500); + fields[TestAllTypes.SingleStringFieldNumber].SetValue(message, "It's a string"); + fields[TestAllTypes.SingleBytesFieldNumber].SetValue(message, ByteString.CopyFrom(99, 98, 97)); + fields[TestAllTypes.SingleForeignEnumFieldNumber].SetValue(message, ForeignEnum.FOREIGN_FOO); + fields[TestAllTypes.SingleForeignMessageFieldNumber].SetValue(message, new ForeignMessage { C = 12345 }); + fields[TestAllTypes.SingleDoubleFieldNumber].SetValue(message, 20150701.5); + + var expected = new TestAllTypes(GetSampleMessage()) + { + SingleBool = false, + SingleInt32 = 500, + SingleString = "It's a string", + SingleBytes = ByteString.CopyFrom(99, 98, 97), + SingleForeignEnum = ForeignEnum.FOREIGN_FOO, + SingleForeignMessage = new ForeignMessage { C = 12345 }, + SingleDouble = 20150701.5 + }; + + Assert.AreEqual(expected, message); + } + + [Test] + public void Reflection_SetValue_SingleFields_WrongType() + { + var message = GetSampleMessage(); + var fields = message.Fields; + Assert.Throws(() => fields[TestAllTypes.SingleBoolFieldNumber].SetValue(message, "This isn't a bool")); + } + + [Test] + public void Reflection_SetValue_MapFields() + { + var message = new TestMap(); + var fields = message.Fields; + Assert.Throws(() => fields[TestMap.MapStringStringFieldNumber].SetValue(message, new Dictionary())); + } + + [Test] + public void Reflection_SetValue_RepeatedFields() + { + var message = GetSampleMessage(); + var fields = message.Fields; + Assert.Throws(() => fields[TestAllTypes.RepeatedDoubleFieldNumber].SetValue(message, new double[10])); + } + + [Test] + public void Reflection_GetValue_IncorrectType() + { + var message = GetSampleMessage(); + Assert.Throws(() => message.Fields[TestAllTypes.SingleBoolFieldNumber].GetValue(new TestMap())); + } } } diff --git a/csharp/src/ProtocolBuffers/Collections/MapField.cs b/csharp/src/ProtocolBuffers/Collections/MapField.cs index 6d1097a6..779ff061 100644 --- a/csharp/src/ProtocolBuffers/Collections/MapField.cs +++ b/csharp/src/ProtocolBuffers/Collections/MapField.cs @@ -48,7 +48,7 @@ namespace Google.Protobuf.Collections /// /// Key type in the map. Must be a type supported by Protocol Buffer map keys. /// Value type in the map. Must be a type supported by Protocol Buffers. - public sealed class MapField : IDeepCloneable>, IFreezable, IDictionary, IEquatable> + public sealed class MapField : IDeepCloneable>, IFreezable, IDictionary, IEquatable>, IDictionary { // TODO: Don't create the map/list until we have an entry. (Assume many maps will be empty.) private bool frozen; @@ -64,7 +64,7 @@ namespace Google.Protobuf.Collections { foreach (var pair in list) { - clone.Add(pair.Key, pair.Value == null ? pair.Value : ((IDeepCloneable) pair.Value).Clone()); + clone.Add(pair.Key, pair.Value == null ? pair.Value : ((IDeepCloneable)pair.Value).Clone()); } } else @@ -309,7 +309,7 @@ namespace Google.Protobuf.Collections /// /// Stream to read from /// Codec describing how the key/value pairs are encoded - public void AddEntriesFrom(CodedInputStream input, Codec codec) + public void AddEntriesFrom(CodedInputStream input, Codec codec) { var adapter = new Codec.MessageAdapter(codec); do @@ -318,7 +318,7 @@ namespace Google.Protobuf.Collections input.ReadMessage(adapter); this[adapter.Key] = adapter.Value; } while (input.MaybeConsumeTag(codec.MapTag)); - } + } public void WriteTo(CodedOutputStream output, Codec codec) { @@ -350,6 +350,67 @@ namespace Google.Protobuf.Collections return size; } + #region IDictionary explicit interface implementation + void IDictionary.Add(object key, object value) + { + Add((TKey)key, (TValue)value); + } + + bool IDictionary.Contains(object key) + { + if (!(key is TKey)) + { + return false; + } + return ContainsKey((TKey)key); + } + + IDictionaryEnumerator IDictionary.GetEnumerator() + { + throw new NotImplementedException(); + } + + void IDictionary.Remove(object key) + { + if (!(key is TKey)) + { + return; + } + Remove((TKey)key); + } + + void ICollection.CopyTo(Array array, int index) + { + throw new NotImplementedException(); + } + + bool IDictionary.IsFixedSize { get { return IsFrozen; } } + + ICollection IDictionary.Keys { get { return (ICollection)Keys; } } + + ICollection IDictionary.Values { get { return (ICollection)Values; } } + + bool ICollection.IsSynchronized { get { return false; } } + + object ICollection.SyncRoot { get { return null; } } + + object IDictionary.this[object key] + { + get + { + if (!(key is TKey)) + { + return null; + } + TValue value; + TryGetValue((TKey)key, out value); + return value; + } + + set { this[(TKey)key] = (TValue)value; } + } + #endregion + /// /// A codec for a specific map field. This contains all the information required to encoded and /// decode the nested messages. diff --git a/csharp/src/ProtocolBuffers/Collections/RepeatedField.cs b/csharp/src/ProtocolBuffers/Collections/RepeatedField.cs index ed311494..ebc711de 100644 --- a/csharp/src/ProtocolBuffers/Collections/RepeatedField.cs +++ b/csharp/src/ProtocolBuffers/Collections/RepeatedField.cs @@ -41,7 +41,7 @@ namespace Google.Protobuf.Collections /// restrictions (no null values) and capabilities (deep cloning and freezing). /// /// The element type of the repeated field. - public sealed class RepeatedField : IList, IDeepCloneable>, IEquatable>, IFreezable + public sealed class RepeatedField : IList, IList, IDeepCloneable>, IEquatable>, IFreezable { private static readonly T[] EmptyArray = new T[0]; @@ -415,7 +415,66 @@ namespace Google.Protobuf.Collections array[index] = value; } } - + + #region Explicit interface implementation for IList and ICollection. + bool IList.IsFixedSize { get { return IsFrozen; } } + + void ICollection.CopyTo(Array array, int index) + { + ThrowHelper.ThrowIfNull(array, "array"); + T[] strongArray = array as T[]; + if (strongArray == null) + { + throw new ArgumentException("Array is of incorrect type", "array"); + } + CopyTo(strongArray, index); + } + + bool ICollection.IsSynchronized { get { return false; } } + + object ICollection.SyncRoot { get { return null; } } + + object IList.this[int index] + { + get { return this[index]; } + set { this[index] = (T)value; } + } + + int IList.Add(object value) + { + Add((T) value); + return count - 1; + } + + bool IList.Contains(object value) + { + return (value is T && Contains((T)value)); + } + + int IList.IndexOf(object value) + { + if (!(value is T)) + { + return -1; + } + return IndexOf((T)value); + } + + void IList.Insert(int index, object value) + { + Insert(index, (T) value); + } + + void IList.Remove(object value) + { + if (!(value is T)) + { + return; + } + Remove((T)value); + } + #endregion + public struct Enumerator : IEnumerator { private int index; diff --git a/csharp/src/ProtocolBuffers/Descriptors/FieldDescriptor.cs b/csharp/src/ProtocolBuffers/Descriptors/FieldDescriptor.cs index 2f2c5806..3b36a280 100644 --- a/csharp/src/ProtocolBuffers/Descriptors/FieldDescriptor.cs +++ b/csharp/src/ProtocolBuffers/Descriptors/FieldDescriptor.cs @@ -131,6 +131,11 @@ namespace Google.Protobuf.Descriptors get { return Proto.Label == FieldDescriptorProto.Types.Label.LABEL_REPEATED; } } + public bool IsMap + { + get { return fieldType == FieldType.Message && messageType.Options != null && messageType.Options.MapEntry; } + } + public bool IsPacked { get { return Proto.Options.Packed; } diff --git a/csharp/src/ProtocolBuffers/Descriptors/FileDescriptor.cs b/csharp/src/ProtocolBuffers/Descriptors/FileDescriptor.cs index 7da14a54..a6320a31 100644 --- a/csharp/src/ProtocolBuffers/Descriptors/FileDescriptor.cs +++ b/csharp/src/ProtocolBuffers/Descriptors/FileDescriptor.cs @@ -336,33 +336,8 @@ namespace Google.Protobuf.Descriptors } } - /// - /// This method is to be called by generated code only. It is equivalent - /// to BuildFrom except that the FileDescriptorProto is encoded in - /// protocol buffer wire format. This overload is maintained for backward - /// compatibility with source code generated before the custom options were available - /// (and working). - /// - public static FileDescriptor InternalBuildGeneratedFileFrom(byte[] descriptorData, FileDescriptor[] dependencies) - { - return InternalBuildGeneratedFileFrom(descriptorData, dependencies, x => { }); - } - - /// - /// This delegate should be used by generated code only. When calling - /// FileDescriptor.InternalBuildGeneratedFileFrom, the caller can provide - /// a callback which assigns the global variables defined in the generated code - /// which point at parts of the FileDescriptor. The callback returns an - /// Extension Registry which contains any extensions which might be used in - /// the descriptor - that is, extensions of the various "Options" messages defined - /// in descriptor.proto. The callback may also return null to indicate that - /// no extensions are used in the descriptor. - /// - public delegate void InternalDescriptorAssigner(FileDescriptor descriptor); - public static FileDescriptor InternalBuildGeneratedFileFrom(byte[] descriptorData, - FileDescriptor[] dependencies, - InternalDescriptorAssigner descriptorAssigner) + FileDescriptor[] dependencies) { FileDescriptorProto proto; try @@ -374,20 +349,16 @@ namespace Google.Protobuf.Descriptors throw new ArgumentException("Failed to parse protocol buffer descriptor for generated code.", e); } - FileDescriptor result; try { // When building descriptors for generated code, we allow unknown // dependencies by default. - result = BuildFrom(proto, dependencies, true); + return BuildFrom(proto, dependencies, true); } catch (DescriptorValidationException e) { throw new ArgumentException("Invalid embedded descriptor for \"" + proto.Name + "\".", e); } - - descriptorAssigner(result); - return result; } public override string ToString() diff --git a/csharp/src/ProtocolBuffers/FieldAccess/FieldAccessorBase.cs b/csharp/src/ProtocolBuffers/FieldAccess/FieldAccessorBase.cs index 73d777b2..2a3e5b8b 100644 --- a/csharp/src/ProtocolBuffers/FieldAccess/FieldAccessorBase.cs +++ b/csharp/src/ProtocolBuffers/FieldAccess/FieldAccessorBase.cs @@ -32,34 +32,37 @@ using System; using System.Reflection; +using Google.Protobuf.Descriptors; namespace Google.Protobuf.FieldAccess { /// /// Base class for field accessors. /// - /// Type of message containing the field - internal abstract class FieldAccessorBase : IFieldAccessor where T : IMessage + internal abstract class FieldAccessorBase : IFieldAccessor { - private readonly Func getValueDelegate; + private readonly Func getValueDelegate; + private readonly FieldDescriptor descriptor; - internal FieldAccessorBase(string name) + internal FieldAccessorBase(Type type, string propertyName, FieldDescriptor descriptor) { - PropertyInfo property = typeof(T).GetProperty(name); + PropertyInfo property = type.GetProperty(propertyName); if (property == null || !property.CanRead) { throw new ArgumentException("Not all required properties/methods available"); } - getValueDelegate = ReflectionUtil.CreateUpcastDelegate(property.GetGetMethod()); + this.descriptor = descriptor; + getValueDelegate = ReflectionUtil.CreateFuncObjectObject(property.GetGetMethod()); } - public object GetValue(T message) + public FieldDescriptor Descriptor { get { return descriptor; } } + + public object GetValue(object message) { return getValueDelegate(message); } - public abstract bool HasValue(T message); - public abstract void Clear(T message); - public abstract void SetValue(T message, object value); + public abstract void Clear(object message); + public abstract void SetValue(object message, object value); } } diff --git a/csharp/src/ProtocolBuffers/FieldAccess/FieldAccessorTable.cs b/csharp/src/ProtocolBuffers/FieldAccess/FieldAccessorTable.cs index 6379ff25..57ea9c87 100644 --- a/csharp/src/ProtocolBuffers/FieldAccess/FieldAccessorTable.cs +++ b/csharp/src/ProtocolBuffers/FieldAccess/FieldAccessorTable.cs @@ -31,6 +31,7 @@ #endregion using System; +using System.Collections.ObjectModel; using Google.Protobuf.Descriptors; namespace Google.Protobuf.FieldAccess @@ -38,34 +39,43 @@ namespace Google.Protobuf.FieldAccess /// /// Provides access to fields in generated messages via reflection. /// - public sealed class FieldAccessorTable where T : IMessage + public sealed class FieldAccessorTable { - private readonly IFieldAccessor[] accessors; + private readonly ReadOnlyCollection accessors; private readonly MessageDescriptor descriptor; /// /// Constructs a FieldAccessorTable for a particular message class. /// Only one FieldAccessorTable should be constructed per class. /// + /// 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(MessageDescriptor descriptor, string[] propertyNames) + public FieldAccessorTable(Type type, MessageDescriptor descriptor, string[] propertyNames) { this.descriptor = descriptor; - accessors = new IFieldAccessor[descriptor.Fields.Count]; - bool supportFieldPresence = descriptor.File.Syntax == FileDescriptor.ProtoSyntax.Proto2; - for (int i = 0; i < accessors.Length; i++) + var accessorsArray = new IFieldAccessor[descriptor.Fields.Count]; + for (int i = 0; i < accessorsArray.Length; i++) { var field = descriptor.Fields[i]; var name = propertyNames[i]; - accessors[i] = field.IsRepeated - ? (IFieldAccessor) new RepeatedFieldAccessor(propertyNames[i]) - : new SingleFieldAccessor(field, name, supportFieldPresence); + accessorsArray[i] = + field.IsMap ? new MapFieldAccessor(type, name, field) + : field.IsRepeated ? new RepeatedFieldAccessor(type, name, field) + : (IFieldAccessor) new SingleFieldAccessor(type, name, field); } + accessors = new ReadOnlyCollection(accessorsArray); // TODO(jonskeet): Oneof support } - internal IFieldAccessor this[int fieldNumber] + // TODO: Validate the name here... should possibly make this type a more "general reflection access" type, + // bearing in mind the oneof parts to come as well. + /// + /// Returns all of the field accessors for the message type. + /// + public ReadOnlyCollection Accessors { get { return accessors; } } + + public IFieldAccessor this[int fieldNumber] { get { @@ -74,7 +84,7 @@ namespace Google.Protobuf.FieldAccess } } - internal IFieldAccessor this[FieldDescriptor field] + internal IFieldAccessor this[FieldDescriptor field] { get { diff --git a/csharp/src/ProtocolBuffers/FieldAccess/IFieldAccessor.cs b/csharp/src/ProtocolBuffers/FieldAccess/IFieldAccessor.cs index 61838543..77e7146d 100644 --- a/csharp/src/ProtocolBuffers/FieldAccess/IFieldAccessor.cs +++ b/csharp/src/ProtocolBuffers/FieldAccess/IFieldAccessor.cs @@ -30,39 +30,41 @@ // OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. #endregion +using Google.Protobuf.Descriptors; + namespace Google.Protobuf.FieldAccess { /// - /// Allows fields to be reflectively accessed in a smart manner. - /// The property descriptors for each field are created once and then cached. - /// In addition, this interface holds knowledge of repeated fields, builders etc. + /// Allows fields to be reflectively accessed. /// - internal interface IFieldAccessor where T : IMessage + public interface IFieldAccessor { /// - /// Indicates whether the specified message contains the field. For primitive fields - /// declared in proto3-syntax messages, this simply checks whether the value is the default one. + /// Returns the descriptor associated with this field. /// - /// The field is a repeated field, or a single primitive field. - bool HasValue(T message); + FieldDescriptor Descriptor { get; } /// /// Clears the field in the specified message. (For repeated fields, /// this clears the list.) /// - void Clear(T message); + void Clear(object message); /// /// Fetches the field value. For repeated values, this will be an - /// implementation. + /// implementation. For map values, this will be an + /// implementation. /// - object GetValue(T message); + object GetValue(object message); /// - /// Mutator for single fields only. (Repeated fields must be mutated - /// by fetching the list, then mutating that.) + /// Mutator for single "simple" fields only. /// - /// The field is a repeated field. - void SetValue(T message, object value); + /// + /// Repeated fields are mutated by fetching the value and manipulating it as a list. + /// Map fields are mutated by fetching the value and manipulating it as a dictionary. + /// + /// The field is not a "simple" field, or the message is frozen. + void SetValue(object message, object value); } } \ No newline at end of file diff --git a/csharp/src/ProtocolBuffers/FieldAccess/MapFieldAccessor.cs b/csharp/src/ProtocolBuffers/FieldAccess/MapFieldAccessor.cs new file mode 100644 index 00000000..100dbb37 --- /dev/null +++ b/csharp/src/ProtocolBuffers/FieldAccess/MapFieldAccessor.cs @@ -0,0 +1,59 @@ +#region Copyright notice and license +// Protocol Buffers - Google's data interchange format +// Copyright 2015 Google Inc. All rights reserved. +// https://developers.google.com/protocol-buffers/ +// +// Redistribution and use in source and binary forms, with or without +// modification, are permitted provided that the following conditions are +// met: +// +// * Redistributions of source code must retain the above copyright +// notice, this list of conditions and the following disclaimer. +// * Redistributions in binary form must reproduce the above +// copyright notice, this list of conditions and the following disclaimer +// in the documentation and/or other materials provided with the +// distribution. +// * Neither the name of Google Inc. nor the names of its +// contributors may be used to endorse or promote products derived from +// this software without specific prior written permission. +// +// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. +#endregion + +using System; +using System.Collections; +using Google.Protobuf.Descriptors; + +namespace Google.Protobuf.FieldAccess +{ + /// + /// Accessor for map fields. + /// + internal sealed class MapFieldAccessor : FieldAccessorBase + { + internal MapFieldAccessor(Type type, string propertyName, FieldDescriptor descriptor) : base(type, propertyName, descriptor) + { + } + + public override void Clear(object message) + { + IDictionary list = (IDictionary) GetValue(message); + list.Clear(); + } + + public override void SetValue(object message, object value) + { + throw new InvalidOperationException("SetValue is not implemented for map fields"); + } + } +} diff --git a/csharp/src/ProtocolBuffers/FieldAccess/ReflectionUtil.cs b/csharp/src/ProtocolBuffers/FieldAccess/ReflectionUtil.cs index 29399b0c..d3053920 100644 --- a/csharp/src/ProtocolBuffers/FieldAccess/ReflectionUtil.cs +++ b/csharp/src/ProtocolBuffers/FieldAccess/ReflectionUtil.cs @@ -31,6 +31,7 @@ #endregion using System; +using System.Linq.Expressions; using System.Reflection; namespace Google.Protobuf.FieldAccess @@ -51,101 +52,42 @@ namespace Google.Protobuf.FieldAccess internal static readonly Type[] EmptyTypes = new Type[0]; /// - /// Creates a delegate which will execute the given method and then return - /// the result as an object. + /// Creates a delegate which will cast the argument to the appropriate method target type, + /// call the method on it, then convert the result to object. /// - public static Func CreateUpcastDelegate(MethodInfo method) + internal static Func CreateFuncObjectObject(MethodInfo method) { - // The tricky bit is invoking CreateCreateUpcastDelegateImpl with the right type parameters - MethodInfo openImpl = typeof(ReflectionUtil).GetMethod("CreateUpcastDelegateImpl"); - MethodInfo closedImpl = openImpl.MakeGenericMethod(typeof(T), method.ReturnType); - return (Func) closedImpl.Invoke(null, new object[] {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(object)); + return Expression.Lambda>(upcast, parameter).Compile(); } - + /// - /// Method used solely for implementing CreateUpcastDelegate. Public to avoid trust issues - /// in low-trust scenarios. + /// 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. /// - public static Func CreateUpcastDelegateImpl(MethodInfo method) + internal static Action CreateActionObjectObject(MethodInfo method) { - // Convert the reflection call into an open delegate, i.e. instead of calling x.Method() - // we'll call getter(x). - Func getter = ReflectionUtil.CreateDelegateFunc(method); - - // Implicit upcast to object (within the delegate) - return source => getter(source); + ParameterExpression targetParameter = Expression.Parameter(typeof(object), "target"); + ParameterExpression argParameter = Expression.Parameter(typeof(object), "arg"); + Expression castTarget = Expression.Convert(targetParameter, method.DeclaringType); + Expression castArgument = Expression.Convert(argParameter, method.GetParameters()[0].ParameterType); + Expression call = Expression.Call(castTarget, method, castArgument); + return Expression.Lambda>(call, targetParameter, argParameter).Compile(); } /// - /// Creates a delegate which will execute the given method after casting the parameter - /// down from object to the required parameter type. + /// Creates a delegate which will execute the given method after casting the first argument to + /// the target type of the method. /// - public static Action CreateDowncastDelegate(MethodInfo method) - { - MethodInfo openImpl = typeof(ReflectionUtil).GetMethod("CreateDowncastDelegateImpl"); - MethodInfo closedImpl = openImpl.MakeGenericMethod(typeof(T), method.GetParameters()[0].ParameterType); - return (Action) closedImpl.Invoke(null, new object[] {method}); - } - - public static Action CreateDowncastDelegateImpl(MethodInfo method) - { - // Convert the reflection call into an open delegate, i.e. instead of calling x.Method(y) we'll - // call Method(x, y) - Action call = ReflectionUtil.CreateDelegateAction(method); - - return (source, parameter) => call(source, (TParam) parameter); - } - - /// - /// Creates a delegate which will execute the given method after casting the parameter - /// down from object to the required parameter type. - /// - public static Action CreateDowncastDelegateIgnoringReturn(MethodInfo method) - { - MethodInfo openImpl = typeof(ReflectionUtil).GetMethod("CreateDowncastDelegateIgnoringReturnImpl"); - MethodInfo closedImpl = openImpl.MakeGenericMethod(typeof(T), method.GetParameters()[0].ParameterType, - method.ReturnType); - return (Action) closedImpl.Invoke(null, new object[] {method}); - } - - public static Action CreateDowncastDelegateIgnoringReturnImpl( - MethodInfo method) - { - // Convert the reflection call into an open delegate, i.e. instead of calling x.Method(y) we'll - // call Method(x, y) - Func call = ReflectionUtil.CreateDelegateFunc(method); - - return delegate(TSource source, object parameter) { call(source, (TParam) parameter); }; - } - - internal static Func CreateDelegateFunc(MethodInfo method) - { - object tdelegate = Delegate.CreateDelegate(typeof(Func), null, method); - return (Func)tdelegate; - } - - internal static Func CreateDelegateFunc(MethodInfo method) - { - object tdelegate = Delegate.CreateDelegate(typeof(Func), null, method); - return (Func)tdelegate; - } - - internal static Func CreateDelegateFunc(MethodInfo method) - { - object tdelegate = Delegate.CreateDelegate(typeof(Func), null, method); - return (Func)tdelegate; - } - - internal static Action CreateDelegateAction(MethodInfo method) - { - object tdelegate = Delegate.CreateDelegate(typeof(Action), null, method); - return (Action)tdelegate; - } - - internal static Action CreateDelegateAction(MethodInfo method) + internal static Action CreateActionObject(MethodInfo method) { - object tdelegate = Delegate.CreateDelegate(typeof(Action), null, method); - return (Action)tdelegate; + ParameterExpression targetParameter = Expression.Parameter(typeof(object), "target"); + Expression castTarget = Expression.Convert(targetParameter, method.DeclaringType); + Expression call = Expression.Call(castTarget, method); + return Expression.Lambda>(call, targetParameter).Compile(); } } } \ No newline at end of file diff --git a/csharp/src/ProtocolBuffers/FieldAccess/RepeatedFieldAccessor.cs b/csharp/src/ProtocolBuffers/FieldAccess/RepeatedFieldAccessor.cs index b12278f9..8d7ecbaf 100644 --- a/csharp/src/ProtocolBuffers/FieldAccess/RepeatedFieldAccessor.cs +++ b/csharp/src/ProtocolBuffers/FieldAccess/RepeatedFieldAccessor.cs @@ -32,33 +32,28 @@ using System; using System.Collections; +using Google.Protobuf.Descriptors; namespace Google.Protobuf.FieldAccess { /// /// Accessor for repeated fields. /// - /// The type of message containing the field. - internal sealed class RepeatedFieldAccessor : FieldAccessorBase where T : IMessage + internal sealed class RepeatedFieldAccessor : FieldAccessorBase { - internal RepeatedFieldAccessor(string name) : base(name) + internal RepeatedFieldAccessor(Type type, string propertyName, FieldDescriptor descriptor) : base(type, propertyName, descriptor) { } - public override void Clear(T message) + public override void Clear(object message) { IList list = (IList) GetValue(message); list.Clear(); } - public override bool HasValue(T message) + public override void SetValue(object message, object value) { - throw new NotImplementedException("HasValue is not implemented for repeated fields"); - } - - public override void SetValue(T message, object value) - { - throw new NotImplementedException("SetValue is not implemented for repeated fields"); + throw new InvalidOperationException("SetValue is not implemented for repeated fields"); } } diff --git a/csharp/src/ProtocolBuffers/FieldAccess/SingleFieldAccessor.cs b/csharp/src/ProtocolBuffers/FieldAccess/SingleFieldAccessor.cs index 7a8f726e..cdc89e8d 100644 --- a/csharp/src/ProtocolBuffers/FieldAccess/SingleFieldAccessor.cs +++ b/csharp/src/ProtocolBuffers/FieldAccess/SingleFieldAccessor.cs @@ -39,76 +39,46 @@ namespace Google.Protobuf.FieldAccess /// /// Accessor for single fields. /// - /// The type of message containing the field. - internal sealed class SingleFieldAccessor : FieldAccessorBase where T : IMessage + internal sealed class SingleFieldAccessor : FieldAccessorBase { // All the work here is actually done in the constructor - it creates the appropriate delegates. // There are various cases to consider, based on the property type (message, string/bytes, or "genuine" primitive) // and proto2 vs proto3 for non-message types, as proto3 doesn't support "full" presence detection or default // values. - private readonly Action setValueDelegate; - private readonly Action clearDelegate; - private readonly Func hasValueDelegate; + private readonly Action setValueDelegate; + private readonly Action clearDelegate; - internal SingleFieldAccessor(FieldDescriptor descriptor, string name, bool supportsFieldPresence) : base(name) + internal SingleFieldAccessor(Type type, string propertyName, FieldDescriptor descriptor) : base(type, propertyName, descriptor) { - PropertyInfo property = typeof(T).GetProperty(name); + PropertyInfo property = type.GetProperty(propertyName); // We know there *is* such a property, or the base class constructor would have thrown. We should be able to write // to it though. if (!property.CanWrite) { throw new ArgumentException("Not all required properties/methods available"); } - setValueDelegate = ReflectionUtil.CreateDowncastDelegate(property.GetSetMethod()); + setValueDelegate = ReflectionUtil.CreateActionObjectObject(property.GetSetMethod()); var clrType = property.PropertyType; + + // TODO: What should clear on a oneof member do? Clear the oneof? - if (typeof(IMessage).IsAssignableFrom(clrType)) - { - // Message types are simple - we only need to detect nullity. - clearDelegate = message => SetValue(message, null); - hasValueDelegate = message => GetValue(message) == null; - } - - if (supportsFieldPresence) - { - // Proto2: we expect a HasFoo property and a ClearFoo method. - // For strings and byte arrays, setting the property to null would have the equivalent effect, - // but we generate the method for consistency, which makes this simpler. - PropertyInfo hasProperty = typeof(T).GetProperty("Has" + name); - MethodInfo clearMethod = typeof(T).GetMethod("Clear" + name); - if (hasProperty == null || clearMethod == null || !hasProperty.CanRead) - { - throw new ArgumentException("Not all required properties/methods available"); - } - hasValueDelegate = ReflectionUtil.CreateDelegateFunc(hasProperty.GetGetMethod()); - clearDelegate = ReflectionUtil.CreateDelegateAction(clearMethod); - } - else - { - /* - // TODO(jonskeet): Reimplement. We need a better way of working out default values. - // Proto3: for field detection, we just need the default value of the field (0, "", byte[0] etc) - // To clear a field, we set the value to that default. - object defaultValue = descriptor.DefaultValue; - hasValueDelegate = message => GetValue(message).Equals(defaultValue); - clearDelegate = message => SetValue(message, defaultValue); - */ - } - } - - public override bool HasValue(T message) - { - return hasValueDelegate(message); + // TODO: Validate that this is a reasonable single field? (Should be a value type, a message type, or string/ByteString.) + object defaultValue = + typeof(IMessage).IsAssignableFrom(clrType) ? null + : clrType == typeof(string) ? "" + : clrType == typeof(ByteString) ? ByteString.Empty + : Activator.CreateInstance(clrType); + clearDelegate = message => SetValue(message, defaultValue); } - public override void Clear(T message) + public override void Clear(object message) { clearDelegate(message); } - public override void SetValue(T message, object value) + public override void SetValue(object message, object value) { setValueDelegate(message, value); } diff --git a/csharp/src/ProtocolBuffers/IMessage.cs b/csharp/src/ProtocolBuffers/IMessage.cs index 27bcc117..ad44668c 100644 --- a/csharp/src/ProtocolBuffers/IMessage.cs +++ b/csharp/src/ProtocolBuffers/IMessage.cs @@ -40,12 +40,11 @@ namespace Google.Protobuf // TODO(jonskeet): Split these interfaces into separate files when we're happy with them. /// - /// Reflection support for a specific message type. message + /// Reflection support for a specific message type. /// - /// The message type being reflected. - public interface IReflectedMessage where T : IMessage + public interface IReflectedMessage { - FieldAccessorTable Fields { get; } + FieldAccessorTable Fields { get; } // TODO(jonskeet): Descriptor? Or a single property which has "all you need for reflection"? } diff --git a/csharp/src/ProtocolBuffers/ProtocolBuffers.csproj b/csharp/src/ProtocolBuffers/ProtocolBuffers.csproj index 1e7408ea..4078589e 100644 --- a/csharp/src/ProtocolBuffers/ProtocolBuffers.csproj +++ b/csharp/src/ProtocolBuffers/ProtocolBuffers.csproj @@ -79,6 +79,7 @@ + -- cgit v1.2.3