[ggj][codgen] fix: add HTTP's additional_bindings to GrpcServiceStub …

…call settings (#591) * fix: fix dep ordering in Bazel dedupe rules * fix: use lowerCamelCase param names in ServiceClient JavaDocs * fix: add HTTP's additional_bindings to GrpcServiceStub call settings
googleapis · Dec 7, 2020 · 767d1c2 · 767d1c2
1 parent ab20de9
commit 767d1c2
Show file tree

Hide file tree

Showing 5 changed files with 66 additions and 9 deletions.
diff --git a/src/main/java/com/google/api/generator/gapic/protoparser/HttpRuleParser.java b/src/main/java/com/google/api/generator/gapic/protoparser/HttpRuleParser.java
@@ -26,9 +26,11 @@
 import com.google.protobuf.Descriptors.MethodDescriptor;
 import java.util.ArrayList;
 import java.util.Collections;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
+import java.util.Set;
 
 public class HttpRuleParser {
   private static final String ASTERISK = "*";
@@ -48,11 +50,20 @@ public static Optional<List<String>> parseHttpBindings(
     }
 
     // Get pattern.
-    List<String> bindings = getPatternBindings(httpRule);
-    if (bindings.isEmpty()) {
+    Set<String> uniqueBindings = getPatternBindings(httpRule);
+    if (uniqueBindings.isEmpty()) {
       return Optional.empty();
     }
 
+    if (httpRule.getAdditionalBindingsCount() > 0) {
+      for (HttpRule additionalRule : httpRule.getAdditionalBindingsList()) {
+        uniqueBindings.addAll(getPatternBindings(additionalRule));
+      }
+    }
+
+    List<String> bindings = new ArrayList<>(uniqueBindings);
+    Collections.sort(bindings);
+
     // Binding validation.
     for (String binding : bindings) {
       // Handle foo.bar cases by descending into the subfields.
@@ -77,7 +88,7 @@ public static Optional<List<String>> parseHttpBindings(
     return Optional.of(bindings);
   }
 
-  private static List<String> getPatternBindings(HttpRule httpRule) {
+  private static Set<String> getPatternBindings(HttpRule httpRule) {
     String pattern = null;
     // Assign a temp variable to prevent the formatter from removing the import.
     PatternCase patternCase = httpRule.getPatternCase();
@@ -100,12 +111,11 @@ private static List<String> getPatternBindings(HttpRule httpRule) {
       case CUSTOM: // Invalid pattern.
         // Fall through.
       default:
-        return Collections.emptyList();
+        return Collections.emptySet();
     }
 
     PathTemplate template = PathTemplate.create(pattern);
-    List<String> bindings = new ArrayList<String>(template.vars());
-    Collections.sort(bindings);
+    Set<String> bindings = new HashSet<String>(template.vars());
     return bindings;
   }
 

diff --git a/src/test/java/com/google/api/generator/gapic/composer/goldens/GrpcTestingStub.golden b/src/test/java/com/google/api/generator/gapic/composer/goldens/GrpcTestingStub.golden
@@ -262,7 +262,11 @@ public class GrpcTestingStub extends TestingStub {
                   @Override
                   public Map<String, String> extract(VerifyTestRequest request) {
                     ImmutableMap.Builder<String, String> params = ImmutableMap.builder();
+                    params.put("answer", String.valueOf(request.getAnswer()));
+                    params.put("foo", String.valueOf(request.getFoo()));
                     params.put("name", String.valueOf(request.getName()));
+                    params.put(
+                        "test_to_verify.name", String.valueOf(request.getTestToVerify().getName()));
                     return params.build();
                   }
                 })

diff --git a/src/test/java/com/google/api/generator/gapic/composer/goldens/TestingClientTest.golden b/src/test/java/com/google/api/generator/gapic/composer/goldens/TestingClientTest.golden
@@ -465,6 +465,8 @@ public class TestingClientTest {
             .setName(TestName.of("[SESSION]", "[SHARD_ID]", "[TEST_ID]").toString())
             .setAnswer(ByteString.EMPTY)
             .addAllAnswers(new ArrayList<ByteString>())
+            .setFoo("foo101574")
+            .setTestToVerify(com.google.showcase.v1beta1.Test.newBuilder().build())
             .build();
 
     VerifyTestResponse actualResponse = client.verifyTest(request);
@@ -477,6 +479,8 @@ public class TestingClientTest {
     Assert.assertEquals(request.getName(), actualRequest.getName());
     Assert.assertEquals(request.getAnswer(), actualRequest.getAnswer());
     Assert.assertEquals(request.getAnswersList(), actualRequest.getAnswersList());
+    Assert.assertEquals(request.getFoo(), actualRequest.getFoo());
+    Assert.assertEquals(request.getTestToVerify(), actualRequest.getTestToVerify());
     Assert.assertTrue(
         channelProvider.isHeaderSent(
             ApiClientHeaderProvider.getDefaultApiClientHeaderKey(),
@@ -494,6 +498,8 @@ public class TestingClientTest {
               .setName(TestName.of("[SESSION]", "[SHARD_ID]", "[TEST_ID]").toString())
               .setAnswer(ByteString.EMPTY)
               .addAllAnswers(new ArrayList<ByteString>())
+              .setFoo("foo101574")
+              .setTestToVerify(com.google.showcase.v1beta1.Test.newBuilder().build())
               .build();
       client.verifyTest(request);
       Assert.fail("No exception raised");

diff --git a/src/test/java/com/google/api/generator/gapic/protoparser/HttpRuleParserTest.java b/src/test/java/com/google/api/generator/gapic/protoparser/HttpRuleParserTest.java
@@ -46,14 +46,33 @@ public void parseHttpAnnotation_basic() {
         HttpRuleParser.parseHttpBindings(rpcMethod, inputMessage, messages);
     assertFalse(httpBindingsOpt.isPresent());
 
-    // VerityTest method.
-    rpcMethod = testingService.getMethods().get(testingService.getMethods().size() - 1);
-    inputMessage = messages.get("VerifyTestRequest");
+    // GetSession method.
+    rpcMethod = testingService.getMethods().get(1);
+    inputMessage = messages.get("GetSessionRequest");
     httpBindingsOpt = HttpRuleParser.parseHttpBindings(rpcMethod, inputMessage, messages);
     assertTrue(httpBindingsOpt.isPresent());
     assertThat(httpBindingsOpt.get()).containsExactly("name");
   }
 
+  @Test
+  public void parseHttpAnnotation_multipleBindings() {
+    FileDescriptor testingFileDescriptor = TestingOuterClass.getDescriptor();
+    ServiceDescriptor testingService = testingFileDescriptor.getServices().get(0);
+    assertEquals("Testing", testingService.getName());
+
+    Map<String, Message> messages = Parser.parseMessages(testingFileDescriptor);
+
+    // VerityTest method.
+    MethodDescriptor rpcMethod =
+        testingService.getMethods().get(testingService.getMethods().size() - 1);
+    Message inputMessage = messages.get("VerifyTestRequest");
+    Optional<List<String>> httpBindingsOpt =
+        HttpRuleParser.parseHttpBindings(rpcMethod, inputMessage, messages);
+    assertTrue(httpBindingsOpt.isPresent());
+    assertThat(httpBindingsOpt.get())
+        .containsExactly("answer", "foo", "name", "test_to_verify.name");
+  }
+
   @Test
   public void parseHttpAnnotation_missingFieldFromMessage() {
     FileDescriptor testingFileDescriptor = TestingOuterClass.getDescriptor();

diff --git a/src/test/java/com/google/api/generator/gapic/testdata/testing.proto b/src/test/java/com/google/api/generator/gapic/testdata/testing.proto
@@ -107,6 +107,19 @@ service Testing {
   rpc VerifyTest(VerifyTestRequest) returns (VerifyTestResponse) {
     option (google.api.http) = {
       post: "/v1beta1/{name=sessions/*/tests/*}:check"
+      body: "*"
+      additional_bindings {
+        post: "/v1beta1/{foo=sessions/*/tests/*}:check"
+        body: "*"
+      }
+      additional_bindings {
+        post: "/v1beta1/{answer=sessions/*/tests/*}:check"
+        body: "*"
+      }
+      additional_bindings {
+        post: "/v1beta1/{test_to_verify.name=sessions/*/tests/*}:check"
+        body: "*"
+      }
     };
   }
 }
@@ -392,6 +405,11 @@ message VerifyTestRequest {
 
   // The answers from the test if multiple are to be checked
   repeated bytes answers = 3;
+
+  // Test fields for HTTP annotations.
+  string foo = 4;
+
+  Test test_to_verify = 5;
 }
 
 message VerifyTestResponse {