Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow FileDescriptors to be parsed with extension registries #8220

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
17 changes: 16 additions & 1 deletion csharp/src/Google.Protobuf.Test/ExtensionSetTest.cs
Expand Up @@ -116,7 +116,22 @@ public void TestClone()
var other = message.Clone();

Assert.AreEqual(message, other);
Assert.AreEqual(message.CalculateSize(), message.CalculateSize());
Assert.AreEqual(message.CalculateSize(), other.CalculateSize());
}

[Test]
public void TestDefaultValueRoundTrip()
{
var message = new TestAllExtensions();
message.SetExtension(OptionalBoolExtension, false);
Assert.IsFalse(message.GetExtension(OptionalBoolExtension));
Assert.IsTrue(message.HasExtension(OptionalBoolExtension));

var bytes = message.ToByteArray();
var registry = new ExtensionRegistry { OptionalBoolExtension };
var parsed = TestAllExtensions.Parser.WithExtensionRegistry(registry).ParseFrom(bytes);
Assert.IsFalse(parsed.GetExtension(OptionalBoolExtension));
Assert.IsTrue(parsed.HasExtension(OptionalBoolExtension));
}
}
}
20 changes: 20 additions & 0 deletions csharp/src/Google.Protobuf.Test/Reflection/DescriptorsTest.cs
Expand Up @@ -35,7 +35,9 @@
using ProtobufUnittest;
using System;
using System.Collections.Generic;
using System.IO;
using System.Linq;
using UnitTest.Issues.TestProtos;

namespace Google.Protobuf.Reflection
{
Expand Down Expand Up @@ -70,6 +72,24 @@ public void FileDescriptor_BuildFromByteStrings()
TestFileDescriptor(converted[2], converted[1], converted[0]);
}

[Test]
public void FileDescriptor_BuildFromByteStrings_WithExtensionRegistry()
{
var extension = UnittestCustomOptionsProto3Extensions.MessageOpt1;

var byteStrings = new[]
{
DescriptorReflection.Descriptor.Proto.ToByteString(),
UnittestCustomOptionsProto3Reflection.Descriptor.Proto.ToByteString()
};
var registry = new ExtensionRegistry { extension };

var descriptor = FileDescriptor.BuildFromByteStrings(byteStrings, registry).Last();
var message = descriptor.MessageTypes.Single(t => t.Name == nameof(TestMessageWithCustomOptions));
var extensionValue = message.GetOptions().GetExtension(extension);
Assert.AreEqual(-56, extensionValue);
}

private void TestFileDescriptor(FileDescriptor file, FileDescriptor importedFile, FileDescriptor importedPublicFile)
{
Assert.AreEqual("unittest_proto3.proto", file.Name);
Expand Down
2 changes: 1 addition & 1 deletion csharp/src/Google.Protobuf/ExtensionValue.cs
Expand Up @@ -59,7 +59,7 @@ internal ExtensionValue(FieldCodec<T> codec)

public int CalculateSize()
{
return codec.CalculateSizeWithTag(field);
return codec.CalculateUnconditionalSizeWithTag(field);
}

public IExtensionValue Clone()
Expand Down
6 changes: 6 additions & 0 deletions csharp/src/Google.Protobuf/FieldCodec.cs
Expand Up @@ -876,6 +876,12 @@ public T Read(ref ParseContext ctx)
/// </summary>
public int CalculateSizeWithTag(T value) => IsDefault(value) ? 0 : ValueSizeCalculator(value) + tagSize;

/// <summary>
/// Calculates the size required to write the given value, with a tag, even
/// if the value is the default.
/// </summary>
internal int CalculateUnconditionalSizeWithTag(T value) => ValueSizeCalculator(value) + tagSize;

private bool IsDefault(T value) => EqualityComparer.Equals(value, DefaultValue);
}
}
19 changes: 17 additions & 2 deletions csharp/src/Google.Protobuf/Reflection/FileDescriptor.cs
Expand Up @@ -481,18 +481,21 @@ private static IEnumerable<Extension> GetAllDependedExtensionsFromMessage(Messag
/// dependencies must come before the descriptor which depends on them. (If A depends on B, and B
/// depends on C, then the descriptors must be presented in the order C, B, A.) This is compatible
/// with the order in which protoc provides descriptors to plugins.</param>
/// <param name="registry">The extension registry to use when parsing, or null if no extensions are required.</param>
/// <returns>The file descriptors corresponding to <paramref name="descriptorData"/>.</returns>
public static IReadOnlyList<FileDescriptor> BuildFromByteStrings(IEnumerable<ByteString> descriptorData)
public static IReadOnlyList<FileDescriptor> BuildFromByteStrings(IEnumerable<ByteString> descriptorData, ExtensionRegistry registry)
{
ProtoPreconditions.CheckNotNull(descriptorData, nameof(descriptorData));

var parser = FileDescriptorProto.Parser.WithExtensionRegistry(registry);

// TODO: See if we can build a single DescriptorPool instead of building lots of them.
// This will all behave correctly, but it's less efficient than we'd like.
var descriptors = new List<FileDescriptor>();
var descriptorsByName = new Dictionary<string, FileDescriptor>();
foreach (var data in descriptorData)
{
var proto = FileDescriptorProto.Parser.ParseFrom(data);
var proto = parser.ParseFrom(data);
var dependencies = new List<FileDescriptor>();
foreach (var dependencyName in proto.Dependency)
{
Expand All @@ -518,6 +521,18 @@ public static IReadOnlyList<FileDescriptor> BuildFromByteStrings(IEnumerable<Byt
return new ReadOnlyCollection<FileDescriptor>(descriptors);
}

/// <summary>
/// Converts the given descriptor binary data into FileDescriptor objects.
/// Note: reflection using the returned FileDescriptors is not currently supported.
/// </summary>
/// <param name="descriptorData">The binary file descriptor proto data. Must not be null, and any
/// dependencies must come before the descriptor which depends on them. (If A depends on B, and B
/// depends on C, then the descriptors must be presented in the order C, B, A.) This is compatible
/// with the order in which protoc provides descriptors to plugins.</param>
/// <returns>The file descriptors corresponding to <paramref name="descriptorData"/>.</returns>
public static IReadOnlyList<FileDescriptor> BuildFromByteStrings(IEnumerable<ByteString> descriptorData) =>
BuildFromByteStrings(descriptorData, null);

/// <summary>
/// Returns a <see cref="System.String" /> that represents this instance.
/// </summary>
Expand Down