From ceaaddbd08651df694e7b11834ae71556aa366aa Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Wed, 7 Oct 2020 16:18:54 -0700 Subject: [PATCH 1/5] fix: refactor requestBuilder into separate method in ServiceClientClassComposer --- .../composer/ServiceClientClassComposer.java | 138 +++--- .../api/generator/gapic/composer/BUILD.bazel | 4 +- .../ServiceClientClassComposerTest.java | 39 +- .../composer/goldens/IdentityClient.golden | 407 ++++++++++++++++++ .../api/generator/gapic/testdata/BUILD.bazel | 1 + .../generator/gapic/testdata/identity.proto | 192 +++++++++ 6 files changed, 705 insertions(+), 76 deletions(-) create mode 100644 src/test/java/com/google/api/generator/gapic/composer/goldens/IdentityClient.golden create mode 100644 src/test/java/com/google/api/generator/gapic/testdata/identity.proto diff --git a/src/main/java/com/google/api/generator/gapic/composer/ServiceClientClassComposer.java b/src/main/java/com/google/api/generator/gapic/composer/ServiceClientClassComposer.java index 177fffb162..eb9d4abc99 100644 --- a/src/main/java/com/google/api/generator/gapic/composer/ServiceClientClassComposer.java +++ b/src/main/java/com/google/api/generator/gapic/composer/ServiceClientClassComposer.java @@ -64,6 +64,7 @@ import com.google.api.generator.gapic.model.MethodArgument; import com.google.api.generator.gapic.model.Service; import com.google.api.generator.gapic.utils.JavaStyle; +import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Preconditions; import com.google.common.base.Strings; import com.google.common.util.concurrent.MoreExecutors; @@ -90,6 +91,9 @@ public class ServiceClientClassComposer implements ClassComposer { private static final String PAGED_CALLABLE_NAME_PATTERN = "%sPagedCallable"; private static final String OPERATION_CALLABLE_NAME_PATTERN = "%sOperationCallable"; + private static final Reference LIST_REFERENCE = ConcreteReference.withClazz(List.class); + private static final Reference MAP_REFERENCE = ConcreteReference.withClazz(Map.class); + private enum CallableMethodKind { REGULAR, LRO, @@ -499,8 +503,6 @@ private static List createMethodVariants( } String methodInputTypeName = methodInputType.reference().name(); - Reference listRef = ConcreteReference.withClazz(List.class); - Reference mapRef = ConcreteReference.withClazz(Map.class); // Make the method signature order deterministic, which helps with unit testing and per-version // diffs. @@ -544,71 +546,12 @@ private static List createMethodVariants( .setIsDecl(true) .build(); - MethodInvocationExpr newBuilderExpr = - MethodInvocationExpr.builder() - .setMethodName("newBuilder") - .setStaticReferenceType(methodInputType) - .build(); - // TODO(miraleung): Handle nested arguments and descending setters here. - for (MethodArgument argument : signature) { - String argumentName = JavaStyle.toLowerCamelCase(argument.name()); - TypeNode argumentType = argument.type(); - String setterMethodVariantPattern = "set%s"; - if (TypeNode.isReferenceType(argumentType)) { - if (listRef.isSupertypeOrEquals(argumentType.reference())) { - setterMethodVariantPattern = "addAll%s"; - } else if (mapRef.isSupertypeOrEquals(argumentType.reference())) { - setterMethodVariantPattern = "putAll%s"; - } - } - String setterMethodName = - String.format(setterMethodVariantPattern, JavaStyle.toUpperCamelCase(argumentName)); - - Expr argVarExpr = - VariableExpr.withVariable( - Variable.builder().setName(argumentName).setType(argumentType).build()); - - if (argument.isResourceNameHelper()) { - MethodInvocationExpr isNullCheckExpr = - MethodInvocationExpr.builder() - .setStaticReferenceType(types.get("Objects")) - .setMethodName("isNull") - .setArguments(Arrays.asList(argVarExpr)) - .setReturnType(TypeNode.BOOLEAN) - .build(); - Expr nullExpr = ValueExpr.withValue(NullObjectValue.create()); - MethodInvocationExpr toStringExpr = - MethodInvocationExpr.builder() - .setExprReferenceExpr(argVarExpr) - .setMethodName("toString") - .setReturnType(TypeNode.STRING) - .build(); - argVarExpr = - TernaryExpr.builder() - .setConditionExpr(isNullCheckExpr) - .setThenExpr(nullExpr) - .setElseExpr(toStringExpr) - .build(); - } + Expr requestBuilderExpr = createRequestBuilderExpr(method, signature, types); - newBuilderExpr = - MethodInvocationExpr.builder() - .setMethodName(setterMethodName) - .setArguments(Arrays.asList(argVarExpr)) - .setExprReferenceExpr(newBuilderExpr) - .build(); - } - - MethodInvocationExpr builderExpr = - MethodInvocationExpr.builder() - .setMethodName("build") - .setExprReferenceExpr(newBuilderExpr) - .setReturnType(methodInputType) - .build(); AssignmentExpr requestAssignmentExpr = AssignmentExpr.builder() .setVariableExpr(requestVarExpr) - .setValueExpr(builderExpr) + .setValueExpr(requestBuilderExpr) .build(); List statements = Arrays.asList(ExprStatement.withExpr(requestAssignmentExpr)); @@ -1373,6 +1316,75 @@ private static ClassDefinition createNestedRpcFixedSizeCollectionClass( .build(); } + @VisibleForTesting + static Expr createRequestBuilderExpr( + Method method, List signature, Map types) { + TypeNode methodInputType = method.inputType(); + MethodInvocationExpr newBuilderExpr = + MethodInvocationExpr.builder() + .setMethodName("newBuilder") + .setStaticReferenceType(methodInputType) + .build(); + // TODO(miraleung): Handle nested arguments and descending setters here. + for (MethodArgument argument : signature) { + String argumentName = JavaStyle.toLowerCamelCase(argument.name()); + TypeNode argumentType = argument.type(); + String setterMethodVariantPattern = "set%s"; + if (TypeNode.isReferenceType(argumentType)) { + if (LIST_REFERENCE.isSupertypeOrEquals(argumentType.reference())) { + setterMethodVariantPattern = "addAll%s"; + } else if (MAP_REFERENCE.isSupertypeOrEquals(argumentType.reference())) { + setterMethodVariantPattern = "putAll%s"; + } + } + String setterMethodName = + String.format(setterMethodVariantPattern, JavaStyle.toUpperCamelCase(argumentName)); + + Expr argVarExpr = + VariableExpr.withVariable( + Variable.builder().setName(argumentName).setType(argumentType).build()); + + if (argument.isResourceNameHelper()) { + MethodInvocationExpr isNullCheckExpr = + MethodInvocationExpr.builder() + .setStaticReferenceType(types.get("Objects")) + .setMethodName("isNull") + .setArguments(Arrays.asList(argVarExpr)) + .setReturnType(TypeNode.BOOLEAN) + .build(); + Expr nullExpr = ValueExpr.withValue(NullObjectValue.create()); + MethodInvocationExpr toStringExpr = + MethodInvocationExpr.builder() + .setExprReferenceExpr(argVarExpr) + .setMethodName("toString") + .setReturnType(TypeNode.STRING) + .build(); + argVarExpr = + TernaryExpr.builder() + .setConditionExpr(isNullCheckExpr) + .setThenExpr(nullExpr) + .setElseExpr(toStringExpr) + .build(); + } + + newBuilderExpr = + MethodInvocationExpr.builder() + .setMethodName(setterMethodName) + .setArguments(Arrays.asList(argVarExpr)) + .setExprReferenceExpr(newBuilderExpr) + .build(); + } + + MethodInvocationExpr builderExpr = + MethodInvocationExpr.builder() + .setMethodName("build") + .setExprReferenceExpr(newBuilderExpr) + .setReturnType(methodInputType) + .build(); + + return builderExpr; + } + private static Map createTypes( Service service, Map messageTypes) { Map types = new HashMap<>(); diff --git a/src/test/java/com/google/api/generator/gapic/composer/BUILD.bazel b/src/test/java/com/google/api/generator/gapic/composer/BUILD.bazel index d04f29e1c9..3a689014d3 100644 --- a/src/test/java/com/google/api/generator/gapic/composer/BUILD.bazel +++ b/src/test/java/com/google/api/generator/gapic/composer/BUILD.bazel @@ -11,6 +11,8 @@ UPDATE_GOLDENS_TESTS = [ "MockServiceClassComposerTest", "MockServiceImplClassComposerTest", "ResourceNameHelperClassComposerTest", + "ServiceClientClassComposerTest", + "ServiceClientTestClassComposerTest", "ServiceSettingsClassComposerTest", "ServiceStubSettingsClassComposerTest", "ServiceStubClassComposerTest", @@ -20,8 +22,6 @@ TESTS = UPDATE_GOLDENS_TESTS + [ "DefaultValueComposerTest", "ResourceNameTokenizerTest", "RetrySettingsComposerTest", - "ServiceClientClassComposerTest", - "ServiceClientTestClassComposerTest", ] TEST_DEPS = [ diff --git a/src/test/java/com/google/api/generator/gapic/composer/ServiceClientClassComposerTest.java b/src/test/java/com/google/api/generator/gapic/composer/ServiceClientClassComposerTest.java index f0c0d70102..7e2f8d24d0 100644 --- a/src/test/java/com/google/api/generator/gapic/composer/ServiceClientClassComposerTest.java +++ b/src/test/java/com/google/api/generator/gapic/composer/ServiceClientClassComposerTest.java @@ -27,28 +27,22 @@ import com.google.protobuf.Descriptors.FileDescriptor; import com.google.protobuf.Descriptors.ServiceDescriptor; import com.google.showcase.v1beta1.EchoOuterClass; +import com.google.showcase.v1beta1.IdentityOuterClass; import java.nio.file.Path; import java.nio.file.Paths; import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Set; -import org.junit.Before; import org.junit.Test; public class ServiceClientClassComposerTest { - private ServiceDescriptor echoService; - private FileDescriptor echoFileDescriptor; - - @Before - public void setUp() { - echoFileDescriptor = EchoOuterClass.getDescriptor(); - echoService = echoFileDescriptor.getServices().get(0); - assertEquals(echoService.getName(), "Echo"); - } - @Test public void generateServiceClasses() { + FileDescriptor echoFileDescriptor = EchoOuterClass.getDescriptor(); + ServiceDescriptor echoService = echoFileDescriptor.getServices().get(0); + assertEquals(echoService.getName(), "Echo"); + Map messageTypes = Parser.parseMessages(echoFileDescriptor); Map resourceNames = Parser.parseResourceNames(echoFileDescriptor); Set outputResourceNames = new HashSet<>(); @@ -65,4 +59,27 @@ public void generateServiceClasses() { Path goldenFilePath = Paths.get(ComposerConstants.GOLDENFILES_DIRECTORY, "EchoClient.golden"); assertCodeEquals(goldenFilePath, visitor.write()); } + + @Test + public void generateServiceClasses_methodSignatureHasNestedFields() { + FileDescriptor fileDescriptor = IdentityOuterClass.getDescriptor(); + ServiceDescriptor identityService = fileDescriptor.getServices().get(0); + assertEquals(identityService.getName(), "Identity"); + + Map messageTypes = Parser.parseMessages(fileDescriptor); + Map resourceNames = Parser.parseResourceNames(fileDescriptor); + Set outputResourceNames = new HashSet<>(); + List services = + Parser.parseService(fileDescriptor, messageTypes, resourceNames, outputResourceNames); + + Service protoService = services.get(0); + GapicClass clazz = ServiceClientClassComposer.instance().generate(protoService, messageTypes); + + JavaWriterVisitor visitor = new JavaWriterVisitor(); + clazz.classDefinition().accept(visitor); + Utils.saveCodegenToFile(this.getClass(), "IdentityClient.golden", visitor.write()); + Path goldenFilePath = + Paths.get(ComposerConstants.GOLDENFILES_DIRECTORY, "IdentityClient.golden"); + assertCodeEquals(goldenFilePath, visitor.write()); + } } diff --git a/src/test/java/com/google/api/generator/gapic/composer/goldens/IdentityClient.golden b/src/test/java/com/google/api/generator/gapic/composer/goldens/IdentityClient.golden new file mode 100644 index 0000000000..0f6f1a418d --- /dev/null +++ b/src/test/java/com/google/api/generator/gapic/composer/goldens/IdentityClient.golden @@ -0,0 +1,407 @@ +package com.google.showcase.v1beta1; + +import com.google.api.core.ApiFunction; +import com.google.api.core.ApiFuture; +import com.google.api.core.ApiFutures; +import com.google.api.core.BetaApi; +import com.google.api.gax.core.BackgroundResource; +import com.google.api.gax.paging.AbstractFixedSizeCollection; +import com.google.api.gax.paging.AbstractPage; +import com.google.api.gax.paging.AbstractPagedListResponse; +import com.google.api.gax.rpc.PageContext; +import com.google.api.gax.rpc.UnaryCallable; +import com.google.common.util.concurrent.MoreExecutors; +import com.google.protobuf.Empty; +import com.google.showcase.v1beta1.stub.IdentityStub; +import com.google.showcase.v1beta1.stub.IdentityStubSettings; +import java.io.IOException; +import java.util.List; +import java.util.Objects; +import java.util.concurrent.TimeUnit; +import javax.annotation.Generated; + +// AUTO-GENERATED DOCUMENTATION AND CLASS. +/** + * This class provides the ability to make remote calls to the backing service through method calls + * that map to API methods. Sample code to get started: + * + *

Note: close() needs to be called on the echoClient object to clean up resources such as + * threads. In the example above, try-with-resources is used, which automatically calls close(). + * + *

The surface of this class includes several types of Java methods for each of the API's + * methods: + * + *

    + *
  1. A "flattened" method. With this type of method, the fields of the request type have been + * converted into function parameters. It may be the case that not all fields are available as + * parameters, and not every API method will have a flattened method entry point. + *
  2. A "request object" method. This type of method only takes one parameter, a request object, + * which must be constructed before the call. Not every API method will have a request object + * method. + *
  3. A "callable" method. This type of method takes no parameters and returns an immutable API + * callable object, which can be used to initiate calls to the service. + *
+ * + *

See the individual methods for example code. + * + *

Many parameters require resource names to be formatted in a particular way. To assist with + * these names, this class includes a format method for each type of name, and additionally a parse + * method to extract the individual identifiers contained within names that are returned. + * + *

This class can be customized by passing in a custom instance of IdentitySettings to create(). + * For example: + * + *

To customize credentials: + * + *

To customize the endpoint: + */ +@BetaApi +@Generated("by gapic-generator") +public class IdentityClient implements BackgroundResource { + private final IdentitySettings settings; + private final IdentityStub stub; + + /** Constructs an instance of EchoClient with default settings. */ + public static final IdentityClient create() throws IOException { + return create(IdentitySettings.newBuilder().build()); + } + + /** + * Constructs an instance of EchoClient, using the given settings. The channels are created based + * on the settings passed in, or defaults for any settings that are not set. + */ + public static final IdentityClient create(IdentitySettings settings) throws IOException { + return new IdentityClient(settings); + } + + /** + * Constructs an instance of EchoClient, using the given stub for making calls. This is for + * advanced usage - prefer using create(IdentitySettings). + */ + @BetaApi("A restructuring of stub classes is planned, so this may break in the future") + public static final IdentityClient create(IdentityStub stub) { + return new IdentityClient(stub); + } + + /** + * Constructs an instance of EchoClient, using the given settings. This is protected so that it is + * easy to make a subclass, but otherwise, the static factory methods should be preferred. + */ + protected IdentityClient(IdentitySettings settings) throws IOException { + this.settings = settings; + this.stub = ((IdentityStubSettings) settings.getStubSettings()).createStub(); + } + + @BetaApi("A restructuring of stub classes is planned, so this may break in the future") + protected IdentityClient(IdentityStub stub) { + this.settings = null; + this.stub = stub; + } + + public final IdentitySettings getSettings() { + return settings; + } + + @BetaApi("A restructuring of stub classes is planned, so this may break in the future") + public IdentityStub getStub() { + return stub; + } + + // AUTO-GENERATED DOCUMENTATION AND METHOD. + /** + * Sample code: + * + * @param parent + * @param display_name + * @param email + * @throws com.google.api.gax.rpc.ApiException if the remote call fails + */ + public final User createUser(String parent, String displayName, String email) { + CreateUserRequest request = + CreateUserRequest.newBuilder() + .setParent(parent) + .setDisplayName(displayName) + .setEmail(email) + .build(); + return createUser(request); + } + + // AUTO-GENERATED DOCUMENTATION AND METHOD. + /** + * Sample code: + * + * @param parent + * @param display_name + * @param email + * @param age + * @param nickname + * @param enable_notifications + * @param height_feet + * @throws com.google.api.gax.rpc.ApiException if the remote call fails + */ + public final User createUser( + String parent, + String displayName, + String email, + int age, + String nickname, + boolean enableNotifications, + double heightFeet) { + CreateUserRequest request = + CreateUserRequest.newBuilder() + .setParent(parent) + .setDisplayName(displayName) + .setEmail(email) + .setAge(age) + .setNickname(nickname) + .setEnableNotifications(enableNotifications) + .setHeightFeet(heightFeet) + .build(); + return createUser(request); + } + + // AUTO-GENERATED DOCUMENTATION AND METHOD. + /** + * Sample code: + * + * @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 User createUser(CreateUserRequest request) { + return createUserCallable().call(request); + } + + // AUTO-GENERATED DOCUMENTATION AND METHOD. + /** Sample code: */ + public final UnaryCallable createUserCallable() { + return stub.createUserCallable(); + } + + // AUTO-GENERATED DOCUMENTATION AND METHOD. + /** + * Sample code: + * + * @param name + * @throws com.google.api.gax.rpc.ApiException if the remote call fails + */ + public final User getUser(UserName name) { + GetUserRequest request = + GetUserRequest.newBuilder().setName(Objects.isNull(name) ? null : name.toString()).build(); + return getUser(request); + } + + // AUTO-GENERATED DOCUMENTATION AND METHOD. + /** + * Sample code: + * + * @param name + * @throws com.google.api.gax.rpc.ApiException if the remote call fails + */ + public final User getUser(String name) { + GetUserRequest request = GetUserRequest.newBuilder().setName(name).build(); + return getUser(request); + } + + // AUTO-GENERATED DOCUMENTATION AND METHOD. + /** + * Sample code: + * + * @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 User getUser(GetUserRequest request) { + return getUserCallable().call(request); + } + + // AUTO-GENERATED DOCUMENTATION AND METHOD. + /** Sample code: */ + public final UnaryCallable getUserCallable() { + return stub.getUserCallable(); + } + + // AUTO-GENERATED DOCUMENTATION AND METHOD. + /** + * Sample code: + * + * @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 User updateUser(UpdateUserRequest request) { + return updateUserCallable().call(request); + } + + // AUTO-GENERATED DOCUMENTATION AND METHOD. + /** Sample code: */ + public final UnaryCallable updateUserCallable() { + return stub.updateUserCallable(); + } + + // AUTO-GENERATED DOCUMENTATION AND METHOD. + /** + * Sample code: + * + * @param name + * @throws com.google.api.gax.rpc.ApiException if the remote call fails + */ + public final Empty deleteUser(UserName name) { + DeleteUserRequest request = + DeleteUserRequest.newBuilder() + .setName(Objects.isNull(name) ? null : name.toString()) + .build(); + return deleteUser(request); + } + + // AUTO-GENERATED DOCUMENTATION AND METHOD. + /** + * Sample code: + * + * @param name + * @throws com.google.api.gax.rpc.ApiException if the remote call fails + */ + public final Empty deleteUser(String name) { + DeleteUserRequest request = DeleteUserRequest.newBuilder().setName(name).build(); + return deleteUser(request); + } + + // AUTO-GENERATED DOCUMENTATION AND METHOD. + /** + * Sample code: + * + * @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 Empty deleteUser(DeleteUserRequest request) { + return deleteUserCallable().call(request); + } + + // AUTO-GENERATED DOCUMENTATION AND METHOD. + /** Sample code: */ + public final UnaryCallable deleteUserCallable() { + return stub.deleteUserCallable(); + } + + // AUTO-GENERATED DOCUMENTATION AND METHOD. + /** + * Sample code: + * + * @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 ListUsersPagedResponse listUsers(ListUsersRequest request) { + return listUsersPagedCallable().call(request); + } + + // AUTO-GENERATED DOCUMENTATION AND METHOD. + /** Sample code: */ + public final UnaryCallable listUsersPagedCallable() { + return stub.listUsersPagedCallable(); + } + + // AUTO-GENERATED DOCUMENTATION AND METHOD. + /** Sample code: */ + public final UnaryCallable listUsersCallable() { + return stub.listUsersCallable(); + } + + @Override + public final void close() { + stub.close(); + } + + @Override + public void shutdown() { + stub.shutdown(); + } + + @Override + public boolean isShutdown() { + return stub.isShutdown(); + } + + @Override + public boolean isTerminated() { + return stub.isTerminated(); + } + + @Override + public void shutdownNow() { + stub.shutdownNow(); + } + + @Override + public boolean awaitTermination(long duration, TimeUnit unit) throws InterruptedException { + return stub.awaitTermination(duration, unit); + } + + public static class ListUsersPagedResponse + extends AbstractPagedListResponse< + ListUsersRequest, ListUsersResponse, User, ListUsersPage, ListUsersFixedSizeCollection> { + + public static ApiFuture createAsync( + PageContext context, + ApiFuture futureResponse) { + ApiFuture futurePage = + ListUsersPage.createEmptyPage().createPageAsync(context, futureResponse); + return ApiFutures.transform( + futurePage, + new ApiFunction() { + @Override + public ListUsersPagedResponse apply(ListUsersPage input) { + return new ListUsersPagedResponse(input); + } + }, + MoreExecutors.directExecutor()); + } + + private ListUsersPagedResponse(ListUsersPage page) { + super(page, ListUsersFixedSizeCollection.createEmptyCollection()); + } + } + + public static class ListUsersPage + extends AbstractPage { + + private ListUsersPage( + PageContext context, + ListUsersResponse response) { + super(context, response); + } + + private static ListUsersPage createEmptyPage() { + return new ListUsersPage(null, null); + } + + @Override + protected ListUsersPage createPage( + PageContext context, + ListUsersResponse response) { + return new ListUsersPage(context, response); + } + + @Override + public ApiFuture createPageAsync( + PageContext context, + ApiFuture futureResponse) { + return super.createPageAsync(context, futureResponse); + } + } + + public static class ListUsersFixedSizeCollection + extends AbstractFixedSizeCollection< + ListUsersRequest, ListUsersResponse, User, ListUsersPage, ListUsersFixedSizeCollection> { + + private ListUsersFixedSizeCollection(List pages, int collectionSize) { + super(pages, collectionSize); + } + + private static ListUsersFixedSizeCollection createEmptyCollection() { + return new ListUsersFixedSizeCollection(null, 0); + } + + @Override + protected ListUsersFixedSizeCollection createCollection( + List pages, int collectionSize) { + return new ListUsersFixedSizeCollection(pages, collectionSize); + } + } +} diff --git a/src/test/java/com/google/api/generator/gapic/testdata/BUILD.bazel b/src/test/java/com/google/api/generator/gapic/testdata/BUILD.bazel index a846eaf638..9682b5c414 100644 --- a/src/test/java/com/google/api/generator/gapic/testdata/BUILD.bazel +++ b/src/test/java/com/google/api/generator/gapic/testdata/BUILD.bazel @@ -32,6 +32,7 @@ proto_library( name = "showcase_proto", srcs = [ "echo.proto", + "identity.proto", "testing.proto", ], deps = [ diff --git a/src/test/java/com/google/api/generator/gapic/testdata/identity.proto b/src/test/java/com/google/api/generator/gapic/testdata/identity.proto new file mode 100644 index 0000000000..5ed47e8c59 --- /dev/null +++ b/src/test/java/com/google/api/generator/gapic/testdata/identity.proto @@ -0,0 +1,192 @@ +// Copyright 2018 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +syntax = "proto3"; + +import "google/api/annotations.proto"; +import "google/api/client.proto"; +import "google/api/field_behavior.proto"; +import "google/api/resource.proto"; +import "google/protobuf/empty.proto"; +import "google/protobuf/field_mask.proto"; +import "google/protobuf/timestamp.proto"; + +package google.showcase.v1beta1; + +option go_package = "github.com/googleapis/gapic-showcase/server/genproto"; +option java_package = "com.google.showcase.v1beta1"; +option java_multiple_files = true; + +// A simple identity service. +service Identity { + // This service is meant to only run locally on the port 7469 (keypad digits + // for "show"). + option (google.api.default_host) = "localhost:7469"; + option (google.api.oauth_scopes) = + "https://www.googleapis.com/auth/cloud-platform"; + + // Creates a user. + rpc CreateUser(CreateUserRequest) returns (User) { + option (google.api.http) = { + post: "/v1beta1/{parent=users}" + body: "*" + }; + option (google.api.method_signature) = + "parent,user.display_name,user.email"; + option (google.api.method_signature) = + "parent,user.display_name,user.email,user.age,user.nickname,user.enable_notifications,user.height_feet"; + } + + // Retrieves the User with the given uri. + rpc GetUser(GetUserRequest) returns (User) { + option (google.api.http) = { + get: "/v1beta1/{name=users/*}" + }; + option (google.api.method_signature) = "name"; + } + + // Updates a user. + rpc UpdateUser(UpdateUserRequest) returns (User) { + option (google.api.http) = { + patch: "/v1beta1/{user.name=users/*}" + body: "*" + }; + } + + // Deletes a user, their profile, and all of their authored messages. + rpc DeleteUser(DeleteUserRequest) returns (google.protobuf.Empty) { + option (google.api.http) = { + delete: "/v1beta1/{name=users/*}" + }; + option (google.api.method_signature) = "name"; + } + + // Lists all users. + rpc ListUsers(ListUsersRequest) returns (ListUsersResponse) { + option (google.api.http) = { + get: "/v1beta1/users" + }; + } +} + +// A user. +message User { + option (google.api.resource) = { + type: "showcase.googleapis.com/User" + pattern: "users/{user}" + }; + + // The resource name of the user. + string name = 1; + + // The display_name of the user. + string display_name = 2 [(google.api.field_behavior) = REQUIRED]; + + // The email address of the user. + string email = 3 [(google.api.field_behavior) = REQUIRED]; + + // The timestamp at which the user was created. + google.protobuf.Timestamp create_time = 4 + [(google.api.field_behavior) = OUTPUT_ONLY]; + + // The latest timestamp at which the user was updated. + google.protobuf.Timestamp update_time = 5 + [(google.api.field_behavior) = OUTPUT_ONLY]; + + // The age of the use in years. + optional int32 age = 6; + + // The height of the user in feet. + optional double height_feet = 7; + + // The nickname of the user. + // + // (-- aip.dev/not-precedent: An empty string is a valid nickname. + // Ordinarily, proto3_optional should not be used on a `string` field. --) + optional string nickname = 8; + + // Enables the receiving of notifications. The default is true if unset. + // + // (-- aip.dev/not-precedent: The default for the feature is true. + // Ordinarily, the default for a `bool` field should be false. --) + optional bool enable_notifications = 9; +} + +// The request message for the google.showcase.v1beta1.Identity\CreateUser +// method. +message CreateUserRequest { + string parent = 1 [ + (google.api.resource_reference).child_type = "showcase.googleapis.com/User", + (google.api.field_behavior) = REQUIRED + ]; + // The user to create. + User user = 2 [(google.api.field_behavior) = REQUIRED]; +} + +// The request message for the google.showcase.v1beta1.Identity\GetUser +// method. +message GetUserRequest { + // The resource name of the requested user. + string name = 1 [ + (google.api.resource_reference).type = "showcase.googleapis.com/User", + (google.api.field_behavior) = REQUIRED + ]; +} + +// The request message for the google.showcase.v1beta1.Identity\UpdateUser +// method. +message UpdateUserRequest { + // The user to update. + User user = 1; + + // The field mask to determine wich fields are to be updated. If empty, the + // server will assume all fields are to be updated. + google.protobuf.FieldMask update_mask = 2; +} + +// The request message for the google.showcase.v1beta1.Identity\DeleteUser +// method. +message DeleteUserRequest { + // The resource name of the user to delete. + string name = 1 [ + (google.api.resource_reference).type = "showcase.googleapis.com/User", + (google.api.field_behavior) = REQUIRED + ]; +} + +// The request message for the google.showcase.v1beta1.Identity\ListUsers +// method. +message ListUsersRequest { + // The maximum number of users to return. Server may return fewer users + // than requested. If unspecified, server will pick an appropriate default. + int32 page_size = 1; + + // The value of google.showcase.v1beta1.ListUsersResponse.next_page_token + // returned from the previous call to + // `google.showcase.v1beta1.Identity\ListUsers` method. + string page_token = 2; +} + +// The response message for the google.showcase.v1beta1.Identity\ListUsers +// method. +message ListUsersResponse { + // The list of users. + repeated User users = 1; + + // A token to retrieve next page of results. + // Pass this value in ListUsersRequest.page_token field in the subsequent + // call to `google.showcase.v1beta1.Message\ListUsers` method to retrieve the + // next page of results. + string next_page_token = 2; +} From 5142602d50e64cc80bd353801cf958935a9f5722 Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Thu, 8 Oct 2020 11:26:00 -0700 Subject: [PATCH 2/5] feat: add EmptyLineStatement --- .../generator/engine/ast/AstNodeVisitor.java | 2 ++ .../engine/ast/EmptyLineStatement.java | 28 +++++++++++++++++++ .../engine/writer/ImportWriterVisitor.java | 14 +++++++--- .../engine/writer/JavaWriterVisitor.java | 6 ++++ .../writer/ImportWriterVisitorTest.java | 9 ++++++ .../engine/writer/JavaWriterVisitorTest.java | 8 ++++++ 6 files changed, 63 insertions(+), 4 deletions(-) create mode 100644 src/main/java/com/google/api/generator/engine/ast/EmptyLineStatement.java diff --git a/src/main/java/com/google/api/generator/engine/ast/AstNodeVisitor.java b/src/main/java/com/google/api/generator/engine/ast/AstNodeVisitor.java index 32b0642619..4f885f01d7 100644 --- a/src/main/java/com/google/api/generator/engine/ast/AstNodeVisitor.java +++ b/src/main/java/com/google/api/generator/engine/ast/AstNodeVisitor.java @@ -87,6 +87,8 @@ public interface AstNodeVisitor { public void visit(CommentStatement commentStatement); + public void visit(EmptyLineStatement emptyLineStatement); + /** =============================== OTHER =============================== */ public void visit(MethodDefinition methodDefinition); diff --git a/src/main/java/com/google/api/generator/engine/ast/EmptyLineStatement.java b/src/main/java/com/google/api/generator/engine/ast/EmptyLineStatement.java new file mode 100644 index 0000000000..f2688a2b9e --- /dev/null +++ b/src/main/java/com/google/api/generator/engine/ast/EmptyLineStatement.java @@ -0,0 +1,28 @@ +// Copyright 2020 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.api.generator.engine.ast; + +public class EmptyLineStatement implements Statement { + private EmptyLineStatement() {} + + @Override + public void accept(AstNodeVisitor visitor) { + visitor.visit(this); + } + + public static EmptyLineStatement create() { + return new EmptyLineStatement(); + } +} diff --git a/src/main/java/com/google/api/generator/engine/writer/ImportWriterVisitor.java b/src/main/java/com/google/api/generator/engine/writer/ImportWriterVisitor.java index fa2648a545..2a340c974b 100644 --- a/src/main/java/com/google/api/generator/engine/writer/ImportWriterVisitor.java +++ b/src/main/java/com/google/api/generator/engine/writer/ImportWriterVisitor.java @@ -25,6 +25,7 @@ import com.google.api.generator.engine.ast.CastExpr; import com.google.api.generator.engine.ast.ClassDefinition; import com.google.api.generator.engine.ast.CommentStatement; +import com.google.api.generator.engine.ast.EmptyLineStatement; import com.google.api.generator.engine.ast.EnumRefExpr; import com.google.api.generator.engine.ast.Expr; import com.google.api.generator.engine.ast.ExprStatement; @@ -316,23 +317,28 @@ public void visit(SynchronizedStatement synchronizedStatement) { @Override public void visit(CommentStatement commentStatement) { - // Do nothing + // Nothing to do. + } + + @Override + public void visit(EmptyLineStatement emptyLineStatement) { + // Nothing to do. } /** =============================== COMMENT =============================== */ @Override public void visit(LineComment lineComment) { - // Do nothing + // Nothing to do. } @Override public void visit(BlockComment blockComment) { - // Do nothing + // Nothing to do. } @Override public void visit(JavaDocComment javaDocComment) { - // Do nothing + // Nothing to do. } /** =============================== OTHER =============================== */ diff --git a/src/main/java/com/google/api/generator/engine/writer/JavaWriterVisitor.java b/src/main/java/com/google/api/generator/engine/writer/JavaWriterVisitor.java index 538d4a62b0..0ef76ee89e 100644 --- a/src/main/java/com/google/api/generator/engine/writer/JavaWriterVisitor.java +++ b/src/main/java/com/google/api/generator/engine/writer/JavaWriterVisitor.java @@ -25,6 +25,7 @@ import com.google.api.generator.engine.ast.CastExpr; import com.google.api.generator.engine.ast.ClassDefinition; import com.google.api.generator.engine.ast.CommentStatement; +import com.google.api.generator.engine.ast.EmptyLineStatement; import com.google.api.generator.engine.ast.EnumRefExpr; import com.google.api.generator.engine.ast.Expr; import com.google.api.generator.engine.ast.ExprStatement; @@ -628,6 +629,11 @@ public void visit(CommentStatement commentStatement) { commentStatement.comment().accept(this); } + @Override + public void visit(EmptyLineStatement emptyLineStatement) { + newline(); + } + /** =============================== COMMENT =============================== */ public void visit(LineComment lineComment) { // Split comments by new line and add `//` to each line. diff --git a/src/test/java/com/google/api/generator/engine/writer/ImportWriterVisitorTest.java b/src/test/java/com/google/api/generator/engine/writer/ImportWriterVisitorTest.java index 5a936b61b3..e9801aa6d6 100644 --- a/src/test/java/com/google/api/generator/engine/writer/ImportWriterVisitorTest.java +++ b/src/test/java/com/google/api/generator/engine/writer/ImportWriterVisitorTest.java @@ -14,6 +14,7 @@ package com.google.api.generator.engine.writer; +import static com.google.common.truth.Truth.assertThat; import static junit.framework.Assert.assertEquals; import static junit.framework.Assert.assertFalse; import static junit.framework.Assert.assertTrue; @@ -25,6 +26,7 @@ import com.google.api.generator.engine.ast.CastExpr; import com.google.api.generator.engine.ast.ClassDefinition; import com.google.api.generator.engine.ast.ConcreteReference; +import com.google.api.generator.engine.ast.EmptyLineStatement; import com.google.api.generator.engine.ast.EnumRefExpr; import com.google.api.generator.engine.ast.Expr; import com.google.api.generator.engine.ast.ExprStatement; @@ -1020,6 +1022,13 @@ public void writeLogicalOperationExprImports() { "import com.google.api.generator.engine.ast.UnaryOperationExpr;\n\n"); } + @Test + public void writeEmptyLineStatementImports() { + EmptyLineStatement statement = EmptyLineStatement.create(); + statement.accept(writerVisitor); + assertThat(writerVisitor.write()).isEmpty(); + } + private static TypeNode createType(Class clazz) { return TypeNode.withReference(ConcreteReference.withClazz(clazz)); } diff --git a/src/test/java/com/google/api/generator/engine/writer/JavaWriterVisitorTest.java b/src/test/java/com/google/api/generator/engine/writer/JavaWriterVisitorTest.java index bfa090c228..fcee9a12a0 100644 --- a/src/test/java/com/google/api/generator/engine/writer/JavaWriterVisitorTest.java +++ b/src/test/java/com/google/api/generator/engine/writer/JavaWriterVisitorTest.java @@ -28,6 +28,7 @@ import com.google.api.generator.engine.ast.ClassDefinition; import com.google.api.generator.engine.ast.CommentStatement; import com.google.api.generator.engine.ast.ConcreteReference; +import com.google.api.generator.engine.ast.EmptyLineStatement; import com.google.api.generator.engine.ast.EnumRefExpr; import com.google.api.generator.engine.ast.Expr; import com.google.api.generator.engine.ast.ExprStatement; @@ -2209,6 +2210,13 @@ public void writeAssignmentOperationExpr_xorAssignment() { assertThat(writerVisitor.write()).isEqualTo("h ^= Objects.hashCode(fixedValue)"); } + @Test + public void writeEmptyLineStatement() { + EmptyLineStatement statement = EmptyLineStatement.create(); + statement.accept(writerVisitor); + assertEquals(writerVisitor.write(), "\n"); + } + private static String createLines(int numLines) { return new String(new char[numLines]).replace("\0", "%s"); } From 88c25fcfc76504b2cca8ba0b3b4759011926ebf9 Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Thu, 8 Oct 2020 12:32:04 -0700 Subject: [PATCH 3/5] fix: space out lines in ServiceStubSettings --- .../generator/engine/ast/ClassDefinition.java | 1 + .../ServiceStubSettingsClassComposer.java | 264 ++++++++++-------- .../composer/goldens/EchoStubSettings.golden | 15 + .../LoggingServiceV2StubSettings.golden | 20 ++ .../goldens/PublisherStubSettings.golden | 24 ++ 5 files changed, 206 insertions(+), 118 deletions(-) diff --git a/src/main/java/com/google/api/generator/engine/ast/ClassDefinition.java b/src/main/java/com/google/api/generator/engine/ast/ClassDefinition.java index f5f9f0feb1..76c797c7d3 100644 --- a/src/main/java/com/google/api/generator/engine/ast/ClassDefinition.java +++ b/src/main/java/com/google/api/generator/engine/ast/ClassDefinition.java @@ -174,6 +174,7 @@ public ClassDefinition build() { for (Statement statement : classDef.statements()) { Preconditions.checkState( statement instanceof CommentStatement + || statement instanceof EmptyLineStatement || statement instanceof ExprStatement || statement instanceof BlockStatement, "Class statement type must be either an expression, block, or comment statement"); diff --git a/src/main/java/com/google/api/generator/gapic/composer/ServiceStubSettingsClassComposer.java b/src/main/java/com/google/api/generator/gapic/composer/ServiceStubSettingsClassComposer.java index fe8456bd00..06058813f5 100644 --- a/src/main/java/com/google/api/generator/gapic/composer/ServiceStubSettingsClassComposer.java +++ b/src/main/java/com/google/api/generator/gapic/composer/ServiceStubSettingsClassComposer.java @@ -58,6 +58,7 @@ import com.google.api.generator.engine.ast.ClassDefinition; import com.google.api.generator.engine.ast.CommentStatement; import com.google.api.generator.engine.ast.ConcreteReference; +import com.google.api.generator.engine.ast.EmptyLineStatement; import com.google.api.generator.engine.ast.Expr; import com.google.api.generator.engine.ast.ExprStatement; import com.google.api.generator.engine.ast.IfStatement; @@ -113,6 +114,8 @@ // TODO(miraleung): Refactor ClassComposer's interface. public class ServiceStubSettingsClassComposer { + private static final Statement EMPTY_LINE_STATEMENT = EmptyLineStatement.create(); + private static final String BATCHING_DESC_PATTERN = "%s_BATCHING_DESC"; private static final String CLASS_NAME_PATTERN = "%sStubSettings"; private static final String GRPC_SERVICE_STUB_PATTERN = "Grpc%sStub"; @@ -301,11 +304,13 @@ private static List createClassStatements( .setIsFinal(true) .build())) .collect(Collectors.toList())); + statements.add(EMPTY_LINE_STATEMENT); - statements.addAll( - createPagingStaticAssignExprs(service, serviceConfig, messageTypes, types).stream() - .map(e -> exprToStatementFn.apply(e)) - .collect(Collectors.toList())); + for (Expr pagingAssignExpr : + createPagingStaticAssignExprs(service, serviceConfig, messageTypes, types)) { + statements.add(exprToStatementFn.apply(pagingAssignExpr)); + statements.add(EMPTY_LINE_STATEMENT); + } for (Method method : service.methods()) { Optional batchingSettingOpt = @@ -318,6 +323,7 @@ private static List createClassStatements( BatchingDescriptorComposer.createBatchingDescriptorFieldDeclExpr( method, batchingSettingOpt.get(), messageTypes))); } + statements.add(EMPTY_LINE_STATEMENT); } return statements; @@ -328,7 +334,6 @@ private static List createPagingStaticAssignExprs( GapicServiceConfig serviceConfig, Map messageTypes, Map types) { - // TODO(miraleung): Add a test case for several such statements. List descExprs = new ArrayList<>(); List factoryExprs = new ArrayList<>(); for (Method method : service.methods()) { @@ -1108,8 +1113,9 @@ private static MethodDefinition createClassConstructor( .setArguments(settingsBuilderVarExpr) .build(); - List bodyExprs = new ArrayList<>(); - bodyExprs.add(superCtorExpr); + List bodyStatements = new ArrayList<>(); + bodyStatements.add(ExprStatement.withExpr(superCtorExpr)); + bodyStatements.add(EMPTY_LINE_STATEMENT); Function, AssignmentExpr> varInitExprFn = e -> @@ -1126,9 +1132,9 @@ private static MethodDefinition createClassConstructor( .setReturnType(e.getValue().type()) .build()) .build(); - bodyExprs.addAll( + bodyStatements.addAll( methodSettingsMemberVarExprs.entrySet().stream() - .map(e -> varInitExprFn.apply(e)) + .map(e -> ExprStatement.withExpr(varInitExprFn.apply(e))) .collect(Collectors.toList())); return MethodDefinition.constructorBuilder() @@ -1136,8 +1142,7 @@ private static MethodDefinition createClassConstructor( .setReturnType(thisType) .setArguments(settingsBuilderVarExpr.toBuilder().setIsDecl(true).build()) .setThrowsExceptions(Arrays.asList(TypeNode.withExceptionClazz(IOException.class))) - .setBody( - bodyExprs.stream().map(e -> ExprStatement.withExpr(e)).collect(Collectors.toList())) + .setBody(bodyStatements) .build(); } @@ -1258,7 +1263,7 @@ private static MethodDefinition createNestedClassInitDefaultsMethod( VariableExpr.withVariable( Variable.builder().setType(builderType).setName("builder").build()); - List bodyExprs = new ArrayList<>(); + List bodyStatements = new ArrayList<>(); // Iterate through methods twice to so we can have LRO expressions appear last. for (Method method : service.methods()) { Method.Stream streamKind = method.stream(); @@ -1275,32 +1280,38 @@ private static MethodDefinition createNestedClassInitDefaultsMethod( service.name(), method.name())); String settingsGetterMethodName = String.format("%sSettings", JavaStyle.toLowerCamelCase(method.name())); - bodyExprs.add( - RetrySettingsComposer.createBatchingBuilderSettingsExpr( - settingsGetterMethodName, batchingSettingOpt.get(), builderVarExpr)); + bodyStatements.add( + ExprStatement.withExpr( + RetrySettingsComposer.createBatchingBuilderSettingsExpr( + settingsGetterMethodName, batchingSettingOpt.get(), builderVarExpr))); + bodyStatements.add(EMPTY_LINE_STATEMENT); } - bodyExprs.add( - RetrySettingsComposer.createSimpleBuilderSettingsExpr( - service, - serviceConfig, - method, - builderVarExpr, - NESTED_RETRYABLE_CODE_DEFINITIONS_VAR_EXPR, - NESTED_RETRY_PARAM_DEFINITIONS_VAR_EXPR)); + bodyStatements.add( + ExprStatement.withExpr( + RetrySettingsComposer.createSimpleBuilderSettingsExpr( + service, + serviceConfig, + method, + builderVarExpr, + NESTED_RETRYABLE_CODE_DEFINITIONS_VAR_EXPR, + NESTED_RETRY_PARAM_DEFINITIONS_VAR_EXPR))); + bodyStatements.add(EMPTY_LINE_STATEMENT); } for (Method method : service.methods()) { if (!method.hasLro()) { continue; } - bodyExprs.add( - RetrySettingsComposer.createLroSettingsBuilderExpr( - service, - serviceConfig, - method, - builderVarExpr, - NESTED_RETRYABLE_CODE_DEFINITIONS_VAR_EXPR, - NESTED_RETRY_PARAM_DEFINITIONS_VAR_EXPR)); + bodyStatements.add( + ExprStatement.withExpr( + RetrySettingsComposer.createLroSettingsBuilderExpr( + service, + serviceConfig, + method, + builderVarExpr, + NESTED_RETRYABLE_CODE_DEFINITIONS_VAR_EXPR, + NESTED_RETRY_PARAM_DEFINITIONS_VAR_EXPR))); + bodyStatements.add(EMPTY_LINE_STATEMENT); } return MethodDefinition.builder() @@ -1309,8 +1320,7 @@ private static MethodDefinition createNestedClassInitDefaultsMethod( .setReturnType(builderType) .setName("initDefaults") .setArguments(builderVarExpr.toBuilder().setIsDecl(true).build()) - .setBody( - bodyExprs.stream().map(e -> ExprStatement.withExpr(e)).collect(Collectors.toList())) + .setBody(bodyStatements) .setReturnExpr(builderVarExpr) .build(); } @@ -1372,13 +1382,16 @@ private static List createNestedClassConstructorMethods( .setName(t.reference().enclosingClassName()) .setPakkage(t.reference().pakkage()) .build()); - List ctorBodyExprs = new ArrayList<>(); - ctorBodyExprs.add( - ReferenceConstructorExpr.superBuilder() - .setType(builderType) - .setArguments(clientContextVarExpr) - .build()); - ctorBodyExprs.addAll( + List ctorBodyStatements = new ArrayList<>(); + ctorBodyStatements.add( + ExprStatement.withExpr( + ReferenceConstructorExpr.superBuilder() + .setType(builderType) + .setArguments(clientContextVarExpr) + .build())); + ctorBodyStatements.add(EMPTY_LINE_STATEMENT); + + ctorBodyStatements.addAll( nestedMethodSettingsMemberVarExprs.entrySet().stream() .map( e -> { @@ -1394,19 +1407,21 @@ private static List createNestedClassConstructorMethods( if (!isPagedCallSettingsBuilderFn.apply(varType)) { if (!isBatchingCallSettingsBuilderFn.apply(varType)) { boolean isUnaryCallSettings = isUnaryCallSettingsBuilderFn.apply(varType); - return AssignmentExpr.builder() - .setVariableExpr(varExpr) - .setValueExpr( - MethodInvocationExpr.builder() - .setStaticReferenceType( - builderToCallSettingsFn.apply(varExpr.type())) - .setMethodName( - isUnaryCallSettings - ? "newUnaryCallSettingsBuilder" - : "newBuilder") - .setReturnType(varExpr.type()) - .build()) - .build(); + Expr builderExpr = + AssignmentExpr.builder() + .setVariableExpr(varExpr) + .setValueExpr( + MethodInvocationExpr.builder() + .setStaticReferenceType( + builderToCallSettingsFn.apply(varExpr.type())) + .setMethodName( + isUnaryCallSettings + ? "newUnaryCallSettingsBuilder" + : "newBuilder") + .setReturnType(varExpr.type()) + .build()) + .build(); + return ExprStatement.withExpr(builderExpr); } Expr newBatchingSettingsExpr = MethodInvocationExpr.builder() @@ -1441,10 +1456,12 @@ private static List createNestedClassConstructorMethods( .setReturnType(varType) .build(); - return AssignmentExpr.builder() - .setVariableExpr(varExpr) - .setValueExpr(batchingSettingsBuilderExpr) - .build(); + Expr builderExpr = + AssignmentExpr.builder() + .setVariableExpr(varExpr) + .setValueExpr(batchingSettingsBuilderExpr) + .build(); + return ExprStatement.withExpr(builderExpr); } String memberVarName = String.format( @@ -1455,16 +1472,19 @@ private static List createNestedClassConstructorMethods( .setType(STATIC_TYPES.get("PagedListResponseFactory")) .setName(memberVarName) .build()); - return AssignmentExpr.builder() - .setVariableExpr(varExpr) - .setValueExpr( - MethodInvocationExpr.builder() - .setStaticReferenceType(builderToCallSettingsFn.apply(varExpr.type())) - .setMethodName("newBuilder") - .setArguments(argVar) - .setReturnType(varExpr.type()) - .build()) - .build(); + Expr builderExpr = + AssignmentExpr.builder() + .setVariableExpr(varExpr) + .setValueExpr( + MethodInvocationExpr.builder() + .setStaticReferenceType( + builderToCallSettingsFn.apply(varExpr.type())) + .setMethodName("newBuilder") + .setArguments(argVar) + .setReturnType(varExpr.type()) + .build()) + .build(); + return ExprStatement.withExpr(builderExpr); }) .collect(Collectors.toList())); @@ -1491,23 +1511,23 @@ private static List createNestedClassConstructorMethods( .setReturnType(NESTED_UNARY_METHOD_SETTINGS_BUILDERS_VAR_EXPR.type()) .build()) .build(); - ctorBodyExprs.add(unaryMethodSettingsBuildersAssignExpr); + ctorBodyStatements.add(EMPTY_LINE_STATEMENT); - ctorBodyExprs.add( - MethodInvocationExpr.builder() - .setMethodName("initDefaults") - .setArguments(ValueExpr.withValue(ThisObjectValue.withType(builderType))) - .build()); + ctorBodyStatements.add(ExprStatement.withExpr(unaryMethodSettingsBuildersAssignExpr)); + + ctorBodyStatements.add( + ExprStatement.withExpr( + MethodInvocationExpr.builder() + .setMethodName("initDefaults") + .setArguments(ValueExpr.withValue(ThisObjectValue.withType(builderType))) + .build())); ctorMethods.add( MethodDefinition.constructorBuilder() .setScope(ScopeNode.PROTECTED) .setReturnType(builderType) .setArguments(clientContextVarExpr.toBuilder().setIsDecl(true).build()) - .setBody( - ctorBodyExprs.stream() - .map(e -> ExprStatement.withExpr(e)) - .collect(Collectors.toList())) + .setBody(ctorBodyStatements) .build()); // Third constructor that takes a ServivceStubSettings. @@ -1515,43 +1535,46 @@ private static List createNestedClassConstructorMethods( VariableExpr settingsVarExpr = VariableExpr.withVariable( Variable.builder().setType(outerSettingsType).setName("settings").build()); - ctorBodyExprs = new ArrayList<>(); - ctorBodyExprs.add( - ReferenceConstructorExpr.superBuilder() - .setType(builderType) - .setArguments(settingsVarExpr) - .build()); + ctorBodyStatements = new ArrayList<>(); + ctorBodyStatements.add( + ExprStatement.withExpr( + ReferenceConstructorExpr.superBuilder() + .setType(builderType) + .setArguments(settingsVarExpr) + .build())); + ctorBodyStatements.add(EMPTY_LINE_STATEMENT); + // TODO(cleanup): Technically this should actually use the outer class's Settings // members to avoid decoupling variable names. - ctorBodyExprs.addAll( + ctorBodyStatements.addAll( nestedMethodSettingsMemberVarExprs.values().stream() .map( v -> - AssignmentExpr.builder() - .setVariableExpr(v) - .setValueExpr( - MethodInvocationExpr.builder() - .setExprReferenceExpr( - VariableExpr.builder() - .setExprReferenceExpr(settingsVarExpr) - .setVariable(v.variable()) - .build()) - .setMethodName("toBuilder") - .setReturnType(v.type()) - .build()) - .build()) + ExprStatement.withExpr( + AssignmentExpr.builder() + .setVariableExpr(v) + .setValueExpr( + MethodInvocationExpr.builder() + .setExprReferenceExpr( + VariableExpr.builder() + .setExprReferenceExpr(settingsVarExpr) + .setVariable(v.variable()) + .build()) + .setMethodName("toBuilder") + .setReturnType(v.type()) + .build()) + .build())) .collect(Collectors.toList())); - ctorBodyExprs.add(unaryMethodSettingsBuildersAssignExpr); + ctorBodyStatements.add(EMPTY_LINE_STATEMENT); + + ctorBodyStatements.add(ExprStatement.withExpr(unaryMethodSettingsBuildersAssignExpr)); ctorMethods.add( MethodDefinition.constructorBuilder() .setScope(ScopeNode.PROTECTED) .setReturnType(builderType) .setArguments(settingsVarExpr.toBuilder().setIsDecl(true).build()) - .setBody( - ctorBodyExprs.stream() - .map(e -> ExprStatement.withExpr(e)) - .collect(Collectors.toList())) + .setBody(ctorBodyStatements) .build()); return ctorMethods; @@ -1559,27 +1582,30 @@ private static List createNestedClassConstructorMethods( private static MethodDefinition createNestedClassCreateDefaultMethod( Map types) { - List bodyExprs = new ArrayList<>(); + List bodyStatements = new ArrayList<>(); // Initialize the builder: Builder builder = new Builder((ClientContext) null); TypeNode builderType = types.get(NESTED_BUILDER_CLASS_NAME); VariableExpr builderVarExpr = VariableExpr.withVariable( Variable.builder().setType(builderType).setName("builder").build()); - bodyExprs.add( - AssignmentExpr.builder() - .setVariableExpr(builderVarExpr.toBuilder().setIsDecl(true).build()) - .setValueExpr( - NewObjectExpr.builder() - .setType(builderType) - .setArguments( - CastExpr.builder() - .setType(STATIC_TYPES.get("ClientContext")) - .setExpr(ValueExpr.withValue(NullObjectValue.create())) - .build()) - .build()) - .build()); + bodyStatements.add( + ExprStatement.withExpr( + AssignmentExpr.builder() + .setVariableExpr(builderVarExpr.toBuilder().setIsDecl(true).build()) + .setValueExpr( + NewObjectExpr.builder() + .setType(builderType) + .setArguments( + CastExpr.builder() + .setType(STATIC_TYPES.get("ClientContext")) + .setExpr(ValueExpr.withValue(NullObjectValue.create())) + .build()) + .build()) + .build())); + bodyStatements.add(EMPTY_LINE_STATEMENT); + List bodyExprs = new ArrayList<>(); bodyExprs.add( MethodInvocationExpr.builder() .setExprReferenceExpr(builderVarExpr) @@ -1625,6 +1651,9 @@ private static MethodDefinition createNestedClassCreateDefaultMethod( .setArguments( MethodInvocationExpr.builder().setMethodName("getDefaultEndpoint").build()) .build()); + bodyStatements.addAll( + bodyExprs.stream().map(e -> ExprStatement.withExpr(e)).collect(Collectors.toList())); + bodyStatements.add(EMPTY_LINE_STATEMENT); Expr returnExpr = MethodInvocationExpr.builder() @@ -1638,8 +1667,7 @@ private static MethodDefinition createNestedClassCreateDefaultMethod( .setIsStatic(true) .setReturnType(builderType) .setName("createDefault") - .setBody( - bodyExprs.stream().map(e -> ExprStatement.withExpr(e)).collect(Collectors.toList())) + .setBody(bodyStatements) .setReturnExpr(returnExpr) .build(); } diff --git a/src/test/java/com/google/api/generator/gapic/composer/goldens/EchoStubSettings.golden b/src/test/java/com/google/api/generator/gapic/composer/goldens/EchoStubSettings.golden index d55d7a9f3f..0895544aba 100644 --- a/src/test/java/com/google/api/generator/gapic/composer/goldens/EchoStubSettings.golden +++ b/src/test/java/com/google/api/generator/gapic/composer/goldens/EchoStubSettings.golden @@ -86,6 +86,7 @@ public class EchoStubSettings extends StubSettings { private final OperationCallSettings waitOperationSettings; private final UnaryCallSettings blockSettings; + private static final PagedListDescriptor PAGED_EXPAND_PAGE_STR_DESC = new PagedListDescriptor() { @@ -121,6 +122,7 @@ public class EchoStubSettings extends StubSettings { : payload.getResponsesList(); } }; + private static final PagedListResponseFactory< PagedExpandRequest, PagedExpandResponse, PagedExpandPagedResponse> PAGED_EXPAND_PAGE_STR_FACT = @@ -251,6 +253,7 @@ public class EchoStubSettings extends StubSettings { protected EchoStubSettings(Builder settingsBuilder) throws IOException { super(settingsBuilder); + echoSettings = settingsBuilder.echoSettings().build(); expandSettings = settingsBuilder.expandSettings().build(); collectSettings = settingsBuilder.collectSettings().build(); @@ -326,6 +329,7 @@ public class EchoStubSettings extends StubSettings { protected Builder(ClientContext clientContext) { super(clientContext); + echoSettings = UnaryCallSettings.newUnaryCallSettingsBuilder(); expandSettings = ServerStreamingCallSettings.newBuilder(); collectSettings = StreamingCallSettings.newBuilder(); @@ -335,6 +339,7 @@ public class EchoStubSettings extends StubSettings { waitSettings = UnaryCallSettings.newUnaryCallSettingsBuilder(); waitOperationSettings = OperationCallSettings.newBuilder(); blockSettings = UnaryCallSettings.newUnaryCallSettingsBuilder(); + unaryMethodSettingsBuilders = ImmutableList.>of( echoSettings, pagedExpandSettings, waitSettings, blockSettings); @@ -343,6 +348,7 @@ public class EchoStubSettings extends StubSettings { protected Builder(EchoStubSettings settings) { super(settings); + echoSettings = settings.echoSettings.toBuilder(); expandSettings = settings.expandSettings.toBuilder(); collectSettings = settings.collectSettings.toBuilder(); @@ -352,6 +358,7 @@ public class EchoStubSettings extends StubSettings { waitSettings = settings.waitSettings.toBuilder(); waitOperationSettings = settings.waitOperationSettings.toBuilder(); blockSettings = settings.blockSettings.toBuilder(); + unaryMethodSettingsBuilders = ImmutableList.>of( echoSettings, pagedExpandSettings, waitSettings, blockSettings); @@ -359,10 +366,12 @@ public class EchoStubSettings extends StubSettings { private static Builder createDefault() { Builder builder = new Builder(((ClientContext) null)); + builder.setTransportChannelProvider(defaultTransportChannelProvider()); builder.setCredentialsProvider(defaultCredentialsProviderBuilder().build()); builder.setInternalHeaderProvider(defaultApiClientHeaderProviderBuilder().build()); builder.setEndpoint(getDefaultEndpoint()); + return initDefaults(builder); } @@ -371,22 +380,27 @@ public class EchoStubSettings extends StubSettings { .echoSettings() .setRetryableCodes(RETRYABLE_CODE_DEFINITIONS.get("retry_policy_1_codes")) .setRetrySettings(RETRY_PARAM_DEFINITIONS.get("retry_policy_1_params")); + builder .expandSettings() .setRetryableCodes(RETRYABLE_CODE_DEFINITIONS.get("retry_policy_1_codes")) .setRetrySettings(RETRY_PARAM_DEFINITIONS.get("retry_policy_1_params")); + builder .pagedExpandSettings() .setRetryableCodes(RETRYABLE_CODE_DEFINITIONS.get("retry_policy_1_codes")) .setRetrySettings(RETRY_PARAM_DEFINITIONS.get("retry_policy_1_params")); + builder .waitSettings() .setRetryableCodes(RETRYABLE_CODE_DEFINITIONS.get("no_retry_0_codes")) .setRetrySettings(RETRY_PARAM_DEFINITIONS.get("no_retry_0_params")); + builder .blockSettings() .setRetryableCodes(RETRYABLE_CODE_DEFINITIONS.get("no_retry_0_codes")) .setRetrySettings(RETRY_PARAM_DEFINITIONS.get("no_retry_0_params")); + builder .waitOperationSettings() .setInitialCallSettings( @@ -409,6 +423,7 @@ public class EchoStubSettings extends StubSettings { .setMaxRpcTimeout(Duration.ZERO) .setTotalTimeout(Duration.ofMillis(300000L)) .build())); + return builder; } diff --git a/src/test/java/com/google/api/generator/gapic/composer/goldens/LoggingServiceV2StubSettings.golden b/src/test/java/com/google/api/generator/gapic/composer/goldens/LoggingServiceV2StubSettings.golden index 9b34b02c44..29dc2b3ac5 100644 --- a/src/test/java/com/google/api/generator/gapic/composer/goldens/LoggingServiceV2StubSettings.golden +++ b/src/test/java/com/google/api/generator/gapic/composer/goldens/LoggingServiceV2StubSettings.golden @@ -100,6 +100,7 @@ public class LoggingServiceV2StubSettings extends StubSettings listLogsSettings; + private static final PagedListDescriptor LIST_LOG_ENTRIES_PAGE_STR_DESC = new PagedListDescriptor() { @@ -136,6 +137,7 @@ public class LoggingServiceV2StubSettings extends StubSettings LIST_LOGS_PAGE_STR_DESC = new PagedListDescriptor() { @@ -219,6 +222,7 @@ public class LoggingServiceV2StubSettings extends StubSettings LIST_LOG_ENTRIES_PAGE_STR_FACT = @@ -235,6 +239,7 @@ public class LoggingServiceV2StubSettings extends StubSettings LIST_LOGS_PAGE_STR_FACT = @@ -282,6 +288,7 @@ public class LoggingServiceV2StubSettings extends StubSettings WRITE_LOG_ENTRIES_BATCHING_DESC = new BatchingDescriptor() { @@ -443,6 +450,7 @@ public class LoggingServiceV2StubSettings extends StubSettings>of( deleteLogSettings, @@ -529,12 +539,14 @@ public class LoggingServiceV2StubSettings extends StubSettings>of( deleteLogSettings, @@ -546,10 +558,12 @@ public class LoggingServiceV2StubSettings extends StubSettings { private final UnaryCallSettings deleteTopicSettings; private final UnaryCallSettings detachSubscriptionSettings; + private static final PagedListDescriptor LIST_TOPICS_PAGE_STR_DESC = new PagedListDescriptor() { @@ -140,6 +141,7 @@ public class PublisherStubSettings extends StubSettings { : payload.getTopicsList(); } }; + private static final PagedListDescriptor< ListTopicSubscriptionsRequest, ListTopicSubscriptionsResponse, String> LIST_TOPIC_SUBSCRIPTIONS_PAGE_STR_DESC = @@ -181,6 +183,7 @@ public class PublisherStubSettings extends StubSettings { : payload.getSubscriptionsList(); } }; + private static final PagedListDescriptor< ListTopicSnapshotsRequest, ListTopicSnapshotsResponse, String> LIST_TOPIC_SNAPSHOTS_PAGE_STR_DESC = @@ -219,6 +222,7 @@ public class PublisherStubSettings extends StubSettings { : payload.getSnapshotsList(); } }; + private static final PagedListResponseFactory< ListTopicsRequest, ListTopicsResponse, ListTopicsPagedResponse> LIST_TOPICS_PAGE_STR_FACT = @@ -235,6 +239,7 @@ public class PublisherStubSettings extends StubSettings { return ListTopicsPagedResponse.createAsync(pageContext, futureResponse); } }; + private static final PagedListResponseFactory< ListTopicSubscriptionsRequest, ListTopicSubscriptionsResponse, @@ -258,6 +263,7 @@ public class PublisherStubSettings extends StubSettings { return ListTopicSubscriptionsPagedResponse.createAsync(pageContext, futureResponse); } }; + private static final PagedListResponseFactory< ListTopicSnapshotsRequest, ListTopicSnapshotsResponse, ListTopicSnapshotsPagedResponse> LIST_TOPIC_SNAPSHOTS_PAGE_STR_FACT = @@ -278,6 +284,7 @@ public class PublisherStubSettings extends StubSettings { return ListTopicSnapshotsPagedResponse.createAsync(pageContext, futureResponse); } }; + private static final BatchingDescriptor PUBLISH_BATCHING_DESC = new BatchingDescriptor() { @Override @@ -463,6 +470,7 @@ public class PublisherStubSettings extends StubSettings { protected PublisherStubSettings(Builder settingsBuilder) throws IOException { super(settingsBuilder); + createTopicSettings = settingsBuilder.createTopicSettings().build(); updateTopicSettings = settingsBuilder.updateTopicSettings().build(); publishSettings = settingsBuilder.publishSettings().build(); @@ -570,6 +578,7 @@ public class PublisherStubSettings extends StubSettings { protected Builder(ClientContext clientContext) { super(clientContext); + createTopicSettings = UnaryCallSettings.newUnaryCallSettingsBuilder(); updateTopicSettings = UnaryCallSettings.newUnaryCallSettingsBuilder(); publishSettings = @@ -582,6 +591,7 @@ public class PublisherStubSettings extends StubSettings { listTopicSnapshotsSettings = PagedCallSettings.newBuilder(LIST_TOPIC_SNAPSHOTS_PAGE_STR_FACT); deleteTopicSettings = UnaryCallSettings.newUnaryCallSettingsBuilder(); detachSubscriptionSettings = UnaryCallSettings.newUnaryCallSettingsBuilder(); + unaryMethodSettingsBuilders = ImmutableList.>of( createTopicSettings, @@ -598,6 +608,7 @@ public class PublisherStubSettings extends StubSettings { protected Builder(PublisherStubSettings settings) { super(settings); + createTopicSettings = settings.createTopicSettings.toBuilder(); updateTopicSettings = settings.updateTopicSettings.toBuilder(); publishSettings = settings.publishSettings.toBuilder(); @@ -607,6 +618,7 @@ public class PublisherStubSettings extends StubSettings { listTopicSnapshotsSettings = settings.listTopicSnapshotsSettings.toBuilder(); deleteTopicSettings = settings.deleteTopicSettings.toBuilder(); detachSubscriptionSettings = settings.detachSubscriptionSettings.toBuilder(); + unaryMethodSettingsBuilders = ImmutableList.>of( createTopicSettings, @@ -622,10 +634,12 @@ public class PublisherStubSettings extends StubSettings { private static Builder createDefault() { Builder builder = new Builder(((ClientContext) null)); + builder.setTransportChannelProvider(defaultTransportChannelProvider()); builder.setCredentialsProvider(defaultCredentialsProviderBuilder().build()); builder.setInternalHeaderProvider(defaultApiClientHeaderProviderBuilder().build()); builder.setEndpoint(getDefaultEndpoint()); + return initDefaults(builder); } @@ -634,10 +648,12 @@ public class PublisherStubSettings extends StubSettings { .createTopicSettings() .setRetryableCodes(RETRYABLE_CODE_DEFINITIONS.get("retry_policy_0_codes")) .setRetrySettings(RETRY_PARAM_DEFINITIONS.get("retry_policy_0_params")); + builder .updateTopicSettings() .setRetryableCodes(RETRYABLE_CODE_DEFINITIONS.get("retry_policy_0_codes")) .setRetrySettings(RETRY_PARAM_DEFINITIONS.get("retry_policy_0_params")); + builder .publishSettings() .setBatchingSettings( @@ -650,34 +666,42 @@ public class PublisherStubSettings extends StubSettings { .setLimitExceededBehavior(FlowController.LimitExceededBehavior.Ignore) .build()) .build()); + builder .publishSettings() .setRetryableCodes(RETRYABLE_CODE_DEFINITIONS.get("retry_policy_1_codes")) .setRetrySettings(RETRY_PARAM_DEFINITIONS.get("retry_policy_1_params")); + builder .getTopicSettings() .setRetryableCodes(RETRYABLE_CODE_DEFINITIONS.get("retry_policy_2_codes")) .setRetrySettings(RETRY_PARAM_DEFINITIONS.get("retry_policy_2_params")); + builder .listTopicsSettings() .setRetryableCodes(RETRYABLE_CODE_DEFINITIONS.get("retry_policy_2_codes")) .setRetrySettings(RETRY_PARAM_DEFINITIONS.get("retry_policy_2_params")); + builder .listTopicSubscriptionsSettings() .setRetryableCodes(RETRYABLE_CODE_DEFINITIONS.get("retry_policy_2_codes")) .setRetrySettings(RETRY_PARAM_DEFINITIONS.get("retry_policy_2_params")); + builder .listTopicSnapshotsSettings() .setRetryableCodes(RETRYABLE_CODE_DEFINITIONS.get("retry_policy_2_codes")) .setRetrySettings(RETRY_PARAM_DEFINITIONS.get("retry_policy_2_params")); + builder .deleteTopicSettings() .setRetryableCodes(RETRYABLE_CODE_DEFINITIONS.get("retry_policy_0_codes")) .setRetrySettings(RETRY_PARAM_DEFINITIONS.get("retry_policy_0_params")); + builder .detachSubscriptionSettings() .setRetryableCodes(RETRYABLE_CODE_DEFINITIONS.get("retry_policy_0_codes")) .setRetrySettings(RETRY_PARAM_DEFINITIONS.get("retry_policy_0_params")); + return builder; } From 88b15e55082bbf8cde4c548203110dcfc7f0c9e1 Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Thu, 8 Oct 2020 13:22:27 -0700 Subject: [PATCH 4/5] fix: space out codegen in GrpcServiceStub --- .../GrpcServiceStubClassComposer.java | 53 ++++++++++++++++--- .../composer/goldens/GrpcEchoStub.golden | 12 +++++ 2 files changed, 57 insertions(+), 8 deletions(-) diff --git a/src/main/java/com/google/api/generator/gapic/composer/GrpcServiceStubClassComposer.java b/src/main/java/com/google/api/generator/gapic/composer/GrpcServiceStubClassComposer.java index 30b05204f9..73f0780209 100644 --- a/src/main/java/com/google/api/generator/gapic/composer/GrpcServiceStubClassComposer.java +++ b/src/main/java/com/google/api/generator/gapic/composer/GrpcServiceStubClassComposer.java @@ -28,6 +28,7 @@ import com.google.api.generator.engine.ast.AssignmentExpr; import com.google.api.generator.engine.ast.ClassDefinition; import com.google.api.generator.engine.ast.ConcreteReference; +import com.google.api.generator.engine.ast.EmptyLineStatement; import com.google.api.generator.engine.ast.EnumRefExpr; import com.google.api.generator.engine.ast.Expr; import com.google.api.generator.engine.ast.ExprStatement; @@ -68,6 +69,8 @@ import javax.annotation.Generated; public class GrpcServiceStubClassComposer implements ClassComposer { + private static final Statement EMPTY_LINE_STATEMENT = EmptyLineStatement.create(); + private static final String CLASS_NAME_PATTERN = "Grpc%sStub"; private static final String GRPC_SERVICE_CALLABLE_FACTORY_PATTERN = "Grpc%sCallableFactory"; private static final String METHOD_DESCRIPTOR_NAME_PATTERN = "%sMethodDescriptor"; @@ -131,11 +134,12 @@ public GapicClass generate(Service service, Map ignore) { .setType(staticTypes.get("GrpcStubCallableFactory")) .build())); - List classStatements = new ArrayList<>(); - classStatements.addAll( - createMethodDescriptorVariableDecls(service, protoMethodNameToDescriptorVarExprs)); - classStatements.addAll(createClassMemberFieldDeclarations(callableClassMemberVarExprs)); - classStatements.addAll(createClassMemberFieldDeclarations(classMemberVarExprs)); + List classStatements = + createClassStatements( + service, + protoMethodNameToDescriptorVarExprs, + callableClassMemberVarExprs, + classMemberVarExprs); ClassDefinition classDef = ClassDefinition.builder() @@ -158,6 +162,25 @@ public GapicClass generate(Service service, Map ignore) { return GapicClass.create(kind, classDef); } + private static List createClassStatements( + Service service, + Map protoMethodNameToDescriptorVarExprs, + Map callableClassMemberVarExprs, + Map classMemberVarExprs) { + List classStatements = new ArrayList<>(); + for (Statement statement : + createMethodDescriptorVariableDecls(service, protoMethodNameToDescriptorVarExprs)) { + classStatements.add(statement); + classStatements.add(EMPTY_LINE_STATEMENT); + } + + classStatements.addAll(createClassMemberFieldDeclarations(callableClassMemberVarExprs)); + classStatements.add(EMPTY_LINE_STATEMENT); + + classStatements.addAll(createClassMemberFieldDeclarations(classMemberVarExprs)); + return classStatements; + } + private static List createMethodDescriptorVariableDecls( Service service, Map protoMethodNameToDescriptorVarExprs) { return service.methods().stream() @@ -520,6 +543,7 @@ private static List createConstructorMethods( Expr thisExpr = ValueExpr.withValue(ThisObjectValue.withType(types.get(getThisClassName(service.name())))); // Body of the second constructor method. + List secondCtorStatements = new ArrayList<>(); List secondCtorExprs = new ArrayList<>(); secondCtorExprs.add( AssignmentExpr.builder() @@ -544,6 +568,10 @@ private static List createConstructorMethods( .setReturnType(operationsStubClassVarExpr.type()) .build()) .build()); + secondCtorStatements.addAll( + secondCtorExprs.stream().map(e -> ExprStatement.withExpr(e)).collect(Collectors.toList())); + secondCtorExprs.clear(); + secondCtorStatements.add(EMPTY_LINE_STATEMENT); // Transport settings local variables. Map javaStyleMethodNameToTransportSettingsVarExprs = @@ -578,6 +606,10 @@ private static List createConstructorMethods( JavaStyle.toLowerCamelCase(m.name())), protoMethodNameToDescriptorVarExprs.get(m.name()))) .collect(Collectors.toList())); + secondCtorStatements.addAll( + secondCtorExprs.stream().map(e -> ExprStatement.withExpr(e)).collect(Collectors.toList())); + secondCtorExprs.clear(); + secondCtorStatements.add(EMPTY_LINE_STATEMENT); // Initialize Callable variables. secondCtorExprs.addAll( @@ -594,6 +626,10 @@ private static List createConstructorMethods( thisExpr, javaStyleMethodNameToTransportSettingsVarExprs)) .collect(Collectors.toList())); + secondCtorStatements.addAll( + secondCtorExprs.stream().map(e -> ExprStatement.withExpr(e)).collect(Collectors.toList())); + secondCtorExprs.clear(); + secondCtorStatements.add(EMPTY_LINE_STATEMENT); // Instantiate backgroundResources. MethodInvocationExpr getBackgroundResourcesMethodExpr = @@ -612,14 +648,15 @@ private static List createConstructorMethods( .setArguments(Arrays.asList(getBackgroundResourcesMethodExpr)) .build()) .build()); + secondCtorStatements.addAll( + secondCtorExprs.stream().map(e -> ExprStatement.withExpr(e)).collect(Collectors.toList())); + secondCtorExprs.clear(); // Second constructor method. MethodDefinition secondCtor = ctorMakerFn.apply( Arrays.asList(settingsVarExpr, clientContextVarExpr, callableFactoryVarExpr), - secondCtorExprs.stream() - .map(e -> ExprStatement.withExpr(e)) - .collect(Collectors.toList())); + secondCtorStatements); return Arrays.asList(firstCtor, secondCtor); } diff --git a/src/test/java/com/google/api/generator/gapic/composer/goldens/GrpcEchoStub.golden b/src/test/java/com/google/api/generator/gapic/composer/goldens/GrpcEchoStub.golden index fdc6daf449..6cfd500bc0 100644 --- a/src/test/java/com/google/api/generator/gapic/composer/goldens/GrpcEchoStub.golden +++ b/src/test/java/com/google/api/generator/gapic/composer/goldens/GrpcEchoStub.golden @@ -45,6 +45,7 @@ public class GrpcEchoStub extends EchoStub { .setRequestMarshaller(ProtoUtils.marshaller(EchoRequest.getDefaultInstance())) .setResponseMarshaller(ProtoUtils.marshaller(EchoResponse.getDefaultInstance())) .build(); + private static final MethodDescriptor expandMethodDescriptor = MethodDescriptor.newBuilder() .setType(MethodDescriptor.MethodType.SERVER_STREAMING) @@ -52,6 +53,7 @@ public class GrpcEchoStub extends EchoStub { .setRequestMarshaller(ProtoUtils.marshaller(ExpandRequest.getDefaultInstance())) .setResponseMarshaller(ProtoUtils.marshaller(EchoResponse.getDefaultInstance())) .build(); + private static final MethodDescriptor collectMethodDescriptor = MethodDescriptor.newBuilder() .setType(MethodDescriptor.MethodType.CLIENT_STREAMING) @@ -59,6 +61,7 @@ public class GrpcEchoStub extends EchoStub { .setRequestMarshaller(ProtoUtils.marshaller(EchoRequest.getDefaultInstance())) .setResponseMarshaller(ProtoUtils.marshaller(EchoResponse.getDefaultInstance())) .build(); + private static final MethodDescriptor chatMethodDescriptor = MethodDescriptor.newBuilder() .setType(MethodDescriptor.MethodType.BIDI_STREAMING) @@ -66,6 +69,7 @@ public class GrpcEchoStub extends EchoStub { .setRequestMarshaller(ProtoUtils.marshaller(EchoRequest.getDefaultInstance())) .setResponseMarshaller(ProtoUtils.marshaller(EchoResponse.getDefaultInstance())) .build(); + private static final MethodDescriptor chatAgainMethodDescriptor = MethodDescriptor.newBuilder() .setType(MethodDescriptor.MethodType.BIDI_STREAMING) @@ -73,6 +77,7 @@ public class GrpcEchoStub extends EchoStub { .setRequestMarshaller(ProtoUtils.marshaller(EchoRequest.getDefaultInstance())) .setResponseMarshaller(ProtoUtils.marshaller(EchoResponse.getDefaultInstance())) .build(); + private static final MethodDescriptor pagedExpandMethodDescriptor = MethodDescriptor.newBuilder() @@ -82,6 +87,7 @@ public class GrpcEchoStub extends EchoStub { .setResponseMarshaller( ProtoUtils.marshaller(PagedExpandResponse.getDefaultInstance())) .build(); + private static final MethodDescriptor waitMethodDescriptor = MethodDescriptor.newBuilder() .setType(MethodDescriptor.MethodType.UNARY) @@ -89,6 +95,7 @@ public class GrpcEchoStub extends EchoStub { .setRequestMarshaller(ProtoUtils.marshaller(WaitRequest.getDefaultInstance())) .setResponseMarshaller(ProtoUtils.marshaller(Operation.getDefaultInstance())) .build(); + private static final MethodDescriptor blockMethodDescriptor = MethodDescriptor.newBuilder() .setType(MethodDescriptor.MethodType.UNARY) @@ -96,6 +103,7 @@ public class GrpcEchoStub extends EchoStub { .setRequestMarshaller(ProtoUtils.marshaller(BlockRequest.getDefaultInstance())) .setResponseMarshaller(ProtoUtils.marshaller(BlockResponse.getDefaultInstance())) .build(); + private final UnaryCallable echoCallable; private final ServerStreamingCallable expandCallable; private final ClientStreamingCallable collectCallable; @@ -107,6 +115,7 @@ public class GrpcEchoStub extends EchoStub { private final UnaryCallable waitCallable; private final OperationCallable waitOperationCallable; private final UnaryCallable blockCallable; + private final BackgroundResource backgroundResources; private final GrpcOperationsStub operationsStub; private final GrpcStubCallableFactory callableFactory; @@ -136,6 +145,7 @@ public class GrpcEchoStub extends EchoStub { throws IOException { this.callableFactory = callableFactory; this.operationsStub = GrpcOperationsStub.create(clientContext, callableFactory); + GrpcCallSettings echoTransportSettings = GrpcCallSettings.newBuilder() .setMethodDescriptor(echoMethodDescriptor) @@ -168,6 +178,7 @@ public class GrpcEchoStub extends EchoStub { GrpcCallSettings.newBuilder() .setMethodDescriptor(blockMethodDescriptor) .build(); + this.echoCallable = callableFactory.createUnaryCallable( echoTransportSettings, settings.echoSettings(), clientContext); @@ -198,6 +209,7 @@ public class GrpcEchoStub extends EchoStub { this.blockCallable = callableFactory.createUnaryCallable( blockTransportSettings, settings.blockSettings(), clientContext); + this.backgroundResources = new BackgroundResourceAggregation(clientContext.getBackgroundResources()); } From c004e9cdb40c55012b594d6075e160d14d07e595 Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Fri, 9 Oct 2020 18:14:59 -0700 Subject: [PATCH 5/5] fix: privately-scoped vars in ServiceClientTest codegen (#377) --- .../gapic/composer/ServiceClientTestClassComposer.java | 2 +- .../gapic/composer/goldens/EchoClientTest.golden | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/main/java/com/google/api/generator/gapic/composer/ServiceClientTestClassComposer.java b/src/main/java/com/google/api/generator/gapic/composer/ServiceClientTestClassComposer.java index 39c8836c6c..b200f74b5b 100644 --- a/src/main/java/com/google/api/generator/gapic/composer/ServiceClientTestClassComposer.java +++ b/src/main/java/com/google/api/generator/gapic/composer/ServiceClientTestClassComposer.java @@ -186,7 +186,7 @@ private static List createClassMemberFieldDecls( ExprStatement.withExpr( v.toBuilder() .setIsDecl(true) - .setScope(ScopeNode.PUBLIC) + .setScope(ScopeNode.PRIVATE) .setIsStatic(v.type().reference().name().startsWith("Mock")) .build())) .collect(Collectors.toList()); diff --git a/src/test/java/com/google/api/generator/gapic/composer/goldens/EchoClientTest.golden b/src/test/java/com/google/api/generator/gapic/composer/goldens/EchoClientTest.golden index c1cf340662..1c026587a5 100644 --- a/src/test/java/com/google/api/generator/gapic/composer/goldens/EchoClientTest.golden +++ b/src/test/java/com/google/api/generator/gapic/composer/goldens/EchoClientTest.golden @@ -37,10 +37,10 @@ import org.junit.Test; @Generated("by gapic-generator-java") public class EchoClientTest { - public static MockServiceHelper mockServiceHelper; - public static MockEcho mockEcho; - public EchoClient client; - public LocalChannelProvider channelProvider; + private static MockServiceHelper mockServiceHelper; + private static MockEcho mockEcho; + private EchoClient client; + private LocalChannelProvider channelProvider; @BeforeClass public static void startStaticServer() {