Commit 20bf6a56 authored by Jon Skeet's avatar Jon Skeet

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 :)
parent 7b5c3967
<?xml version="1.0" encoding="utf-8"?>
<?xml version="1.0" encoding="utf-8"?>
<Project ToolsVersion="4.0" DefaultTargets="Build" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
<PropertyGroup>
<Configuration Condition=" '$(Configuration)' == '' ">Debug</Configuration>
......@@ -82,6 +82,7 @@
<Compile Include="Collections\RepeatedFieldTest.cs" />
<Compile Include="JsonFormatterTest.cs" />
<Compile Include="Reflection\DescriptorsTest.cs" />
<Compile Include="Reflection\FieldAccessTest.cs" />
<Compile Include="SampleEnum.cs" />
<Compile Include="SampleMessages.cs" />
<Compile Include="TestProtos\MapUnittestProto3.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<MessageDescriptor>("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
This diff is collapsed.
......@@ -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]);
......
......@@ -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.
/// </summary>
/// <param name="name">The unqualified type name to look for.</param>
/// <typeparam name="T">The type of descriptor to look for (or ITypeDescriptor for any)</typeparam>
/// <typeparam name="T">The type of descriptor to look for</typeparam>
/// <returns>The type's descriptor, or null if not found.</returns>
public T FindTypeByName<T>(String name)
where T : class, IDescriptor
......
......@@ -61,10 +61,10 @@ namespace Google.Protobuf.Reflection
private readonly IList<MessageDescriptor> nestedTypes;
private readonly IList<EnumDescriptor> enumTypes;
private readonly IList<FieldDescriptor> fields;
private readonly FieldAccessorCollection fieldAccessors;
private readonly IList<OneofDescriptor> oneofs;
// CLR representation of the type described by this descriptor, if any.
private readonly Type generatedType;
private IDictionary<int, IFieldAccessor> 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);
}
/// <summary>
......@@ -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?
/// <value>
/// 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.
/// </value>
public IList<FieldDescriptor> Fields
{
get { return fields; }
}
/// <value>
/// A collection of accessors, which can be retrieved by name or field number.
/// </value>
public FieldAccessorCollection FieldAccessors
{
get { return fieldAccessors; }
}
/// <value>
/// An unmodifiable list of this message type's nested types.
/// </value>
......@@ -164,13 +178,6 @@ namespace Google.Protobuf.Reflection
get { return oneofs; }
}
/// <summary>
/// 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.
/// </summary>
public IDictionary<int, IFieldAccessor> FieldAccessorsByFieldNumber { get { return fieldAccessorsByFieldNumber; } }
/// <summary>
/// Finds a field by field name.
/// </summary>
......@@ -222,8 +229,61 @@ namespace Google.Protobuf.Reflection
{
oneof.CrossLink();
}
}
/// <summary>
/// A collection to simplify retrieving the field accessor for a particular field.
/// </summary>
public sealed class FieldAccessorCollection
{
private readonly MessageDescriptor messageDescriptor;
internal FieldAccessorCollection(MessageDescriptor messageDescriptor)
{
this.messageDescriptor = messageDescriptor;
}
fieldAccessorsByFieldNumber = new ReadOnlyDictionary<int, IFieldAccessor>(fields.ToDictionary(field => field.FieldNumber, field => field.Accessor));
/// <summary>
/// Retrieves the accessor for the field with the given number.
/// </summary>
/// <param name="number">Number of the field to retrieve the accessor for</param>
/// <returns>The accessor for the given field, or null if reflective field access is
/// not supported for the field.</returns>
/// <exception cref="KeyNotFoundException">The message descriptor does not contain a field
/// with the given number</exception>
public IFieldAccessor this[int number]
{
get
{
var fieldDescriptor = messageDescriptor.FindFieldByNumber(number);
if (fieldDescriptor == null)
{
throw new KeyNotFoundException("No such field number");
}
return fieldDescriptor.Accessor;
}
}
/// <summary>
/// Retrieves the accessor for the field with the given name.
/// </summary>
/// <param name="number">Number of the field to retrieve the accessor for</param>
/// <returns>The accessor for the given field, or null if reflective field access is
/// not supported for the field.</returns>
/// <exception cref="KeyNotFoundException">The message descriptor does not contain a field
/// with the given name</exception>
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
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment