fix: Check additional_binding's custom FieldsExtractor #1565

lqiu96 · 2023-03-27T20:45:44Z

Thank you for opening a Pull Request! For general contributing guidelines, please refer to contributing guide

Before submitting your PR, there are a few things you can do to make sure it goes smoothly:

Make sure to open an issue as a bug/issue before writing your code! That way we can discuss the change, evaluate designs, and agree on the general idea
Ensure the tests and linter pass
Code coverage does not decrease (if any source code was changed)
Appropriate docs were updated (if necessary)

Fixes #1440 ☕️

This Part 2 of the fix. Part 1 was resolved here: #1440

...ava/gax-httpjson/src/main/java/com/google/api/gax/httpjson/ProtoMessageRequestFormatter.java

lqiu96 · 2023-03-27T20:54:33Z

...ava/gax-httpjson/src/main/java/com/google/api/gax/httpjson/ProtoMessageRequestFormatter.java

  private ProtoMessageRequestFormatter(
      FieldsExtractor<RequestT, String> requestBodyExtractor,
      FieldsExtractor<RequestT, Map<String, List<String>>> queryParamsExtractor,
      String rawPath,
      PathTemplate pathTemplate,
      FieldsExtractor<RequestT, Map<String, String>> pathVarsExtractor,
-      List<String> additionalRawPaths,
-      List<PathTemplate> additionalPathTemplates) {
+      Map<PathTemplate, FieldsExtractor<RequestT, Map<String, String>>>
+          additionalPathsExtractorMap) {


This is a private constructor which should give us some flexibility to update the parameters.

lqiu96 · 2023-03-27T21:06:22Z

...pjson/src/main/java/com/google/api/gax/httpjson/longrunning/stub/HttpJsonOperationsStub.java

+  private final FieldsExtractor<ListOperationsRequest, Map<String, String>>
+      LRO_LIST_PATH_EXTRACTOR =
+          request -> {
+            Map<String, String> fields = new HashMap<>();
+            ProtoRestSerializer<ListOperationsRequest> serializer = ProtoRestSerializer.create();
+            serializer.putPathParam(fields, "name", request.getName());
+            return fields;
+          };
+  private final FieldsExtractor<GetOperationRequest, Map<String, String>> LRO_GET_PATH_EXTRACTOR =
+      request -> {
+        Map<String, String> fields = new HashMap<>();
+        ProtoRestSerializer<GetOperationRequest> serializer = ProtoRestSerializer.create();
+        serializer.putPathParam(fields, "name", request.getName());
+        return fields;
+      };
+
+  private final FieldsExtractor<DeleteOperationRequest, Map<String, String>>
+      LRO_DELETE_PATH_EXTRACTOR =
+          request -> {
+            Map<String, String> fields = new HashMap<>();
+            ProtoRestSerializer<DeleteOperationRequest> serializer = ProtoRestSerializer.create();
+            serializer.putPathParam(fields, "name", request.getName());
+            return fields;
+          };
+
+  private final FieldsExtractor<CancelOperationRequest, Map<String, String>>
+      LRO_CANCEL_PATH_EXTRACTOR =
+          request -> {
+            Map<String, String> fields = new HashMap<>();
+            ProtoRestSerializer<CancelOperationRequest> serializer = ProtoRestSerializer.create();
+            serializer.putPathParam(fields, "name", request.getName());
+            return fields;
+          };


OperationsStub has a default FieldsExtractor for each RPC (difference being that the request type for each FieldsExtractor is different). Each request type in the proto file has the name field, so we should be able to the default FieldsExtractor even for custom paths: https://github.com/googleapis/googleapis/blob/d5a51f6baba66f5dde236211d41d66efa1e4e7ca/google/longrunning/operations.proto#L171

lqiu96 · 2023-03-27T21:32:30Z

gax-java/gax-httpjson/src/test/java/com/google/api/gax/httpjson/HttpJsonDirectCallableTest.java

+                  .setAdditionalPathsExtractorMap(
+                      ImmutableMap
+                          .<PathTemplate, FieldsExtractor<Field, Map<String, String>>>builder()
+                          .put(
+                              PathTemplate.create("/fake/v1/name/{name=john/*}"),
+                              request -> {
+                                Map<String, String> fields = new HashMap<>();
+                                ProtoRestSerializer<Field> serializer =
+                                    ProtoRestSerializer.create();
+                                serializer.putPathParam(fields, "name", request.getName());
+                                return fields;
+                              })
+                          .build())


The test cases use john/imTheBestField so they match against the additionalPathExtractor()

sonarqubecloud · 2023-03-27T21:36:15Z

[gapic-generator-java-root] Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

100.0% Coverage
0.0% Duplication

sonarqubecloud · 2023-03-27T21:38:56Z

[java_showcase_integration_tests] SonarCloud Quality Gate failed.

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

25.6% Coverage
0.0% Duplication

sonarqubecloud · 2023-03-27T21:42:56Z

[java_showcase_unit_tests] SonarCloud Quality Gate failed.

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

43.4% Coverage
0.0% Duplication

lqiu96 · 2023-03-27T21:47:26Z

...-generator-java/src/main/java/com/google/api/generator/gapic/protoparser/HttpRuleParser.java

+        .setAdditionalBindings(
+            httpRule.getAdditionalBindingsList().stream()
+                .map(x -> parseHttpRuleHelper(x, inputMessageOpt, messageTypes))
+                .collect(Collectors.toList()))


setAdditionalPatterns above just gets the HttpVerb associated with the additional_bindings list. setAdditionalBindings recursively calls this function to get the nested additional_bindings.

lqiu96 · 2023-03-28T20:59:14Z

Error:

16:29:34:562 [ERROR] Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.10.1:compile (default-compile) on project google-cloud-kms: Compilation failure: Compilation failure:
16:29:34:563 [ERROR] /tmpfs/src/github/gapic-generator-java/google-cloud-java/java-kms/google-cloud-kms/src/main/java/com/google/cloud/kms/v1/stub/HttpJsonEkmServiceStub.java:[380,23] cannot find symbol
16:29:34:563 [ERROR]   symbol:   method setAdditionalPaths(java.lang.String,java.lang.String,java.lang.String,java.lang.String)
16:29:34:563 [ERROR]   location: class com.google.api.gax.httpjson.ProtoMessageRequestFormatter.Builder<com.google.iam.v1.SetIamPolicyRequest>
16:29:34:563 [ERROR] /tmpfs/src/github/gapic-generator-java/google-cloud-java/java-kms/google-cloud-kms/src/main/java/com/google/cloud/kms/v1/stub/HttpJsonEkmServiceStub.java:[422,23] cannot find symbol
16:29:34:564 [ERROR]   symbol:   method setAdditionalPaths(java.lang.String,java.lang.String,java.lang.String,java.lang.String)
16:29:34:564 [ERROR]   location: class com.google.api.gax.httpjson.ProtoMessageRequestFormatter.Builder<com.google.iam.v1.GetIamPolicyRequest>
16:29:34:564 [ERROR] /tmpfs/src/github/gapic-generator-java/google-cloud-java/java-kms/google-cloud-kms/src/main/java/com/google/cloud/kms/v1/stub/HttpJsonEkmServiceStub.java:[461,23] cannot find symbol
16:29:34:565 [ERROR]   symbol:   method setAdditionalPaths(java.lang.String,java.lang.String,java.lang.String,java.lang.String)
16:29:34:565 [ERROR]   location: class com.google.api.gax.httpjson.ProtoMessageRequestFormatter.Builder<com.google.iam.v1.TestIamPermissionsRequest>
16:29:34:565 [ERROR] /tmpfs/src/github/gapic-generator-java/google-cloud-java/java-kms/google-cloud-kms/src/main/java/com/google/cloud/kms/v1/stub/HttpJsonKeyManagementServiceStub.java:[1164,23] cannot find symbol
16:29:34:565 [ERROR]   symbol:   method setAdditionalPaths(java.lang.String,java.lang.String,java.lang.String,java.lang.String)
16:29:34:566 [ERROR]   location: class com.google.api.gax.httpjson.ProtoMessageRequestFormatter.Builder<com.google.iam.v1.SetIamPolicyRequest>
16:29:34:566 [ERROR] /tmpfs/src/github/gapic-generator-java/google-cloud-java/java-kms/google-cloud-kms/src/main/java/com/google/cloud/kms/v1/stub/HttpJsonKeyManagementServiceStub.java:[1206,23] cannot find symbol
16:29:34:566 [ERROR]   symbol:   method setAdditionalPaths(java.lang.String,java.lang.String,java.lang.String,java.lang.String)
16:29:34:566 [ERROR]   location: class com.google.api.gax.httpjson.ProtoMessageRequestFormatter.Builder<com.google.iam.v1.GetIamPolicyRequest>
16:29:34:567 [ERROR] /tmpfs/src/github/gapic-generator-java/google-cloud-java/java-kms/google-cloud-kms/src/main/java/com/google/cloud/kms/v1/stub/HttpJsonKeyManagementServiceStub.java:[1245,23] cannot find symbol
16:29:34:567 [ERROR]   symbol:   method setAdditionalPaths(java.lang.String,java.lang.String,java.lang.String,java.lang.String)
16:29:34:567 [ERROR]   location: class com.google.api.gax.httpjson.ProtoMessageRequestFormatter.Builder<com.google.iam.v1.TestIamPermissionsRequest>
16:29:34:568 [ERROR] -> [Help 1]

Seems to be due to the breaking changes from this PR (updating setAdditionalPaths to setAdditionalPathsExtractorMap.

sonarqubecloud · 2023-05-09T02:37:57Z

[gapic-generator-java-root] SonarCloud Quality Gate failed.

0 Bugs
0 Vulnerabilities
0 Security Hotspots
2 Code Smells

59.4% Coverage
0.0% Duplication

sonarqubecloud · 2023-05-09T02:40:57Z

[java_showcase_integration_tests] SonarCloud Quality Gate failed.

0 Bugs
0 Vulnerabilities
0 Security Hotspots
2 Code Smells

40.0% Coverage
0.0% Duplication

sonarqubecloud · 2023-05-09T02:45:08Z

[java_showcase_unit_tests] SonarCloud Quality Gate failed.

0 Bugs
0 Vulnerabilities
0 Security Hotspots
2 Code Smells

43.8% Coverage
0.0% Duplication

lqiu96 · 2023-05-10T15:10:41Z

Closing this issue for now. Will re-open if there is a need for this fix.

lqiu96 added 10 commits March 27, 2023 16:35

chore: Add additional bindings logic

7db8f57

chore: Update showcase goldens

fe94626

chore: Fix additional bindings logic

d6d6914

chore: Update tests

a693d25

chore: Update operations to use additional bindings mapping logic

3efafb1

chore: Update proto message request formatter

747255f

chore: Remove unused method

e46e3c4

chore: Clean up ProtoMessageRequestFormatter

f8e831e

chore: Clean up the files

f66fe66

chore: Update ProtoMessageRequestFormatter

1bf0a9c

product-auto-label bot added the size: l Pull request size is large. label Mar 27, 2023

lqiu96 commented Mar 27, 2023

View reviewed changes

...ava/gax-httpjson/src/main/java/com/google/api/gax/httpjson/ProtoMessageRequestFormatter.java Show resolved Hide resolved

chore: Add back public method

357be41

lqiu96 commented Mar 27, 2023

View reviewed changes

chore: Address code smell

e18d8b8

lqiu96 commented Mar 27, 2023

View reviewed changes

lqiu96 added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Mar 28, 2023

kokoro-team removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Mar 28, 2023

lqiu96 added 6 commits May 1, 2023 17:14

chore: Resolve merge conflicts

2dd4eb7

chore: Fix tests

6beb873

Merge branch 'main' into main-http_additional_bindings_part_2

c0d5c23

chore: Add back public methods

570683b

chore: Add test case for additionalBindings

48751ff

chore: Fix tests

f49db68

lqiu96 closed this May 10, 2023

lqiu96 mentioned this pull request Sep 6, 2023

GAPIC HttpJson additional_bindings annotation is not being used #1440

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: Check additional_binding's custom FieldsExtractor #1565

fix: Check additional_binding's custom FieldsExtractor #1565

lqiu96 commented Mar 27, 2023 •

edited

Loading

lqiu96 Mar 27, 2023

lqiu96 Mar 27, 2023

lqiu96 Mar 27, 2023

sonarqubecloud bot commented Mar 27, 2023

sonarqubecloud bot commented Mar 27, 2023

sonarqubecloud bot commented Mar 27, 2023

lqiu96 Mar 27, 2023

lqiu96 commented Mar 28, 2023

sonarqubecloud bot commented May 9, 2023

sonarqubecloud bot commented May 9, 2023

sonarqubecloud bot commented May 9, 2023

lqiu96 commented May 10, 2023

fix: Check additional_binding's custom FieldsExtractor #1565

fix: Check additional_binding's custom FieldsExtractor #1565

Conversation

lqiu96 commented Mar 27, 2023 • edited Loading

lqiu96 Mar 27, 2023

Choose a reason for hiding this comment

lqiu96 Mar 27, 2023

Choose a reason for hiding this comment

lqiu96 Mar 27, 2023

Choose a reason for hiding this comment

sonarqubecloud bot commented Mar 27, 2023

sonarqubecloud bot commented Mar 27, 2023

sonarqubecloud bot commented Mar 27, 2023

lqiu96 Mar 27, 2023

Choose a reason for hiding this comment

lqiu96 commented Mar 28, 2023

sonarqubecloud bot commented May 9, 2023

sonarqubecloud bot commented May 9, 2023

sonarqubecloud bot commented May 9, 2023

lqiu96 commented May 10, 2023

lqiu96 commented Mar 27, 2023 •

edited

Loading