fix: support reroute_to_grpc_interface with grpc_service_config (#3272)

googleapis · Sep 2, 2020 · 231ad97 · 231ad97
1 parent 6c4b62a
commit 231ad97
Show file tree

Hide file tree

Showing 5 changed files with 30 additions and 12 deletions.
diff --git a/src/main/java/com/google/api/codegen/config/GapicInterfaceConfig.java b/src/main/java/com/google/api/codegen/config/GapicInterfaceConfig.java
@@ -153,7 +153,8 @@ static GapicInterfaceConfig createInterfaceConfig(
               retryCodesConfig,
               retrySettingsDefinition.keySet(),
               protoParser,
-              grpcGapicRetryMapping);
+              grpcGapicRetryMapping,
+              interfaceInput.getInterfaceConfigProto().getName());
       if (methodConfigsMap == null) {
         diagCollector.addDiag(
             Diag.error(SimpleLocation.TOPLEVEL, "Error constructing methodConfigMap"));
@@ -245,7 +246,8 @@ private static ImmutableMap<String, GapicMethodConfig> createMethodConfigMap(
       RetryCodesConfig retryCodesConfig,
       ImmutableSet<String> retryParamsConfigNames,
       ProtoParser protoParser,
-      GrpcGapicRetryMapping retryMapping) {
+      GrpcGapicRetryMapping retryMapping,
+      String gapicInterfaceName) {
     Map<String, GapicMethodConfig> methodConfigMapBuilder = new LinkedHashMap<>();
 
     for (Entry<Method, MethodConfigProto> methodEntry : methodsToGenerate.entrySet()) {
@@ -265,7 +267,8 @@ private static ImmutableMap<String, GapicMethodConfig> createMethodConfigMap(
                 retryCodesConfig,
                 retryParamsConfigNames,
                 protoParser,
-                retryMapping);
+                retryMapping,
+                gapicInterfaceName);
       } else {
         methodConfig =
             GapicMethodConfig.createGapicMethodConfigFromGapicYaml(

diff --git a/src/main/java/com/google/api/codegen/config/GapicMethodConfig.java b/src/main/java/com/google/api/codegen/config/GapicMethodConfig.java
@@ -75,7 +75,8 @@ private static GapicMethodConfig.Builder createCommonMethodConfig(
       ProtoMethodModel methodModel,
       RetryCodesConfig retryCodesConfig,
       ImmutableSet<String> retryParamsConfigNames,
-      GrpcGapicRetryMapping retryMapping) {
+      GrpcGapicRetryMapping retryMapping,
+      String gapicInterfaceName) {
 
     GrpcStreamingConfig grpcStreaming = null;
     if (isGrpcStreamingMethod(methodModel)) {
@@ -102,7 +103,17 @@ private static GapicMethodConfig.Builder createCommonMethodConfig(
     if (retryMapping != null) {
       // use the gRPC ServiceConfig retry as the source
       retryCodesName = retryCodesConfig.getMethodRetryNames().get(methodModel.getSimpleName());
-      retryParamsName = retryMapping.methodParamsMap().get(methodModel.getFullName());
+
+      // The retry_params are mapped to full-qualified names. If the GAPIC config uses the
+      // reroute_to_grpc_interface option, we should use the GAPIC interface name to look up
+      // the retry_params, because the retry config should name them using the same GAPIC interface
+      // name.
+      String fullName = methodModel.getFullName();
+      if (!Strings.isNullOrEmpty(methodConfigProto.getRerouteToGrpcInterface())
+          && !Strings.isNullOrEmpty(gapicInterfaceName)) {
+        fullName = gapicInterfaceName + "." + methodModel.getSimpleName();
+      }
+      retryParamsName = retryMapping.methodParamsMap().get(fullName);
 
       // unknown/unspecified methods get no retry codes or params
       if (Strings.isNullOrEmpty(retryCodesName)) {
@@ -188,7 +199,8 @@ static GapicMethodConfig createGapicMethodConfigFromProto(
       RetryCodesConfig retryCodesConfig,
       ImmutableSet<String> retryParamsConfigNames,
       ProtoParser protoParser,
-      GrpcGapicRetryMapping retryMapping) {
+      GrpcGapicRetryMapping retryMapping,
+      String gapicInterfaceName) {
     int previousErrors = diagCollector.getErrorCount();
 
     ProtoMethodModel methodModel = new ProtoMethodModel(method);
@@ -206,7 +218,8 @@ static GapicMethodConfig createGapicMethodConfigFromProto(
                 methodModel,
                 retryCodesConfig,
                 retryParamsConfigNames,
-                retryMapping)
+                retryMapping,
+                gapicInterfaceName)
             .setPageStreaming(
                 PageStreamingConfig.createPageStreamingConfig(
                     diagCollector,
@@ -279,6 +292,7 @@ static GapicMethodConfig createGapicMethodConfigFromGapicYaml(
                 methodModel,
                 retryCodesConfig,
                 retryParamsConfigNames,
+                null,
                 null)
             .setPageStreaming(
                 PageStreamingConfig.createPageStreamingConfig(

diff --git a/...ogle/api/codegen/protoannotations/testdata/java_library_with_grpc_service_config.baseline b/...ogle/api/codegen/protoannotations/testdata/java_library_with_grpc_service_config.baseline
@@ -14761,8 +14761,8 @@ public class LibraryServiceStubSettings extends StubSettings<LibraryServiceStubS
           .setRetrySettings(RETRY_PARAM_DEFINITIONS.get("retry_policy_1_params"));
 
       builder.addLabelSettings()
-          .setRetryableCodes(RETRYABLE_CODE_DEFINITIONS.get("no_retry_codes"))
-          .setRetrySettings(RETRY_PARAM_DEFINITIONS.get("no_retry_params"));
+          .setRetryableCodes(RETRYABLE_CODE_DEFINITIONS.get("no_retry_1_codes"))
+          .setRetrySettings(RETRY_PARAM_DEFINITIONS.get("no_retry_1_params"));
 
       builder.getBigBookSettings()
           .setRetryableCodes(RETRYABLE_CODE_DEFINITIONS.get("no_retry_1_codes"))

diff --git a/...oogle/api/codegen/protoannotations/testdata/php_library_with_grpc_service_config.baseline b/...oogle/api/codegen/protoannotations/testdata/php_library_with_grpc_service_config.baseline
@@ -6244,8 +6244,8 @@ class MyProtoClient extends MyProtoGapicClient
         },
         "AddLabel": {
           "timeout_millis": 60000,
-          "retry_codes_name": "no_retry_codes",
-          "retry_params_name": "no_retry_params"
+          "retry_codes_name": "no_retry_1_codes",
+          "retry_params_name": "no_retry_1_params"
         },
         "GetBigBook": {
           "timeout_millis": 60000,

diff --git a/...est/java/com/google/api/codegen/testsrc/protoannotations/library_grpc_service_config.json b/...est/java/com/google/api/codegen/testsrc/protoannotations/library_grpc_service_config.json
@@ -32,7 +32,8 @@
       { "service": "google.example.library.v1.LibraryService", "method": "GetBigBook" },
       { "service": "google.example.library.v1.LibraryService", "method": "GetBigNothing" },
       { "service": "google.example.library.v1.LibraryService", "method": "TestOptionalRequiredFlatteningParams" },
-      { "service": "google.example.library.v1.LibraryService", "method": "BabbleAboutBook" }
+      { "service": "google.example.library.v1.LibraryService", "method": "BabbleAboutBook" },
+      { "service": "google.example.library.v1.LibraryService", "method": "AddLabel" }
     ],
 
     "waitForReady": true,