From 59eeebee870332ea2b9085688ba524c69311f662 Mon Sep 17 00:00:00 2001 From: Jon Skeet Date: Fri, 17 Jul 2015 08:26:04 +0100 Subject: First pass at the big rename from ProtocolBuffers to Google.Protobuf. We'll see what I've missed when CI fails... --- .../Reflection/DescriptorsTest.cs | 223 +++++++++++++++++++++ 1 file changed, 223 insertions(+) create mode 100644 csharp/src/Google.Protobuf.Test/Reflection/DescriptorsTest.cs (limited to 'csharp/src/Google.Protobuf.Test/Reflection') diff --git a/csharp/src/Google.Protobuf.Test/Reflection/DescriptorsTest.cs b/csharp/src/Google.Protobuf.Test/Reflection/DescriptorsTest.cs new file mode 100644 index 00000000..0db01a5e --- /dev/null +++ b/csharp/src/Google.Protobuf.Test/Reflection/DescriptorsTest.cs @@ -0,0 +1,223 @@ +#region Copyright notice and license +// Protocol Buffers - Google's data interchange format +// Copyright 2008 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.Linq; +using Google.Protobuf.TestProtos; +using NUnit.Framework; + +namespace Google.Protobuf.Reflection +{ + /// + /// Tests for descriptors. (Not in its own namespace or broken up into individual classes as the + /// size doesn't warrant it. On the other hand, this makes me feel a bit dirty...) + /// + public class DescriptorsTest + { + [Test] + public void FileDescriptor() + { + FileDescriptor file = UnittestProto3.Descriptor; + + Assert.AreEqual("google/protobuf/unittest_proto3.proto", file.Name); + Assert.AreEqual("protobuf_unittest", file.Package); + + Assert.AreEqual("UnittestProto", file.Proto.Options.JavaOuterClassname); + Assert.AreEqual("google/protobuf/unittest_proto3.proto", file.Proto.Name); + + // unittest.proto doesn't have any public imports, but unittest_import.proto does. + Assert.AreEqual(0, file.PublicDependencies.Count); + Assert.AreEqual(1, UnittestImportProto3.Descriptor.PublicDependencies.Count); + Assert.AreEqual(UnittestImportPublicProto3.Descriptor, UnittestImportProto3.Descriptor.PublicDependencies[0]); + + Assert.AreEqual(1, file.Dependencies.Count); + Assert.AreEqual(UnittestImportProto3.Descriptor, file.Dependencies[0]); + + MessageDescriptor messageType = TestAllTypes.Descriptor; + Assert.AreEqual(messageType, file.MessageTypes[0]); + Assert.AreEqual(messageType, file.FindTypeByName("TestAllTypes")); + Assert.Null(file.FindTypeByName("NoSuchType")); + Assert.Null(file.FindTypeByName("protobuf_unittest.TestAllTypes")); + for (int i = 0; i < file.MessageTypes.Count; i++) + { + Assert.AreEqual(i, file.MessageTypes[i].Index); + } + + Assert.AreEqual(file.EnumTypes[0], file.FindTypeByName("ForeignEnum")); + Assert.Null(file.FindTypeByName("NoSuchType")); + Assert.Null(file.FindTypeByName("protobuf_unittest.ForeignEnum")); + Assert.AreEqual(1, UnittestImportProto3.Descriptor.EnumTypes.Count); + Assert.AreEqual("ImportEnum", UnittestImportProto3.Descriptor.EnumTypes[0].Name); + for (int i = 0; i < file.EnumTypes.Count; i++) + { + Assert.AreEqual(i, file.EnumTypes[i].Index); + } + } + + [Test] + public void MessageDescriptor() + { + MessageDescriptor messageType = TestAllTypes.Descriptor; + MessageDescriptor nestedType = TestAllTypes.Types.NestedMessage.Descriptor; + + Assert.AreEqual("TestAllTypes", messageType.Name); + Assert.AreEqual("protobuf_unittest.TestAllTypes", messageType.FullName); + Assert.AreEqual(UnittestProto3.Descriptor, messageType.File); + Assert.IsNull(messageType.ContainingType); + Assert.IsNull(messageType.Proto.Options); + + Assert.AreEqual("TestAllTypes", messageType.Name); + + Assert.AreEqual("NestedMessage", nestedType.Name); + Assert.AreEqual("protobuf_unittest.TestAllTypes.NestedMessage", nestedType.FullName); + Assert.AreEqual(UnittestProto3.Descriptor, nestedType.File); + Assert.AreEqual(messageType, nestedType.ContainingType); + + FieldDescriptor field = messageType.Fields[0]; + Assert.AreEqual("single_int32", field.Name); + Assert.AreEqual(field, messageType.FindDescriptor("single_int32")); + Assert.Null(messageType.FindDescriptor("no_such_field")); + Assert.AreEqual(field, messageType.FindFieldByNumber(1)); + Assert.Null(messageType.FindFieldByNumber(571283)); + for (int i = 0; i < messageType.Fields.Count; i++) + { + Assert.AreEqual(i, messageType.Fields[i].Index); + } + + Assert.AreEqual(nestedType, messageType.NestedTypes[0]); + Assert.AreEqual(nestedType, messageType.FindDescriptor("NestedMessage")); + Assert.Null(messageType.FindDescriptor("NoSuchType")); + for (int i = 0; i < messageType.NestedTypes.Count; i++) + { + Assert.AreEqual(i, messageType.NestedTypes[i].Index); + } + + Assert.AreEqual(messageType.EnumTypes[0], messageType.FindDescriptor("NestedEnum")); + Assert.Null(messageType.FindDescriptor("NoSuchType")); + for (int i = 0; i < messageType.EnumTypes.Count; i++) + { + Assert.AreEqual(i, messageType.EnumTypes[i].Index); + } + } + + [Test] + public void FieldDescriptor() + { + MessageDescriptor messageType = TestAllTypes.Descriptor; + FieldDescriptor primitiveField = messageType.FindDescriptor("single_int32"); + FieldDescriptor enumField = messageType.FindDescriptor("single_nested_enum"); + FieldDescriptor messageField = messageType.FindDescriptor("single_foreign_message"); + + Assert.AreEqual("single_int32", primitiveField.Name); + Assert.AreEqual("protobuf_unittest.TestAllTypes.single_int32", + primitiveField.FullName); + Assert.AreEqual(1, primitiveField.FieldNumber); + Assert.AreEqual(messageType, primitiveField.ContainingType); + Assert.AreEqual(UnittestProto3.Descriptor, primitiveField.File); + Assert.AreEqual(FieldType.Int32, primitiveField.FieldType); + Assert.IsNull(primitiveField.Proto.Options); + + Assert.AreEqual("single_nested_enum", enumField.Name); + Assert.AreEqual(FieldType.Enum, enumField.FieldType); + // Assert.AreEqual(TestAllTypes.Types.NestedEnum.DescriptorProtoFile, enumField.EnumType); + + Assert.AreEqual("single_foreign_message", messageField.Name); + Assert.AreEqual(FieldType.Message, messageField.FieldType); + Assert.AreEqual(ForeignMessage.Descriptor, messageField.MessageType); + } + + [Test] + public void FieldDescriptorLabel() + { + FieldDescriptor singleField = + TestAllTypes.Descriptor.FindDescriptor("single_int32"); + FieldDescriptor repeatedField = + TestAllTypes.Descriptor.FindDescriptor("repeated_int32"); + + Assert.IsFalse(singleField.IsRepeated); + Assert.IsTrue(repeatedField.IsRepeated); + } + + [Test] + public void EnumDescriptor() + { + // Note: this test is a bit different to the Java version because there's no static way of getting to the descriptor + EnumDescriptor enumType = UnittestProto3.Descriptor.FindTypeByName("ForeignEnum"); + EnumDescriptor nestedType = TestAllTypes.Descriptor.FindDescriptor("NestedEnum"); + + Assert.AreEqual("ForeignEnum", enumType.Name); + Assert.AreEqual("protobuf_unittest.ForeignEnum", enumType.FullName); + Assert.AreEqual(UnittestProto3.Descriptor, enumType.File); + Assert.Null(enumType.ContainingType); + Assert.Null(enumType.Proto.Options); + + Assert.AreEqual("NestedEnum", nestedType.Name); + Assert.AreEqual("protobuf_unittest.TestAllTypes.NestedEnum", + nestedType.FullName); + Assert.AreEqual(UnittestProto3.Descriptor, nestedType.File); + Assert.AreEqual(TestAllTypes.Descriptor, nestedType.ContainingType); + + EnumValueDescriptor value = enumType.FindValueByName("FOREIGN_FOO"); + Assert.AreEqual(value, enumType.Values[1]); + Assert.AreEqual("FOREIGN_FOO", value.Name); + Assert.AreEqual(4, value.Number); + Assert.AreEqual((int) ForeignEnum.FOREIGN_FOO, value.Number); + Assert.AreEqual(value, enumType.FindValueByNumber(4)); + Assert.Null(enumType.FindValueByName("NO_SUCH_VALUE")); + for (int i = 0; i < enumType.Values.Count; i++) + { + Assert.AreEqual(i, enumType.Values[i].Index); + } + } + + [Test] + public void OneofDescriptor() + { + OneofDescriptor descriptor = TestAllTypes.Descriptor.FindDescriptor("oneof_field"); + Assert.AreEqual("oneof_field", descriptor.Name); + Assert.AreEqual("protobuf_unittest.TestAllTypes.oneof_field", descriptor.FullName); + + var expectedFields = new[] { + TestAllTypes.OneofBytesFieldNumber, + TestAllTypes.OneofNestedMessageFieldNumber, + TestAllTypes.OneofStringFieldNumber, + TestAllTypes.OneofUint32FieldNumber } + .Select(fieldNumber => TestAllTypes.Descriptor.FindFieldByNumber(fieldNumber)) + .ToList(); + foreach (var field in expectedFields) + { + Assert.AreSame(descriptor, field.ContainingOneof); + } + + CollectionAssert.AreEquivalent(expectedFields, descriptor.Fields); + } + } +} \ No newline at end of file -- cgit v1.2.3 From 53c399a1d65df65e9f83a70b55041a01cf8d7489 Mon Sep 17 00:00:00 2001 From: Jon Skeet Date: Mon, 20 Jul 2015 19:24:31 +0100 Subject: Revamp to reflection. Changes in brief: 1. Descriptor is now the entry point for all reflection. 2. IReflectedMessage has gone; there's now a Descriptor property in IMessage, which is explicitly implemented (due to the static property). 3. FieldAccessorTable has gone away 4. IFieldAccessor and OneofFieldAccessor still exist; we *could* put the functionality straight into FieldDescriptor and OneofDescriptor... I'm unsure about that. 5. There's a temporary property MessageDescriptor.FieldAccessorsByFieldNumber to make the test changes small - we probably want this to go away 6. Discovery for delegates is now via attributes applied to properties and the Clear method of a oneof I'm happy with 1-3. 4 I'm unsure about - feedback welcome. 5 will go away 6 I'm unsure about, both in design and implementation. Should we have a ProtobufMessageAttribute too? Should we find all the relevant attributes in MessageDescriptor and pass them down, to avoid an O(N^2) scenario? Generated code changes coming in the next commit. --- .../Google.Protobuf.Test/GeneratedMessageTest.cs | 47 ++++++----- .../Reflection/DescriptorsTest.cs | 1 + .../WellKnownTypes/WrappersTest.cs | 6 +- csharp/src/Google.Protobuf/Collections/MapField.cs | 3 + csharp/src/Google.Protobuf/Google.Protobuf.csproj | 3 +- csharp/src/Google.Protobuf/IMessage.cs | 18 ++-- csharp/src/Google.Protobuf/JsonFormatter.cs | 26 +++--- .../Google.Protobuf/Reflection/DescriptorUtil.cs | 5 +- .../Google.Protobuf/Reflection/EnumDescriptor.cs | 10 ++- .../Reflection/FieldAccessorBase.cs | 7 +- .../Reflection/FieldAccessorTable.cs | 97 ---------------------- .../Google.Protobuf/Reflection/FieldDescriptor.cs | 28 +++++++ .../Google.Protobuf/Reflection/FileDescriptor.cs | 36 ++++++-- .../Google.Protobuf/Reflection/MapFieldAccessor.cs | 3 +- .../Reflection/MessageDescriptor.cs | 34 +++++++- .../Google.Protobuf/Reflection/OneofAccessor.cs | 10 +-- .../Google.Protobuf/Reflection/OneofDescriptor.cs | 34 ++++++++ .../Reflection/ProtobufFieldAttribute.cs | 58 +++++++++++++ .../Reflection/ProtobufOneofAttribute.cs | 52 ++++++++++++ .../Google.Protobuf/Reflection/ReflectionUtil.cs | 20 +++++ .../Reflection/RepeatedFieldAccessor.cs | 3 +- .../Reflection/SingleFieldAccessor.cs | 5 +- .../protobuf/compiler/csharp/csharp_field_base.cc | 2 +- .../protobuf/compiler/csharp/csharp_helpers.h | 6 +- .../protobuf/compiler/csharp/csharp_map_field.cc | 1 + .../protobuf/compiler/csharp/csharp_message.cc | 84 ++----------------- .../protobuf/compiler/csharp/csharp_message.h | 2 - .../compiler/csharp/csharp_message_field.cc | 2 + .../compiler/csharp/csharp_primitive_field.cc | 2 + .../compiler/csharp/csharp_repeated_enum_field.cc | 3 +- .../csharp/csharp_repeated_message_field.cc | 1 + .../csharp/csharp_repeated_primitive_field.cc | 1 + .../compiler/csharp/csharp_umbrella_class.cc | 37 ++++++--- .../compiler/csharp/csharp_umbrella_class.h | 1 + .../compiler/csharp/csharp_wrapper_field.cc | 2 + 35 files changed, 372 insertions(+), 278 deletions(-) delete mode 100644 csharp/src/Google.Protobuf/Reflection/FieldAccessorTable.cs create mode 100644 csharp/src/Google.Protobuf/Reflection/ProtobufFieldAttribute.cs create mode 100644 csharp/src/Google.Protobuf/Reflection/ProtobufOneofAttribute.cs (limited to 'csharp/src/Google.Protobuf.Test/Reflection') diff --git a/csharp/src/Google.Protobuf.Test/GeneratedMessageTest.cs b/csharp/src/Google.Protobuf.Test/GeneratedMessageTest.cs index acb20b15..180976d1 100644 --- a/csharp/src/Google.Protobuf.Test/GeneratedMessageTest.cs +++ b/csharp/src/Google.Protobuf.Test/GeneratedMessageTest.cs @@ -604,7 +604,7 @@ namespace Google.Protobuf public void Reflection_GetValue() { var message = SampleMessages.CreateFullTestAllTypes(); - var fields = ((IReflectedMessage) message).Fields; + var fields = TestAllTypes.Descriptor.FieldAccessorsByFieldNumber; 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)); @@ -639,7 +639,7 @@ namespace Google.Protobuf // Just a single map field, for the same reason var mapMessage = new TestMap { MapStringString = { { "key1", "value1" }, { "key2", "value2" } } }; - fields = ((IReflectedMessage) mapMessage).Fields; + fields = TestMap.Descriptor.FieldAccessorsByFieldNumber; var dictionary = (IDictionary) fields[TestMap.MapStringStringFieldNumber].GetValue(mapMessage); Assert.AreEqual(mapMessage.MapStringString, dictionary); Assert.AreEqual("value1", dictionary["key1"]); @@ -648,8 +648,8 @@ namespace Google.Protobuf [Test] public void Reflection_Clear() { - IReflectedMessage message = SampleMessages.CreateFullTestAllTypes(); - var fields = message.Fields; + var message = SampleMessages.CreateFullTestAllTypes(); + var fields = TestAllTypes.Descriptor.FieldAccessorsByFieldNumber; fields[TestAllTypes.SingleBoolFieldNumber].Clear(message); fields[TestAllTypes.SingleInt32FieldNumber].Clear(message); fields[TestAllTypes.SingleStringFieldNumber].Clear(message); @@ -673,7 +673,7 @@ namespace Google.Protobuf // Separately, maps. var mapMessage = new TestMap { MapStringString = { { "key1", "value1" }, { "key2", "value2" } } }; - fields = ((IReflectedMessage) mapMessage).Fields; + fields = TestMap.Descriptor.FieldAccessorsByFieldNumber; fields[TestMap.MapStringStringFieldNumber].Clear(mapMessage); Assert.AreEqual(0, mapMessage.MapStringString.Count); } @@ -682,8 +682,8 @@ namespace Google.Protobuf public void Reflection_SetValue_SingleFields() { // Just a sample (primitives, messages, enums, strings, byte strings) - IReflectedMessage message = SampleMessages.CreateFullTestAllTypes(); - var fields = message.Fields; + var message = SampleMessages.CreateFullTestAllTypes(); + var fields = TestAllTypes.Descriptor.FieldAccessorsByFieldNumber; fields[TestAllTypes.SingleBoolFieldNumber].SetValue(message, false); fields[TestAllTypes.SingleInt32FieldNumber].SetValue(message, 500); fields[TestAllTypes.SingleStringFieldNumber].SetValue(message, "It's a string"); @@ -709,51 +709,52 @@ namespace Google.Protobuf [Test] public void Reflection_SetValue_SingleFields_WrongType() { - IReflectedMessage message = SampleMessages.CreateFullTestAllTypes(); - var fields = message.Fields; + IMessage message = SampleMessages.CreateFullTestAllTypes(); + var fields = message.Descriptor.FieldAccessorsByFieldNumber; Assert.Throws(() => fields[TestAllTypes.SingleBoolFieldNumber].SetValue(message, "This isn't a bool")); } [Test] public void Reflection_SetValue_MapFields() { - IReflectedMessage message = new TestMap(); - var fields = message.Fields; + IMessage message = new TestMap(); + var fields = message.Descriptor.FieldAccessorsByFieldNumber; Assert.Throws(() => fields[TestMap.MapStringStringFieldNumber].SetValue(message, new Dictionary())); } [Test] public void Reflection_SetValue_RepeatedFields() { - IReflectedMessage message = SampleMessages.CreateFullTestAllTypes(); - var fields = message.Fields; + IMessage message = SampleMessages.CreateFullTestAllTypes(); + var fields = message.Descriptor.FieldAccessorsByFieldNumber; Assert.Throws(() => fields[TestAllTypes.RepeatedDoubleFieldNumber].SetValue(message, new double[10])); } [Test] public void Reflection_GetValue_IncorrectType() { - IReflectedMessage message = SampleMessages.CreateFullTestAllTypes(); - Assert.Throws(() => message.Fields[TestAllTypes.SingleBoolFieldNumber].GetValue(new TestMap())); + IMessage message = SampleMessages.CreateFullTestAllTypes(); + var fields = message.Descriptor.FieldAccessorsByFieldNumber; + Assert.Throws(() => fields[TestAllTypes.SingleBoolFieldNumber].GetValue(new TestMap())); } [Test] public void Reflection_Oneof() { var message = new TestAllTypes(); - var fields = ((IReflectedMessage) 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)); + var descriptor = TestAllTypes.Descriptor; + Assert.AreEqual(1, descriptor.Oneofs.Count); + var oneof = descriptor.Oneofs[0]; + Assert.AreEqual("oneof_field", oneof.Name); + Assert.IsNull(oneof.Accessor.GetCaseFieldDescriptor(message)); message.OneofString = "foo"; - Assert.AreSame(fields[TestAllTypes.OneofStringFieldNumber].Descriptor, oneof.GetCaseFieldDescriptor(message)); + Assert.AreSame(descriptor.FieldAccessorsByFieldNumber[TestAllTypes.OneofStringFieldNumber].Descriptor, oneof.Accessor.GetCaseFieldDescriptor(message)); message.OneofUint32 = 10; - Assert.AreSame(fields[TestAllTypes.OneofUint32FieldNumber].Descriptor, oneof.GetCaseFieldDescriptor(message)); + Assert.AreSame(descriptor.FieldAccessorsByFieldNumber[TestAllTypes.OneofUint32FieldNumber].Descriptor, oneof.Accessor.GetCaseFieldDescriptor(message)); - oneof.Clear(message); + oneof.Accessor.Clear(message); Assert.AreEqual(TestAllTypes.OneofFieldOneofCase.None, message.OneofFieldCase); } } diff --git a/csharp/src/Google.Protobuf.Test/Reflection/DescriptorsTest.cs b/csharp/src/Google.Protobuf.Test/Reflection/DescriptorsTest.cs index 0db01a5e..0aff0a6c 100644 --- a/csharp/src/Google.Protobuf.Test/Reflection/DescriptorsTest.cs +++ b/csharp/src/Google.Protobuf.Test/Reflection/DescriptorsTest.cs @@ -62,6 +62,7 @@ namespace Google.Protobuf.Reflection Assert.AreEqual(UnittestImportProto3.Descriptor, file.Dependencies[0]); MessageDescriptor messageType = TestAllTypes.Descriptor; + Assert.AreSame(typeof(TestAllTypes), messageType.GeneratedType); Assert.AreEqual(messageType, file.MessageTypes[0]); Assert.AreEqual(messageType, file.FindTypeByName("TestAllTypes")); Assert.Null(file.FindTypeByName("NoSuchType")); diff --git a/csharp/src/Google.Protobuf.Test/WellKnownTypes/WrappersTest.cs b/csharp/src/Google.Protobuf.Test/WellKnownTypes/WrappersTest.cs index ad88c4eb..c617db36 100644 --- a/csharp/src/Google.Protobuf.Test/WellKnownTypes/WrappersTest.cs +++ b/csharp/src/Google.Protobuf.Test/WellKnownTypes/WrappersTest.cs @@ -192,7 +192,7 @@ namespace Google.Protobuf.WellKnownTypes Uint32Field = 3, Uint64Field = 4 }; - var fields = ((IReflectedMessage) message).Fields; + var fields = TestWellKnownTypes.Descriptor.FieldAccessorsByFieldNumber; Assert.AreEqual("x", fields[TestWellKnownTypes.StringFieldFieldNumber].GetValue(message)); Assert.AreEqual(ByteString.CopyFrom(1, 2, 3), fields[TestWellKnownTypes.BytesFieldFieldNumber].GetValue(message)); @@ -216,7 +216,7 @@ namespace Google.Protobuf.WellKnownTypes { // Just a single example... note that we can't have a null value here var message = new RepeatedWellKnownTypes { Int32Field = { 1, 2 } }; - var fields = ((IReflectedMessage) message).Fields; + var fields = RepeatedWellKnownTypes.Descriptor.FieldAccessorsByFieldNumber; var list = (IList) fields[RepeatedWellKnownTypes.Int32FieldFieldNumber].GetValue(message); CollectionAssert.AreEqual(new[] { 1, 2 }, list); } @@ -226,7 +226,7 @@ namespace Google.Protobuf.WellKnownTypes { // Just a single example... note that we can't have a null value here var message = new MapWellKnownTypes { Int32Field = { { 1, 2 }, { 3, null } } }; - var fields = ((IReflectedMessage) message).Fields; + var fields = MapWellKnownTypes.Descriptor.FieldAccessorsByFieldNumber; var dictionary = (IDictionary) fields[MapWellKnownTypes.Int32FieldFieldNumber].GetValue(message); Assert.AreEqual(2, dictionary[1]); Assert.IsNull(dictionary[3]); diff --git a/csharp/src/Google.Protobuf/Collections/MapField.cs b/csharp/src/Google.Protobuf/Collections/MapField.cs index f5e4fd3a..9ca5104d 100644 --- a/csharp/src/Google.Protobuf/Collections/MapField.cs +++ b/csharp/src/Google.Protobuf/Collections/MapField.cs @@ -30,6 +30,7 @@ // OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. #endregion +using Google.Protobuf.Reflection; using System; using System.Collections; using System.Collections.Generic; @@ -559,6 +560,8 @@ namespace Google.Protobuf.Collections { return codec.keyCodec.CalculateSizeWithTag(Key) + codec.valueCodec.CalculateSizeWithTag(Value); } + + MessageDescriptor IMessage.Descriptor { get { return null; } } } } } diff --git a/csharp/src/Google.Protobuf/Google.Protobuf.csproj b/csharp/src/Google.Protobuf/Google.Protobuf.csproj index 431668fe..50756299 100644 --- a/csharp/src/Google.Protobuf/Google.Protobuf.csproj +++ b/csharp/src/Google.Protobuf/Google.Protobuf.csproj @@ -78,7 +78,6 @@ - @@ -91,6 +90,8 @@ + + diff --git a/csharp/src/Google.Protobuf/IMessage.cs b/csharp/src/Google.Protobuf/IMessage.cs index 8c4a4aaf..d179c386 100644 --- a/csharp/src/Google.Protobuf/IMessage.cs +++ b/csharp/src/Google.Protobuf/IMessage.cs @@ -39,15 +39,6 @@ namespace Google.Protobuf // TODO(jonskeet): Do we want a "weak" (non-generic) version of IReflectedMessage? // TODO(jonskeet): Split these interfaces into separate files when we're happy with them. - /// - /// Reflection support for accessing field values. - /// - public interface IReflectedMessage : IMessage - { - FieldAccessorTable Fields { get; } - // TODO(jonskeet): Descriptor? Or a single property which has "all you need for reflection"? - } - /// /// Interface for a Protocol Buffers message, supporting /// basic operations required for serialization. @@ -73,6 +64,13 @@ namespace Google.Protobuf /// The number of bytes required to write this message /// to a coded output stream. int CalculateSize(); + + /// + /// Descriptor for this message. All instances are expected to return the same descriptor, + /// and for generated types this will be an explicitly-implemented member, returning the + /// same value as the static property declared on the type. + /// + MessageDescriptor Descriptor { get; } } /// @@ -81,7 +79,7 @@ namespace Google.Protobuf /// the implementation class. /// /// The message type. - public interface IMessage : IReflectedMessage, IEquatable, IDeepCloneable, IFreezable where T : IMessage + public interface IMessage : IMessage, IEquatable, IDeepCloneable, IFreezable where T : IMessage { /// /// Merges the given message into this one. diff --git a/csharp/src/Google.Protobuf/JsonFormatter.cs b/csharp/src/Google.Protobuf/JsonFormatter.cs index a06e6545..7f13e33e 100644 --- a/csharp/src/Google.Protobuf/JsonFormatter.cs +++ b/csharp/src/Google.Protobuf/JsonFormatter.cs @@ -118,7 +118,7 @@ namespace Google.Protobuf this.settings = settings; } - public string Format(IReflectedMessage message) + public string Format(IMessage message) { ThrowHelper.ThrowIfNull(message, "message"); StringBuilder builder = new StringBuilder(); @@ -129,7 +129,7 @@ namespace Google.Protobuf return builder.ToString(); } - private void WriteMessage(StringBuilder builder, IReflectedMessage message) + private void WriteMessage(StringBuilder builder, IMessage message) { if (message == null) { @@ -137,15 +137,15 @@ namespace Google.Protobuf return; } builder.Append("{ "); - var fields = message.Fields; + var fields = message.Descriptor.Fields; bool first = true; // First non-oneof fields - foreach (var accessor in fields.Accessors) + foreach (var field in fields) { - var descriptor = accessor.Descriptor; + var accessor = field.Accessor; // Oneofs are written later // TODO: Change to write out fields in order, interleaving oneofs appropriately (as per binary format) - if (descriptor.ContainingOneof != null) + if (field.ContainingOneof != null) { continue; } @@ -156,7 +156,7 @@ namespace Google.Protobuf continue; } // Omit awkward (single) values such as unknown enum values - if (!descriptor.IsRepeated && !descriptor.IsMap && !CanWriteSingleValue(accessor.Descriptor, value)) + if (!field.IsRepeated && !field.IsMap && !CanWriteSingleValue(accessor.Descriptor, value)) { continue; } @@ -173,15 +173,15 @@ namespace Google.Protobuf } // Now oneofs - foreach (var accessor in fields.Oneofs) + foreach (var oneof in message.Descriptor.Oneofs) { + var accessor = oneof.Accessor; var fieldDescriptor = accessor.GetCaseFieldDescriptor(message); if (fieldDescriptor == null) { continue; } - var fieldAccessor = fields[fieldDescriptor.FieldNumber]; - object value = fieldAccessor.GetValue(message); + object value = fieldDescriptor.Accessor.GetValue(message); // Omit awkward (single) values such as unknown enum values if (!fieldDescriptor.IsRepeated && !fieldDescriptor.IsMap && !CanWriteSingleValue(fieldDescriptor, value)) { @@ -194,7 +194,7 @@ namespace Google.Protobuf } WriteString(builder, ToCamelCase(fieldDescriptor.Name)); builder.Append(": "); - WriteValue(builder, fieldAccessor, value); + WriteValue(builder, fieldDescriptor.Accessor, value); first = false; } builder.Append(first ? "}" : " }"); @@ -385,7 +385,7 @@ namespace Google.Protobuf } else { - WriteMessage(builder, (IReflectedMessage) value); + WriteMessage(builder, (IMessage) value); } break; default: @@ -406,7 +406,7 @@ namespace Google.Protobuf WriteSingleValue(builder, descriptor.MessageType.FindFieldByNumber(1), value); return; } - WriteMessage(builder, (IReflectedMessage) value); + WriteMessage(builder, (IMessage) value); } private void WriteList(StringBuilder builder, IFieldAccessor accessor, IList list) diff --git a/csharp/src/Google.Protobuf/Reflection/DescriptorUtil.cs b/csharp/src/Google.Protobuf/Reflection/DescriptorUtil.cs index af31dfb1..f5570fc4 100644 --- a/csharp/src/Google.Protobuf/Reflection/DescriptorUtil.cs +++ b/csharp/src/Google.Protobuf/Reflection/DescriptorUtil.cs @@ -50,9 +50,8 @@ namespace Google.Protobuf.Reflection /// Converts the given array into a read-only list, applying the specified conversion to /// each input element. /// - internal static IList ConvertAndMakeReadOnly(IList input, - IndexedConverter - converter) + internal static IList ConvertAndMakeReadOnly + (IList input, IndexedConverter converter) { TOutput[] array = new TOutput[input.Count]; for (int i = 0; i < array.Length; i++) diff --git a/csharp/src/Google.Protobuf/Reflection/EnumDescriptor.cs b/csharp/src/Google.Protobuf/Reflection/EnumDescriptor.cs index bf8f8c83..285f26f3 100644 --- a/csharp/src/Google.Protobuf/Reflection/EnumDescriptor.cs +++ b/csharp/src/Google.Protobuf/Reflection/EnumDescriptor.cs @@ -30,6 +30,7 @@ // OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. #endregion +using System; using System.Collections.Generic; namespace Google.Protobuf.Reflection @@ -42,11 +43,13 @@ namespace Google.Protobuf.Reflection private readonly EnumDescriptorProto proto; private readonly MessageDescriptor containingType; private readonly IList values; + private readonly Type generatedType; - internal EnumDescriptor(EnumDescriptorProto proto, FileDescriptor file, MessageDescriptor parent, int index) + internal EnumDescriptor(EnumDescriptorProto proto, FileDescriptor file, MessageDescriptor parent, int index, Type generatedType) : base(file, file.ComputeFullName(parent, proto.Name), index) { this.proto = proto; + this.generatedType = generatedType; containingType = parent; if (proto.Value.Count == 0) @@ -69,6 +72,11 @@ namespace Google.Protobuf.Reflection /// public override string Name { get { return proto.Name; } } + /// + /// The generated type for this enum, or null if the descriptor does not represent a generated type. + /// + public Type GeneratedType { get { return generatedType; } } + /// /// If this is a nested type, get the outer descriptor, otherwise null. /// diff --git a/csharp/src/Google.Protobuf/Reflection/FieldAccessorBase.cs b/csharp/src/Google.Protobuf/Reflection/FieldAccessorBase.cs index 39a63b47..0893dc3d 100644 --- a/csharp/src/Google.Protobuf/Reflection/FieldAccessorBase.cs +++ b/csharp/src/Google.Protobuf/Reflection/FieldAccessorBase.cs @@ -43,13 +43,8 @@ namespace Google.Protobuf.Reflection private readonly Func getValueDelegate; private readonly FieldDescriptor descriptor; - internal FieldAccessorBase(Type type, string propertyName, FieldDescriptor descriptor) + internal FieldAccessorBase(PropertyInfo property, FieldDescriptor descriptor) { - PropertyInfo property = type.GetProperty(propertyName); - if (property == null || !property.CanRead) - { - throw new ArgumentException("Not all required properties/methods available"); - } this.descriptor = descriptor; getValueDelegate = ReflectionUtil.CreateFuncObjectObject(property.GetGetMethod()); } diff --git a/csharp/src/Google.Protobuf/Reflection/FieldAccessorTable.cs b/csharp/src/Google.Protobuf/Reflection/FieldAccessorTable.cs deleted file mode 100644 index 24fcbc64..00000000 --- a/csharp/src/Google.Protobuf/Reflection/FieldAccessorTable.cs +++ /dev/null @@ -1,97 +0,0 @@ -#region Copyright notice and license -// Protocol Buffers - Google's data interchange format -// Copyright 2008 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.ObjectModel; - -namespace Google.Protobuf.Reflection -{ - /// - /// Provides access to fields in generated messages via reflection. - /// - public sealed class FieldAccessorTable - { - private readonly ReadOnlyCollection accessors; - private readonly ReadOnlyCollection oneofs; - 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(Type type, MessageDescriptor descriptor, string[] propertyNames, string[] oneofPropertyNames) - { - this.descriptor = descriptor; - var accessorsArray = new IFieldAccessor[descriptor.Fields.Count]; - for (int i = 0; i < accessorsArray.Length; i++) - { - var field = descriptor.Fields[i]; - var name = propertyNames[i]; - 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); - 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, - // 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 ReadOnlyCollection Oneofs { get { return oneofs; } } - - // TODO: Review this, as it's easy to get confused between FieldNumber and Index. - // Currently only used to get an accessor related to a oneof... maybe just make that simpler? - public IFieldAccessor this[int fieldNumber] - { - get - { - FieldDescriptor field = descriptor.FindFieldByNumber(fieldNumber); - return accessors[field.Index]; - } - } - } -} \ No newline at end of file diff --git a/csharp/src/Google.Protobuf/Reflection/FieldDescriptor.cs b/csharp/src/Google.Protobuf/Reflection/FieldDescriptor.cs index 3d9d0d75..57378e4c 100644 --- a/csharp/src/Google.Protobuf/Reflection/FieldDescriptor.cs +++ b/csharp/src/Google.Protobuf/Reflection/FieldDescriptor.cs @@ -31,6 +31,7 @@ #endregion using System; +using System.Linq; namespace Google.Protobuf.Reflection { @@ -45,6 +46,7 @@ namespace Google.Protobuf.Reflection private readonly MessageDescriptor containingType; private readonly OneofDescriptor containingOneof; private FieldType fieldType; + private IFieldAccessor accessor; internal FieldDescriptor(FieldDescriptorProto proto, FileDescriptor file, MessageDescriptor parent, int index) @@ -82,6 +84,8 @@ namespace Google.Protobuf.Reflection public override string Name { get { return proto.Name; } } internal FieldDescriptorProto Proto { get { return proto; } } + + public IFieldAccessor Accessor { get { return accessor; } } /// /// Maps a field type as included in the .proto file to a FieldType. @@ -287,6 +291,30 @@ namespace Google.Protobuf.Reflection { throw new DescriptorValidationException(this, "MessageSet format is not supported."); } + + accessor = CreateAccessor(); + } + + private IFieldAccessor CreateAccessor() + { + // TODO: Check the performance of this with some large protos. Each message is O(N^2) in the number of fields, + // which isn't great... + if (containingType.GeneratedType == null) + { + return null; + } + var property = containingType + .GeneratedType + .GetProperties() + .FirstOrDefault(p => p.IsDefined(typeof(ProtobufFieldAttribute), false) && + p.GetCustomAttributes(typeof(ProtobufFieldAttribute), false).Cast().Single().Number == FieldNumber); + if (property == null) + { + return null; + } + return IsMap ? new MapFieldAccessor(property, this) + : IsRepeated ? new RepeatedFieldAccessor(property, this) + : (IFieldAccessor) new SingleFieldAccessor(property, this); } } } \ No newline at end of file diff --git a/csharp/src/Google.Protobuf/Reflection/FileDescriptor.cs b/csharp/src/Google.Protobuf/Reflection/FileDescriptor.cs index db393480..a10e617b 100644 --- a/csharp/src/Google.Protobuf/Reflection/FileDescriptor.cs +++ b/csharp/src/Google.Protobuf/Reflection/FileDescriptor.cs @@ -62,11 +62,12 @@ namespace Google.Protobuf.Reflection get { return proto.Syntax == "proto3" ? ProtoSyntax.Proto3 : ProtoSyntax.Proto2; } } - private FileDescriptor(FileDescriptorProto proto, FileDescriptor[] dependencies, DescriptorPool pool, bool allowUnknownDependencies) + private FileDescriptor(FileDescriptorProto proto, FileDescriptor[] dependencies, DescriptorPool pool, bool allowUnknownDependencies, Type[] generatedTypes) { this.pool = pool; this.proto = proto; this.dependencies = new ReadOnlyCollection((FileDescriptor[]) dependencies.Clone()); + IEnumerator generatedTypeIterator = generatedTypes == null ? null : ((IEnumerable)generatedTypes).GetEnumerator(); publicDependencies = DeterminePublicDependencies(this, proto, dependencies, allowUnknownDependencies); @@ -74,15 +75,21 @@ namespace Google.Protobuf.Reflection messageTypes = DescriptorUtil.ConvertAndMakeReadOnly(proto.MessageType, (message, index) => - new MessageDescriptor(message, this, null, index)); + new MessageDescriptor(message, this, null, index, generatedTypeIterator)); enumTypes = DescriptorUtil.ConvertAndMakeReadOnly(proto.EnumType, (enumType, index) => - new EnumDescriptor(enumType, this, null, index)); + new EnumDescriptor(enumType, this, null, index, ReflectionUtil.GetNextType(generatedTypeIterator))); services = DescriptorUtil.ConvertAndMakeReadOnly(proto.Service, (service, index) => new ServiceDescriptor(service, this, index)); + + // We should now have consumed all the generated types. + if (generatedTypeIterator != null && generatedTypeIterator.MoveNext()) + { + throw new ArgumentException("More generated types left over after consuming all expected ones", "generatedTypes"); + } } /// @@ -265,7 +272,7 @@ namespace Google.Protobuf.Reflection /// If is not /// a valid descriptor. This can occur for a number of reasons, such as a field /// having an undefined type or because two messages were defined with the same name. - private static FileDescriptor BuildFrom(FileDescriptorProto proto, FileDescriptor[] dependencies, bool allowUnknownDependencies) + private static FileDescriptor BuildFrom(FileDescriptorProto proto, FileDescriptor[] dependencies, bool allowUnknownDependencies, Type[] generatedTypes) { // Building descriptors involves two steps: translating and linking. // In the translation step (implemented by FileDescriptor's @@ -282,7 +289,7 @@ namespace Google.Protobuf.Reflection } DescriptorPool pool = new DescriptorPool(dependencies); - FileDescriptor result = new FileDescriptor(proto, dependencies, pool, allowUnknownDependencies); + FileDescriptor result = new FileDescriptor(proto, dependencies, pool, allowUnknownDependencies, generatedTypes); // TODO(jonskeet): Reinstate these checks, or get rid of them entirely. They aren't in the Java code, // and fail for the CustomOptions test right now. (We get "descriptor.proto" vs "google/protobuf/descriptor.proto".) @@ -319,8 +326,23 @@ namespace Google.Protobuf.Reflection } } + /// + /// Creates an instance for generated code. + /// + /// + /// The parameter should be null for descriptors which don't correspond to + /// generated types. Otherwise, the array should be represent all the generated types in the file: messages then + /// enums. Within each message, there can be nested messages and enums, which must be specified "inline" in the array: + /// containing message, nested messages, nested enums - and of course each nested message may contain *more* nested messages, + /// etc. All messages within the descriptor should be represented, even if they do not have a generated type - any + /// type without a corresponding generated type (such as map entries) should respond to a null element. + /// For example, a file with a messages OuterMessage and InnerMessage, and enums OuterEnum and InnerEnum (where + /// InnerMessage and InnerEnum are nested within InnerMessage) would result in an array of + /// OuterMessage, InnerMessage, InnerEnum, OuterEnum. + /// public static FileDescriptor InternalBuildGeneratedFileFrom(byte[] descriptorData, - FileDescriptor[] dependencies) + FileDescriptor[] dependencies, + Type[] generatedTypes) { FileDescriptorProto proto; try @@ -336,7 +358,7 @@ namespace Google.Protobuf.Reflection { // When building descriptors for generated code, we allow unknown // dependencies by default. - return BuildFrom(proto, dependencies, true); + return BuildFrom(proto, dependencies, true, generatedTypes); } catch (DescriptorValidationException e) { diff --git a/csharp/src/Google.Protobuf/Reflection/MapFieldAccessor.cs b/csharp/src/Google.Protobuf/Reflection/MapFieldAccessor.cs index 317fbd8d..6df4c5f0 100644 --- a/csharp/src/Google.Protobuf/Reflection/MapFieldAccessor.cs +++ b/csharp/src/Google.Protobuf/Reflection/MapFieldAccessor.cs @@ -32,6 +32,7 @@ using System; using System.Collections; +using System.Reflection; namespace Google.Protobuf.Reflection { @@ -40,7 +41,7 @@ namespace Google.Protobuf.Reflection /// internal sealed class MapFieldAccessor : FieldAccessorBase { - internal MapFieldAccessor(Type type, string propertyName, FieldDescriptor descriptor) : base(type, propertyName, descriptor) + internal MapFieldAccessor(PropertyInfo property, FieldDescriptor descriptor) : base(property, descriptor) { } diff --git a/csharp/src/Google.Protobuf/Reflection/MessageDescriptor.cs b/csharp/src/Google.Protobuf/Reflection/MessageDescriptor.cs index 1c22c460..9413cf61 100644 --- a/csharp/src/Google.Protobuf/Reflection/MessageDescriptor.cs +++ b/csharp/src/Google.Protobuf/Reflection/MessageDescriptor.cs @@ -30,8 +30,10 @@ // OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. #endregion +using Google.Protobuf.Collections; using System; using System.Collections.Generic; +using System.Linq; namespace Google.Protobuf.Reflection { @@ -60,11 +62,15 @@ namespace Google.Protobuf.Reflection private readonly IList enumTypes; private readonly IList fields; private readonly IList oneofs; + // CLR representation of the type described by this descriptor, if any. + private readonly Type generatedType; + private IDictionary fieldAccessorsByFieldNumber; - internal MessageDescriptor(DescriptorProto proto, FileDescriptor file, MessageDescriptor parent, int typeIndex) + internal MessageDescriptor(DescriptorProto proto, FileDescriptor file, MessageDescriptor parent, int typeIndex, IEnumerator generatedTypeIterator) : base(file, file.ComputeFullName(parent, proto.Name), typeIndex) { this.proto = proto; + generatedType = ReflectionUtil.GetNextType(generatedTypeIterator); containingType = parent; oneofs = DescriptorUtil.ConvertAndMakeReadOnly(proto.OneofDecl, @@ -73,11 +79,11 @@ namespace Google.Protobuf.Reflection nestedTypes = DescriptorUtil.ConvertAndMakeReadOnly(proto.NestedType, (type, index) => - new MessageDescriptor(type, file, this, index)); + new MessageDescriptor(type, file, this, index, generatedTypeIterator)); enumTypes = DescriptorUtil.ConvertAndMakeReadOnly(proto.EnumType, (type, index) => - new EnumDescriptor(type, file, this, index)); + new EnumDescriptor(type, file, this, index, ReflectionUtil.GetNextType(generatedTypeIterator))); // TODO(jonskeet): Sort fields first? fields = DescriptorUtil.ConvertAndMakeReadOnly(proto.Field, @@ -85,6 +91,14 @@ namespace Google.Protobuf.Reflection new FieldDescriptor(field, file, this, index)); file.DescriptorPool.AddSymbol(this); } + + /// + /// Returns the total number of nested types and enums, recursively. + /// + private int CountTotalGeneratedTypes() + { + return nestedTypes.Sum(nested => nested.CountTotalGeneratedTypes()) + enumTypes.Count; + } /// /// The brief name of the descriptor's target. @@ -93,6 +107,11 @@ namespace Google.Protobuf.Reflection internal DescriptorProto Proto { get { return proto; } } + /// + /// The generated type for this message, or null if the descriptor does not represent a generated type. + /// + public Type GeneratedType { get { return generatedType; } } + /// /// Returns whether this message is one of the "well known types" which may have runtime/protoc support. /// @@ -141,6 +160,13 @@ namespace Google.Protobuf.Reflection get { return oneofs; } } + /// + /// Returns a map from field number to accessor. + /// TODO: Revisit this. It's mostly in place to make the transition from FieldAccessorTable + /// to descriptor-based reflection simple in terms of tests. Work out what we really want. + /// + public IDictionary FieldAccessorsByFieldNumber { get { return fieldAccessorsByFieldNumber; } } + /// /// Finds a field by field name. /// @@ -192,6 +218,8 @@ namespace Google.Protobuf.Reflection { oneof.CrossLink(); } + + fieldAccessorsByFieldNumber = new ReadOnlyDictionary(fields.ToDictionary(field => field.FieldNumber, field => field.Accessor)); } } } \ No newline at end of file diff --git a/csharp/src/Google.Protobuf/Reflection/OneofAccessor.cs b/csharp/src/Google.Protobuf/Reflection/OneofAccessor.cs index 7a11d36b..20cbea92 100644 --- a/csharp/src/Google.Protobuf/Reflection/OneofAccessor.cs +++ b/csharp/src/Google.Protobuf/Reflection/OneofAccessor.cs @@ -44,18 +44,16 @@ namespace Google.Protobuf.Reflection private readonly Action clearDelegate; private OneofDescriptor descriptor; - internal OneofAccessor(Type type, string propertyName, OneofDescriptor descriptor) + internal OneofAccessor(PropertyInfo caseProperty, MethodInfo clearMethod, OneofDescriptor descriptor) { - PropertyInfo property = type.GetProperty(propertyName + "Case"); - if (property == null || !property.CanRead) + if (!caseProperty.CanRead) { - throw new ArgumentException("Not all required properties/methods available"); + throw new ArgumentException("Cannot read from property"); } this.descriptor = descriptor; - caseDelegate = ReflectionUtil.CreateFuncObjectT(property.GetGetMethod()); + caseDelegate = ReflectionUtil.CreateFuncObjectT(caseProperty.GetGetMethod()); this.descriptor = descriptor; - MethodInfo clearMethod = type.GetMethod("Clear" + propertyName); clearDelegate = ReflectionUtil.CreateActionObject(clearMethod); } diff --git a/csharp/src/Google.Protobuf/Reflection/OneofDescriptor.cs b/csharp/src/Google.Protobuf/Reflection/OneofDescriptor.cs index e92dc8bb..b4cc0791 100644 --- a/csharp/src/Google.Protobuf/Reflection/OneofDescriptor.cs +++ b/csharp/src/Google.Protobuf/Reflection/OneofDescriptor.cs @@ -32,6 +32,7 @@ using System.Collections.Generic; using System.Collections.ObjectModel; +using System.Linq; namespace Google.Protobuf.Reflection { @@ -40,6 +41,7 @@ namespace Google.Protobuf.Reflection private readonly OneofDescriptorProto proto; private MessageDescriptor containingType; private IList fields; + private OneofAccessor accessor; internal OneofDescriptor(OneofDescriptorProto proto, FileDescriptor file, MessageDescriptor parent, int index) : base(file, file.ComputeFullName(parent, proto.Name), index) @@ -62,6 +64,8 @@ namespace Google.Protobuf.Reflection public IList Fields { get { return fields; } } + public OneofAccessor Accessor { get { return accessor; } } + internal void CrossLink() { List fieldCollection = new List(); @@ -73,6 +77,36 @@ namespace Google.Protobuf.Reflection } } fields = new ReadOnlyCollection(fieldCollection); + accessor = CreateAccessor(); + } + + private OneofAccessor CreateAccessor() + { + if (containingType.GeneratedType == null) + { + return null; + } + var caseProperty = containingType + .GeneratedType + .GetProperties() + .FirstOrDefault(p => p.IsDefined(typeof(ProtobufOneofAttribute), false) && + p.GetCustomAttributes(typeof(ProtobufOneofAttribute), false).Cast().Single().Name == Name); + if (caseProperty == null) + { + return null; + } + + var clearMethod = containingType + .GeneratedType + .GetMethods() + .FirstOrDefault(p => p.IsDefined(typeof(ProtobufOneofAttribute), false) && + p.GetCustomAttributes(typeof(ProtobufOneofAttribute), false).Cast().Single().Name == Name); + if (clearMethod == null) + { + return null; + } + + return new OneofAccessor(caseProperty, clearMethod, this); } } } diff --git a/csharp/src/Google.Protobuf/Reflection/ProtobufFieldAttribute.cs b/csharp/src/Google.Protobuf/Reflection/ProtobufFieldAttribute.cs new file mode 100644 index 00000000..a59caea7 --- /dev/null +++ b/csharp/src/Google.Protobuf/Reflection/ProtobufFieldAttribute.cs @@ -0,0 +1,58 @@ +#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; + +namespace Google.Protobuf.Reflection +{ + /// + /// Attribute applied to a generated property corresponding to a field in a .proto file. + /// + [AttributeUsage(AttributeTargets.Property, AllowMultiple = false)] + public sealed class ProtobufFieldAttribute : Attribute + { + /// + /// The field number in the original .proto file. + /// + public int Number { get; set; } + + /// + /// The field name in the original .proto file. + /// + public string Name { get; set; } + + public ProtobufFieldAttribute(int number, string name) + { + this.Number = number; + this.Name = name; + } + } +} diff --git a/csharp/src/Google.Protobuf/Reflection/ProtobufOneofAttribute.cs b/csharp/src/Google.Protobuf/Reflection/ProtobufOneofAttribute.cs new file mode 100644 index 00000000..74ad8c53 --- /dev/null +++ b/csharp/src/Google.Protobuf/Reflection/ProtobufOneofAttribute.cs @@ -0,0 +1,52 @@ +#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; + +namespace Google.Protobuf.Reflection +{ + /// + /// Attribute applied to the "case" property or "clear" method corresponding to a oneof in a .proto file. + /// + [AttributeUsage(AttributeTargets.Property | AttributeTargets.Method, AllowMultiple = false)] + public sealed class ProtobufOneofAttribute : Attribute + { + /// + /// The oneof name in the original .proto file. + /// + public string Name { get; set; } + + public ProtobufOneofAttribute(string name) + { + this.Name = name; + } + } +} diff --git a/csharp/src/Google.Protobuf/Reflection/ReflectionUtil.cs b/csharp/src/Google.Protobuf/Reflection/ReflectionUtil.cs index d0dc3e8b..ec222dc1 100644 --- a/csharp/src/Google.Protobuf/Reflection/ReflectionUtil.cs +++ b/csharp/src/Google.Protobuf/Reflection/ReflectionUtil.cs @@ -31,6 +31,7 @@ #endregion using System; +using System.Collections.Generic; using System.Linq.Expressions; using System.Reflection; @@ -102,5 +103,24 @@ namespace Google.Protobuf.Reflection Expression call = Expression.Call(castTarget, method); return Expression.Lambda>(call, targetParameter).Compile(); } + + /// + /// Returns the next type from an iterator of types, unless the iterator is a null reference, + /// in which case null is returned. + /// + internal static Type GetNextType(IEnumerator generatedTypeIterator) + { + if (generatedTypeIterator == null) + { + return null; + } + if (!generatedTypeIterator.MoveNext()) + { + // This parameter name corresponds to any public method supplying the generated types to start with. + throw new ArgumentException("More generated types left over after consuming all expected ones", "generatedTypes"); + } + return generatedTypeIterator.Current; + } + } } \ No newline at end of file diff --git a/csharp/src/Google.Protobuf/Reflection/RepeatedFieldAccessor.cs b/csharp/src/Google.Protobuf/Reflection/RepeatedFieldAccessor.cs index 0ada7567..acb3c8d5 100644 --- a/csharp/src/Google.Protobuf/Reflection/RepeatedFieldAccessor.cs +++ b/csharp/src/Google.Protobuf/Reflection/RepeatedFieldAccessor.cs @@ -32,6 +32,7 @@ using System; using System.Collections; +using System.Reflection; namespace Google.Protobuf.Reflection { @@ -40,7 +41,7 @@ namespace Google.Protobuf.Reflection /// internal sealed class RepeatedFieldAccessor : FieldAccessorBase { - internal RepeatedFieldAccessor(Type type, string propertyName, FieldDescriptor descriptor) : base(type, propertyName, descriptor) + internal RepeatedFieldAccessor(PropertyInfo property, FieldDescriptor descriptor) : base(property, descriptor) { } diff --git a/csharp/src/Google.Protobuf/Reflection/SingleFieldAccessor.cs b/csharp/src/Google.Protobuf/Reflection/SingleFieldAccessor.cs index 8c24e46e..f00a51ba 100644 --- a/csharp/src/Google.Protobuf/Reflection/SingleFieldAccessor.cs +++ b/csharp/src/Google.Protobuf/Reflection/SingleFieldAccessor.cs @@ -48,11 +48,8 @@ namespace Google.Protobuf.Reflection private readonly Action setValueDelegate; private readonly Action clearDelegate; - internal SingleFieldAccessor(Type type, string propertyName, FieldDescriptor descriptor) : base(type, propertyName, descriptor) + internal SingleFieldAccessor(PropertyInfo property, FieldDescriptor descriptor) : base(property, descriptor) { - 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"); diff --git a/src/google/protobuf/compiler/csharp/csharp_field_base.cc b/src/google/protobuf/compiler/csharp/csharp_field_base.cc index 89d4eb18..d327e267 100644 --- a/src/google/protobuf/compiler/csharp/csharp_field_base.cc +++ b/src/google/protobuf/compiler/csharp/csharp_field_base.cc @@ -74,6 +74,7 @@ void FieldGeneratorBase::SetCommonFieldVariables( (*variables)["property_name"] = property_name(); (*variables)["type_name"] = type_name(); + (*variables)["original_name"] = descriptor_->name(); (*variables)["name"] = name(); (*variables)["descriptor_name"] = descriptor_->name(); (*variables)["default_value"] = default_value(); @@ -85,7 +86,6 @@ void FieldGeneratorBase::SetCommonFieldVariables( } (*variables)["capitalized_type_name"] = capitalized_type_name(); (*variables)["number"] = number(); - (*variables)["field_ordinal"] = field_ordinal(); (*variables)["has_property_check"] = (*variables)["property_name"] + " != " + (*variables)["default_value"]; (*variables)["other_has_property_check"] = "other." + diff --git a/src/google/protobuf/compiler/csharp/csharp_helpers.h b/src/google/protobuf/compiler/csharp/csharp_helpers.h index 653ff992..7737ffe2 100644 --- a/src/google/protobuf/compiler/csharp/csharp_helpers.h +++ b/src/google/protobuf/compiler/csharp/csharp_helpers.h @@ -77,6 +77,8 @@ std::string GetFullUmbrellaClassName(const FileDescriptor* descriptor); std::string GetQualifiedUmbrellaClassName(const FileDescriptor* descriptor); +std::string GetClassName(const Descriptor* descriptor); + std::string GetClassName(const EnumDescriptor* descriptor); std::string GetFieldName(const FieldDescriptor* descriptor); @@ -119,10 +121,6 @@ inline bool IsDescriptorProto(const FileDescriptor* descriptor) { return descriptor->name() == "google/protobuf/descriptor_proto_file.proto"; } -inline bool IsMapEntry(const Descriptor* descriptor) { - return descriptor->options().map_entry(); -} - inline bool IsWrapperType(const FieldDescriptor* descriptor) { return descriptor->type() == FieldDescriptor::TYPE_MESSAGE && descriptor->message_type()->file()->name() == "google/protobuf/wrappers.proto"; diff --git a/src/google/protobuf/compiler/csharp/csharp_map_field.cc b/src/google/protobuf/compiler/csharp/csharp_map_field.cc index cba24a59..bdbfd92b 100644 --- a/src/google/protobuf/compiler/csharp/csharp_map_field.cc +++ b/src/google/protobuf/compiler/csharp/csharp_map_field.cc @@ -79,6 +79,7 @@ void MapFieldGenerator::GenerateMembers(io::Printer* printer) { AddDeprecatedFlag(printer); printer->Print( variables_, + "[pbr::ProtobufField($number$, \"$original_name$\")]\n" "$access_level$ pbc::MapField<$key_type_name$, $value_type_name$> $property_name$ {\n" " get { return $name$_; }\n" "}\n"); diff --git a/src/google/protobuf/compiler/csharp/csharp_message.cc b/src/google/protobuf/compiler/csharp/csharp_message.cc index 10cf3585..42fd5065 100644 --- a/src/google/protobuf/compiler/csharp/csharp_message.cc +++ b/src/google/protobuf/compiler/csharp/csharp_message.cc @@ -96,88 +96,11 @@ const std::vector& MessageGenerator::fields_by_number() return fields_by_number_; } -/// Get an identifier that uniquely identifies this type within the file. -/// This is used to declare static variables related to this type at the -/// outermost file scope. -std::string GetUniqueFileScopeIdentifier(const Descriptor* descriptor) { - std::string result = descriptor->full_name(); - std::replace(result.begin(), result.end(), '.', '_'); - return "static_" + result; -} - -void MessageGenerator::GenerateStaticVariables(io::Printer* printer) { - // Because descriptor.proto (Google.Protobuf.DescriptorProtos) is - // used in the construction of descriptors, we have a tricky bootstrapping - // problem. To help control static initialization order, we make sure all - // descriptors and other static data that depends on them are members of - // the proto-descriptor class. This way, they will be initialized in - // a deterministic order. - - std::string identifier = GetUniqueFileScopeIdentifier(descriptor_); - - // The descriptor for this type. - printer->Print( - "internal static pbr::FieldAccessorTable internal__$identifier$__FieldAccessorTable;\n", - "identifier", GetUniqueFileScopeIdentifier(descriptor_), - "full_class_name", full_class_name()); - - for (int i = 0; i < descriptor_->nested_type_count(); i++) { - // Don't generate accessor table fields for maps... - if (!IsMapEntryMessage(descriptor_->nested_type(i))) { - MessageGenerator messageGenerator(descriptor_->nested_type(i)); - messageGenerator.GenerateStaticVariables(printer); - } - } -} - -void MessageGenerator::GenerateStaticVariableInitializers(io::Printer* printer) { - map vars; - vars["identifier"] = GetUniqueFileScopeIdentifier(descriptor_); - vars["full_class_name"] = full_class_name(); - - // Work out how to get to the message descriptor (which may be multiply nested) from the file - // descriptor. - string descriptor_chain; - const Descriptor* current_descriptor = descriptor_; - while (current_descriptor->containing_type()) { - descriptor_chain = ".NestedTypes[" + SimpleItoa(current_descriptor->index()) + "]" + descriptor_chain; - current_descriptor = current_descriptor->containing_type(); - } - descriptor_chain = "descriptor.MessageTypes[" + SimpleItoa(current_descriptor->index()) + "]" + descriptor_chain; - vars["descriptor_chain"] = descriptor_chain; - - printer->Print( - vars, - "internal__$identifier$__FieldAccessorTable = \n" - " new pbr::FieldAccessorTable(typeof($full_class_name$), $descriptor_chain$,\n"); - printer->Print(" new string[] { "); - for (int i = 0; i < descriptor_->field_count(); i++) { - 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", - UnderscoresToCamelCase(descriptor_->oneof_decl(i)->name(), true)); - } - printer->Print("});\n"); - - // Generate static member initializers for all non-map-entry nested types. - for (int i = 0; i < descriptor_->nested_type_count(); i++) { - if (!IsMapEntryMessage(descriptor_->nested_type(i))) { - MessageGenerator messageGenerator(descriptor_->nested_type(i)); - messageGenerator.GenerateStaticVariableInitializers(printer); - } - } -} - void MessageGenerator::Generate(io::Printer* printer) { map vars; vars["class_name"] = class_name(); vars["access_level"] = class_access_level(); vars["umbrella_class_name"] = GetFullUmbrellaClassName(descriptor_->file()); - vars["identifier"] = GetUniqueFileScopeIdentifier(descriptor_); printer->Print( "[global::System.Diagnostics.DebuggerNonUserCodeAttribute()]\n"); @@ -221,8 +144,8 @@ void MessageGenerator::Generate(io::Printer* printer) { " get { return $descriptor_accessor$; }\n" "}\n" "\n" - "pbr::FieldAccessorTable pb::IReflectedMessage.Fields {\n" - " get { return $umbrella_class_name$.internal__$identifier$__FieldAccessorTable; }\n" + "pbr::MessageDescriptor pb::IMessage.Descriptor {\n" + " get { return Descriptor; }\n" "}\n" "\n" "private bool _frozen = false;\n" @@ -258,6 +181,7 @@ void MessageGenerator::Generate(io::Printer* printer) { for (int i = 0; i < descriptor_->oneof_decl_count(); i++) { vars["name"] = UnderscoresToCamelCase(descriptor_->oneof_decl(i)->name(), false); vars["property_name"] = UnderscoresToCamelCase(descriptor_->oneof_decl(i)->name(), true); + vars["original_name"] = descriptor_->oneof_decl(i)->name(); printer->Print( vars, "private object $name$_;\n" @@ -275,9 +199,11 @@ void MessageGenerator::Generate(io::Printer* printer) { printer->Print( vars, "private $property_name$OneofCase $name$Case_ = $property_name$OneofCase.None;\n" + "[pbr::ProtobufOneof(\"$original_name$\")]\n" "public $property_name$OneofCase $property_name$Case {\n" " get { return $name$Case_; }\n" "}\n\n" + "[pbr::ProtobufOneof(\"$original_name$\")]\n" "public void Clear$property_name$() {\n" " pb::Freezable.CheckMutable(this);\n" " $name$Case_ = $property_name$OneofCase.None;\n" diff --git a/src/google/protobuf/compiler/csharp/csharp_message.h b/src/google/protobuf/compiler/csharp/csharp_message.h index fbe8a3be..f0c49ac9 100644 --- a/src/google/protobuf/compiler/csharp/csharp_message.h +++ b/src/google/protobuf/compiler/csharp/csharp_message.h @@ -53,8 +53,6 @@ class MessageGenerator : public SourceGeneratorBase { void GenerateCloningCode(io::Printer* printer); void GenerateFreezingCode(io::Printer* printer); void GenerateFrameworkMethods(io::Printer* printer); - void GenerateStaticVariables(io::Printer* printer); - void GenerateStaticVariableInitializers(io::Printer* printer); void Generate(io::Printer* printer); private: diff --git a/src/google/protobuf/compiler/csharp/csharp_message_field.cc b/src/google/protobuf/compiler/csharp/csharp_message_field.cc index d2c3a88b..c2b6ff76 100644 --- a/src/google/protobuf/compiler/csharp/csharp_message_field.cc +++ b/src/google/protobuf/compiler/csharp/csharp_message_field.cc @@ -64,6 +64,7 @@ void MessageFieldGenerator::GenerateMembers(io::Printer* printer) { AddDeprecatedFlag(printer); printer->Print( variables_, + "[pbr::ProtobufField($number$, \"$original_name$\")]\n" "$access_level$ $type_name$ $property_name$ {\n" " get { return $name$_; }\n" " set {\n" @@ -158,6 +159,7 @@ void MessageOneofFieldGenerator::GenerateMembers(io::Printer* printer) { AddDeprecatedFlag(printer); printer->Print( variables_, + "[pbr::ProtobufField($number$, \"$original_name$\")]\n" "$access_level$ $type_name$ $property_name$ {\n" " get { return $has_property_check$ ? ($type_name$) $oneof_name$_ : null; }\n" " set {\n" diff --git a/src/google/protobuf/compiler/csharp/csharp_primitive_field.cc b/src/google/protobuf/compiler/csharp/csharp_primitive_field.cc index 4454ef02..2c9338bd 100644 --- a/src/google/protobuf/compiler/csharp/csharp_primitive_field.cc +++ b/src/google/protobuf/compiler/csharp/csharp_primitive_field.cc @@ -71,6 +71,7 @@ void PrimitiveFieldGenerator::GenerateMembers(io::Printer* printer) { AddDeprecatedFlag(printer); printer->Print( variables_, + "[pbr::ProtobufField($number$, \"$original_name$\")]\n" "$access_level$ $type_name$ $property_name$ {\n" " get { return $name$_; }\n" " set {\n" @@ -174,6 +175,7 @@ void PrimitiveOneofFieldGenerator::GenerateMembers(io::Printer* printer) { AddDeprecatedFlag(printer); printer->Print( variables_, + "[pbr::ProtobufField($number$, \"$original_name$\")]\n" "$access_level$ $type_name$ $property_name$ {\n" " get { return $has_property_check$ ? ($type_name$) $oneof_name$_ : $default_value$; }\n" " set {\n" diff --git a/src/google/protobuf/compiler/csharp/csharp_repeated_enum_field.cc b/src/google/protobuf/compiler/csharp/csharp_repeated_enum_field.cc index d5fc6d98..60d06154 100644 --- a/src/google/protobuf/compiler/csharp/csharp_repeated_enum_field.cc +++ b/src/google/protobuf/compiler/csharp/csharp_repeated_enum_field.cc @@ -59,12 +59,13 @@ void RepeatedEnumFieldGenerator::GenerateMembers(io::Printer* printer) { printer->Print( variables_, "private static readonly pb::FieldCodec<$type_name$> _repeated_$name$_codec\n" - " = pb::FieldCodec.ForEnum($tag$, x => (int) x, x => ($type_name$) x);"); + " = pb::FieldCodec.ForEnum($tag$, x => (int) x, x => ($type_name$) x);\n"); printer->Print(variables_, "private readonly pbc::RepeatedField<$type_name$> $name$_ = new pbc::RepeatedField<$type_name$>();\n"); AddDeprecatedFlag(printer); printer->Print( variables_, + "[pbr::ProtobufField($number$, \"$original_name$\")]\n" "$access_level$ pbc::RepeatedField<$type_name$> $property_name$ {\n" " get { return $name$_; }\n" "}\n"); diff --git a/src/google/protobuf/compiler/csharp/csharp_repeated_message_field.cc b/src/google/protobuf/compiler/csharp/csharp_repeated_message_field.cc index d939fc79..921798b0 100644 --- a/src/google/protobuf/compiler/csharp/csharp_repeated_message_field.cc +++ b/src/google/protobuf/compiler/csharp/csharp_repeated_message_field.cc @@ -78,6 +78,7 @@ void RepeatedMessageFieldGenerator::GenerateMembers(io::Printer* printer) { AddDeprecatedFlag(printer); printer->Print( variables_, + "[pbr::ProtobufField($number$, \"$original_name$\")]\n" "$access_level$ pbc::RepeatedField<$type_name$> $property_name$ {\n" " get { return $name$_; }\n" "}\n"); diff --git a/src/google/protobuf/compiler/csharp/csharp_repeated_primitive_field.cc b/src/google/protobuf/compiler/csharp/csharp_repeated_primitive_field.cc index 5b5d9b3d..bcfb9936 100644 --- a/src/google/protobuf/compiler/csharp/csharp_repeated_primitive_field.cc +++ b/src/google/protobuf/compiler/csharp/csharp_repeated_primitive_field.cc @@ -65,6 +65,7 @@ void RepeatedPrimitiveFieldGenerator::GenerateMembers(io::Printer* printer) { AddDeprecatedFlag(printer); printer->Print( variables_, + "[pbr::ProtobufField($number$, \"$original_name$\")]\n" "$access_level$ pbc::RepeatedField<$type_name$> $property_name$ {\n" " get { return $name$_; }\n" "}\n"); diff --git a/src/google/protobuf/compiler/csharp/csharp_umbrella_class.cc b/src/google/protobuf/compiler/csharp/csharp_umbrella_class.cc index 57271d17..3f39250e 100644 --- a/src/google/protobuf/compiler/csharp/csharp_umbrella_class.cc +++ b/src/google/protobuf/compiler/csharp/csharp_umbrella_class.cc @@ -63,12 +63,6 @@ UmbrellaClassGenerator::~UmbrellaClassGenerator() { void UmbrellaClassGenerator::Generate(io::Printer* printer) { WriteIntroduction(printer); - printer->Print("#region Static variables\n"); - for (int i = 0; i < file_->message_type_count(); i++) { - MessageGenerator messageGenerator(file_->message_type(i)); - messageGenerator.GenerateStaticVariables(printer); - } - printer->Print("#endregion\n"); WriteDescriptor(printer); // Close the class declaration. printer->Outdent(); @@ -183,24 +177,43 @@ void UmbrellaClassGenerator::WriteDescriptor(io::Printer* printer) { // Invoke InternalBuildGeneratedFileFrom() to build the file. printer->Print( "descriptor = pbr::FileDescriptor.InternalBuildGeneratedFileFrom(descriptorData,\n"); - printer->Print(" new pbr::FileDescriptor[] {\n"); + printer->Print(" new pbr::FileDescriptor[] { "); for (int i = 0; i < file_->dependency_count(); i++) { printer->Print( - " $full_umbrella_class_name$.Descriptor, \n", + "$full_umbrella_class_name$.Descriptor, ", "full_umbrella_class_name", GetFullUmbrellaClassName(file_->dependency(i))); } - printer->Print(" });\n"); - // Then invoke any other static variable initializers, e.g. field accessors. + // Specify all the generated types (messages and enums), recursively, as an array. + printer->Print("},\n" + " new global::System.Type[] { "); for (int i = 0; i < file_->message_type_count(); i++) { - MessageGenerator messageGenerator(file_->message_type(i)); - messageGenerator.GenerateStaticVariableInitializers(printer); + WriteTypeLiterals(file_->message_type(i), printer); } + for (int i = 0; i < file_->enum_type_count(); i++) { + printer->Print("typeof($type_name$), ", "type_name", GetClassName(file_->enum_type(i))); + } + printer->Print("});\n"); + printer->Outdent(); printer->Print("}\n"); printer->Print("#endregion\n\n"); } +void UmbrellaClassGenerator::WriteTypeLiterals(const Descriptor* descriptor, io::Printer* printer) { + if (IsMapEntryMessage(descriptor)) { + printer->Print("null, "); + return; + } + printer->Print("typeof($type_name$), ", "type_name", GetClassName(descriptor)); + for (int i = 0; i < descriptor->nested_type_count(); i++) { + WriteTypeLiterals(descriptor->nested_type(i), printer); + } + for (int i = 0; i < descriptor->enum_type_count(); i++) { + printer->Print("typeof($type_name$), ", "type_name", GetClassName(descriptor->enum_type(i))); + } +} + } // namespace csharp } // namespace compiler } // namespace protobuf diff --git a/src/google/protobuf/compiler/csharp/csharp_umbrella_class.h b/src/google/protobuf/compiler/csharp/csharp_umbrella_class.h index f7533d2d..db1092a9 100644 --- a/src/google/protobuf/compiler/csharp/csharp_umbrella_class.h +++ b/src/google/protobuf/compiler/csharp/csharp_umbrella_class.h @@ -57,6 +57,7 @@ class UmbrellaClassGenerator : public SourceGeneratorBase { void WriteIntroduction(io::Printer* printer); void WriteDescriptor(io::Printer* printer); + void WriteTypeLiterals(const Descriptor* descriptor, io::Printer* printer); GOOGLE_DISALLOW_EVIL_CONSTRUCTORS(UmbrellaClassGenerator); }; diff --git a/src/google/protobuf/compiler/csharp/csharp_wrapper_field.cc b/src/google/protobuf/compiler/csharp/csharp_wrapper_field.cc index 75ef5e50..d6cd0a10 100644 --- a/src/google/protobuf/compiler/csharp/csharp_wrapper_field.cc +++ b/src/google/protobuf/compiler/csharp/csharp_wrapper_field.cc @@ -73,6 +73,7 @@ void WrapperFieldGenerator::GenerateMembers(io::Printer* printer) { AddDeprecatedFlag(printer); printer->Print( variables_, + "[pbr::ProtobufField($number$, \"$original_name$\")]\n" "$access_level$ $type_name$ $property_name$ {\n" " get { return $name$_; }\n" " set {\n" @@ -169,6 +170,7 @@ void WrapperOneofFieldGenerator::GenerateMembers(io::Printer* printer) { AddDeprecatedFlag(printer); printer->Print( variables_, + "[pbr::ProtobufField($number$, \"$original_name$\")]\n" "$access_level$ $type_name$ $property_name$ {\n" " get { return $has_property_check$ ? ($type_name$) $oneof_name$_ : ($type_name$) null; }\n" " set {\n" -- cgit v1.2.3 From 20bf6a563a94e5772fdb7b1bd6530404b2ea2c0b Mon Sep 17 00:00:00 2001 From: Jon Skeet Date: Wed, 22 Jul 2015 15:14:38 +0100 Subject: First pass at making field access simpler. This is definitely not ready to ship - I'm "troubled" by the disconnect between a list of fields in declaration order, and a mapping of field accessors by field number/name. Discussion required, but I find that easier when we've got code to look at :) --- .../Google.Protobuf.Test/GeneratedMessageTest.cs | 160 --------------- .../Google.Protobuf.Test.csproj | 3 +- .../Reflection/DescriptorsTest.cs | 15 ++ .../Reflection/FieldAccessTest.cs | 218 +++++++++++++++++++++ .../WellKnownTypes/WrappersTest.cs | 6 +- .../Google.Protobuf/Reflection/FileDescriptor.cs | 2 +- .../Reflection/MessageDescriptor.cs | 80 +++++++- 7 files changed, 309 insertions(+), 175 deletions(-) create mode 100644 csharp/src/Google.Protobuf.Test/Reflection/FieldAccessTest.cs (limited to 'csharp/src/Google.Protobuf.Test/Reflection') diff --git a/csharp/src/Google.Protobuf.Test/GeneratedMessageTest.cs b/csharp/src/Google.Protobuf.Test/GeneratedMessageTest.cs index 180976d1..528a4023 100644 --- a/csharp/src/Google.Protobuf.Test/GeneratedMessageTest.cs +++ b/csharp/src/Google.Protobuf.Test/GeneratedMessageTest.cs @@ -597,165 +597,5 @@ 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 = SampleMessages.CreateFullTestAllTypes(); - var fields = TestAllTypes.Descriptor.FieldAccessorsByFieldNumber; - 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" } } }; - fields = TestMap.Descriptor.FieldAccessorsByFieldNumber; - var dictionary = (IDictionary) fields[TestMap.MapStringStringFieldNumber].GetValue(mapMessage); - Assert.AreEqual(mapMessage.MapStringString, dictionary); - Assert.AreEqual("value1", dictionary["key1"]); - } - - [Test] - public void Reflection_Clear() - { - var message = SampleMessages.CreateFullTestAllTypes(); - var fields = TestAllTypes.Descriptor.FieldAccessorsByFieldNumber; - 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(SampleMessages.CreateFullTestAllTypes()) - { - 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" } } }; - fields = TestMap.Descriptor.FieldAccessorsByFieldNumber; - 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 = SampleMessages.CreateFullTestAllTypes(); - var fields = TestAllTypes.Descriptor.FieldAccessorsByFieldNumber; - 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(SampleMessages.CreateFullTestAllTypes()) - { - 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() - { - IMessage message = SampleMessages.CreateFullTestAllTypes(); - var fields = message.Descriptor.FieldAccessorsByFieldNumber; - Assert.Throws(() => fields[TestAllTypes.SingleBoolFieldNumber].SetValue(message, "This isn't a bool")); - } - - [Test] - public void Reflection_SetValue_MapFields() - { - IMessage message = new TestMap(); - var fields = message.Descriptor.FieldAccessorsByFieldNumber; - Assert.Throws(() => fields[TestMap.MapStringStringFieldNumber].SetValue(message, new Dictionary())); - } - - [Test] - public void Reflection_SetValue_RepeatedFields() - { - IMessage message = SampleMessages.CreateFullTestAllTypes(); - var fields = message.Descriptor.FieldAccessorsByFieldNumber; - Assert.Throws(() => fields[TestAllTypes.RepeatedDoubleFieldNumber].SetValue(message, new double[10])); - } - - [Test] - public void Reflection_GetValue_IncorrectType() - { - IMessage message = SampleMessages.CreateFullTestAllTypes(); - var fields = message.Descriptor.FieldAccessorsByFieldNumber; - Assert.Throws(() => fields[TestAllTypes.SingleBoolFieldNumber].GetValue(new TestMap())); - } - - [Test] - public void Reflection_Oneof() - { - var message = new TestAllTypes(); - var descriptor = TestAllTypes.Descriptor; - Assert.AreEqual(1, descriptor.Oneofs.Count); - var oneof = descriptor.Oneofs[0]; - Assert.AreEqual("oneof_field", oneof.Name); - Assert.IsNull(oneof.Accessor.GetCaseFieldDescriptor(message)); - - message.OneofString = "foo"; - Assert.AreSame(descriptor.FieldAccessorsByFieldNumber[TestAllTypes.OneofStringFieldNumber].Descriptor, oneof.Accessor.GetCaseFieldDescriptor(message)); - - message.OneofUint32 = 10; - Assert.AreSame(descriptor.FieldAccessorsByFieldNumber[TestAllTypes.OneofUint32FieldNumber].Descriptor, oneof.Accessor.GetCaseFieldDescriptor(message)); - - oneof.Accessor.Clear(message); - Assert.AreEqual(TestAllTypes.OneofFieldOneofCase.None, message.OneofFieldCase); - } } } diff --git a/csharp/src/Google.Protobuf.Test/Google.Protobuf.Test.csproj b/csharp/src/Google.Protobuf.Test/Google.Protobuf.Test.csproj index 6d8b4de2..2522901e 100644 --- a/csharp/src/Google.Protobuf.Test/Google.Protobuf.Test.csproj +++ b/csharp/src/Google.Protobuf.Test/Google.Protobuf.Test.csproj @@ -1,4 +1,4 @@ - + Debug @@ -82,6 +82,7 @@ + diff --git a/csharp/src/Google.Protobuf.Test/Reflection/DescriptorsTest.cs b/csharp/src/Google.Protobuf.Test/Reflection/DescriptorsTest.cs index 0aff0a6c..ed4291a4 100644 --- a/csharp/src/Google.Protobuf.Test/Reflection/DescriptorsTest.cs +++ b/csharp/src/Google.Protobuf.Test/Reflection/DescriptorsTest.cs @@ -33,6 +33,7 @@ using System.Linq; using Google.Protobuf.TestProtos; using NUnit.Framework; +using UnitTest.Issues.TestProtos; namespace Google.Protobuf.Reflection { @@ -220,5 +221,19 @@ namespace Google.Protobuf.Reflection CollectionAssert.AreEquivalent(expectedFields, descriptor.Fields); } + + [Test] + public void ConstructionWithoutGeneratedCodeInfo() + { + var data = UnittestIssues.Descriptor.Proto.ToByteArray(); + var newDescriptor = Google.Protobuf.Reflection.FileDescriptor.InternalBuildGeneratedFileFrom(data, new Reflection.FileDescriptor[] { }, null); + + // We should still be able to get at a field... + var messageDescriptor = newDescriptor.FindTypeByName("ItemField"); + var fieldDescriptor = messageDescriptor.FindFieldByName("item"); + // But there shouldn't be an accessor (or a generated type for the message) + Assert.IsNull(fieldDescriptor.Accessor); + Assert.IsNull(messageDescriptor.GeneratedType); + } } } \ No newline at end of file diff --git a/csharp/src/Google.Protobuf.Test/Reflection/FieldAccessTest.cs b/csharp/src/Google.Protobuf.Test/Reflection/FieldAccessTest.cs new file mode 100644 index 00000000..5d6e777f --- /dev/null +++ b/csharp/src/Google.Protobuf.Test/Reflection/FieldAccessTest.cs @@ -0,0 +1,218 @@ +#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 Google.Protobuf.TestProtos; +using NUnit.Framework; +using System; +using System.Collections; +using System.Collections.Generic; + +namespace Google.Protobuf.Reflection +{ + public class FieldAccessTest + { + [Test] + public void GetValue() + { + var message = SampleMessages.CreateFullTestAllTypes(); + var fields = TestAllTypes.Descriptor.FieldAccessors; + 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" } } }; + fields = TestMap.Descriptor.FieldAccessors; + var dictionary = (IDictionary) fields[TestMap.MapStringStringFieldNumber].GetValue(mapMessage); + Assert.AreEqual(mapMessage.MapStringString, dictionary); + Assert.AreEqual("value1", dictionary["key1"]); + } + + [Test] + public void Clear() + { + var message = SampleMessages.CreateFullTestAllTypes(); + var fields = TestAllTypes.Descriptor.FieldAccessors; + 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(SampleMessages.CreateFullTestAllTypes()) + { + 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" } } }; + fields = TestMap.Descriptor.FieldAccessors; + fields[TestMap.MapStringStringFieldNumber].Clear(mapMessage); + Assert.AreEqual(0, mapMessage.MapStringString.Count); + } + + [Test] + public void SetValue_SingleFields() + { + // Just a sample (primitives, messages, enums, strings, byte strings) + var message = SampleMessages.CreateFullTestAllTypes(); + var fields = TestAllTypes.Descriptor.FieldAccessors; + 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(SampleMessages.CreateFullTestAllTypes()) + { + 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 SetValue_SingleFields_WrongType() + { + IMessage message = SampleMessages.CreateFullTestAllTypes(); + var fields = message.Descriptor.FieldAccessors; + Assert.Throws(() => fields[TestAllTypes.SingleBoolFieldNumber].SetValue(message, "This isn't a bool")); + } + + [Test] + public void SetValue_MapFields() + { + IMessage message = new TestMap(); + var fields = message.Descriptor.FieldAccessors; + Assert.Throws(() => fields[TestMap.MapStringStringFieldNumber].SetValue(message, new Dictionary())); + } + + [Test] + public void SetValue_RepeatedFields() + { + IMessage message = SampleMessages.CreateFullTestAllTypes(); + var fields = message.Descriptor.FieldAccessors; + Assert.Throws(() => fields[TestAllTypes.RepeatedDoubleFieldNumber].SetValue(message, new double[10])); + } + + [Test] + public void GetValue_IncorrectType() + { + IMessage message = SampleMessages.CreateFullTestAllTypes(); + var fields = message.Descriptor.FieldAccessors; + Assert.Throws(() => fields[TestAllTypes.SingleBoolFieldNumber].GetValue(new TestMap())); + } + + [Test] + public void Oneof() + { + var message = new TestAllTypes(); + var descriptor = TestAllTypes.Descriptor; + Assert.AreEqual(1, descriptor.Oneofs.Count); + var oneof = descriptor.Oneofs[0]; + Assert.AreEqual("oneof_field", oneof.Name); + Assert.IsNull(oneof.Accessor.GetCaseFieldDescriptor(message)); + + message.OneofString = "foo"; + Assert.AreSame(descriptor.FieldAccessors[TestAllTypes.OneofStringFieldNumber].Descriptor, oneof.Accessor.GetCaseFieldDescriptor(message)); + + message.OneofUint32 = 10; + Assert.AreSame(descriptor.FieldAccessors[TestAllTypes.OneofUint32FieldNumber].Descriptor, oneof.Accessor.GetCaseFieldDescriptor(message)); + + oneof.Accessor.Clear(message); + Assert.AreEqual(TestAllTypes.OneofFieldOneofCase.None, message.OneofFieldCase); + } + + [Test] + public void FieldAccessor_ByName() + { + var descriptor = TestAllTypes.Descriptor; + Assert.AreSame( + descriptor.FieldAccessors[TestAllTypes.SingleBoolFieldNumber], + descriptor.FieldAccessors["single_bool"]); + } + + [Test] + public void FieldAccessor_NotFound() + { + var descriptor = TestAllTypes.Descriptor; + Assert.Throws(() => descriptor.FieldAccessors[999999].ToString()); + Assert.Throws(() => descriptor.FieldAccessors["not found"].ToString()); + } + } +} diff --git a/csharp/src/Google.Protobuf.Test/WellKnownTypes/WrappersTest.cs b/csharp/src/Google.Protobuf.Test/WellKnownTypes/WrappersTest.cs index c617db36..a8288483 100644 --- a/csharp/src/Google.Protobuf.Test/WellKnownTypes/WrappersTest.cs +++ b/csharp/src/Google.Protobuf.Test/WellKnownTypes/WrappersTest.cs @@ -192,7 +192,7 @@ namespace Google.Protobuf.WellKnownTypes Uint32Field = 3, Uint64Field = 4 }; - var fields = TestWellKnownTypes.Descriptor.FieldAccessorsByFieldNumber; + var fields = TestWellKnownTypes.Descriptor.FieldAccessors; Assert.AreEqual("x", fields[TestWellKnownTypes.StringFieldFieldNumber].GetValue(message)); Assert.AreEqual(ByteString.CopyFrom(1, 2, 3), fields[TestWellKnownTypes.BytesFieldFieldNumber].GetValue(message)); @@ -216,7 +216,7 @@ namespace Google.Protobuf.WellKnownTypes { // Just a single example... note that we can't have a null value here var message = new RepeatedWellKnownTypes { Int32Field = { 1, 2 } }; - var fields = RepeatedWellKnownTypes.Descriptor.FieldAccessorsByFieldNumber; + var fields = RepeatedWellKnownTypes.Descriptor.FieldAccessors; var list = (IList) fields[RepeatedWellKnownTypes.Int32FieldFieldNumber].GetValue(message); CollectionAssert.AreEqual(new[] { 1, 2 }, list); } @@ -226,7 +226,7 @@ namespace Google.Protobuf.WellKnownTypes { // Just a single example... note that we can't have a null value here var message = new MapWellKnownTypes { Int32Field = { { 1, 2 }, { 3, null } } }; - var fields = MapWellKnownTypes.Descriptor.FieldAccessorsByFieldNumber; + var fields = MapWellKnownTypes.Descriptor.FieldAccessors; var dictionary = (IDictionary) fields[MapWellKnownTypes.Int32FieldFieldNumber].GetValue(message); Assert.AreEqual(2, dictionary[1]); Assert.IsNull(dictionary[3]); diff --git a/csharp/src/Google.Protobuf/Reflection/FileDescriptor.cs b/csharp/src/Google.Protobuf/Reflection/FileDescriptor.cs index 041d4711..718c4797 100644 --- a/csharp/src/Google.Protobuf/Reflection/FileDescriptor.cs +++ b/csharp/src/Google.Protobuf/Reflection/FileDescriptor.cs @@ -231,7 +231,7 @@ namespace Google.Protobuf.Reflection /// Finds a type (message, enum, service or extension) in the file by name. Does not find nested types. /// /// The unqualified type name to look for. - /// The type of descriptor to look for (or ITypeDescriptor for any) + /// The type of descriptor to look for /// The type's descriptor, or null if not found. public T FindTypeByName(String name) where T : class, IDescriptor diff --git a/csharp/src/Google.Protobuf/Reflection/MessageDescriptor.cs b/csharp/src/Google.Protobuf/Reflection/MessageDescriptor.cs index b29b4b20..909a31ff 100644 --- a/csharp/src/Google.Protobuf/Reflection/MessageDescriptor.cs +++ b/csharp/src/Google.Protobuf/Reflection/MessageDescriptor.cs @@ -61,10 +61,10 @@ namespace Google.Protobuf.Reflection private readonly IList nestedTypes; private readonly IList enumTypes; private readonly IList fields; + private readonly FieldAccessorCollection fieldAccessors; private readonly IList oneofs; // CLR representation of the type described by this descriptor, if any. private readonly Type generatedType; - private IDictionary fieldAccessorsByFieldNumber; internal MessageDescriptor(DescriptorProto proto, FileDescriptor file, MessageDescriptor parent, int typeIndex, GeneratedCodeInfo generatedCodeInfo) : base(file, file.ComputeFullName(parent, proto.Name), typeIndex) @@ -94,6 +94,7 @@ namespace Google.Protobuf.Reflection (field, index) => new FieldDescriptor(field, file, this, index, generatedCodeInfo == null ? null : generatedCodeInfo.PropertyNames[index])); file.DescriptorPool.AddSymbol(this); + fieldAccessors = new FieldAccessorCollection(this); } /// @@ -135,14 +136,27 @@ namespace Google.Protobuf.Reflection get { return containingType; } } + // TODO: It's confusing that FieldAccessors[x] doesn't retrieve the accessor + // for Fields[x]. We should think about this further... how often does a user really + // want the fields in declaration order? + /// - /// An unmodifiable list of this message type's fields. + /// An unmodifiable list of this message type's fields, in the declaration order + /// within the .proto file. /// public IList Fields { get { return fields; } } + /// + /// A collection of accessors, which can be retrieved by name or field number. + /// + public FieldAccessorCollection FieldAccessors + { + get { return fieldAccessors; } + } + /// /// An unmodifiable list of this message type's nested types. /// @@ -164,13 +178,6 @@ namespace Google.Protobuf.Reflection get { return oneofs; } } - /// - /// Returns a map from field number to accessor. - /// TODO: Revisit this. It's mostly in place to make the transition from FieldAccessorTable - /// to descriptor-based reflection simple in terms of tests. Work out what we really want. - /// - public IDictionary FieldAccessorsByFieldNumber { get { return fieldAccessorsByFieldNumber; } } - /// /// Finds a field by field name. /// @@ -222,8 +229,61 @@ namespace Google.Protobuf.Reflection { oneof.CrossLink(); } + } + + /// + /// A collection to simplify retrieving the field accessor for a particular field. + /// + public sealed class FieldAccessorCollection + { + private readonly MessageDescriptor messageDescriptor; + + internal FieldAccessorCollection(MessageDescriptor messageDescriptor) + { + this.messageDescriptor = messageDescriptor; + } - fieldAccessorsByFieldNumber = new ReadOnlyDictionary(fields.ToDictionary(field => field.FieldNumber, field => field.Accessor)); + /// + /// Retrieves the accessor for the field with the given number. + /// + /// Number of the field to retrieve the accessor for + /// The accessor for the given field, or null if reflective field access is + /// not supported for the field. + /// The message descriptor does not contain a field + /// with the given number + public IFieldAccessor this[int number] + { + get + { + var fieldDescriptor = messageDescriptor.FindFieldByNumber(number); + if (fieldDescriptor == null) + { + throw new KeyNotFoundException("No such field number"); + } + return fieldDescriptor.Accessor; + } + } + + /// + /// Retrieves the accessor for the field with the given name. + /// + /// Number of the field to retrieve the accessor for + /// The accessor for the given field, or null if reflective field access is + /// not supported for the field. + /// The message descriptor does not contain a field + /// with the given name + public IFieldAccessor this[string name] + { + get + { + var fieldDescriptor = messageDescriptor.FindFieldByName(name); + if (fieldDescriptor == null) + { + throw new KeyNotFoundException("No such field name"); + } + return fieldDescriptor.Accessor; + } + } } } } \ No newline at end of file -- cgit v1.2.3 From 5e0cfc9a4796ab449e042098e914ace9a635c0b6 Mon Sep 17 00:00:00 2001 From: Jon Skeet Date: Wed, 22 Jul 2015 18:43:15 +0100 Subject: Added newlines --- csharp/src/Google.Protobuf.Test/Reflection/DescriptorsTest.cs | 2 +- csharp/src/Google.Protobuf/Reflection/MessageDescriptor.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) (limited to 'csharp/src/Google.Protobuf.Test/Reflection') diff --git a/csharp/src/Google.Protobuf.Test/Reflection/DescriptorsTest.cs b/csharp/src/Google.Protobuf.Test/Reflection/DescriptorsTest.cs index ed4291a4..f4128e08 100644 --- a/csharp/src/Google.Protobuf.Test/Reflection/DescriptorsTest.cs +++ b/csharp/src/Google.Protobuf.Test/Reflection/DescriptorsTest.cs @@ -236,4 +236,4 @@ namespace Google.Protobuf.Reflection Assert.IsNull(messageDescriptor.GeneratedType); } } -} \ No newline at end of file +} diff --git a/csharp/src/Google.Protobuf/Reflection/MessageDescriptor.cs b/csharp/src/Google.Protobuf/Reflection/MessageDescriptor.cs index 909a31ff..aa585c77 100644 --- a/csharp/src/Google.Protobuf/Reflection/MessageDescriptor.cs +++ b/csharp/src/Google.Protobuf/Reflection/MessageDescriptor.cs @@ -286,4 +286,4 @@ namespace Google.Protobuf.Reflection } } } -} \ No newline at end of file +} -- cgit v1.2.3 From c1c6b2d0d579d863c2ff3709a0053039801f5430 Mon Sep 17 00:00:00 2001 From: Jon Skeet Date: Wed, 22 Jul 2015 19:57:29 +0100 Subject: Implemented Jan's suggestion of FieldCollection, replacing FieldAccessorCollection. I think Jan was actually suggesting keeping both, but that feels redundant to me. The test diff is misleading here IMO, because I wouldn't expect real code using reflection to use several accessors one after another like this, unless it was within a loop. Evidence to the contrary would be welcome :) This change also incidentally goes part way to fixing the issue of the JSON formatter not writing out the fields in field number order - with this change, it does except for oneofs, which we can fix in a follow-up change. I haven't actually added a test with a message with fields deliberately out of order - I'm happy to do so though. It feels like it would make sense to be in google/src/protobuf, but it's not entirely clear what the rules of engagement are for adding new messages there. (unittest_proto3.proto?) --- .../Reflection/DescriptorsTest.cs | 7 +- .../Reflection/FieldAccessTest.cs | 128 ++++++++++----------- .../WellKnownTypes/WrappersTest.cs | 32 +++--- csharp/src/Google.Protobuf/JsonFormatter.cs | 2 +- .../Reflection/MessageDescriptor.cs | 76 ++++++------ .../Google.Protobuf/Reflection/OneofDescriptor.cs | 2 +- 6 files changed, 128 insertions(+), 119 deletions(-) (limited to 'csharp/src/Google.Protobuf.Test/Reflection') diff --git a/csharp/src/Google.Protobuf.Test/Reflection/DescriptorsTest.cs b/csharp/src/Google.Protobuf.Test/Reflection/DescriptorsTest.cs index f4128e08..5b6dc0fb 100644 --- a/csharp/src/Google.Protobuf.Test/Reflection/DescriptorsTest.cs +++ b/csharp/src/Google.Protobuf.Test/Reflection/DescriptorsTest.cs @@ -103,15 +103,16 @@ namespace Google.Protobuf.Reflection Assert.AreEqual(UnittestProto3.Descriptor, nestedType.File); Assert.AreEqual(messageType, nestedType.ContainingType); - FieldDescriptor field = messageType.Fields[0]; + FieldDescriptor field = messageType.Fields.InDeclarationOrder()[0]; Assert.AreEqual("single_int32", field.Name); Assert.AreEqual(field, messageType.FindDescriptor("single_int32")); Assert.Null(messageType.FindDescriptor("no_such_field")); Assert.AreEqual(field, messageType.FindFieldByNumber(1)); Assert.Null(messageType.FindFieldByNumber(571283)); - for (int i = 0; i < messageType.Fields.Count; i++) + var fieldsInDeclarationOrder = messageType.Fields.InDeclarationOrder(); + for (int i = 0; i < fieldsInDeclarationOrder.Count; i++) { - Assert.AreEqual(i, messageType.Fields[i].Index); + Assert.AreEqual(i, fieldsInDeclarationOrder[i].Index); } Assert.AreEqual(nestedType, messageType.NestedTypes[0]); diff --git a/csharp/src/Google.Protobuf.Test/Reflection/FieldAccessTest.cs b/csharp/src/Google.Protobuf.Test/Reflection/FieldAccessTest.cs index 5d6e777f..6e1d804e 100644 --- a/csharp/src/Google.Protobuf.Test/Reflection/FieldAccessTest.cs +++ b/csharp/src/Google.Protobuf.Test/Reflection/FieldAccessTest.cs @@ -44,43 +44,43 @@ namespace Google.Protobuf.Reflection public void GetValue() { var message = SampleMessages.CreateFullTestAllTypes(); - var fields = TestAllTypes.Descriptor.FieldAccessors; - 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)); + var fields = TestAllTypes.Descriptor.Fields; + Assert.AreEqual(message.SingleBool, fields[TestAllTypes.SingleBoolFieldNumber].Accessor.GetValue(message)); + Assert.AreEqual(message.SingleBytes, fields[TestAllTypes.SingleBytesFieldNumber].Accessor.GetValue(message)); + Assert.AreEqual(message.SingleDouble, fields[TestAllTypes.SingleDoubleFieldNumber].Accessor.GetValue(message)); + Assert.AreEqual(message.SingleFixed32, fields[TestAllTypes.SingleFixed32FieldNumber].Accessor.GetValue(message)); + Assert.AreEqual(message.SingleFixed64, fields[TestAllTypes.SingleFixed64FieldNumber].Accessor.GetValue(message)); + Assert.AreEqual(message.SingleFloat, fields[TestAllTypes.SingleFloatFieldNumber].Accessor.GetValue(message)); + Assert.AreEqual(message.SingleForeignEnum, fields[TestAllTypes.SingleForeignEnumFieldNumber].Accessor.GetValue(message)); + Assert.AreEqual(message.SingleForeignMessage, fields[TestAllTypes.SingleForeignMessageFieldNumber].Accessor.GetValue(message)); + Assert.AreEqual(message.SingleImportEnum, fields[TestAllTypes.SingleImportEnumFieldNumber].Accessor.GetValue(message)); + Assert.AreEqual(message.SingleImportMessage, fields[TestAllTypes.SingleImportMessageFieldNumber].Accessor.GetValue(message)); + Assert.AreEqual(message.SingleInt32, fields[TestAllTypes.SingleInt32FieldNumber].Accessor.GetValue(message)); + Assert.AreEqual(message.SingleInt64, fields[TestAllTypes.SingleInt64FieldNumber].Accessor.GetValue(message)); + Assert.AreEqual(message.SingleNestedEnum, fields[TestAllTypes.SingleNestedEnumFieldNumber].Accessor.GetValue(message)); + Assert.AreEqual(message.SingleNestedMessage, fields[TestAllTypes.SingleNestedMessageFieldNumber].Accessor.GetValue(message)); + Assert.AreEqual(message.SinglePublicImportMessage, fields[TestAllTypes.SinglePublicImportMessageFieldNumber].Accessor.GetValue(message)); + Assert.AreEqual(message.SingleSint32, fields[TestAllTypes.SingleSint32FieldNumber].Accessor.GetValue(message)); + Assert.AreEqual(message.SingleSint64, fields[TestAllTypes.SingleSint64FieldNumber].Accessor.GetValue(message)); + Assert.AreEqual(message.SingleString, fields[TestAllTypes.SingleStringFieldNumber].Accessor.GetValue(message)); + Assert.AreEqual(message.SingleSfixed32, fields[TestAllTypes.SingleSfixed32FieldNumber].Accessor.GetValue(message)); + Assert.AreEqual(message.SingleSfixed64, fields[TestAllTypes.SingleSfixed64FieldNumber].Accessor.GetValue(message)); + Assert.AreEqual(message.SingleUint32, fields[TestAllTypes.SingleUint32FieldNumber].Accessor.GetValue(message)); + Assert.AreEqual(message.SingleUint64, fields[TestAllTypes.SingleUint64FieldNumber].Accessor.GetValue(message)); + Assert.AreEqual(message.OneofBytes, fields[TestAllTypes.OneofBytesFieldNumber].Accessor.GetValue(message)); + Assert.AreEqual(message.OneofString, fields[TestAllTypes.OneofStringFieldNumber].Accessor.GetValue(message)); + Assert.AreEqual(message.OneofNestedMessage, fields[TestAllTypes.OneofNestedMessageFieldNumber].Accessor.GetValue(message)); + Assert.AreEqual(message.OneofUint32, fields[TestAllTypes.OneofUint32FieldNumber].Accessor.GetValue(message)); // Just one example for repeated fields - they're all just returning the list - var list = (IList) fields[TestAllTypes.RepeatedInt32FieldNumber].GetValue(message); + var list = (IList) fields[TestAllTypes.RepeatedInt32FieldNumber].Accessor.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" } } }; - fields = TestMap.Descriptor.FieldAccessors; - var dictionary = (IDictionary) fields[TestMap.MapStringStringFieldNumber].GetValue(mapMessage); + fields = TestMap.Descriptor.Fields; + var dictionary = (IDictionary) fields[TestMap.MapStringStringFieldNumber].Accessor.GetValue(mapMessage); Assert.AreEqual(mapMessage.MapStringString, dictionary); Assert.AreEqual("value1", dictionary["key1"]); } @@ -89,14 +89,14 @@ namespace Google.Protobuf.Reflection public void Clear() { var message = SampleMessages.CreateFullTestAllTypes(); - var fields = TestAllTypes.Descriptor.FieldAccessors; - 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 fields = TestAllTypes.Descriptor.Fields; + fields[TestAllTypes.SingleBoolFieldNumber].Accessor.Clear(message); + fields[TestAllTypes.SingleInt32FieldNumber].Accessor.Clear(message); + fields[TestAllTypes.SingleStringFieldNumber].Accessor.Clear(message); + fields[TestAllTypes.SingleBytesFieldNumber].Accessor.Clear(message); + fields[TestAllTypes.SingleForeignEnumFieldNumber].Accessor.Clear(message); + fields[TestAllTypes.SingleForeignMessageFieldNumber].Accessor.Clear(message); + fields[TestAllTypes.RepeatedDoubleFieldNumber].Accessor.Clear(message); var expected = new TestAllTypes(SampleMessages.CreateFullTestAllTypes()) { @@ -113,8 +113,8 @@ namespace Google.Protobuf.Reflection // Separately, maps. var mapMessage = new TestMap { MapStringString = { { "key1", "value1" }, { "key2", "value2" } } }; - fields = TestMap.Descriptor.FieldAccessors; - fields[TestMap.MapStringStringFieldNumber].Clear(mapMessage); + fields = TestMap.Descriptor.Fields; + fields[TestMap.MapStringStringFieldNumber].Accessor.Clear(mapMessage); Assert.AreEqual(0, mapMessage.MapStringString.Count); } @@ -123,14 +123,14 @@ namespace Google.Protobuf.Reflection { // Just a sample (primitives, messages, enums, strings, byte strings) var message = SampleMessages.CreateFullTestAllTypes(); - var fields = TestAllTypes.Descriptor.FieldAccessors; - 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 fields = TestAllTypes.Descriptor.Fields; + fields[TestAllTypes.SingleBoolFieldNumber].Accessor.SetValue(message, false); + fields[TestAllTypes.SingleInt32FieldNumber].Accessor.SetValue(message, 500); + fields[TestAllTypes.SingleStringFieldNumber].Accessor.SetValue(message, "It's a string"); + fields[TestAllTypes.SingleBytesFieldNumber].Accessor.SetValue(message, ByteString.CopyFrom(99, 98, 97)); + fields[TestAllTypes.SingleForeignEnumFieldNumber].Accessor.SetValue(message, ForeignEnum.FOREIGN_FOO); + fields[TestAllTypes.SingleForeignMessageFieldNumber].Accessor.SetValue(message, new ForeignMessage { C = 12345 }); + fields[TestAllTypes.SingleDoubleFieldNumber].Accessor.SetValue(message, 20150701.5); var expected = new TestAllTypes(SampleMessages.CreateFullTestAllTypes()) { @@ -150,32 +150,32 @@ namespace Google.Protobuf.Reflection public void SetValue_SingleFields_WrongType() { IMessage message = SampleMessages.CreateFullTestAllTypes(); - var fields = message.Descriptor.FieldAccessors; - Assert.Throws(() => fields[TestAllTypes.SingleBoolFieldNumber].SetValue(message, "This isn't a bool")); + var fields = message.Descriptor.Fields; + Assert.Throws(() => fields[TestAllTypes.SingleBoolFieldNumber].Accessor.SetValue(message, "This isn't a bool")); } [Test] public void SetValue_MapFields() { IMessage message = new TestMap(); - var fields = message.Descriptor.FieldAccessors; - Assert.Throws(() => fields[TestMap.MapStringStringFieldNumber].SetValue(message, new Dictionary())); + var fields = message.Descriptor.Fields; + Assert.Throws(() => fields[TestMap.MapStringStringFieldNumber].Accessor.SetValue(message, new Dictionary())); } [Test] public void SetValue_RepeatedFields() { IMessage message = SampleMessages.CreateFullTestAllTypes(); - var fields = message.Descriptor.FieldAccessors; - Assert.Throws(() => fields[TestAllTypes.RepeatedDoubleFieldNumber].SetValue(message, new double[10])); + var fields = message.Descriptor.Fields; + Assert.Throws(() => fields[TestAllTypes.RepeatedDoubleFieldNumber].Accessor.SetValue(message, new double[10])); } [Test] public void GetValue_IncorrectType() { IMessage message = SampleMessages.CreateFullTestAllTypes(); - var fields = message.Descriptor.FieldAccessors; - Assert.Throws(() => fields[TestAllTypes.SingleBoolFieldNumber].GetValue(new TestMap())); + var fields = message.Descriptor.Fields; + Assert.Throws(() => fields[TestAllTypes.SingleBoolFieldNumber].Accessor.GetValue(new TestMap())); } [Test] @@ -189,30 +189,30 @@ namespace Google.Protobuf.Reflection Assert.IsNull(oneof.Accessor.GetCaseFieldDescriptor(message)); message.OneofString = "foo"; - Assert.AreSame(descriptor.FieldAccessors[TestAllTypes.OneofStringFieldNumber].Descriptor, oneof.Accessor.GetCaseFieldDescriptor(message)); + Assert.AreSame(descriptor.Fields[TestAllTypes.OneofStringFieldNumber], oneof.Accessor.GetCaseFieldDescriptor(message)); message.OneofUint32 = 10; - Assert.AreSame(descriptor.FieldAccessors[TestAllTypes.OneofUint32FieldNumber].Descriptor, oneof.Accessor.GetCaseFieldDescriptor(message)); + Assert.AreSame(descriptor.Fields[TestAllTypes.OneofUint32FieldNumber], oneof.Accessor.GetCaseFieldDescriptor(message)); oneof.Accessor.Clear(message); Assert.AreEqual(TestAllTypes.OneofFieldOneofCase.None, message.OneofFieldCase); } [Test] - public void FieldAccessor_ByName() + public void FieldDescriptor_ByName() { var descriptor = TestAllTypes.Descriptor; Assert.AreSame( - descriptor.FieldAccessors[TestAllTypes.SingleBoolFieldNumber], - descriptor.FieldAccessors["single_bool"]); + descriptor.Fields[TestAllTypes.SingleBoolFieldNumber], + descriptor.Fields["single_bool"]); } [Test] - public void FieldAccessor_NotFound() + public void FieldDescriptor_NotFound() { var descriptor = TestAllTypes.Descriptor; - Assert.Throws(() => descriptor.FieldAccessors[999999].ToString()); - Assert.Throws(() => descriptor.FieldAccessors["not found"].ToString()); + Assert.Throws(() => descriptor.Fields[999999].ToString()); + Assert.Throws(() => descriptor.Fields["not found"].ToString()); } } } diff --git a/csharp/src/Google.Protobuf.Test/WellKnownTypes/WrappersTest.cs b/csharp/src/Google.Protobuf.Test/WellKnownTypes/WrappersTest.cs index a8288483..670bc5f8 100644 --- a/csharp/src/Google.Protobuf.Test/WellKnownTypes/WrappersTest.cs +++ b/csharp/src/Google.Protobuf.Test/WellKnownTypes/WrappersTest.cs @@ -192,23 +192,23 @@ namespace Google.Protobuf.WellKnownTypes Uint32Field = 3, Uint64Field = 4 }; - var fields = TestWellKnownTypes.Descriptor.FieldAccessors; + var fields = TestWellKnownTypes.Descriptor.Fields; - Assert.AreEqual("x", fields[TestWellKnownTypes.StringFieldFieldNumber].GetValue(message)); - Assert.AreEqual(ByteString.CopyFrom(1, 2, 3), fields[TestWellKnownTypes.BytesFieldFieldNumber].GetValue(message)); - Assert.AreEqual(true, fields[TestWellKnownTypes.BoolFieldFieldNumber].GetValue(message)); - Assert.AreEqual(12.5f, fields[TestWellKnownTypes.FloatFieldFieldNumber].GetValue(message)); - Assert.AreEqual(12.25d, fields[TestWellKnownTypes.DoubleFieldFieldNumber].GetValue(message)); - Assert.AreEqual(1, fields[TestWellKnownTypes.Int32FieldFieldNumber].GetValue(message)); - Assert.AreEqual(2L, fields[TestWellKnownTypes.Int64FieldFieldNumber].GetValue(message)); - Assert.AreEqual(3U, fields[TestWellKnownTypes.Uint32FieldFieldNumber].GetValue(message)); - Assert.AreEqual(4UL, fields[TestWellKnownTypes.Uint64FieldFieldNumber].GetValue(message)); + Assert.AreEqual("x", fields[TestWellKnownTypes.StringFieldFieldNumber].Accessor.GetValue(message)); + Assert.AreEqual(ByteString.CopyFrom(1, 2, 3), fields[TestWellKnownTypes.BytesFieldFieldNumber].Accessor.GetValue(message)); + Assert.AreEqual(true, fields[TestWellKnownTypes.BoolFieldFieldNumber].Accessor.GetValue(message)); + Assert.AreEqual(12.5f, fields[TestWellKnownTypes.FloatFieldFieldNumber].Accessor.GetValue(message)); + Assert.AreEqual(12.25d, fields[TestWellKnownTypes.DoubleFieldFieldNumber].Accessor.GetValue(message)); + Assert.AreEqual(1, fields[TestWellKnownTypes.Int32FieldFieldNumber].Accessor.GetValue(message)); + Assert.AreEqual(2L, fields[TestWellKnownTypes.Int64FieldFieldNumber].Accessor.GetValue(message)); + Assert.AreEqual(3U, fields[TestWellKnownTypes.Uint32FieldFieldNumber].Accessor.GetValue(message)); + Assert.AreEqual(4UL, fields[TestWellKnownTypes.Uint64FieldFieldNumber].Accessor.GetValue(message)); // And a couple of null fields... message.StringField = null; message.FloatField = null; - Assert.IsNull(fields[TestWellKnownTypes.StringFieldFieldNumber].GetValue(message)); - Assert.IsNull(fields[TestWellKnownTypes.FloatFieldFieldNumber].GetValue(message)); + Assert.IsNull(fields[TestWellKnownTypes.StringFieldFieldNumber].Accessor.GetValue(message)); + Assert.IsNull(fields[TestWellKnownTypes.FloatFieldFieldNumber].Accessor.GetValue(message)); } [Test] @@ -216,8 +216,8 @@ namespace Google.Protobuf.WellKnownTypes { // Just a single example... note that we can't have a null value here var message = new RepeatedWellKnownTypes { Int32Field = { 1, 2 } }; - var fields = RepeatedWellKnownTypes.Descriptor.FieldAccessors; - var list = (IList) fields[RepeatedWellKnownTypes.Int32FieldFieldNumber].GetValue(message); + var fields = RepeatedWellKnownTypes.Descriptor.Fields; + var list = (IList) fields[RepeatedWellKnownTypes.Int32FieldFieldNumber].Accessor.GetValue(message); CollectionAssert.AreEqual(new[] { 1, 2 }, list); } @@ -226,8 +226,8 @@ namespace Google.Protobuf.WellKnownTypes { // Just a single example... note that we can't have a null value here var message = new MapWellKnownTypes { Int32Field = { { 1, 2 }, { 3, null } } }; - var fields = MapWellKnownTypes.Descriptor.FieldAccessors; - var dictionary = (IDictionary) fields[MapWellKnownTypes.Int32FieldFieldNumber].GetValue(message); + var fields = MapWellKnownTypes.Descriptor.Fields; + var dictionary = (IDictionary) fields[MapWellKnownTypes.Int32FieldFieldNumber].Accessor.GetValue(message); Assert.AreEqual(2, dictionary[1]); Assert.IsNull(dictionary[3]); Assert.IsTrue(dictionary.Contains(3)); diff --git a/csharp/src/Google.Protobuf/JsonFormatter.cs b/csharp/src/Google.Protobuf/JsonFormatter.cs index 7f13e33e..f624b090 100644 --- a/csharp/src/Google.Protobuf/JsonFormatter.cs +++ b/csharp/src/Google.Protobuf/JsonFormatter.cs @@ -140,7 +140,7 @@ namespace Google.Protobuf var fields = message.Descriptor.Fields; bool first = true; // First non-oneof fields - foreach (var field in fields) + foreach (var field in fields.InFieldNumberOrder()) { var accessor = field.Accessor; // Oneofs are written later diff --git a/csharp/src/Google.Protobuf/Reflection/MessageDescriptor.cs b/csharp/src/Google.Protobuf/Reflection/MessageDescriptor.cs index aa585c77..1250774d 100644 --- a/csharp/src/Google.Protobuf/Reflection/MessageDescriptor.cs +++ b/csharp/src/Google.Protobuf/Reflection/MessageDescriptor.cs @@ -33,6 +33,7 @@ using Google.Protobuf.Collections; using System; using System.Collections.Generic; +using System.Collections.ObjectModel; using System.Linq; namespace Google.Protobuf.Reflection @@ -60,8 +61,9 @@ namespace Google.Protobuf.Reflection private readonly MessageDescriptor containingType; private readonly IList nestedTypes; private readonly IList enumTypes; - private readonly IList fields; - private readonly FieldAccessorCollection fieldAccessors; + private readonly IList fieldsInDeclarationOrder; + private readonly IList fieldsInNumberOrder; + private readonly FieldCollection fields; private readonly IList oneofs; // CLR representation of the type described by this descriptor, if any. private readonly Type generatedType; @@ -89,12 +91,13 @@ namespace Google.Protobuf.Reflection (type, index) => new EnumDescriptor(type, file, this, index, generatedCodeInfo == null ? null : generatedCodeInfo.NestedEnums[index])); - fields = DescriptorUtil.ConvertAndMakeReadOnly( + fieldsInDeclarationOrder = DescriptorUtil.ConvertAndMakeReadOnly( proto.Field, (field, index) => new FieldDescriptor(field, file, this, index, generatedCodeInfo == null ? null : generatedCodeInfo.PropertyNames[index])); + fieldsInNumberOrder = new ReadOnlyCollection(fieldsInDeclarationOrder.OrderBy(field => field.FieldNumber).ToArray()); file.DescriptorPool.AddSymbol(this); - fieldAccessors = new FieldAccessorCollection(this); + fields = new FieldCollection(this); } /// @@ -136,27 +139,14 @@ namespace Google.Protobuf.Reflection get { return containingType; } } - // TODO: It's confusing that FieldAccessors[x] doesn't retrieve the accessor - // for Fields[x]. We should think about this further... how often does a user really - // want the fields in declaration order? - /// - /// An unmodifiable list of this message type's fields, in the declaration order - /// within the .proto file. + /// A collection of fields, which can be retrieved by name or field number. /// - public IList Fields + public FieldCollection Fields { get { return fields; } } - /// - /// A collection of accessors, which can be retrieved by name or field number. - /// - public FieldAccessorCollection FieldAccessors - { - get { return fieldAccessors; } - } - /// /// An unmodifiable list of this message type's nested types. /// @@ -220,7 +210,7 @@ namespace Google.Protobuf.Reflection message.CrossLink(); } - foreach (FieldDescriptor field in fields) + foreach (FieldDescriptor field in fieldsInDeclarationOrder) { field.CrossLink(); } @@ -234,24 +224,43 @@ namespace Google.Protobuf.Reflection /// /// A collection to simplify retrieving the field accessor for a particular field. /// - public sealed class FieldAccessorCollection + public sealed class FieldCollection { private readonly MessageDescriptor messageDescriptor; - internal FieldAccessorCollection(MessageDescriptor messageDescriptor) + internal FieldCollection(MessageDescriptor messageDescriptor) { this.messageDescriptor = messageDescriptor; } + /// + /// Returns the fields in the message as an immutable list, in the order in which they + /// are declared in the source .proto file. + /// + public IList InDeclarationOrder() + { + return messageDescriptor.fieldsInDeclarationOrder; + } + + /// + /// Returns the fields in the message as an immutable list, in ascending field number + /// order. Field numbers need not be contiguous, so there is no direct mapping from the + /// index in the list to the field number; to retrieve a field by field number, it is better + /// to use the indexer. + /// + public IList InFieldNumberOrder() + { + return messageDescriptor.fieldsInDeclarationOrder; + } + /// - /// Retrieves the accessor for the field with the given number. + /// Retrieves the descriptor for the field with the given number. /// - /// Number of the field to retrieve the accessor for - /// The accessor for the given field, or null if reflective field access is - /// not supported for the field. + /// Number of the field to retrieve the descriptor for + /// The accessor for the given field /// The message descriptor does not contain a field /// with the given number - public IFieldAccessor this[int number] + public FieldDescriptor this[int number] { get { @@ -260,19 +269,18 @@ namespace Google.Protobuf.Reflection { throw new KeyNotFoundException("No such field number"); } - return fieldDescriptor.Accessor; + return fieldDescriptor; } } /// - /// Retrieves the accessor for the field with the given name. + /// Retrieves the descriptor for the field with the given name. /// - /// Number of the field to retrieve the accessor for - /// The accessor for the given field, or null if reflective field access is - /// not supported for the field. + /// Number of the field to retrieve the descriptor for + /// The descriptor for the given field /// The message descriptor does not contain a field /// with the given name - public IFieldAccessor this[string name] + public FieldDescriptor this[string name] { get { @@ -281,7 +289,7 @@ namespace Google.Protobuf.Reflection { throw new KeyNotFoundException("No such field name"); } - return fieldDescriptor.Accessor; + return fieldDescriptor; } } } diff --git a/csharp/src/Google.Protobuf/Reflection/OneofDescriptor.cs b/csharp/src/Google.Protobuf/Reflection/OneofDescriptor.cs index a79d9de4..cd4c5534 100644 --- a/csharp/src/Google.Protobuf/Reflection/OneofDescriptor.cs +++ b/csharp/src/Google.Protobuf/Reflection/OneofDescriptor.cs @@ -70,7 +70,7 @@ namespace Google.Protobuf.Reflection internal void CrossLink() { List fieldCollection = new List(); - foreach (var field in ContainingType.Fields) + foreach (var field in ContainingType.Fields.InDeclarationOrder()) { if (field.ContainingOneof == this) { -- cgit v1.2.3 From 3b8c83eff151aaf1e3d931b2dc9c98684211cde1 Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Fri, 24 Jul 2015 20:27:35 -0700 Subject: expose original binary data for filedescriptor --- .../Reflection/DescriptorsTest.cs | 2 ++ .../Google.Protobuf/Reflection/FileDescriptor.cs | 23 ++++++++++++++++++---- 2 files changed, 21 insertions(+), 4 deletions(-) (limited to 'csharp/src/Google.Protobuf.Test/Reflection') diff --git a/csharp/src/Google.Protobuf.Test/Reflection/DescriptorsTest.cs b/csharp/src/Google.Protobuf.Test/Reflection/DescriptorsTest.cs index 5b6dc0fb..e84a0727 100644 --- a/csharp/src/Google.Protobuf.Test/Reflection/DescriptorsTest.cs +++ b/csharp/src/Google.Protobuf.Test/Reflection/DescriptorsTest.cs @@ -82,6 +82,8 @@ namespace Google.Protobuf.Reflection { Assert.AreEqual(i, file.EnumTypes[i].Index); } + + Assert.AreEqual(10, file.SerializedData[0]); } [Test] diff --git a/csharp/src/Google.Protobuf/Reflection/FileDescriptor.cs b/csharp/src/Google.Protobuf/Reflection/FileDescriptor.cs index 718c4797..c17c4cc4 100644 --- a/csharp/src/Google.Protobuf/Reflection/FileDescriptor.cs +++ b/csharp/src/Google.Protobuf/Reflection/FileDescriptor.cs @@ -43,6 +43,7 @@ namespace Google.Protobuf.Reflection /// public sealed class FileDescriptor : IDescriptor { + private readonly ByteString descriptorData; private readonly FileDescriptorProto proto; private readonly IList messageTypes; private readonly IList enumTypes; @@ -62,8 +63,9 @@ namespace Google.Protobuf.Reflection get { return proto.Syntax == "proto3" ? ProtoSyntax.Proto3 : ProtoSyntax.Proto2; } } - private FileDescriptor(FileDescriptorProto proto, FileDescriptor[] dependencies, DescriptorPool pool, bool allowUnknownDependencies, GeneratedCodeInfo generatedCodeInfo) + private FileDescriptor(ByteString descriptorData, FileDescriptorProto proto, FileDescriptor[] dependencies, DescriptorPool pool, bool allowUnknownDependencies, GeneratedCodeInfo generatedCodeInfo) { + this.descriptorData = descriptorData; this.pool = pool; this.proto = proto; this.dependencies = new ReadOnlyCollection((FileDescriptor[]) dependencies.Clone()); @@ -203,6 +205,14 @@ namespace Google.Protobuf.Reflection get { return publicDependencies; } } + /// + /// The original serialized binary form of this descriptor. + /// + public ByteString SerializedData + { + get { return descriptorData; } + } + /// /// Implementation of IDescriptor.FullName - just returns the same as Name. /// @@ -257,6 +267,9 @@ namespace Google.Protobuf.Reflection /// /// Builds a FileDescriptor from its protocol buffer representation. /// + /// The original serialized descriptor data. + /// We have only limited proto2 support, so serializing FileDescriptorProto + /// would not necessarily give us this. /// The protocol message form of the FileDescriptor. /// FileDescriptors corresponding to all of the /// file's dependencies, in the exact order listed in the .proto file. May be null, @@ -266,7 +279,7 @@ namespace Google.Protobuf.Reflection /// If is not /// a valid descriptor. This can occur for a number of reasons, such as a field /// having an undefined type or because two messages were defined with the same name. - private static FileDescriptor BuildFrom(FileDescriptorProto proto, FileDescriptor[] dependencies, bool allowUnknownDependencies, GeneratedCodeInfo generatedCodeInfo) + private static FileDescriptor BuildFrom(ByteString descriptorData, FileDescriptorProto proto, FileDescriptor[] dependencies, bool allowUnknownDependencies, GeneratedCodeInfo generatedCodeInfo) { // Building descriptors involves two steps: translating and linking. // In the translation step (implemented by FileDescriptor's @@ -283,7 +296,7 @@ namespace Google.Protobuf.Reflection } DescriptorPool pool = new DescriptorPool(dependencies); - FileDescriptor result = new FileDescriptor(proto, dependencies, pool, allowUnknownDependencies, generatedCodeInfo); + FileDescriptor result = new FileDescriptor(descriptorData, proto, dependencies, pool, allowUnknownDependencies, generatedCodeInfo); // TODO(jonskeet): Reinstate these checks, or get rid of them entirely. They aren't in the Java code, // and fail for the CustomOptions test right now. (We get "descriptor.proto" vs "google/protobuf/descriptor.proto".) @@ -342,11 +355,13 @@ namespace Google.Protobuf.Reflection throw new ArgumentException("Failed to parse protocol buffer descriptor for generated code.", e); } + + try { // When building descriptors for generated code, we allow unknown // dependencies by default. - return BuildFrom(proto, dependencies, true, generatedCodeInfo); + return BuildFrom(ByteString.CopyFrom(descriptorData), proto, dependencies, true, generatedCodeInfo); } catch (DescriptorValidationException e) { -- cgit v1.2.3 From fd02e45b2a2f38da25ee2d202ed911b684fe30ac Mon Sep 17 00:00:00 2001 From: Jon Skeet Date: Fri, 31 Jul 2015 10:32:04 +0100 Subject: Fix trivial bug in field orderings. (Shows the benefit of unit testing even code "too simple to fail"...) --- .../src/Google.Protobuf.Test/Reflection/DescriptorsTest.cs | 13 +++++++++++++ csharp/src/Google.Protobuf/Reflection/MessageDescriptor.cs | 2 +- 2 files changed, 14 insertions(+), 1 deletion(-) (limited to 'csharp/src/Google.Protobuf.Test/Reflection') diff --git a/csharp/src/Google.Protobuf.Test/Reflection/DescriptorsTest.cs b/csharp/src/Google.Protobuf.Test/Reflection/DescriptorsTest.cs index e84a0727..643816e6 100644 --- a/csharp/src/Google.Protobuf.Test/Reflection/DescriptorsTest.cs +++ b/csharp/src/Google.Protobuf.Test/Reflection/DescriptorsTest.cs @@ -238,5 +238,18 @@ namespace Google.Protobuf.Reflection Assert.IsNull(fieldDescriptor.Accessor); Assert.IsNull(messageDescriptor.GeneratedType); } + + // From TestFieldOrdering: + // string my_string = 11; + // int64 my_int = 1; + // float my_float = 101; + // NestedMessage single_nested_message = 200; + [Test] + public void FieldListOrderings() + { + var fields = TestFieldOrderings.Descriptor.Fields; + Assert.AreEqual(new[] { 11, 1, 101, 200 }, fields.InDeclarationOrder().Select(x => x.FieldNumber)); + Assert.AreEqual(new[] { 1, 11, 101, 200 }, fields.InFieldNumberOrder().Select(x => x.FieldNumber)); + } } } diff --git a/csharp/src/Google.Protobuf/Reflection/MessageDescriptor.cs b/csharp/src/Google.Protobuf/Reflection/MessageDescriptor.cs index 1250774d..0b562de1 100644 --- a/csharp/src/Google.Protobuf/Reflection/MessageDescriptor.cs +++ b/csharp/src/Google.Protobuf/Reflection/MessageDescriptor.cs @@ -250,7 +250,7 @@ namespace Google.Protobuf.Reflection /// public IList InFieldNumberOrder() { - return messageDescriptor.fieldsInDeclarationOrder; + return messageDescriptor.fieldsInNumberOrder; } /// -- cgit v1.2.3 From 547d8e8221b8ea7ff7010cbc601f4e5742ff1e51 Mon Sep 17 00:00:00 2001 From: Jon Skeet Date: Fri, 7 Aug 2015 13:37:21 +0100 Subject: Make FieldDescriptor.IsPacked work appropriately. This is a bit of a grotty hack, as we need to sort of fake proto2 field presence, but with only a proto3 version of the descriptor messages (a bit like oneof detection). Should be okay, but will need to be careful of this if we ever implement proto2. --- .../src/Google.Protobuf.Test/Reflection/FieldAccessTest.cs | 2 +- csharp/src/Google.Protobuf/Reflection/FieldDescriptor.cs | 8 +++++--- csharp/src/Google.Protobuf/Reflection/PartialClasses.cs | 12 ++++++++++++ 3 files changed, 18 insertions(+), 4 deletions(-) (limited to 'csharp/src/Google.Protobuf.Test/Reflection') diff --git a/csharp/src/Google.Protobuf.Test/Reflection/FieldAccessTest.cs b/csharp/src/Google.Protobuf.Test/Reflection/FieldAccessTest.cs index 6e1d804e..936e41c6 100644 --- a/csharp/src/Google.Protobuf.Test/Reflection/FieldAccessTest.cs +++ b/csharp/src/Google.Protobuf.Test/Reflection/FieldAccessTest.cs @@ -213,6 +213,6 @@ namespace Google.Protobuf.Reflection var descriptor = TestAllTypes.Descriptor; Assert.Throws(() => descriptor.Fields[999999].ToString()); Assert.Throws(() => descriptor.Fields["not found"].ToString()); - } + } } } diff --git a/csharp/src/Google.Protobuf/Reflection/FieldDescriptor.cs b/csharp/src/Google.Protobuf/Reflection/FieldDescriptor.cs index bb8e9bbb..901cbf47 100644 --- a/csharp/src/Google.Protobuf/Reflection/FieldDescriptor.cs +++ b/csharp/src/Google.Protobuf/Reflection/FieldDescriptor.cs @@ -168,14 +168,16 @@ namespace Google.Protobuf.Reflection get { return fieldType == FieldType.Message && messageType.Proto.Options != null && messageType.Proto.Options.MapEntry; } } - // TODO(jonskeet): Check whether this is correct with proto3, where we default to packed... - /// /// Returns true if this field is a packed, repeated field; false otherwise. /// public bool IsPacked { - get { return Proto.Options != null && Proto.Options.Packed; } + // Note the || rather than && here - we're effectively defaulting to packed, because that *is* + // the default in proto3, which is all we support. We may give the wrong result for the protos + // within descriptor.proto, but that's okay, as they're never exposed and we don't use IsPacked + // within the runtime. + get { return Proto.Options == null || Proto.Options.Packed; } } /// diff --git a/csharp/src/Google.Protobuf/Reflection/PartialClasses.cs b/csharp/src/Google.Protobuf/Reflection/PartialClasses.cs index c7ed4342..8c055d6d 100644 --- a/csharp/src/Google.Protobuf/Reflection/PartialClasses.cs +++ b/csharp/src/Google.Protobuf/Reflection/PartialClasses.cs @@ -44,4 +44,16 @@ namespace Google.Protobuf.Reflection OneofIndex = -1; } } + + internal partial class FieldOptions + { + // We can't tell the difference between "explicitly set to false" and "not set" + // in proto3, but we need to tell the difference for FieldDescriptor.IsPacked. + // This won't work if we ever need to support proto2, but at that point we'll be + // able to remove this hack and use field presence instead. + partial void OnConstruction() + { + Packed = true; + } + } } \ No newline at end of file -- cgit v1.2.3