From 229da5d94cf7db060abf3ea006a20d1ade804597 Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Wed, 19 May 2021 15:05:35 -0700 Subject: [PATCH] fix(mixins): Gate mixin RPC on HTTP rules, add yaml doc/http overrides [ggj] (#727) * fix(mixins): Gate mixin RPC on HTTP rules, override with doc/http in yaml * fix: comment, request params --- .../gapic/protoparser/HttpRuleParser.java | 29 +++++-- .../generator/gapic/protoparser/Parser.java | 84 +++++++++++++++---- .../apis/kms/v1/cloudkms_test_mixins_v1.yaml | 37 ++++---- .../kms/v1/KeyManagementServiceClient.java | 67 +-------------- .../v1/KeyManagementServiceClientTest.java | 54 +----------- .../kms/v1/KeyManagementServiceSettings.java | 11 --- .../google/cloud/kms/v1/gapic_metadata.json | 3 - .../v1/stub/GrpcKeyManagementServiceStub.java | 34 +------- .../kms/v1/stub/KeyManagementServiceStub.java | 5 -- .../KeyManagementServiceStubSettings.java | 23 ----- .../com/google/iam/v1/MockIAMPolicyImpl.java | 20 ----- 11 files changed, 121 insertions(+), 246 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 index 51e84cb57b..cd2ae640c9 100644 --- a/src/main/java/com/google/api/generator/gapic/protoparser/HttpRuleParser.java +++ b/src/main/java/com/google/api/generator/gapic/protoparser/HttpRuleParser.java @@ -50,15 +50,24 @@ public static Optional> parseHttpBindings( checkHttpFieldIsValid(httpRule.getBody(), inputMessage, true); } + return parseHttpRuleHelper(httpRule, Optional.of(inputMessage), messageTypes); + } + + public static Optional> parseHttpRule(HttpRule httpRule) { + return parseHttpRuleHelper(httpRule, Optional.empty(), Collections.emptyMap()); + } + + private static Optional> parseHttpRuleHelper( + HttpRule httpRule, Optional inputMessageOpt, Map messageTypes) { // Get pattern. - Set uniqueBindings = getPatternBindings(httpRule); + Set uniqueBindings = getHttpVerbPattern(httpRule); if (uniqueBindings.isEmpty()) { return Optional.empty(); } if (httpRule.getAdditionalBindingsCount() > 0) { for (HttpRule additionalRule : httpRule.getAdditionalBindingsList()) { - uniqueBindings.addAll(getPatternBindings(additionalRule)); + uniqueBindings.addAll(getHttpVerbPattern(additionalRule)); } } @@ -69,19 +78,23 @@ public static Optional> parseHttpBindings( for (String binding : bindings) { // Handle foo.bar cases by descending into the subfields. String[] descendantBindings = binding.split("\\."); - Message containingMessage = inputMessage; + Optional containingMessageOpt = inputMessageOpt; for (int i = 0; i < descendantBindings.length; i++) { String subField = descendantBindings[i]; + if (!containingMessageOpt.isPresent()) { + continue; + } + if (i < descendantBindings.length - 1) { - Field field = containingMessage.fieldMap().get(subField); - containingMessage = messageTypes.get(field.type().reference().fullName()); + Field field = containingMessageOpt.get().fieldMap().get(subField); + containingMessageOpt = Optional.of(messageTypes.get(field.type().reference().fullName())); Preconditions.checkNotNull( - containingMessage, + containingMessageOpt.get(), String.format( "No containing message found for field %s with type %s", field.name(), field.type().reference().simpleName())); } else { - checkHttpFieldIsValid(subField, containingMessage, false); + checkHttpFieldIsValid(subField, containingMessageOpt.get(), false); } } } @@ -89,7 +102,7 @@ public static Optional> parseHttpBindings( return Optional.of(bindings); } - private static Set getPatternBindings(HttpRule httpRule) { + private static Set getHttpVerbPattern(HttpRule httpRule) { String pattern = null; // Assign a temp variable to prevent the formatter from removing the import. PatternCase patternCase = httpRule.getPatternCase(); diff --git a/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java b/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java index cf74a492ea..41ce3c1041 100644 --- a/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java +++ b/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java @@ -15,6 +15,8 @@ package com.google.api.generator.gapic.protoparser; import com.google.api.ClientProto; +import com.google.api.DocumentationRule; +import com.google.api.HttpRule; import com.google.api.ResourceDescriptor; import com.google.api.ResourceProto; import com.google.api.generator.engine.ast.TypeNode; @@ -239,6 +241,33 @@ public static List parseServices( .filter(a -> MIXIN_ALLOWLIST.contains(a.getName())) .map(a -> a.getName()) .collect(Collectors.toSet()); + // Holds the methods to be mixed in. + // Key: proto_package.ServiceName.RpcName. + // Value: HTTP rules, which clobber those in the proto. + // Assumes that http.rules.selector always specifies RPC names in the above format. + Map> mixedInMethodsToHttpRules = new HashMap<>(); + Map mixedInMethodsToDocs = new HashMap<>(); + // Parse HTTP rules and documentation, which will override the proto. + if (serviceYamlProtoOpt.isPresent()) { + for (HttpRule httpRule : serviceYamlProtoOpt.get().getHttp().getRulesList()) { + Optional> httpBindingsOpt = HttpRuleParser.parseHttpRule(httpRule); + if (!httpBindingsOpt.isPresent()) { + continue; + } + for (String rpcFullNameRaw : httpRule.getSelector().split(",")) { + String rpcFullName = rpcFullNameRaw.trim(); + mixedInMethodsToHttpRules.put(rpcFullName, httpBindingsOpt.get()); + } + } + for (DocumentationRule docRule : + serviceYamlProtoOpt.get().getDocumentation().getRulesList()) { + for (String rpcFullNameRaw : docRule.getSelector().split(",")) { + String rpcFullName = rpcFullNameRaw.trim(); + mixedInMethodsToDocs.put(rpcFullName, docRule.getDescription()); + } + } + } + Set apiDefinedRpcs = new HashSet<>(); for (Service service : services) { if (blockedCodegenMixinApis.contains(service)) { @@ -252,28 +281,55 @@ public static List parseServices( if (servicesContainBlocklistedApi && !mixedInApis.isEmpty()) { for (int i = 0; i < services.size(); i++) { Service originalService = services.get(i); - List updatedMethods = new ArrayList<>(originalService.methods()); + List updatedOriginalServiceMethods = new ArrayList<>(originalService.methods()); // If mixin APIs are present, add the methods to all other services. for (Service mixinService : blockedCodegenMixinApis) { - if (!mixedInApis.contains( - String.format("%s.%s", mixinService.protoPakkage(), mixinService.name()))) { + final String mixinServiceFullName = serviceFullNameFn.apply(mixinService); + if (!mixedInApis.contains(mixinServiceFullName)) { continue; } - mixinService - .methods() + Function methodToFullProtoNameFn = + m -> String.format("%s.%s", mixinServiceFullName, m.name()); + // Filter mixed-in methods based on those listed in the HTTP rules section of + // service.yaml. + List updatedMixinMethods = + mixinService.methods().stream() + // Mixin method inclusion is based on the HTTP rules list in service.yaml. + .filter( + m -> mixedInMethodsToHttpRules.containsKey(methodToFullProtoNameFn.apply(m))) + .map( + m -> { + // HTTP rules and RPC documentation in the service.yaml file take + // precedence. + String fullMethodName = methodToFullProtoNameFn.apply(m); + List httpBindings = + mixedInMethodsToHttpRules.containsKey(fullMethodName) + ? mixedInMethodsToHttpRules.get(fullMethodName) + : m.httpBindings(); + String docs = + mixedInMethodsToDocs.containsKey(fullMethodName) + ? mixedInMethodsToDocs.get(fullMethodName) + : m.description(); + return m.toBuilder() + .setHttpBindings(httpBindings) + .setDescription(docs) + .build(); + }) + .collect(Collectors.toList()); + // Overridden RPCs defined in the protos take precedence. + updatedMixinMethods.stream() + .filter(m -> !apiDefinedRpcs.contains(m.name())) .forEach( - m -> { - // Overridden RPCs defined in the protos take precedence. - if (!apiDefinedRpcs.contains(m.name())) { - updatedMethods.add( + m -> + updatedOriginalServiceMethods.add( m.toBuilder() .setMixedInApiName(serviceFullNameFn.apply(mixinService)) - .build()); - } - }); - outputMixinServiceSet.add(mixinService); + .build())); + outputMixinServiceSet.add( + mixinService.toBuilder().setMethods(updatedMixinMethods).build()); } - services.set(i, originalService.toBuilder().setMethods(updatedMethods).build()); + services.set( + i, originalService.toBuilder().setMethods(updatedOriginalServiceMethods).build()); } } diff --git a/test/integration/apis/kms/v1/cloudkms_test_mixins_v1.yaml b/test/integration/apis/kms/v1/cloudkms_test_mixins_v1.yaml index 4190b62d0b..2828ca13f0 100644 --- a/test/integration/apis/kms/v1/cloudkms_test_mixins_v1.yaml +++ b/test/integration/apis/kms/v1/cloudkms_test_mixins_v1.yaml @@ -21,6 +21,7 @@ documentation: Gets the access control policy for a resource. Returns an empty policy if the resource exists and does not have a policy set. + # This RPC shouldn't appear in the proto even though the documentation field is set. - selector: google.iam.v1.IAMPolicy.SetIamPolicy description: |- Sets the access control policy on the specified resource. Replaces @@ -29,31 +30,37 @@ documentation: Can return `NOT_FOUND`, `INVALID_ARGUMENT`, and `PERMISSION_DENIED` errors. + # Test the overriding of comments. - selector: google.iam.v1.IAMPolicy.TestIamPermissions description: |- - Returns permissions that a caller has on the specified resource. If the - resource does not exist, this will return an empty set of - permissions, not a `NOT_FOUND` error. - - Note: This operation is designed to be used for building - permission-aware UIs and command-line tools, not for authorization - checking. This operation may "fail open" without warning. + This is a different comment for TestIamPermissions in the yaml file that should clobber the documentation in iam_policy.proto. http: rules: + - selector: google.cloud.location.Locations.GetLocation + get: "/v1/{name=locations/*}" + body: '*' + # Test different HTTP verbs. + - selector: google.cloud.location.Locations.ListLocations + get: "/v1/{filter=*/*}/locations" + body: '*' + additional_bindings: + - post: '/v1/{page_size=*}' + body: '*' - selector: google.iam.v1.IAMPolicy.GetIamPolicy get: '/v1/{resource=projects/*/locations/*/keyRings/*}:getIamPolicy' additional_bindings: - get: '/v1/{resource=projects/*/locations/*/keyRings/*/cryptoKeys/*}:getIamPolicy' - get: '/v1/{resource=projects/*/locations/*/keyRings/*/importJobs/*}:getIamPolicy' - - selector: google.iam.v1.IAMPolicy.SetIamPolicy - post: '/v1/{resource=projects/*/locations/*/keyRings/*}:setIamPolicy' - body: '*' - additional_bindings: - - post: '/v1/{resource=projects/*/locations/*/keyRings/*/cryptoKeys/*}:setIamPolicy' - body: '*' - - post: '/v1/{resource=projects/*/locations/*/keyRings/*/importJobs/*}:setIamPolicy' - body: '*' + # Test the omission of SetIamPolicy - this should no longer appear in the generated client. +# - selector: google.iam.v1.IAMPolicy.SetIamPolicy +# post: '/v1/{resource=projects/*/locations/*/keyRings/*}:setIamPolicy' +# body: '*' +# additional_bindings: +# - post: '/v1/{resource=projects/*/locations/*/keyRings/*/cryptoKeys/*}:setIamPolicy' +# body: '*' +# - post: '/v1/{resource=projects/*/locations/*/keyRings/*/importJobs/*}:setIamPolicy' +# body: '*' - selector: google.iam.v1.IAMPolicy.TestIamPermissions post: '/v1/{resource=projects/*/locations/*/keyRings/*}:testIamPermissions' body: '*' diff --git a/test/integration/goldens/kms/com/google/cloud/kms/v1/KeyManagementServiceClient.java b/test/integration/goldens/kms/com/google/cloud/kms/v1/KeyManagementServiceClient.java index 19ddbcd661..cc3e563cff 100644 --- a/test/integration/goldens/kms/com/google/cloud/kms/v1/KeyManagementServiceClient.java +++ b/test/integration/goldens/kms/com/google/cloud/kms/v1/KeyManagementServiceClient.java @@ -36,7 +36,6 @@ import com.google.common.util.concurrent.MoreExecutors; import com.google.iam.v1.GetIamPolicyRequest; import com.google.iam.v1.Policy; -import com.google.iam.v1.SetIamPolicyRequest; import com.google.iam.v1.TestIamPermissionsRequest; import com.google.iam.v1.TestIamPermissionsResponse; import com.google.protobuf.ByteString; @@ -3334,62 +3333,8 @@ public final UnaryCallable getLocationCallable() { // AUTO-GENERATED DOCUMENTATION AND METHOD. /** - * Sets the access control policy on the specified resource. Replaces any existing policy. - * - *

Sample code: - * - * 

{@code
-   * try (KeyManagementServiceClient keyManagementServiceClient =
-   *     KeyManagementServiceClient.create()) {
-   *   SetIamPolicyRequest request =
-   *       SetIamPolicyRequest.newBuilder()
-   *           .setResource(KeyRingName.of("[PROJECT]", "[LOCATION]", "[KEY_RING]").toString())
-   *           .setPolicy(Policy.newBuilder().build())
-   *           .build();
-   *   Policy response = keyManagementServiceClient.setIamPolicy(request);
-   * }
-   * }
- * - * @param request The request object containing all of the parameters for the API call. - * @throws com.google.api.gax.rpc.ApiException if the remote call fails - */ - public final Policy setIamPolicy(SetIamPolicyRequest request) { - return setIamPolicyCallable().call(request); - } - - // AUTO-GENERATED DOCUMENTATION AND METHOD. - /** - * Sets the access control policy on the specified resource. Replaces any existing policy. - * - *

Sample code: - * - * 

{@code
-   * try (KeyManagementServiceClient keyManagementServiceClient =
-   *     KeyManagementServiceClient.create()) {
-   *   SetIamPolicyRequest request =
-   *       SetIamPolicyRequest.newBuilder()
-   *           .setResource(KeyRingName.of("[PROJECT]", "[LOCATION]", "[KEY_RING]").toString())
-   *           .setPolicy(Policy.newBuilder().build())
-   *           .build();
-   *   ApiFuture future =
-   *       keyManagementServiceClient.setIamPolicyCallable().futureCall(request);
-   *   // Do something.
-   *   Policy response = future.get();
-   * }
-   * }
- */ - public final UnaryCallable setIamPolicyCallable() { - return stub.setIamPolicyCallable(); - } - - // AUTO-GENERATED DOCUMENTATION AND METHOD. - /** - * Returns permissions that a caller has on the specified resource. If the resource does not - * exist, this will return an empty set of permissions, not a NOT_FOUND error. - * - *

Note: This operation is designed to be used for building permission-aware UIs and - * command-line tools, not for authorization checking. This operation may "fail open" without - * warning. + * This is a different comment for TestIamPermissions in the yaml file that should clobber the + * documentation in iam_policy.proto. * *

Sample code: * @@ -3414,12 +3359,8 @@ public final TestIamPermissionsResponse testIamPermissions(TestIamPermissionsReq // AUTO-GENERATED DOCUMENTATION AND METHOD. /** - * Returns permissions that a caller has on the specified resource. If the resource does not - * exist, this will return an empty set of permissions, not a NOT_FOUND error. - * - *

Note: This operation is designed to be used for building permission-aware UIs and - * command-line tools, not for authorization checking. This operation may "fail open" without - * warning. + * This is a different comment for TestIamPermissions in the yaml file that should clobber the + * documentation in iam_policy.proto. * *

Sample code: * diff --git a/test/integration/goldens/kms/com/google/cloud/kms/v1/KeyManagementServiceClientTest.java b/test/integration/goldens/kms/com/google/cloud/kms/v1/KeyManagementServiceClientTest.java index 0d02ab2526..b1952fc8e3 100644 --- a/test/integration/goldens/kms/com/google/cloud/kms/v1/KeyManagementServiceClientTest.java +++ b/test/integration/goldens/kms/com/google/cloud/kms/v1/KeyManagementServiceClientTest.java @@ -41,7 +41,6 @@ import com.google.iam.v1.GetPolicyOptions; import com.google.iam.v1.MockIAMPolicy; import com.google.iam.v1.Policy; -import com.google.iam.v1.SetIamPolicyRequest; import com.google.iam.v1.TestIamPermissionsRequest; import com.google.iam.v1.TestIamPermissionsResponse; import com.google.protobuf.AbstractMessage; @@ -77,12 +76,12 @@ public class KeyManagementServiceClientTest { @BeforeClass public static void startStaticServer() { mockKeyManagementService = new MockKeyManagementService(); - mockLocations = new MockLocations(); mockIAMPolicy = new MockIAMPolicy(); + mockLocations = new MockLocations(); mockServiceHelper = new MockServiceHelper( UUID.randomUUID().toString(), - Arrays.asList(mockKeyManagementService, mockLocations, mockIAMPolicy)); + Arrays.asList(mockKeyManagementService, mockIAMPolicy, mockLocations)); mockServiceHelper.start(); } @@ -2374,55 +2373,6 @@ public void getLocationExceptionTest() throws Exception { } } - @Test - public void setIamPolicyTest() throws Exception { - Policy expectedResponse = - Policy.newBuilder() - .setVersion(351608024) - .addAllBindings(new ArrayList()) - .setEtag(ByteString.EMPTY) - .build(); - mockIAMPolicy.addResponse(expectedResponse); - - SetIamPolicyRequest request = - SetIamPolicyRequest.newBuilder() - .setResource(KeyRingName.of("[PROJECT]", "[LOCATION]", "[KEY_RING]").toString()) - .setPolicy(Policy.newBuilder().build()) - .build(); - - Policy actualResponse = client.setIamPolicy(request); - Assert.assertEquals(expectedResponse, actualResponse); - - List actualRequests = mockIAMPolicy.getRequests(); - Assert.assertEquals(1, actualRequests.size()); - SetIamPolicyRequest actualRequest = ((SetIamPolicyRequest) actualRequests.get(0)); - - Assert.assertEquals(request.getResource(), actualRequest.getResource()); - Assert.assertEquals(request.getPolicy(), actualRequest.getPolicy()); - Assert.assertTrue( - channelProvider.isHeaderSent( - ApiClientHeaderProvider.getDefaultApiClientHeaderKey(), - GaxGrpcProperties.getDefaultApiClientHeaderPattern())); - } - - @Test - public void setIamPolicyExceptionTest() throws Exception { - StatusRuntimeException exception = new StatusRuntimeException(io.grpc.Status.INVALID_ARGUMENT); - mockIAMPolicy.addException(exception); - - try { - SetIamPolicyRequest request = - SetIamPolicyRequest.newBuilder() - .setResource(KeyRingName.of("[PROJECT]", "[LOCATION]", "[KEY_RING]").toString()) - .setPolicy(Policy.newBuilder().build()) - .build(); - client.setIamPolicy(request); - Assert.fail("No exception raised"); - } catch (InvalidArgumentException e) { - // Expected exception. - } - } - @Test public void testIamPermissionsTest() throws Exception { TestIamPermissionsResponse expectedResponse = diff --git a/test/integration/goldens/kms/com/google/cloud/kms/v1/KeyManagementServiceSettings.java b/test/integration/goldens/kms/com/google/cloud/kms/v1/KeyManagementServiceSettings.java index bf38dcd54e..d749c91145 100644 --- a/test/integration/goldens/kms/com/google/cloud/kms/v1/KeyManagementServiceSettings.java +++ b/test/integration/goldens/kms/com/google/cloud/kms/v1/KeyManagementServiceSettings.java @@ -41,7 +41,6 @@ import com.google.cloud.location.Location; import com.google.iam.v1.GetIamPolicyRequest; import com.google.iam.v1.Policy; -import com.google.iam.v1.SetIamPolicyRequest; import com.google.iam.v1.TestIamPermissionsRequest; import com.google.iam.v1.TestIamPermissionsResponse; import java.io.IOException; @@ -233,11 +232,6 @@ public UnaryCallSettings getLocationSettings() { return ((KeyManagementServiceStubSettings) getStubSettings()).getLocationSettings(); } - /** Returns the object with the settings used for calls to setIamPolicy. */ - public UnaryCallSettings setIamPolicySettings() { - return ((KeyManagementServiceStubSettings) getStubSettings()).setIamPolicySettings(); - } - /** Returns the object with the settings used for calls to testIamPermissions. */ public UnaryCallSettings testIamPermissionsSettings() { @@ -494,11 +488,6 @@ public UnaryCallSettings.Builder getLocationSettin return getStubSettingsBuilder().getLocationSettings(); } - /** Returns the builder for the settings used for calls to setIamPolicy. */ - public UnaryCallSettings.Builder setIamPolicySettings() { - return getStubSettingsBuilder().setIamPolicySettings(); - } - /** Returns the builder for the settings used for calls to testIamPermissions. */ public UnaryCallSettings.Builder testIamPermissionsSettings() { diff --git a/test/integration/goldens/kms/com/google/cloud/kms/v1/gapic_metadata.json b/test/integration/goldens/kms/com/google/cloud/kms/v1/gapic_metadata.json index a3c2c93ed7..73abe153f4 100644 --- a/test/integration/goldens/kms/com/google/cloud/kms/v1/gapic_metadata.json +++ b/test/integration/goldens/kms/com/google/cloud/kms/v1/gapic_metadata.json @@ -79,9 +79,6 @@ "RestoreCryptoKeyVersion": { "methods": ["restoreCryptoKeyVersion", "restoreCryptoKeyVersion", "restoreCryptoKeyVersion", "restoreCryptoKeyVersionCallable"] }, - "SetIamPolicy": { - "methods": ["setIamPolicy", "setIamPolicyCallable"] - }, "TestIamPermissions": { "methods": ["testIamPermissions", "testIamPermissionsCallable"] }, diff --git a/test/integration/goldens/kms/com/google/cloud/kms/v1/stub/GrpcKeyManagementServiceStub.java b/test/integration/goldens/kms/com/google/cloud/kms/v1/stub/GrpcKeyManagementServiceStub.java index f2dcc16fa9..19065361ed 100644 --- a/test/integration/goldens/kms/com/google/cloud/kms/v1/stub/GrpcKeyManagementServiceStub.java +++ b/test/integration/goldens/kms/com/google/cloud/kms/v1/stub/GrpcKeyManagementServiceStub.java @@ -72,7 +72,6 @@ import com.google.common.collect.ImmutableMap; import com.google.iam.v1.GetIamPolicyRequest; import com.google.iam.v1.Policy; -import com.google.iam.v1.SetIamPolicyRequest; import com.google.iam.v1.TestIamPermissionsRequest; import com.google.iam.v1.TestIamPermissionsResponse; import com.google.longrunning.stub.GrpcOperationsStub; @@ -345,14 +344,6 @@ public class GrpcKeyManagementServiceStub extends KeyManagementServiceStub { .setResponseMarshaller(ProtoUtils.marshaller(Location.getDefaultInstance())) .build(); - private static final MethodDescriptor setIamPolicyMethodDescriptor = - MethodDescriptor.newBuilder() - .setType(MethodDescriptor.MethodType.UNARY) - .setFullMethodName("google.iam.v1.IAMPolicy/SetIamPolicy") - .setRequestMarshaller(ProtoUtils.marshaller(SetIamPolicyRequest.getDefaultInstance())) - .setResponseMarshaller(ProtoUtils.marshaller(Policy.getDefaultInstance())) - .build(); - private static final MethodDescriptor testIamPermissionsMethodDescriptor = MethodDescriptor.newBuilder() @@ -409,7 +400,6 @@ public class GrpcKeyManagementServiceStub extends KeyManagementServiceStub { private final UnaryCallable listLocationsPagedCallable; private final UnaryCallable getLocationCallable; - private final UnaryCallable setIamPolicyCallable; private final UnaryCallable testIamPermissionsCallable; @@ -793,7 +783,8 @@ public Map extract(GetIamPolicyRequest request) { @Override public Map extract(ListLocationsRequest request) { ImmutableMap.Builder params = ImmutableMap.builder(); - params.put("name", String.valueOf(request.getName())); + params.put("filter", String.valueOf(request.getFilter())); + params.put("page_size", String.valueOf(request.getPageSize())); return params.build(); } }) @@ -811,19 +802,6 @@ public Map extract(GetLocationRequest request) { } }) .build(); - GrpcCallSettings setIamPolicyTransportSettings = - GrpcCallSettings.newBuilder() - .setMethodDescriptor(setIamPolicyMethodDescriptor) - .setParamsExtractor( - new RequestParamsExtractor() { - @Override - public Map extract(SetIamPolicyRequest request) { - ImmutableMap.Builder params = ImmutableMap.builder(); - params.put("resource", String.valueOf(request.getResource())); - return params.build(); - } - }) - .build(); GrpcCallSettings testIamPermissionsTransportSettings = GrpcCallSettings.newBuilder() @@ -952,9 +930,6 @@ public Map extract(TestIamPermissionsRequest request) { this.getLocationCallable = callableFactory.createUnaryCallable( getLocationTransportSettings, settings.getLocationSettings(), clientContext); - this.setIamPolicyCallable = - callableFactory.createUnaryCallable( - setIamPolicyTransportSettings, settings.setIamPolicySettings(), clientContext); this.testIamPermissionsCallable = callableFactory.createUnaryCallable( testIamPermissionsTransportSettings, @@ -1136,11 +1111,6 @@ public UnaryCallable getLocationCallable() { return getLocationCallable; } - @Override - public UnaryCallable setIamPolicyCallable() { - return setIamPolicyCallable; - } - @Override public UnaryCallable testIamPermissionsCallable() { diff --git a/test/integration/goldens/kms/com/google/cloud/kms/v1/stub/KeyManagementServiceStub.java b/test/integration/goldens/kms/com/google/cloud/kms/v1/stub/KeyManagementServiceStub.java index 3c6cb33625..14915e1ae7 100644 --- a/test/integration/goldens/kms/com/google/cloud/kms/v1/stub/KeyManagementServiceStub.java +++ b/test/integration/goldens/kms/com/google/cloud/kms/v1/stub/KeyManagementServiceStub.java @@ -66,7 +66,6 @@ import com.google.cloud.location.Location; import com.google.iam.v1.GetIamPolicyRequest; import com.google.iam.v1.Policy; -import com.google.iam.v1.SetIamPolicyRequest; import com.google.iam.v1.TestIamPermissionsRequest; import com.google.iam.v1.TestIamPermissionsResponse; import javax.annotation.Generated; @@ -218,10 +217,6 @@ public UnaryCallable getLocationCallable() { throw new UnsupportedOperationException("Not implemented: getLocationCallable()"); } - public UnaryCallable setIamPolicyCallable() { - throw new UnsupportedOperationException("Not implemented: setIamPolicyCallable()"); - } - public UnaryCallable testIamPermissionsCallable() { throw new UnsupportedOperationException("Not implemented: testIamPermissionsCallable()"); diff --git a/test/integration/goldens/kms/com/google/cloud/kms/v1/stub/KeyManagementServiceStubSettings.java b/test/integration/goldens/kms/com/google/cloud/kms/v1/stub/KeyManagementServiceStubSettings.java index dc9718e802..5ebc43167f 100644 --- a/test/integration/goldens/kms/com/google/cloud/kms/v1/stub/KeyManagementServiceStubSettings.java +++ b/test/integration/goldens/kms/com/google/cloud/kms/v1/stub/KeyManagementServiceStubSettings.java @@ -90,7 +90,6 @@ import com.google.common.collect.Lists; import com.google.iam.v1.GetIamPolicyRequest; import com.google.iam.v1.Policy; -import com.google.iam.v1.SetIamPolicyRequest; import com.google.iam.v1.TestIamPermissionsRequest; import com.google.iam.v1.TestIamPermissionsResponse; import java.io.IOException; @@ -188,7 +187,6 @@ public class KeyManagementServiceStubSettings ListLocationsRequest, ListLocationsResponse, ListLocationsPagedResponse> listLocationsSettings; private final UnaryCallSettings getLocationSettings; - private final UnaryCallSettings setIamPolicySettings; private final UnaryCallSettings testIamPermissionsSettings; @@ -619,11 +617,6 @@ public UnaryCallSettings getLocationSettings() { return getLocationSettings; } - /** Returns the object with the settings used for calls to setIamPolicy. */ - public UnaryCallSettings setIamPolicySettings() { - return setIamPolicySettings; - } - /** Returns the object with the settings used for calls to testIamPermissions. */ public UnaryCallSettings testIamPermissionsSettings() { @@ -726,7 +719,6 @@ protected KeyManagementServiceStubSettings(Builder settingsBuilder) throws IOExc getIamPolicySettings = settingsBuilder.getIamPolicySettings().build(); listLocationsSettings = settingsBuilder.listLocationsSettings().build(); getLocationSettings = settingsBuilder.getLocationSettings().build(); - setIamPolicySettings = settingsBuilder.setIamPolicySettings().build(); testIamPermissionsSettings = settingsBuilder.testIamPermissionsSettings().build(); } @@ -784,7 +776,6 @@ public static class Builder ListLocationsRequest, ListLocationsResponse, ListLocationsPagedResponse> listLocationsSettings; private final UnaryCallSettings.Builder getLocationSettings; - private final UnaryCallSettings.Builder setIamPolicySettings; private final UnaryCallSettings.Builder testIamPermissionsSettings; private static final ImmutableMap> @@ -867,7 +858,6 @@ protected Builder(ClientContext clientContext) { getIamPolicySettings = UnaryCallSettings.newUnaryCallSettingsBuilder(); listLocationsSettings = PagedCallSettings.newBuilder(LIST_LOCATIONS_PAGE_STR_FACT); getLocationSettings = UnaryCallSettings.newUnaryCallSettingsBuilder(); - setIamPolicySettings = UnaryCallSettings.newUnaryCallSettingsBuilder(); testIamPermissionsSettings = UnaryCallSettings.newUnaryCallSettingsBuilder(); unaryMethodSettingsBuilders = @@ -898,7 +888,6 @@ protected Builder(ClientContext clientContext) { getIamPolicySettings, listLocationsSettings, getLocationSettings, - setIamPolicySettings, testIamPermissionsSettings); initDefaults(this); } @@ -933,7 +922,6 @@ protected Builder(KeyManagementServiceStubSettings settings) { getIamPolicySettings = settings.getIamPolicySettings.toBuilder(); listLocationsSettings = settings.listLocationsSettings.toBuilder(); getLocationSettings = settings.getLocationSettings.toBuilder(); - setIamPolicySettings = settings.setIamPolicySettings.toBuilder(); testIamPermissionsSettings = settings.testIamPermissionsSettings.toBuilder(); unaryMethodSettingsBuilders = @@ -964,7 +952,6 @@ protected Builder(KeyManagementServiceStubSettings settings) { getIamPolicySettings, listLocationsSettings, getLocationSettings, - setIamPolicySettings, testIamPermissionsSettings); } @@ -1110,11 +1097,6 @@ private static Builder initDefaults(Builder builder) { .setRetryableCodes(RETRYABLE_CODE_DEFINITIONS.get("no_retry_codes")) .setRetrySettings(RETRY_PARAM_DEFINITIONS.get("no_retry_params")); - builder - .setIamPolicySettings() - .setRetryableCodes(RETRYABLE_CODE_DEFINITIONS.get("retry_policy_1_codes")) - .setRetrySettings(RETRY_PARAM_DEFINITIONS.get("retry_policy_1_params")); - builder .testIamPermissionsSettings() .setRetryableCodes(RETRYABLE_CODE_DEFINITIONS.get("retry_policy_1_codes")) @@ -1290,11 +1272,6 @@ public UnaryCallSettings.Builder getLocationSettin return getLocationSettings; } - /** Returns the builder for the settings used for calls to setIamPolicy. */ - public UnaryCallSettings.Builder setIamPolicySettings() { - return setIamPolicySettings; - } - /** Returns the builder for the settings used for calls to testIamPermissions. */ public UnaryCallSettings.Builder testIamPermissionsSettings() { diff --git a/test/integration/goldens/kms/com/google/iam/v1/MockIAMPolicyImpl.java b/test/integration/goldens/kms/com/google/iam/v1/MockIAMPolicyImpl.java index fc661c900c..6245f271c9 100644 --- a/test/integration/goldens/kms/com/google/iam/v1/MockIAMPolicyImpl.java +++ b/test/integration/goldens/kms/com/google/iam/v1/MockIAMPolicyImpl.java @@ -58,26 +58,6 @@ public void reset() { responses = new LinkedList<>(); } - @Override - public void setIamPolicy(SetIamPolicyRequest request, StreamObserver responseObserver) { - Object response = responses.poll(); - if (response instanceof Policy) { - requests.add(request); - responseObserver.onNext(((Policy) response)); - responseObserver.onCompleted(); - } else if (response instanceof Exception) { - responseObserver.onError(((Exception) response)); - } else { - responseObserver.onError( - new IllegalArgumentException( - String.format( - "Unrecognized response type %s for method SetIamPolicy, expected %s or %s", - response == null ? "null" : response.getClass().getName(), - Policy.class.getName(), - Exception.class.getName()))); - } - } - @Override public void getIamPolicy(GetIamPolicyRequest request, StreamObserver responseObserver) { Object response = responses.poll();