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') 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