From cbfdfe0fce202b990f9e9aed94cdb56d94541ea9 Mon Sep 17 00:00:00 2001 From: Jon Skeet Date: Fri, 7 Oct 2022 10:52:21 +0100 Subject: [PATCH] feat: Support HTTP rule overrides, primarily for mix-ins --- Google.Api.Gax.Grpc.Tests/ApiMetadataTest.cs | 19 +++++++++ .../Rest/RestMethodTest.cs | 17 ++++++++ Google.Api.Gax.Grpc/ApiMetadata.cs | 40 ++++++++++++++++--- Google.Api.Gax.Grpc/Rest/RestMethod.cs | 5 +++ .../Rest/RestServiceCollection.cs | 3 ++ 5 files changed, 78 insertions(+), 6 deletions(-) diff --git a/Google.Api.Gax.Grpc.Tests/ApiMetadataTest.cs b/Google.Api.Gax.Grpc.Tests/ApiMetadataTest.cs index aba7aca1..1ded2356 100644 --- a/Google.Api.Gax.Grpc.Tests/ApiMetadataTest.cs +++ b/Google.Api.Gax.Grpc.Tests/ApiMetadataTest.cs @@ -5,6 +5,7 @@ * https://developers.google.com/open-source/licenses/bsd */ +using Google.Protobuf; using Google.Protobuf.Reflection; using System.Collections; using System.Collections.Generic; @@ -68,6 +69,24 @@ public void WithRequestNumericEnumJsonEncoding() Assert.False(withFalse.RequestNumericEnumJsonEncoding); } + [Fact] + public void WithHttpRuleOverrides() + { + var original = TestApiMetadata.Test; + var rule = new HttpRule { Get = "/v1/xyz" }; + var overrides = new Dictionary { { "x.y.z", rule.ToByteString() } }; + var withOverrides = original.WithHttpRuleOverrides(overrides); + + // Original metadata has not changed + Assert.Empty(original.HttpRuleOverrides); + Assert.Equal(1, withOverrides.HttpRuleOverrides.Count); + Assert.True(withOverrides.HttpRuleOverrides.TryGetValue("x.y.z", out var ruleBytes)); + + // We could just keep hold of the ByteString, but this demonstrates the expected usage more clearly. + var decoded = HttpRule.Parser.ParseFrom(ruleBytes); + Assert.Equal(rule, decoded); + } + private class CountingSequence : IEnumerable { public int EvaluationCount { get; private set; } = 0; diff --git a/Google.Api.Gax.Grpc.Tests/Rest/RestMethodTest.cs b/Google.Api.Gax.Grpc.Tests/Rest/RestMethodTest.cs index 48fbb984..9a344aae 100644 --- a/Google.Api.Gax.Grpc.Tests/Rest/RestMethodTest.cs +++ b/Google.Api.Gax.Grpc.Tests/Rest/RestMethodTest.cs @@ -7,6 +7,7 @@ using Google.Api.Gax.Grpc.Tests; using Google.Protobuf; +using System.Collections.Generic; using System.Linq; using Xunit; @@ -29,4 +30,20 @@ public void CreateRequest_WithRequestNumericEnumJson(bool value, string expected var httpRequest = restMethod.CreateRequest(request, null); Assert.Equal(httpRequest.RequestUri.ToString(), expectedUri); } + + [Fact] + public void CreateRequest_WithHttpOverrides() + { + var rule = new HttpRule { Get = "/v2/def/{name}" }; + var methodDescriptor = TestServiceReflection.Descriptor.Services + .Single(svc => svc.Name == "Sample") + .FindMethodByName("SimpleMethod"); + var overrides = new Dictionary { { methodDescriptor.FullName, rule.ToByteString() } }; + var apiMetadata = TestApiMetadata.Test.WithHttpRuleOverrides(overrides); + var restMethod = RestMethod.Create(apiMetadata, methodDescriptor, JsonParser.Default); + + var request = new SimpleRequest { Name = "ghi" }; + var httpRequest = restMethod.CreateRequest(request, null); + Assert.Equal("/v2/def/ghi", httpRequest.RequestUri.ToString()); + } } diff --git a/Google.Api.Gax.Grpc/ApiMetadata.cs b/Google.Api.Gax.Grpc/ApiMetadata.cs index 10bc9091..edd8021b 100644 --- a/Google.Api.Gax.Grpc/ApiMetadata.cs +++ b/Google.Api.Gax.Grpc/ApiMetadata.cs @@ -6,9 +6,11 @@ */ using Google.Api.Gax.Grpc.Rest; +using Google.Protobuf; using Google.Protobuf.Reflection; using System; using System.Collections.Generic; +using System.Collections.ObjectModel; using System.Linq; using System.Threading; @@ -20,14 +22,16 @@ namespace Google.Api.Gax.Grpc /// public sealed partial class ApiMetadata { - private Lazy> _fileDescriptorsProvider; + private static readonly IReadOnlyDictionary s_emptyHttpRuleOverrides = new ReadOnlyDictionary(new Dictionary()); + + private readonly Lazy> _fileDescriptorsProvider; /// /// The protobuf descriptors used by this API. /// public IReadOnlyList ProtobufDescriptors => _fileDescriptorsProvider.Value; - private Lazy _typeRegistryProvider; + private readonly Lazy _typeRegistryProvider; /// /// A type registry containing all the types in . @@ -46,12 +50,21 @@ public sealed partial class ApiMetadata /// public bool RequestNumericEnumJsonEncoding { get; } - private ApiMetadata(string name, Lazy> fileDescriptorsProvider, bool requestNumericEnumJsonEncoding) + /// + /// A dictionary (based on ordinal string comparisons) from fully-qualified RPC names + /// to byte strings representing overrides for the HTTP rule. This is designed to support + /// mixins which are hosted at individual APIs, but which are exposed via different URLs + /// to the original mixin definition. This is never null, but may be empty. + /// + public IReadOnlyDictionary HttpRuleOverrides { get; } + + private ApiMetadata(string name, Lazy> fileDescriptorsProvider, bool requestNumericEnumJsonEncoding, IReadOnlyDictionary httpRuleOverrides) { Name = GaxPreconditions.CheckNotNullOrEmpty(name, nameof(name)); RequestNumericEnumJsonEncoding = requestNumericEnumJsonEncoding; _fileDescriptorsProvider = fileDescriptorsProvider; _typeRegistryProvider = new Lazy(() => TypeRegistry.FromFiles(ProtobufDescriptors)); + HttpRuleOverrides = httpRuleOverrides; } /// @@ -62,7 +75,7 @@ private ApiMetadata(string name, Lazy> fileDescrip /// /// The name of the API. Must not be null or empty. /// The protobuf descriptors of the API. Must not be null. - public ApiMetadata(string name, IEnumerable descriptors) : this(name, BuildDescriptorsProviderFromDescriptors(descriptors), false) + public ApiMetadata(string name, IEnumerable descriptors) : this(name, BuildDescriptorsProviderFromDescriptors(descriptors), false, s_emptyHttpRuleOverrides) { } @@ -82,7 +95,7 @@ private static Lazy> BuildDescriptorsProviderFromD /// The name of the API. Must not be null or empty. /// A provider function for the protobuf descriptors of the API. Must not be null, and must not /// return a null value. This will only be called once by this API descriptor, when first requested. - public ApiMetadata(string name, Func> descriptorsProvider) : this(name, BuildDescriptorsProviderFromOtherProvider(descriptorsProvider), false) + public ApiMetadata(string name, Func> descriptorsProvider) : this(name, BuildDescriptorsProviderFromOtherProvider(descriptorsProvider), false, s_emptyHttpRuleOverrides) { } @@ -102,6 +115,21 @@ private static Lazy> BuildDescriptorsProviderFromO /// The desired value of in the new instance. /// The new instance. public ApiMetadata WithRequestNumericEnumJsonEncoding(bool value) => - new ApiMetadata(Name, _fileDescriptorsProvider, value); + new ApiMetadata(Name, _fileDescriptorsProvider, value, HttpRuleOverrides); + + /// + /// Creates a new instance with the same values as this one, other than the given set of HttpRule overrides. + /// + /// The HttpRule overrides for services in this package; typically used to override + /// URLs for the REST transport. Must not be null. Will be cloned in the form of an immutable dictionary, + /// after which the original sequence is discarded. + /// The new instance. + public ApiMetadata WithHttpRuleOverrides(IEnumerable> overrides) + { + GaxPreconditions.CheckNotNull(overrides, nameof(overrides)); + var dict = overrides.ToDictionary(pair => pair.Key, pair => pair.Value, StringComparer.Ordinal); + var readOnlyDict = new ReadOnlyDictionary(dict); + return new ApiMetadata(Name, _fileDescriptorsProvider, RequestNumericEnumJsonEncoding, readOnlyDict); + } } } diff --git a/Google.Api.Gax.Grpc/Rest/RestMethod.cs b/Google.Api.Gax.Grpc/Rest/RestMethod.cs index 0b6b594c..4b5f9dfb 100644 --- a/Google.Api.Gax.Grpc/Rest/RestMethod.cs +++ b/Google.Api.Gax.Grpc/Rest/RestMethod.cs @@ -49,6 +49,11 @@ internal static RestMethod Create(ApiMetadata apiMetadata, MethodDescriptor meth { throw new ArgumentException($"Method {method.Name} in service {method.Service.Name} has no HTTP rule"); } + // If we have an override, it completely replaces the original rule. + if (apiMetadata.HttpRuleOverrides.TryGetValue(method.FullName, out var overrideByteString)) + { + rule = HttpRule.Parser.ParseFrom(overrideByteString); + } var transcoder = new HttpRuleTranscoder(method.FullName, method.InputType, rule); return new RestMethod(apiMetadata, method, parser, transcoder); } diff --git a/Google.Api.Gax.Grpc/Rest/RestServiceCollection.cs b/Google.Api.Gax.Grpc/Rest/RestServiceCollection.cs index 38a1e7fc..bee81b50 100644 --- a/Google.Api.Gax.Grpc/Rest/RestServiceCollection.cs +++ b/Google.Api.Gax.Grpc/Rest/RestServiceCollection.cs @@ -39,6 +39,9 @@ internal static RestServiceCollection Create(ApiMetadata metadata) var methodsByName = services.SelectMany(service => service.Methods) // We don't yet support streaming methods. .Where(x => !x.IsClientStreaming && !x.IsServerStreaming) + // Ignore methods without HTTP annotations. Ideally there wouldn't be any, but + // operations.proto doesn't specify an HTTP rule for WaitOperation. + .Where(x => x.GetOptions()?.GetExtension(AnnotationsExtensions.Http) is not null) .Select(method => RestMethod.Create(metadata, method, parser)) .ToDictionary(restMethod => restMethod.FullName); return new RestServiceCollection(new ReadOnlyDictionary(methodsByName));