From 3054adc4e38718db23a36db0f57a0c9b336c0c4b Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Thu, 20 Aug 2020 14:21:44 -0700 Subject: [PATCH 01/20] feat: parse grpc_service_config.json --- BUILD.bazel | 18 +++ repositories.bzl | 9 ++ .../generator/gapic/protoparser/BUILD.bazel | 2 + .../protoparser/ServiceConfigParser.java | 46 +++++++ .../generator/gapic/protoparser/BUILD.bazel | 7 ++ .../protoparser/ServiceConfigParserTest.java | 113 ++++++++++++++++++ .../api/generator/gapic/testdata/BUILD.bazel | 5 + .../bad_proto_fields_grpc_service_config.json | 23 ++++ .../malformed_grpc_service_config.json | 21 ++++ .../retrying_grpc_service_config.json | 22 ++++ .../showcase_grpc_service_config.json | 85 +++++++++++++ 11 files changed, 351 insertions(+) create mode 100644 src/main/java/com/google/api/generator/gapic/protoparser/ServiceConfigParser.java create mode 100644 src/test/java/com/google/api/generator/gapic/protoparser/ServiceConfigParserTest.java create mode 100644 src/test/java/com/google/api/generator/gapic/testdata/bad_proto_fields_grpc_service_config.json create mode 100644 src/test/java/com/google/api/generator/gapic/testdata/malformed_grpc_service_config.json create mode 100644 src/test/java/com/google/api/generator/gapic/testdata/retrying_grpc_service_config.json create mode 100644 src/test/java/com/google/api/generator/gapic/testdata/showcase_grpc_service_config.json diff --git a/BUILD.bazel b/BUILD.bazel index 189b3e6388..7574fb74b9 100644 --- a/BUILD.bazel +++ b/BUILD.bazel @@ -76,6 +76,24 @@ java_proto_library( ], ) +java_proto_library( + name = "rpc_java_proto", + visibility = ["//:__subpackages__"], + deps = [ + "@com_google_googleapis//google/rpc:code_proto", + "@com_google_googleapis//google/rpc:error_details_proto", + "@com_google_googleapis//google/rpc:status_proto", + ], +) + +java_proto_library( + name = "service_config_java_proto", + visibility = ["//:__subpackages__"], + deps = [ + "@io_grpc_proto//:service_config_proto", + ], +) + # ============= Binary targets ================ java_binary( diff --git a/repositories.bzl b/repositories.bzl index 8b733654ea..cf0a3fa7db 100644 --- a/repositories.bzl +++ b/repositories.bzl @@ -133,6 +133,15 @@ def com_google_api_codegen_repositories(): strip_prefix = "grpc-java-%s" % _io_grpc_version, ) + # grpc-proto doesn't have releases, so we use hashes instead. + _io_grpc_proto_prefix = "0020624375a8ee4c7dd9b3e513e443b90bc28990" # Aug. 20, 2020. + _maybe( + http_archive, + name = "io_grpc_proto", + urls = ["https://github.com/grpc/grpc-proto/archive/%s.zip" % _io_grpc_proto_prefix], + strip_prefix = "grpc-proto-%s" % _io_grpc_proto_prefix, + ) + def _maybe(repo_rule, name, strip_repo_prefix = "", **kwargs): if not name.startswith(strip_repo_prefix): return diff --git a/src/main/java/com/google/api/generator/gapic/protoparser/BUILD.bazel b/src/main/java/com/google/api/generator/gapic/protoparser/BUILD.bazel index 1adf6e418e..e6b95b32be 100644 --- a/src/main/java/com/google/api/generator/gapic/protoparser/BUILD.bazel +++ b/src/main/java/com/google/api/generator/gapic/protoparser/BUILD.bazel @@ -23,6 +23,7 @@ java_library( "//:client_java_proto", "//:longrunning_java_proto", "//:resource_java_proto", + "//:service_config_java_proto", "//src/main/java/com/google/api/generator/engine/ast", "//src/main/java/com/google/api/generator/gapic/model", "//src/main/java/com/google/api/generator/gapic/utils", @@ -30,5 +31,6 @@ java_library( "@com_google_code_findbugs_jsr305//jar", "@com_google_guava_guava//jar", "@com_google_protobuf//:protobuf_java", + "@com_google_protobuf//:protobuf_java_util", ], ) diff --git a/src/main/java/com/google/api/generator/gapic/protoparser/ServiceConfigParser.java b/src/main/java/com/google/api/generator/gapic/protoparser/ServiceConfigParser.java new file mode 100644 index 0000000000..25152f3051 --- /dev/null +++ b/src/main/java/com/google/api/generator/gapic/protoparser/ServiceConfigParser.java @@ -0,0 +1,46 @@ +// 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.gapic.protoparser; + +import com.google.common.base.Strings; +import com.google.protobuf.util.JsonFormat; +import io.grpc.serviceconfig.ServiceConfig; +import java.io.FileNotFoundException; +import java.io.FileReader; +import java.io.IOException; +import java.util.Optional; + +public class ServiceConfigParser { + public static Optional parseFile(String serviceConfigFilePath) { + if (Strings.isNullOrEmpty(serviceConfigFilePath)) { + return Optional.empty(); + } + + ServiceConfig.Builder builder = ServiceConfig.newBuilder(); + FileReader file = null; + try { + file = new FileReader(serviceConfigFilePath); + } catch (FileNotFoundException e) { + return Optional.empty(); + } + + try { + JsonFormat.parser().merge(file, builder); + } catch (IOException e) { + return Optional.empty(); + } + return Optional.of(builder.build()); + } +} diff --git a/src/test/java/com/google/api/generator/gapic/protoparser/BUILD.bazel b/src/test/java/com/google/api/generator/gapic/protoparser/BUILD.bazel index d56165de9b..199099fc25 100644 --- a/src/test/java/com/google/api/generator/gapic/protoparser/BUILD.bazel +++ b/src/test/java/com/google/api/generator/gapic/protoparser/BUILD.bazel @@ -6,6 +6,7 @@ TESTS = [ "PluginArgumentParserTest", "ResourceNameParserTest", "ResourceReferenceParserTest", + "ServiceConfigParserTest", ] filegroup( @@ -16,8 +17,13 @@ filegroup( [java_test( name = test_name, srcs = ["{0}.java".format(test_name)], + data = [ + "//src/test/java/com/google/api/generator/gapic/testdata:service_config_files", + ], test_class = "com.google.api.generator.gapic.protoparser.{0}".format(test_name), deps = [ + "//:rpc_java_proto", + "//:service_config_java_proto", "//src/main/java/com/google/api/generator:autovalue", "//src/main/java/com/google/api/generator/engine/ast", "//src/main/java/com/google/api/generator/gapic:status_java_proto", @@ -27,6 +33,7 @@ filegroup( "//src/test/java/com/google/api/generator/gapic/testdata:showcase_java_proto", "//src/test/java/com/google/api/generator/gapic/testdata:testgapic_java_proto", "@com_google_protobuf//:protobuf_java", + "@com_google_protobuf//:protobuf_java_util", "@com_google_truth_truth//jar", "@junit_junit//jar", ], diff --git a/src/test/java/com/google/api/generator/gapic/protoparser/ServiceConfigParserTest.java b/src/test/java/com/google/api/generator/gapic/protoparser/ServiceConfigParserTest.java new file mode 100644 index 0000000000..d5688d545d --- /dev/null +++ b/src/test/java/com/google/api/generator/gapic/protoparser/ServiceConfigParserTest.java @@ -0,0 +1,113 @@ +// 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.gapic.protoparser; + +import static junit.framework.Assert.assertEquals; +import static junit.framework.Assert.assertFalse; +import static junit.framework.Assert.assertTrue; + +import com.google.protobuf.util.Durations; +import com.google.rpc.Code; +import io.grpc.serviceconfig.MethodConfig; +import io.grpc.serviceconfig.ServiceConfig; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.util.Optional; +import org.junit.Test; + +public class ServiceConfigParserTest { + private static final String JSON_DIRECTORY = + "src/test/java/com/google/api/generator/gapic/testdata/"; + private static final double EPSILON = 1e-4; + + @Test + public void parseServiceConfig_basic() { + String jsonFilename = "retrying_grpc_service_config.json"; + Path jsonPath = Paths.get(JSON_DIRECTORY, jsonFilename); + Optional configOpt = ServiceConfigParser.parseFile(jsonPath.toString()); + assertTrue(configOpt.isPresent()); + + ServiceConfig config = configOpt.get(); + assertEquals(1, config.getMethodConfigList().size()); + + MethodConfig methodConfig = config.getMethodConfigList().get(0); + assertEquals(1, methodConfig.getNameList().size()); + + MethodConfig.Name name = methodConfig.getNameList().get(0); + assertEquals("helloworld.Greeter", name.getService()); + assertEquals("SayHello", name.getMethod()); + + MethodConfig.RetryPolicy retryPolicy = methodConfig.getRetryPolicy(); + assertEquals(5, retryPolicy.getMaxAttempts()); + assertEquals(500, Durations.toMillis(retryPolicy.getInitialBackoff())); + assertEquals(30000, Durations.toMillis(retryPolicy.getMaxBackoff())); + assertEquals(2.0, retryPolicy.getBackoffMultiplier(), EPSILON); + + assertEquals(1, retryPolicy.getRetryableStatusCodesList().size()); + assertEquals(Code.UNAVAILABLE, retryPolicy.getRetryableStatusCodesList().get(0)); + } + + @Test + public void parseServiceConfig_showcase() { + String jsonFilename = "showcase_grpc_service_config.json"; + Path jsonPath = Paths.get(JSON_DIRECTORY, jsonFilename); + Optional configOpt = ServiceConfigParser.parseFile(jsonPath.toString()); + assertTrue(configOpt.isPresent()); + + ServiceConfig config = configOpt.get(); + assertEquals(3, config.getMethodConfigList().size()); + + MethodConfig methodConfig = config.getMethodConfigList().get(0); + assertEquals(2, methodConfig.getNameList().size()); + assertTrue(methodConfig.getNameList().get(0).getMethod().isEmpty()); + assertEquals(5000, Durations.toMillis(methodConfig.getTimeout())); + + methodConfig = config.getMethodConfigList().get(1); + assertEquals(9, methodConfig.getNameList().size()); + } + + @Test + public void parseBadServiceConfig_missingFile() { + String jsonFilename = "does_not_exist_grpc_service_config.json"; + Path jsonPath = Paths.get(JSON_DIRECTORY, jsonFilename); + Optional configOpt = ServiceConfigParser.parseFile(jsonPath.toString()); + assertFalse(configOpt.isPresent()); + } + + @Test + public void parseBadServiceConfig_malformedJson() { + String jsonFilename = "malformed_grpc_service_config.json"; + Path jsonPath = Paths.get(JSON_DIRECTORY, jsonFilename); + Optional configOpt = ServiceConfigParser.parseFile(jsonPath.toString()); + assertFalse(configOpt.isPresent()); + } + + @Test + public void parseBadServiceConfig_badProtoFields() { + String jsonFilename = "bad_proto_fields_grpc_service_config.json"; + Path jsonPath = Paths.get(JSON_DIRECTORY, jsonFilename); + Optional configOpt = ServiceConfigParser.parseFile(jsonPath.toString()); + assertFalse(configOpt.isPresent()); + } + + @Test + public void parseBadServiceConfig_nullOrEmptyPath() { + Optional configOpt = ServiceConfigParser.parseFile(null); + assertFalse(configOpt.isPresent()); + + configOpt = ServiceConfigParser.parseFile(""); + assertFalse(configOpt.isPresent()); + } +} 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 a8eaae1db6..32260413ac 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 @@ -1,5 +1,10 @@ package(default_visibility = ["//visibility:public"]) +filegroup( + name = "service_config_files", + srcs = glob(["*.json"]), +) + proto_library( name = "showcase_proto", srcs = [ diff --git a/src/test/java/com/google/api/generator/gapic/testdata/bad_proto_fields_grpc_service_config.json b/src/test/java/com/google/api/generator/gapic/testdata/bad_proto_fields_grpc_service_config.json new file mode 100644 index 0000000000..66849f50d9 --- /dev/null +++ b/src/test/java/com/google/api/generator/gapic/testdata/bad_proto_fields_grpc_service_config.json @@ -0,0 +1,23 @@ +{ + "methodConfig": [ + { + "foobar": 1, + "name": [ + { + "service": "helloworld.Greeter", + "method": "SayHello" + } + ], + + "retryPolicy": { + "maxAttempts": 5, + "initialBackoff": "0.5s", + "maxBackoff": "30s", + "backoffMultiplier": 2, + "retryableStatusCodes": [ + "UNAVAILABLE" + ] + } + } + ] +} diff --git a/src/test/java/com/google/api/generator/gapic/testdata/malformed_grpc_service_config.json b/src/test/java/com/google/api/generator/gapic/testdata/malformed_grpc_service_config.json new file mode 100644 index 0000000000..cc65725f8a --- /dev/null +++ b/src/test/java/com/google/api/generator/gapic/testdata/malformed_grpc_service_config.json @@ -0,0 +1,21 @@ +{ + "methodConfig": [ + { + { + "service": "helloworld.Greeter", + "method": "SayHello" + } + ], + + "retryPolicy": { + "maxAttempts": 5, + "initialBackoff": "0.5s", + "maxBackoff": "30s", + "backoffMultiplier": 2, + "retryableStatusCodes": [ + "UNAVAILABLE" + ] + } + } + ] +} diff --git a/src/test/java/com/google/api/generator/gapic/testdata/retrying_grpc_service_config.json b/src/test/java/com/google/api/generator/gapic/testdata/retrying_grpc_service_config.json new file mode 100644 index 0000000000..ff115a9ad6 --- /dev/null +++ b/src/test/java/com/google/api/generator/gapic/testdata/retrying_grpc_service_config.json @@ -0,0 +1,22 @@ +{ + "methodConfig": [ + { + "name": [ + { + "service": "helloworld.Greeter", + "method": "SayHello" + } + ], + + "retryPolicy": { + "maxAttempts": 5, + "initialBackoff": "0.5s", + "maxBackoff": "30s", + "backoffMultiplier": 2, + "retryableStatusCodes": [ + "UNAVAILABLE" + ] + } + } + ] +} diff --git a/src/test/java/com/google/api/generator/gapic/testdata/showcase_grpc_service_config.json b/src/test/java/com/google/api/generator/gapic/testdata/showcase_grpc_service_config.json new file mode 100644 index 0000000000..ea16b8994b --- /dev/null +++ b/src/test/java/com/google/api/generator/gapic/testdata/showcase_grpc_service_config.json @@ -0,0 +1,85 @@ +{ + "methodConfig": [ + { + "name": [ + {"service": "google.showcase.v1beta1.Echo"}, + {"service": "google.showcase.v1beta1.Messaging"} + ], + "timeout": "5s" + }, + { + "name": [ + { + "service": "google.showcase.v1beta1.Echo", + "method": "Echo" + }, + { + "service": "google.showcase.v1beta1.Echo", + "method": "Expand" + }, + { + "service": "google.showcase.v1beta1.Echo", + "method": "PagedExpand" + }, + { + "service": "google.showcase.v1beta1.Messaging", + "method": "GetRoom" + }, + { + "service": "google.showcase.v1beta1.Messaging", + "method": "ListRooms" + }, + { + "service": "google.showcase.v1beta1.Messaging", + "method": "GetBlurb" + }, + { + "service": "google.showcase.v1beta1.Messaging", + "method": "ListBlurbs" + }, + { + "service": "google.showcase.v1beta1.Messaging", + "method": "SearchBlurbs" + }, + { + "service": "google.showcase.v1beta1.Messaging", + "method": "Connect" + } + ], + "retryPolicy": { + "maxAttempts": 3, + "maxBackoff": "3s", + "initialBackoff": "0.1s", + "backoffMultiplier": 2, + "retryableStatusCodes": [ + "UNAVAILABLE", + "UNKNOWN" + ] + }, + "timeout": "10s" + }, + { + "name": [ + { + "service": "google.showcase.v1beta1.Identity", + "method": "GetUser" + }, + { + "service": "google.showcase.v1beta1.Identity", + "method": "ListUsers" + } + ], + "retryPolicy": { + "maxAttempts": 5, + "maxBackoff": "3s", + "initialBackoff": "0.2s", + "backoffMultiplier": 2, + "retryableStatusCodes": [ + "UNAVAILABLE", + "UNKNOWN" + ] + }, + "timeout": "5s" + } + ] +} From 6398d2abdb3689daddcb958c4be3be7c05add180 Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Thu, 20 Aug 2020 14:53:16 -0700 Subject: [PATCH 02/20] feat: pipe ServiceConfig into settings codegen --- run.sh | 2 +- .../api/generator/gapic/composer/BUILD.bazel | 1 + .../generator/gapic/composer/Composer.java | 21 +++++++++---------- .../ServiceStubSettingsClassComposer.java | 8 ++++--- .../api/generator/gapic/model/BUILD.bazel | 1 + .../generator/gapic/model/GapicContext.java | 7 +++++++ .../generator/gapic/protoparser/Parser.java | 10 ++++++--- .../api/generator/gapic/composer/BUILD.bazel | 4 ++++ .../ServiceStubSettingsClassComposerTest.java | 18 +++++++++++++++- 9 files changed, 53 insertions(+), 19 deletions(-) diff --git a/run.sh b/run.sh index 08e67c8c4d..46572fe76c 100755 --- a/run.sh +++ b/run.sh @@ -5,7 +5,7 @@ # Usage example here: : << 'EXAMPLE' DIR=~/dev/googleapis/google/showcase/v1beta1 - ./run.sh --g ~/dev/googleapis --p "$DIR"-s "$DIR/showcase_grpc_config.json" + ./run.sh --g ~/dev/googleapis --p "$DIR"-s "$DIR/showcase_grpc_service_config.json" EXAMPLE source gbash.sh diff --git a/src/main/java/com/google/api/generator/gapic/composer/BUILD.bazel b/src/main/java/com/google/api/generator/gapic/composer/BUILD.bazel index e4d8b12f9e..d1847bc155 100644 --- a/src/main/java/com/google/api/generator/gapic/composer/BUILD.bazel +++ b/src/main/java/com/google/api/generator/gapic/composer/BUILD.bazel @@ -13,6 +13,7 @@ java_library( deps = [ "//:longrunning_java_proto", "//:monitored_resource_java_proto", + "//:service_config_java_proto", "//src/main/java/com/google/api/generator/engine/ast", "//src/main/java/com/google/api/generator/gapic:status_java_proto", "//src/main/java/com/google/api/generator/gapic/model", diff --git a/src/main/java/com/google/api/generator/gapic/composer/Composer.java b/src/main/java/com/google/api/generator/gapic/composer/Composer.java index 15aed08d4d..f54150735c 100644 --- a/src/main/java/com/google/api/generator/gapic/composer/Composer.java +++ b/src/main/java/com/google/api/generator/gapic/composer/Composer.java @@ -24,27 +24,31 @@ import com.google.api.generator.gapic.model.Service; import com.google.api.generator.gapic.utils.ApacheLicense; import com.google.common.annotations.VisibleForTesting; +import io.grpc.serviceconfig.ServiceConfig; import java.util.ArrayList; import java.util.List; import java.util.Map; import java.util.Set; import java.util.stream.Collectors; import javax.annotation.Nonnull; +import javax.annotation.Nullable; public class Composer { public static List composeServiceClasses(GapicContext context) { List clazzes = new ArrayList<>(); for (Service service : context.services()) { - clazzes.addAll(generateServiceClasses(service, context.messages())); + clazzes.addAll(generateServiceClasses(service, context.serviceConfig(), context.messages())); } clazzes.addAll(generateResourceNameHelperClasses(context.helperResourceNames())); return addApacheLicense(clazzes); } public static List generateServiceClasses( - @Nonnull Service service, @Nonnull Map messageTypes) { + @Nonnull Service service, + @Nullable ServiceConfig serviceConfig, + @Nonnull Map messageTypes) { List clazzes = new ArrayList<>(); - clazzes.addAll(generateStubClasses(service, messageTypes)); + clazzes.addAll(generateStubClasses(service, serviceConfig, messageTypes)); clazzes.addAll(generateClientSettingsClasses(service, messageTypes)); clazzes.addAll(generateMocksAndTestClasses(service, messageTypes)); // TODO(miraleung): Generate test classes. @@ -60,10 +64,11 @@ public static List generateResourceNameHelperClasses( } public static List generateStubClasses( - Service service, Map messageTypes) { + Service service, ServiceConfig serviceConfig, Map messageTypes) { List clazzes = new ArrayList<>(); clazzes.add(ServiceStubClassComposer.instance().generate(service, messageTypes)); - clazzes.add(generateStubServiceSettings(service)); + clazzes.add( + ServiceStubSettingsClassComposer.instance().generate(service, serviceConfig, messageTypes)); clazzes.add(GrpcServiceCallableFactoryClassComposer.instance().generate(service, messageTypes)); clazzes.add(GrpcServiceStubClassComposer.instance().generate(service, messageTypes)); return clazzes; @@ -86,12 +91,6 @@ public static List generateMocksAndTestClasses( return clazzes; } - /** ====================== STUB CLASSES ==================== */ - private static GapicClass generateStubServiceSettings(Service service) { - return generateGenericClass( - Kind.STUB, String.format("%sStubSettings", service.name()), service); - } - /** ====================== HELPERS ==================== */ // TODO(miraleung): Add method list. private static GapicClass generateGenericClass(Kind kind, String name, Service service) { 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 30e68c260e..f2d4477511 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 @@ -75,6 +75,7 @@ import com.google.common.collect.Lists; import com.google.longrunning.Operation; import com.google.protobuf.Empty; +import io.grpc.serviceconfig.ServiceConfig; import java.io.IOException; import java.util.ArrayList; import java.util.Arrays; @@ -85,7 +86,8 @@ import javax.annotation.Generated; import org.threeten.bp.Duration; -public class ServiceStubSettingsClassComposer implements ClassComposer { +// TODO(miraleung): Refactor ClassComposer's interface. +public class ServiceStubSettingsClassComposer { private static final String CLASS_NAME_PATTERN = "%sStubSettings"; private static final String SLASH = "/"; private static final String LEFT_BRACE = "{"; @@ -104,8 +106,8 @@ public static ServiceStubSettingsClassComposer instance() { return INSTANCE; } - @Override - public GapicClass generate(Service service, Map ignore) { + public GapicClass generate( + Service service, ServiceConfig serviceConfig, Map messageTypes) { String pakkage = String.format("%s.stub", service.pakkage()); Map types = createDynamicTypes(service, pakkage); Map classMemberVarExprs = createClassMemberVarExprs(service, types); diff --git a/src/main/java/com/google/api/generator/gapic/model/BUILD.bazel b/src/main/java/com/google/api/generator/gapic/model/BUILD.bazel index 2460f46eed..e0106a3c27 100644 --- a/src/main/java/com/google/api/generator/gapic/model/BUILD.bazel +++ b/src/main/java/com/google/api/generator/gapic/model/BUILD.bazel @@ -11,6 +11,7 @@ java_library( ":model_files", ], deps = [ + "//:service_config_java_proto", "//src/main/java/com/google/api/generator:autovalue", "//src/main/java/com/google/api/generator/engine/ast", "//src/main/java/com/google/api/generator/gapic/utils", diff --git a/src/main/java/com/google/api/generator/gapic/model/GapicContext.java b/src/main/java/com/google/api/generator/gapic/model/GapicContext.java index 15cf8bb012..0d894884b9 100644 --- a/src/main/java/com/google/api/generator/gapic/model/GapicContext.java +++ b/src/main/java/com/google/api/generator/gapic/model/GapicContext.java @@ -18,9 +18,11 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; +import io.grpc.serviceconfig.ServiceConfig; import java.util.List; import java.util.Map; import java.util.Set; +import javax.annotation.Nullable; @AutoValue public abstract class GapicContext { @@ -34,6 +36,9 @@ public abstract class GapicContext { public abstract ImmutableSet helperResourceNames(); + @Nullable + public abstract ServiceConfig serviceConfig(); + public static Builder builder() { return new AutoValue_GapicContext.Builder(); } @@ -48,6 +53,8 @@ public abstract static class Builder { public abstract Builder setHelperResourceNames(Set helperResourceNames); + public abstract Builder setServiceConfig(ServiceConfig serviceConfig); + public abstract GapicContext build(); } } 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 db90210655..f03079882d 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 @@ -46,6 +46,7 @@ import com.google.protobuf.Descriptors.MethodDescriptor; import com.google.protobuf.Descriptors.ServiceDescriptor; import com.google.protobuf.compiler.PluginProtos.CodeGeneratorRequest; +import io.grpc.serviceconfig.ServiceConfig; import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; @@ -69,9 +70,11 @@ public GapicParserException(String errorMessage) { } public static GapicContext parse(CodeGeneratorRequest request) { - // TODO(miraleung): Actually parse these files. - Optional jsonConfigPathOpt = PluginArgumentParser.parseJsonConfigPath(request); - String jsonConfigPath = jsonConfigPathOpt.isPresent() ? jsonConfigPathOpt.get() : null; + Optional serviceConfigPathOpt = PluginArgumentParser.parseJsonConfigPath(request); + String serviceConfigPath = serviceConfigPathOpt.isPresent() ? serviceConfigPathOpt.get() : null; + Optional serviceConfigOpt = ServiceConfigParser.parseFile(serviceConfigPath); + + // TODO(miraleung): Actually parse the yaml file. Optional gapicYamlConfigPathOpt = PluginArgumentParser.parseGapicYamlConfigPath(request); String gapicYamlConfigPath = @@ -91,6 +94,7 @@ public static GapicContext parse(CodeGeneratorRequest request) { .setMessages(messages) .setResourceNames(resourceNames) .setHelperResourceNames(outputArgResourceNames) + .setServiceConfig(serviceConfigOpt.isPresent() ? serviceConfigOpt.get() : null) .build(); } 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 1f13280759..a0fc9e4a60 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 @@ -22,8 +22,12 @@ filegroup( [java_test( name = test_name, srcs = ["{0}.java".format(test_name)], + data = [ + "//src/test/java/com/google/api/generator/gapic/testdata:service_config_files", + ], test_class = "com.google.api.generator.gapic.composer.{0}".format(test_name), deps = [ + "//:service_config_java_proto", "//src/main/java/com/google/api/generator/engine/ast", "//src/main/java/com/google/api/generator/engine/writer", "//src/main/java/com/google/api/generator/gapic/composer", diff --git a/src/test/java/com/google/api/generator/gapic/composer/ServiceStubSettingsClassComposerTest.java b/src/test/java/com/google/api/generator/gapic/composer/ServiceStubSettingsClassComposerTest.java index df95e95ede..ed59b109f1 100644 --- a/src/test/java/com/google/api/generator/gapic/composer/ServiceStubSettingsClassComposerTest.java +++ b/src/test/java/com/google/api/generator/gapic/composer/ServiceStubSettingsClassComposerTest.java @@ -15,6 +15,7 @@ package com.google.api.generator.gapic.composer; import static junit.framework.Assert.assertEquals; +import static junit.framework.Assert.assertTrue; import com.google.api.generator.engine.writer.JavaWriterVisitor; import com.google.api.generator.gapic.model.GapicClass; @@ -22,17 +23,25 @@ import com.google.api.generator.gapic.model.ResourceName; import com.google.api.generator.gapic.model.Service; import com.google.api.generator.gapic.protoparser.Parser; +import com.google.api.generator.gapic.protoparser.ServiceConfigParser; import com.google.protobuf.Descriptors.FileDescriptor; import com.google.protobuf.Descriptors.ServiceDescriptor; import com.google.showcase.v1beta1.EchoOuterClass; +import io.grpc.serviceconfig.ServiceConfig; +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.Optional; import java.util.Set; import org.junit.Before; import org.junit.Test; public class ServiceStubSettingsClassComposerTest { + private static final String JSON_DIRECTORY = + "src/test/java/com/google/api/generator/gapic/testdata/"; + private ServiceDescriptor echoService; private FileDescriptor echoFileDescriptor; @@ -51,9 +60,16 @@ public void generateServiceClasses() { List services = Parser.parseService(echoFileDescriptor, messageTypes, resourceNames, outputResourceNames); + String jsonFilename = "showcase_grpc_service_config.json"; + Path jsonPath = Paths.get(JSON_DIRECTORY, jsonFilename); + Optional configOpt = ServiceConfigParser.parseFile(jsonPath.toString()); + assertTrue(configOpt.isPresent()); + ServiceConfig config = configOpt.get(); + Service echoProtoService = services.get(0); GapicClass clazz = - ServiceStubSettingsClassComposer.instance().generate(echoProtoService, messageTypes); + ServiceStubSettingsClassComposer.instance() + .generate(echoProtoService, config, messageTypes); JavaWriterVisitor visitor = new JavaWriterVisitor(); clazz.classDefinition().accept(visitor); From 6e03c6a193930eb3cd51ffd9c0e41dff935fce48 Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Fri, 21 Aug 2020 13:22:36 -0700 Subject: [PATCH 03/20] feat: add settings fields to ServiceStubSettings codegen --- .../ServiceStubSettingsClassComposer.java | 122 +++++++++++++++++- .../ServiceStubSettingsClassComposerTest.java | 31 +++++ 2 files changed, 147 insertions(+), 6 deletions(-) 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 f2d4477511..35f5c2cfd1 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 @@ -44,7 +44,9 @@ import com.google.api.gax.rpc.PagedCallSettings; import com.google.api.gax.rpc.PagedListDescriptor; import com.google.api.gax.rpc.PagedListResponseFactory; +import com.google.api.gax.rpc.ServerStreamingCallSettings; import com.google.api.gax.rpc.StatusCode; +import com.google.api.gax.rpc.StreamingCallSettings; import com.google.api.gax.rpc.StubSettings; import com.google.api.gax.rpc.TransportChannelProvider; import com.google.api.gax.rpc.UnaryCallSettings; @@ -57,6 +59,7 @@ import com.google.api.generator.engine.ast.ExprStatement; import com.google.api.generator.engine.ast.MethodDefinition; import com.google.api.generator.engine.ast.MethodInvocationExpr; +import com.google.api.generator.engine.ast.Reference; import com.google.api.generator.engine.ast.ScopeNode; import com.google.api.generator.engine.ast.Statement; import com.google.api.generator.engine.ast.StringObjectValue; @@ -67,8 +70,10 @@ import com.google.api.generator.engine.ast.VariableExpr; import com.google.api.generator.gapic.model.GapicClass; import com.google.api.generator.gapic.model.Message; +import com.google.api.generator.gapic.model.Method; import com.google.api.generator.gapic.model.Service; import com.google.api.generator.gapic.utils.JavaStyle; +import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; @@ -80,6 +85,7 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.HashMap; +import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.stream.Collectors; @@ -89,9 +95,11 @@ // TODO(miraleung): Refactor ClassComposer's interface. public class ServiceStubSettingsClassComposer { private static final String CLASS_NAME_PATTERN = "%sStubSettings"; - private static final String SLASH = "/"; + private static final String PAGED_RESPONSE_TYPE_NAME_PATTERN = "%sPagedResponse"; + private static final String LEFT_BRACE = "{"; private static final String RIGHT_BRACE = "}"; + private static final String SLASH = "/"; private static final ServiceStubSettingsClassComposer INSTANCE = new ServiceStubSettingsClassComposer(); @@ -110,7 +118,8 @@ public GapicClass generate( Service service, ServiceConfig serviceConfig, Map messageTypes) { String pakkage = String.format("%s.stub", service.pakkage()); Map types = createDynamicTypes(service, pakkage); - Map classMemberVarExprs = createClassMemberVarExprs(service, types); + Map methodSettingsMemberVarExprs = + createClassMemberVarExprs(service, types); String className = getThisClassName(service.name()); ClassDefinition classDef = @@ -120,7 +129,7 @@ public GapicClass generate( .setScope(ScopeNode.PUBLIC) .setName(className) .setExtendsType(createExtendsType(service, types)) - .setStatements(createClassStatements(service, types)) + .setStatements(createClassStatements(service, methodSettingsMemberVarExprs, types)) .setMethods(createClassMethods(service, types)) .setNestedClasses(Arrays.asList(createNestedBuilderClass(service, types))) .build(); @@ -147,12 +156,35 @@ private static TypeNode createExtendsType(Service service, Map private static Map createClassMemberVarExprs( Service service, Map types) { - // TODO(miraleung): Fill this out. - return new HashMap(); + // Maintain insertion order. + Map varExprs = new LinkedHashMap<>(); + + // Creates class variables Settings, e.g. echoSettings. + // TODO(miraleung): Handle batching here. + for (Method method : service.methods()) { + TypeNode settingsType = getCallSettingsType(method, types); + String varName = JavaStyle.toLowerCamelCase(String.format("%sSettings", method.name())); + varExprs.put( + varName, + VariableExpr.withVariable( + Variable.builder().setType(settingsType).setName(varName).build())); + if (method.hasLro()) { + settingsType = getOperationCallSettingsType(method); + varName = JavaStyle.toLowerCamelCase(String.format("%sOperationSettings", method.name())); + varExprs.put( + varName, + VariableExpr.withVariable( + Variable.builder().setType(settingsType).setName(varName).build())); + } + } + + return varExprs; } private static List createClassStatements( - Service service, Map types) { + Service service, + Map methodSettingsMemberVarExprs, + Map types) { List memberVars = new ArrayList<>(); // Assign DEFAULT_SERVICE_SCOPES. @@ -192,6 +224,18 @@ private static List createClassStatements( .setValueExpr(listBuilderExpr) .build()); + // Declare settings members. + memberVars.addAll( + methodSettingsMemberVarExprs.values().stream() + .map( + v -> + v.toBuilder() + .setIsDecl(true) + .setScope(ScopeNode.PRIVATE) + .setIsFinal(true) + .build()) + .collect(Collectors.toList())); + // TODO(miraleung): Fill this out. return memberVars.stream().map(e -> ExprStatement.withExpr(e)).collect(Collectors.toList()); } @@ -262,7 +306,9 @@ private static Map createStaticTypes() { ProtoOperationTransformers.class, RequestBuilder.class, RetrySettings.class, + ServerStreamingCallSettings.class, StatusCode.class, + StreamingCallSettings.class, StubSettings.class, TransportChannelProvider.class, UnaryCallSettings.class, @@ -291,6 +337,22 @@ private static Map createDynamicTypes(Service service, String .setIsStaticImport(true) .build())); + // Pagination types. + dynamicTypes.putAll( + service.methods().stream() + .filter(m -> m.isPaged()) + .collect( + Collectors.toMap( + m -> String.format(PAGED_RESPONSE_TYPE_NAME_PATTERN, m.name()), + m -> + TypeNode.withReference( + VaporReference.builder() + .setName(getPagedResponseTypeName(m.name())) + .setPakkage(service.pakkage()) + .setEnclosingClassName(String.format("%sClient", service.name())) + .setIsStaticImport(true) + .build())))); + // TODO(miraleung): Fill this out. return dynamicTypes; } @@ -309,4 +371,52 @@ private static VariableExpr createDefaultServiceScopesVarExpr() { private static String getThisClassName(String serviceName) { return String.format(CLASS_NAME_PATTERN, JavaStyle.toUpperCamelCase(serviceName)); } + + private static String getPagedResponseTypeName(String methodName) { + return String.format(PAGED_RESPONSE_TYPE_NAME_PATTERN, methodName); + } + + private static TypeNode getCallSettingsType(Method method, Map types) { + // Default: No streaming. + TypeNode callSettingsType = + method.isPaged() + ? STATIC_TYPES.get("PagedCallSettings") + : STATIC_TYPES.get("UnaryCallSettings"); + + // Streaming takes precendence over paging, as per the monolith's existing behavior. + switch (method.stream()) { + case SERVER: + callSettingsType = STATIC_TYPES.get("ServerStreamingCallSettings"); + break; + case CLIENT: + // Fall through. + case BIDI: + callSettingsType = STATIC_TYPES.get("StreamingCallSettings"); + break; + case NONE: + // Fall through. + default: + break; + } + + List generics = new ArrayList<>(); + generics.add(method.inputType().reference()); + generics.add(method.outputType().reference()); + if (method.isPaged()) { + generics.add(types.get(getPagedResponseTypeName(method.name())).reference()); + } + return TypeNode.withReference(callSettingsType.reference().copyAndSetGenerics(generics)); + } + + private static TypeNode getOperationCallSettingsType(Method method) { + Preconditions.checkState( + method.hasLro(), + String.format("Cannot get OperationCallSettings for non-LRO method %s", method.name())); + List generics = new ArrayList<>(); + generics.add(method.inputType().reference()); + generics.add(method.lro().responseType().reference()); + generics.add(method.lro().metadataType().reference()); + return TypeNode.withReference( + STATIC_TYPES.get("OperationCallSettings").reference().copyAndSetGenerics(generics)); + } } diff --git a/src/test/java/com/google/api/generator/gapic/composer/ServiceStubSettingsClassComposerTest.java b/src/test/java/com/google/api/generator/gapic/composer/ServiceStubSettingsClassComposerTest.java index ed59b109f1..14ebd09ceb 100644 --- a/src/test/java/com/google/api/generator/gapic/composer/ServiceStubSettingsClassComposerTest.java +++ b/src/test/java/com/google/api/generator/gapic/composer/ServiceStubSettingsClassComposerTest.java @@ -79,10 +79,28 @@ public void generateServiceClasses() { // TODO(miraleung): Update this when a file-diffing test mechanism is in place. private static final String EXPECTED_CLASS_STRING = "package com.google.showcase.v1beta1.stub;\n" + + "\n" + + "import static com.google.showcase.v1beta1.EchoClient.PagedExpandPagedResponse;\n" + "\n" + "import com.google.api.core.BetaApi;\n" + + "import com.google.api.gax.rpc.OperationCallSettings;\n" + + "import com.google.api.gax.rpc.PagedCallSettings;\n" + + "import com.google.api.gax.rpc.ServerStreamingCallSettings;\n" + + "import com.google.api.gax.rpc.StreamingCallSettings;\n" + "import com.google.api.gax.rpc.StubSettings;\n" + + "import com.google.api.gax.rpc.UnaryCallSettings;\n" + "import com.google.common.collect.ImmutableList;\n" + + "import com.google.longrunning.Operation;\n" + + "import com.google.showcase.v1beta1.BlockRequest;\n" + + "import com.google.showcase.v1beta1.BlockResponse;\n" + + "import com.google.showcase.v1beta1.EchoRequest;\n" + + "import com.google.showcase.v1beta1.EchoResponse;\n" + + "import com.google.showcase.v1beta1.ExpandRequest;\n" + + "import com.google.showcase.v1beta1.PagedExpandRequest;\n" + + "import com.google.showcase.v1beta1.PagedExpandResponse;\n" + + "import com.google.showcase.v1beta1.WaitMetadata;\n" + + "import com.google.showcase.v1beta1.WaitRequest;\n" + + "import com.google.showcase.v1beta1.WaitResponse;\n" + "import javax.annotation.Generated;\n" + "\n" + "@BetaApi\n" @@ -91,6 +109,19 @@ public void generateServiceClasses() { + " private static final ImmutableList DEFAULT_SERVICE_SCOPES =\n" + " " + " ImmutableList.builder().add(\"https://www.googleapis.com/auth/cloud-platform\").build();\n" + + " private final UnaryCallSettings echoSettings;\n" + + " private final ServerStreamingCallSettings" + + " expandSettings;\n" + + " private final StreamingCallSettings collectSettings;\n" + + " private final StreamingCallSettings chatSettings;\n" + + " private final StreamingCallSettings chatAgainSettings;\n" + + " private final PagedCallSettings\n" + + " pagedExpandSettings;\n" + + " private final UnaryCallSettings waitSettings;\n" + + " private final OperationCallSettings\n" + + " waitOperationSettings;\n" + + " private final UnaryCallSettings blockSettings;\n" + "\n" + " public static class Builder {}\n" + "}\n"; From f0a851ac6c24bdd699697f01d9e8559b53fdef94 Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Fri, 21 Aug 2020 14:18:20 -0700 Subject: [PATCH 04/20] feat: add settings getters to ServiceStubSettings codegen --- .../ServiceStubSettingsClassComposer.java | 23 ++++++++++- .../ServiceStubSettingsClassComposerTest.java | 40 +++++++++++++++++++ 2 files changed, 61 insertions(+), 2 deletions(-) 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 35f5c2cfd1..da60e6aad7 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 @@ -88,6 +88,7 @@ import java.util.LinkedHashMap; import java.util.List; import java.util.Map; +import java.util.function.Function; import java.util.stream.Collectors; import javax.annotation.Generated; import org.threeten.bp.Duration; @@ -130,7 +131,7 @@ public GapicClass generate( .setName(className) .setExtendsType(createExtendsType(service, types)) .setStatements(createClassStatements(service, methodSettingsMemberVarExprs, types)) - .setMethods(createClassMethods(service, types)) + .setMethods(createClassMethods(service, methodSettingsMemberVarExprs, types)) .setNestedClasses(Arrays.asList(createNestedBuilderClass(service, types))) .build(); return GapicClass.create(GapicClass.Kind.STUB, classDef); @@ -241,12 +242,30 @@ private static List createClassStatements( } private static List createClassMethods( - Service service, Map types) { + Service service, + Map methodSettingsMemberVarExprs, + Map types) { List javaMethods = new ArrayList<>(); + javaMethods.addAll(createMethodSettingsGetterMethods(methodSettingsMemberVarExprs)); // TODO(miraleung): Fill this out. return javaMethods; } + private static List createMethodSettingsGetterMethods( + Map methodSettingsMemberVarExprs) { + Function, MethodDefinition> varToMethodFn = + e -> + MethodDefinition.builder() + .setScope(ScopeNode.PUBLIC) + .setReturnType(e.getValue().type()) + .setName(e.getKey()) + .setReturnExpr(e.getValue()) + .build(); + return methodSettingsMemberVarExprs.entrySet().stream() + .map(e -> varToMethodFn.apply(e)) + .collect(Collectors.toList()); + } + private static ClassDefinition createNestedBuilderClass( Service service, Map types) { String thisClassName = getThisClassName(service.name()); diff --git a/src/test/java/com/google/api/generator/gapic/composer/ServiceStubSettingsClassComposerTest.java b/src/test/java/com/google/api/generator/gapic/composer/ServiceStubSettingsClassComposerTest.java index 14ebd09ceb..19bc536664 100644 --- a/src/test/java/com/google/api/generator/gapic/composer/ServiceStubSettingsClassComposerTest.java +++ b/src/test/java/com/google/api/generator/gapic/composer/ServiceStubSettingsClassComposerTest.java @@ -123,6 +123,46 @@ public void generateServiceClasses() { + " waitOperationSettings;\n" + " private final UnaryCallSettings blockSettings;\n" + "\n" + + " public UnaryCallSettings echoSettings() {\n" + + " return echoSettings;\n" + + " }\n" + + "\n" + + " public ServerStreamingCallSettings expandSettings()" + + " {\n" + + " return expandSettings;\n" + + " }\n" + + "\n" + + " public StreamingCallSettings collectSettings() {\n" + + " return collectSettings;\n" + + " }\n" + + "\n" + + " public StreamingCallSettings chatSettings() {\n" + + " return chatSettings;\n" + + " }\n" + + "\n" + + " public StreamingCallSettings chatAgainSettings() {\n" + + " return chatAgainSettings;\n" + + " }\n" + + "\n" + + " public PagedCallSettings\n" + + " pagedExpandSettings() {\n" + + " return pagedExpandSettings;\n" + + " }\n" + + "\n" + + " public UnaryCallSettings waitSettings() {\n" + + " return waitSettings;\n" + + " }\n" + + "\n" + + " public OperationCallSettings" + + " waitOperationSettings() {\n" + + " return waitOperationSettings;\n" + + " }\n" + + "\n" + + " public UnaryCallSettings blockSettings() {\n" + + " return blockSettings;\n" + + " }\n" + + "\n" + " public static class Builder {}\n" + "}\n"; } From 8448c3be7b163d3af8723a3373bb86df71351f54 Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Fri, 21 Aug 2020 15:31:43 -0700 Subject: [PATCH 05/20] fix!: Refactor ThrowExpr msgs to String-typed exprs --- .../api/generator/engine/ast/ThrowExpr.java | 26 ++++++++++----- .../engine/writer/ImportWriterVisitor.java | 5 +++ .../engine/writer/JavaWriterVisitor.java | 8 ++--- .../ResourceNameHelperClassComposer.java | 2 +- .../composer/ServiceStubClassComposer.java | 2 +- .../generator/engine/ast/ThrowExprTest.java | 31 ++++++++++++++---- .../writer/ImportWriterVisitorTest.java | 32 ++++++++++++++++++- .../engine/writer/JavaWriterVisitorTest.java | 20 ++++++++++-- 8 files changed, 101 insertions(+), 25 deletions(-) diff --git a/src/main/java/com/google/api/generator/engine/ast/ThrowExpr.java b/src/main/java/com/google/api/generator/engine/ast/ThrowExpr.java index d173bb0244..805499b074 100644 --- a/src/main/java/com/google/api/generator/engine/ast/ThrowExpr.java +++ b/src/main/java/com/google/api/generator/engine/ast/ThrowExpr.java @@ -26,7 +26,7 @@ public abstract class ThrowExpr implements Expr { public abstract TypeNode type(); @Nullable - public abstract String message(); + public abstract Expr messageExpr(); @Override public void accept(AstNodeVisitor visitor) { @@ -41,19 +41,29 @@ public static Builder builder() { public abstract static class Builder { public abstract Builder setType(TypeNode type); - public abstract Builder setMessage(String message); + public Builder setMessageExpr(String message) { + return setMessageExpr(ValueExpr.withValue(StringObjectValue.withValue(message))); + } + + public abstract Builder setMessageExpr(Expr expr); // Private. + abstract TypeNode type(); + + abstract Expr messageExpr(); + abstract ThrowExpr autoBuild(); public ThrowExpr build() { - // TODO(miraleung): Escaping message() and the corresponding tests will be done when we switch - // to StringObjectValue. - ThrowExpr throwExpr = autoBuild(); Preconditions.checkState( - TypeNode.isExceptionType(throwExpr.type()), - String.format("Type %s must be an exception type", throwExpr.type())); - return throwExpr; + TypeNode.isExceptionType(type()), + String.format("Type %s must be an exception type", type())); + if (messageExpr() != null) { + Preconditions.checkState( + messageExpr().type().equals(TypeNode.STRING), + String.format("Message expression type must be a string for exception %s", type())); + } + return autoBuild(); } } } 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 6e97990f09..409029b2a0 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 @@ -146,6 +146,8 @@ public void visit(AssignmentExpr assignmentExpr) { @Override public void visit(MethodInvocationExpr methodInvocationExpr) { + // May not actually be used in source, but import it anyway. Unused imports will be removed by + // the formatter. methodInvocationExpr.returnType().accept(this); if (methodInvocationExpr.staticReferenceType() != null) { methodInvocationExpr.staticReferenceType().accept(this); @@ -173,6 +175,9 @@ public void visit(AnonymousClassExpr anonymousClassExpr) { @Override public void visit(ThrowExpr throwExpr) { throwExpr.type().accept(this); + if (throwExpr.messageExpr() != null) { + throwExpr.messageExpr().accept(this); + } } @Override 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 29ded3474b..6d4d19d345 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 @@ -49,7 +49,6 @@ import com.google.api.generator.engine.ast.Variable; import com.google.api.generator.engine.ast.VariableExpr; import com.google.api.generator.engine.ast.WhileStatement; -import com.google.common.base.Strings; import java.util.Arrays; import java.util.Iterator; import java.util.List; @@ -304,11 +303,8 @@ public void visit(ThrowExpr throwExpr) { space(); throwExpr.type().accept(this); leftParen(); - if (!Strings.isNullOrEmpty(throwExpr.message())) { - // TODO(miraleung): Update this when we use StringObjectValue. - buffer.append(ESCAPED_QUOTE); - buffer.append(throwExpr.message()); - buffer.append(ESCAPED_QUOTE); + if (throwExpr.messageExpr() != null) { + throwExpr.messageExpr().accept(this); } rightParen(); } diff --git a/src/main/java/com/google/api/generator/gapic/composer/ResourceNameHelperClassComposer.java b/src/main/java/com/google/api/generator/gapic/composer/ResourceNameHelperClassComposer.java index edef7a7dc2..e6f2a53e4f 100644 --- a/src/main/java/com/google/api/generator/gapic/composer/ResourceNameHelperClassComposer.java +++ b/src/main/java/com/google/api/generator/gapic/composer/ResourceNameHelperClassComposer.java @@ -701,7 +701,7 @@ private static MethodDefinition createParseMethod( ExprStatement.withExpr( ThrowExpr.builder() .setType(STATIC_TYPES.get("ValidationException")) - .setMessage(exceptionMessageString) + .setMessageExpr(exceptionMessageString) .build())); return MethodDefinition.builder() .setScope(ScopeNode.PUBLIC) diff --git a/src/main/java/com/google/api/generator/gapic/composer/ServiceStubClassComposer.java b/src/main/java/com/google/api/generator/gapic/composer/ServiceStubClassComposer.java index a8d76e5750..f2998431ae 100644 --- a/src/main/java/com/google/api/generator/gapic/composer/ServiceStubClassComposer.java +++ b/src/main/java/com/google/api/generator/gapic/composer/ServiceStubClassComposer.java @@ -244,7 +244,7 @@ private static List createThrowUOEBody( ExprStatement.withExpr( ThrowExpr.builder() .setType(types.get("UnsupportedOperationException")) - .setMessage(String.format("Not implemented: %s()", methodName)) + .setMessageExpr(String.format("Not implemented: %s()", methodName)) .build())); } } diff --git a/src/test/java/com/google/api/generator/engine/ast/ThrowExprTest.java b/src/test/java/com/google/api/generator/engine/ast/ThrowExprTest.java index 69d17ceeb7..48202e8552 100644 --- a/src/test/java/com/google/api/generator/engine/ast/ThrowExprTest.java +++ b/src/test/java/com/google/api/generator/engine/ast/ThrowExprTest.java @@ -21,20 +21,29 @@ public class ThrowExprTest { @Test public void createThrowExpr_basic() { - TypeNode npeType = - TypeNode.withReference(ConcreteReference.withClazz(NullPointerException.class)); + TypeNode npeType = TypeNode.withExceptionClazz(NullPointerException.class); ThrowExpr.builder().setType(npeType).build(); // No exception thrown, we're good. } @Test - public void createThrowExpr_basicWithMessage() { - TypeNode npeType = - TypeNode.withReference(ConcreteReference.withClazz(NullPointerException.class)); - ThrowExpr.builder().setType(npeType).setMessage("Some message").build(); + public void createThrowExpr_basicWithStringMessage() { + TypeNode npeType = TypeNode.withExceptionClazz(NullPointerException.class); + ThrowExpr.builder().setType(npeType).setMessageExpr("Some message").build(); // No exception thrown, we're good. + } + @Test + public void createThrowExpr_messageExpr() { + TypeNode npeType = TypeNode.withExceptionClazz(NullPointerException.class); + Expr messageExpr = + MethodInvocationExpr.builder() + .setMethodName("foobar") + .setReturnType(TypeNode.STRING) + .build(); + ThrowExpr.builder().setType(npeType).setMessageExpr(messageExpr).build(); + // No exception thrown, we're good. } @Test @@ -43,4 +52,14 @@ public void createThrowExpr_badExceptionType() { assertThrows( IllegalStateException.class, () -> ThrowExpr.builder().setType(nonExceptionType).build()); } + + @Test + public void createThrowExpr_badMessageExpr() { + TypeNode npeType = TypeNode.withExceptionClazz(NullPointerException.class); + Expr messageExpr = + MethodInvocationExpr.builder().setMethodName("foobar").setReturnType(TypeNode.INT).build(); + assertThrows( + IllegalStateException.class, + () -> ThrowExpr.builder().setType(npeType).setMessageExpr(messageExpr).build()); + } } 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 023b170110..4c74598ee5 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 @@ -27,6 +27,7 @@ import com.google.api.generator.engine.ast.EnumRefExpr; import com.google.api.generator.engine.ast.Expr; import com.google.api.generator.engine.ast.ExprStatement; +import com.google.api.generator.engine.ast.IfStatement; import com.google.api.generator.engine.ast.InstanceofExpr; import com.google.api.generator.engine.ast.MethodDefinition; import com.google.api.generator.engine.ast.MethodInvocationExpr; @@ -589,11 +590,40 @@ public void writeThrowExprImports_basic() { TypeNode exceptionTypes = TypeNode.withReference(ConcreteReference.withClazz(IOException.class)); String message = "Some message asdf"; - ThrowExpr throwExpr = ThrowExpr.builder().setType(exceptionTypes).setMessage(message).build(); + ThrowExpr throwExpr = + ThrowExpr.builder().setType(exceptionTypes).setMessageExpr(message).build(); throwExpr.accept(writerVisitor); assertEquals(writerVisitor.write(), "import java.io.IOException;\n\n"); } + @Test + public void writeThrowExprImports_messageExpr() { + TypeNode npeType = TypeNode.withExceptionClazz(NullPointerException.class); + Expr messageExpr = + MethodInvocationExpr.builder() + .setStaticReferenceType( + TypeNode.withReference(ConcreteReference.withClazz(IfStatement.class))) + .setMethodName("conditionExpr") + .setReturnType(TypeNode.withReference(ConcreteReference.withClazz(Expr.class))) + .build(); + + messageExpr = + MethodInvocationExpr.builder() + .setExprReferenceExpr(messageExpr) + .setMethodName("foobar") + .setReturnType(TypeNode.STRING) + .build(); + ThrowExpr throwExpr = ThrowExpr.builder().setType(npeType).setMessageExpr(messageExpr).build(); + + throwExpr.accept(writerVisitor); + assertEquals( + writerVisitor.write(), + String.format( + createLines(2), + "import com.google.api.generator.engine.ast.Expr;\n", + "import com.google.api.generator.engine.ast.IfStatement;\n\n")); + } + @Test public void writeInstanceofExprImports_basic() { TypeNode exprType = TypeNode.withReference(ConcreteReference.withClazz(Expr.class)); 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 3cc2beac3e..c46bfd1878 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 @@ -435,7 +435,9 @@ public void writeBlockComment_shortLines() { @Test public void writeBlockComment_newLineInBetween() { String content = - "Apache License \nLicensed under the Apache License, Version 2.0 (the \"License\");\n\nyou may not use this file except in compliance with the License."; + "Apache License \n" + + "Licensed under the Apache License, Version 2.0 (the \"License\");\n\n" + + "you may not use this file except in compliance with the License."; BlockComment blockComment = BlockComment.builder().setComment(content).build(); String expected = String.format( @@ -921,11 +923,25 @@ public void writeThrowExpr_basicWithMessage() { TypeNode npeType = TypeNode.withReference(ConcreteReference.withClazz(NullPointerException.class)); String message = "Some message asdf"; - ThrowExpr throwExpr = ThrowExpr.builder().setType(npeType).setMessage(message).build(); + ThrowExpr throwExpr = ThrowExpr.builder().setType(npeType).setMessageExpr(message).build(); throwExpr.accept(writerVisitor); assertEquals(writerVisitor.write(), "throw new NullPointerException(\"Some message asdf\")"); } + @Test + public void writeThrowExpr_messageExpr() { + TypeNode npeType = TypeNode.withExceptionClazz(NullPointerException.class); + Expr messageExpr = + MethodInvocationExpr.builder() + .setMethodName("foobar") + .setReturnType(TypeNode.STRING) + .build(); + ThrowExpr throwExpr = ThrowExpr.builder().setType(npeType).setMessageExpr(messageExpr).build(); + + throwExpr.accept(writerVisitor); + assertEquals(writerVisitor.write(), "throw new NullPointerException(foobar())"); + } + @Test public void writeInstanceofExpr() { Variable variable = Variable.builder().setName("x").setType(TypeNode.STRING).build(); From e9f96c1eeeb59173b0d2b9d3e22a32a8c0bc3b98 Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Fri, 21 Aug 2020 15:34:28 -0700 Subject: [PATCH 06/20] feat: add createStub to ServiceStubSettings codegen --- .../ServiceStubSettingsClassComposer.java | 108 +++++++++++++++++- .../ServiceStubSettingsClassComposerTest.java | 16 +++ 2 files changed, 123 insertions(+), 1 deletion(-) 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 da60e6aad7..f06579bc35 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 @@ -57,12 +57,16 @@ import com.google.api.generator.engine.ast.ConcreteReference; import com.google.api.generator.engine.ast.Expr; import com.google.api.generator.engine.ast.ExprStatement; +import com.google.api.generator.engine.ast.IfStatement; import com.google.api.generator.engine.ast.MethodDefinition; import com.google.api.generator.engine.ast.MethodInvocationExpr; import com.google.api.generator.engine.ast.Reference; +import com.google.api.generator.engine.ast.ReturnExpr; import com.google.api.generator.engine.ast.ScopeNode; import com.google.api.generator.engine.ast.Statement; import com.google.api.generator.engine.ast.StringObjectValue; +import com.google.api.generator.engine.ast.ThisObjectValue; +import com.google.api.generator.engine.ast.ThrowExpr; import com.google.api.generator.engine.ast.TypeNode; import com.google.api.generator.engine.ast.ValueExpr; import com.google.api.generator.engine.ast.VaporReference; @@ -97,6 +101,8 @@ public class ServiceStubSettingsClassComposer { private static final String CLASS_NAME_PATTERN = "%sStubSettings"; private static final String PAGED_RESPONSE_TYPE_NAME_PATTERN = "%sPagedResponse"; + private static final String GRPC_SERVICE_STUB_PATTERN = "Grpc%sStub"; + private static final String STUB_PATTERN = "%sStub"; private static final String LEFT_BRACE = "{"; private static final String RIGHT_BRACE = "}"; @@ -247,6 +253,7 @@ private static List createClassMethods( Map types) { List javaMethods = new ArrayList<>(); javaMethods.addAll(createMethodSettingsGetterMethods(methodSettingsMemberVarExprs)); + javaMethods.add(createCreateStubMethod(service, types)); // TODO(miraleung): Fill this out. return javaMethods; } @@ -266,6 +273,80 @@ private static List createMethodSettingsGetterMethods( .collect(Collectors.toList()); } + private static MethodDefinition createCreateStubMethod( + Service service, Map types) { + // Set up the if-statement. + Expr grpcTransportNameExpr = + MethodInvocationExpr.builder() + .setStaticReferenceType(STATIC_TYPES.get("GrpcTransportChannel")) + .setMethodName("getGrpcTransportName") + .build(); + + Expr getTransportNameExpr = + MethodInvocationExpr.builder().setMethodName("getTransportChannelProvider").build(); + getTransportNameExpr = + MethodInvocationExpr.builder() + .setExprReferenceExpr(getTransportNameExpr) + .setMethodName("getTransportName") + .build(); + + Expr ifConditionExpr = + MethodInvocationExpr.builder() + .setExprReferenceExpr(getTransportNameExpr) + .setMethodName("equals") + .setArguments(grpcTransportNameExpr) + .setReturnType(TypeNode.BOOLEAN) + .build(); + + Expr createExpr = + MethodInvocationExpr.builder() + .setStaticReferenceType(types.get(getGrpcServiceStubTypeName(service.name()))) + .setMethodName("create") + .setArguments( + ValueExpr.withValue( + ThisObjectValue.withType(types.get(getThisClassName(service.name()))))) + .build(); + + IfStatement ifStatement = + IfStatement.builder() + .setConditionExpr(ifConditionExpr) + .setBody(Arrays.asList(ExprStatement.withExpr(ReturnExpr.withExpr(createExpr)))) + .build(); + + // Set up exception throwing. + Expr errorMessageExpr = + MethodInvocationExpr.builder() + .setStaticReferenceType(TypeNode.STRING) + .setMethodName("format") + .setArguments( + ValueExpr.withValue(StringObjectValue.withValue("Transport not supported: %s")), + getTransportNameExpr) + .setReturnType(TypeNode.STRING) + .build(); + TypeNode exceptionType = TypeNode.withExceptionClazz(UnsupportedOperationException.class); + Statement throwStatement = + ExprStatement.withExpr( + ThrowExpr.builder().setType(exceptionType).setMessageExpr(errorMessageExpr).build()); + + // Put the method together. + TypeNode returnType = types.get(getServiceStubTypeName(service.name())); + AnnotationNode annotation = + AnnotationNode.builder() + .setType(STATIC_TYPES.get("BetaApi")) + .setDescription( + "A restructuring of stub classes is planned, so this may break in the future") + .build(); + + return MethodDefinition.builder() + .setAnnotations(Arrays.asList(annotation)) + .setScope(ScopeNode.PUBLIC) + .setReturnType(returnType) + .setName("createStub") + .setThrowsExceptions(Arrays.asList(TypeNode.withExceptionClazz(IOException.class))) + .setBody(Arrays.asList(ifStatement, throwStatement)) + .build(); + } + private static ClassDefinition createNestedBuilderClass( Service service, Map types) { String thisClassName = getThisClassName(service.name()); @@ -342,10 +423,14 @@ private static Map createStaticTypes() { private static Map createDynamicTypes(Service service, String pakkage) { String thisClassName = getThisClassName(service.name()); Map dynamicTypes = new HashMap<>(); + + // This type. dynamicTypes.put( thisClassName, TypeNode.withReference( VaporReference.builder().setName(thisClassName).setPakkage(pakkage).build())); + + // Nested builder class. dynamicTypes.put( "Builder", TypeNode.withReference( @@ -356,6 +441,19 @@ private static Map createDynamicTypes(Service service, String .setIsStaticImport(true) .build())); + // Other generated stub classes. + dynamicTypes.putAll( + Arrays.asList(GRPC_SERVICE_STUB_PATTERN, STUB_PATTERN).stream() + .collect( + Collectors.toMap( + p -> String.format(p, service.name()), + p -> + TypeNode.withReference( + VaporReference.builder() + .setName(String.format(p, service.name())) + .setPakkage(pakkage) + .build())))); + // Pagination types. dynamicTypes.putAll( service.methods().stream() @@ -392,7 +490,15 @@ private static String getThisClassName(String serviceName) { } private static String getPagedResponseTypeName(String methodName) { - return String.format(PAGED_RESPONSE_TYPE_NAME_PATTERN, methodName); + return String.format(PAGED_RESPONSE_TYPE_NAME_PATTERN, JavaStyle.toUpperCamelCase(methodName)); + } + + private static String getServiceStubTypeName(String serviceName) { + return String.format(STUB_PATTERN, JavaStyle.toUpperCamelCase(serviceName)); + } + + private static String getGrpcServiceStubTypeName(String serviceName) { + return String.format(GRPC_SERVICE_STUB_PATTERN, JavaStyle.toUpperCamelCase(serviceName)); } private static TypeNode getCallSettingsType(Method method, Map types) { diff --git a/src/test/java/com/google/api/generator/gapic/composer/ServiceStubSettingsClassComposerTest.java b/src/test/java/com/google/api/generator/gapic/composer/ServiceStubSettingsClassComposerTest.java index 19bc536664..7769713886 100644 --- a/src/test/java/com/google/api/generator/gapic/composer/ServiceStubSettingsClassComposerTest.java +++ b/src/test/java/com/google/api/generator/gapic/composer/ServiceStubSettingsClassComposerTest.java @@ -83,6 +83,7 @@ public void generateServiceClasses() { + "import static com.google.showcase.v1beta1.EchoClient.PagedExpandPagedResponse;\n" + "\n" + "import com.google.api.core.BetaApi;\n" + + "import com.google.api.gax.grpc.GrpcTransportChannel;\n" + "import com.google.api.gax.rpc.OperationCallSettings;\n" + "import com.google.api.gax.rpc.PagedCallSettings;\n" + "import com.google.api.gax.rpc.ServerStreamingCallSettings;\n" @@ -101,6 +102,7 @@ public void generateServiceClasses() { + "import com.google.showcase.v1beta1.WaitMetadata;\n" + "import com.google.showcase.v1beta1.WaitRequest;\n" + "import com.google.showcase.v1beta1.WaitResponse;\n" + + "import java.io.IOException;\n" + "import javax.annotation.Generated;\n" + "\n" + "@BetaApi\n" @@ -163,6 +165,20 @@ public void generateServiceClasses() { + " return blockSettings;\n" + " }\n" + "\n" + + " @BetaApi(\"A restructuring of stub classes is planned, so this may break in the" + + " future\")\n" + + " public EchoStub createStub() throws IOException {\n" + + " if (getTransportChannelProvider()\n" + + " .getTransportName()\n" + + " .equals(GrpcTransportChannel.getGrpcTransportName())) {\n" + + " return GrpcEchoStub.create(this);\n" + + " }\n" + + " throw new UnsupportedOperationException(\n" + + " String.format(\n" + + " \"Transport not supported: %s\"," + + " getTransportChannelProvider().getTransportName()));\n" + + " }\n" + + "\n" + " public static class Builder {}\n" + "}\n"; } From 6e049c015aa2834037c91c27e279794aa6ce2cac Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Fri, 21 Aug 2020 15:58:14 -0700 Subject: [PATCH 07/20] feat: add default* helperMethods to ServiceStubSettings codegen --- .../ServiceStubSettingsClassComposer.java | 53 +++++++++++++++++++ .../ServiceStubSettingsClassComposerTest.java | 15 ++++++ 2 files changed, 68 insertions(+) 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 f06579bc35..315434aab5 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 @@ -254,6 +254,7 @@ private static List createClassMethods( List javaMethods = new ArrayList<>(); javaMethods.addAll(createMethodSettingsGetterMethods(methodSettingsMemberVarExprs)); javaMethods.add(createCreateStubMethod(service, types)); + javaMethods.addAll(createDefaultHelperAndGetterMethods(service, types)); // TODO(miraleung): Fill this out. return javaMethods; } @@ -347,6 +348,58 @@ private static MethodDefinition createCreateStubMethod( .build(); } + private static List createDefaultHelperAndGetterMethods( + Service service, Map types) { + List javaMethods = new ArrayList<>(); + + // Create the defaultExecutorProviderBuilder method. + TypeNode returnType = + TypeNode.withReference( + ConcreteReference.withClazz(InstantiatingExecutorProvider.Builder.class)); + javaMethods.add( + MethodDefinition.builder() + .setScope(ScopeNode.PUBLIC) + .setIsStatic(true) + .setReturnType(returnType) + .setName("defaultExecutorProviderBuilder") + .setReturnExpr( + MethodInvocationExpr.builder() + .setStaticReferenceType(STATIC_TYPES.get("InstantiatingExecutorProvider")) + .setMethodName("newBuilder") + .setReturnType(returnType) + .build()) + .build()); + + // Create the getDefaultEndpoint method. + returnType = TypeNode.STRING; + javaMethods.add( + MethodDefinition.builder() + .setScope(ScopeNode.PUBLIC) + .setIsStatic(true) + .setReturnType(returnType) + .setName("getDefaultEndpoint") + .setReturnExpr(ValueExpr.withValue(StringObjectValue.withValue(service.defaultHost()))) + .build()); + + // Create the getDefaultServiceScopes method. + returnType = + TypeNode.withReference( + ConcreteReference.builder() + .setClazz(List.class) + .setGenerics(Arrays.asList(TypeNode.STRING.reference())) + .build()); + javaMethods.add( + MethodDefinition.builder() + .setScope(ScopeNode.PUBLIC) + .setIsStatic(true) + .setReturnType(returnType) + .setName("getDefaultServiceScopes") + .setReturnExpr(DEFAULT_SERVICE_SCOPES_VAR_EXPR) + .build()); + + return javaMethods; + } + private static ClassDefinition createNestedBuilderClass( Service service, Map types) { String thisClassName = getThisClassName(service.name()); diff --git a/src/test/java/com/google/api/generator/gapic/composer/ServiceStubSettingsClassComposerTest.java b/src/test/java/com/google/api/generator/gapic/composer/ServiceStubSettingsClassComposerTest.java index 7769713886..2d960ffa38 100644 --- a/src/test/java/com/google/api/generator/gapic/composer/ServiceStubSettingsClassComposerTest.java +++ b/src/test/java/com/google/api/generator/gapic/composer/ServiceStubSettingsClassComposerTest.java @@ -83,6 +83,7 @@ public void generateServiceClasses() { + "import static com.google.showcase.v1beta1.EchoClient.PagedExpandPagedResponse;\n" + "\n" + "import com.google.api.core.BetaApi;\n" + + "import com.google.api.gax.core.InstantiatingExecutorProvider;\n" + "import com.google.api.gax.grpc.GrpcTransportChannel;\n" + "import com.google.api.gax.rpc.OperationCallSettings;\n" + "import com.google.api.gax.rpc.PagedCallSettings;\n" @@ -103,6 +104,7 @@ public void generateServiceClasses() { + "import com.google.showcase.v1beta1.WaitRequest;\n" + "import com.google.showcase.v1beta1.WaitResponse;\n" + "import java.io.IOException;\n" + + "import java.util.List;\n" + "import javax.annotation.Generated;\n" + "\n" + "@BetaApi\n" @@ -179,6 +181,19 @@ public void generateServiceClasses() { + " getTransportChannelProvider().getTransportName()));\n" + " }\n" + "\n" + + " public static InstantiatingExecutorProvider.Builder" + + " defaultExecutorProviderBuilder() {\n" + + " return InstantiatingExecutorProvider.newBuilder();\n" + + " }\n" + + "\n" + + " public static String getDefaultEndpoint() {\n" + + " return \"localhost:7469\";\n" + + " }\n" + + "\n" + + " public static List getDefaultServiceScopes() {\n" + + " return DEFAULT_SERVICE_SCOPES;\n" + + " }\n" + + "\n" + " public static class Builder {}\n" + "}\n"; } From 36308fd27685a467385315aa4aadce5272b6e6c9 Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Fri, 21 Aug 2020 16:25:41 -0700 Subject: [PATCH 08/20] feat: add static refs to VariableExpr --- .../generator/engine/ast/VariableExpr.java | 19 +++++++++++ .../engine/writer/ImportWriterVisitor.java | 3 ++ .../engine/writer/JavaWriterVisitor.java | 12 +++++-- .../engine/ast/VariableExprTest.java | 32 +++++++++++++++++++ .../writer/ImportWriterVisitorTest.java | 23 +++++++++++++ .../engine/writer/JavaWriterVisitorTest.java | 15 ++++++++- 6 files changed, 100 insertions(+), 4 deletions(-) diff --git a/src/main/java/com/google/api/generator/engine/ast/VariableExpr.java b/src/main/java/com/google/api/generator/engine/ast/VariableExpr.java index 1999947437..d7ce91e98c 100644 --- a/src/main/java/com/google/api/generator/engine/ast/VariableExpr.java +++ b/src/main/java/com/google/api/generator/engine/ast/VariableExpr.java @@ -28,6 +28,9 @@ public abstract class VariableExpr implements Expr { @Nullable public abstract Expr exprReferenceExpr(); + @Nullable + public abstract TypeNode staticReferenceType(); + /** Variable declaration fields. */ public abstract boolean isDecl(); @@ -85,6 +88,9 @@ public abstract static class Builder { // Optional, but cannot co-exist with a variable declaration. public abstract Builder setExprReferenceExpr(Expr exprReference); + // Optional, but cannot co-exist with an expression reference. + public abstract Builder setStaticReferenceType(TypeNode type); + public abstract Builder setIsDecl(boolean isDecl); public abstract Builder setScope(ScopeNode scope); @@ -126,11 +132,24 @@ public VariableExpr build() { variableExpr.isDecl() ^ (variableExpr.exprReferenceExpr() != null), "Variable references cannot be declared"); } + + Preconditions.checkState( + variableExpr.exprReferenceExpr() == null || variableExpr.staticReferenceType() == null, + "Only the expression reference or the static reference can be set, not both"); + if (variableExpr.exprReferenceExpr() != null) { Preconditions.checkState( TypeNode.isReferenceType(variableExpr.exprReferenceExpr().type()), "Variables can only be referenced from object types"); } + if (variableExpr.staticReferenceType() != null) { + Preconditions.checkState( + TypeNode.isReferenceType(variableExpr.staticReferenceType()), + String.format( + "Static field references can only be done on static types, but instead found %s", + variableExpr.staticReferenceType())); + } + return variableExpr; } } 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 409029b2a0..f8b16a57b3 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 @@ -135,6 +135,9 @@ public void visit(VariableExpr variableExpr) { if (variableExpr.exprReferenceExpr() != null) { variableExpr.exprReferenceExpr().accept(this); } + if (variableExpr.staticReferenceType() != null) { + variableExpr.staticReferenceType().accept(this); + } variableExpr.templateNodes().stream().forEach(n -> n.accept(this)); } 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 6d4d19d345..b246e7234f 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 @@ -201,9 +201,15 @@ public void visit(VariableExpr variableExpr) { rightAngle(); } space(); - } else if (variableExpr.exprReferenceExpr() != null) { - variableExpr.exprReferenceExpr().accept(this); - buffer.append(DOT); + } else { + // Expression or static reference. + if (variableExpr.exprReferenceExpr() != null) { + variableExpr.exprReferenceExpr().accept(this); + buffer.append(DOT); + } else if (variableExpr.staticReferenceType() != null) { + variableExpr.staticReferenceType().accept(this); + buffer.append(DOT); + } } variable.identifier().accept(this); diff --git a/src/test/java/com/google/api/generator/engine/ast/VariableExprTest.java b/src/test/java/com/google/api/generator/engine/ast/VariableExprTest.java index d243959167..75c8241ed8 100644 --- a/src/test/java/com/google/api/generator/engine/ast/VariableExprTest.java +++ b/src/test/java/com/google/api/generator/engine/ast/VariableExprTest.java @@ -34,6 +34,14 @@ public void validVariableExpr_basic() { assertThat(variableExpr.scope()).isEqualTo(ScopeNode.LOCAL); } + @Test + public void validVariableExpr_staticReference() { + VariableExpr.builder() + .setVariable(Variable.builder().setType(TypeNode.INT).setName("MAX_VALUE").build()) + .setStaticReferenceType(TypeNode.INT_OBJECT) + .build(); + } + @Test public void validVariableExpr_withFields() { Variable variable = Variable.builder().setName("x").setType(TypeNode.STRING).build(); @@ -195,4 +203,28 @@ public void invalidVariableExpr_referenceAndDecl() { .setExprReferenceExpr(variableExpr) .build()); } + + @Test + public void invalidVariableExpr_exprAndStaticReference() { + Variable refVariable = Variable.builder().setName("x").setType(TypeNode.STRING_ARRAY).build(); + assertThrows( + IllegalStateException.class, + () -> + VariableExpr.builder() + .setVariable(Variable.builder().setType(TypeNode.INT).setName("MAX_VALUE").build()) + .setExprReferenceExpr(VariableExpr.withVariable(refVariable)) + .setStaticReferenceType(TypeNode.INT_OBJECT) + .build()); + } + + @Test + public void invalidVariableExpr_primitiveStaticReference() { + assertThrows( + IllegalStateException.class, + () -> + VariableExpr.builder() + .setVariable(Variable.builder().setType(TypeNode.INT).setName("MAX_VALUE").build()) + .setStaticReferenceType(TypeNode.INT) + .build()); + } } 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 4c74598ee5..815588b8b4 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 @@ -450,6 +450,29 @@ public void writeVariableExprImports_basic() { String.format(createLines(1), "import com.google.api.generator.engine.ast.Expr;\n\n")); } + @Test + public void writeVariableExprImports_staticReference() { + VariableExpr variableExpr = + VariableExpr.builder() + .setVariable( + Variable.builder() + .setType( + TypeNode.withReference(ConcreteReference.withClazz(AssignmentExpr.class))) + .setName("AN_ASSIGN_EXPR") + .build()) + .setStaticReferenceType( + TypeNode.withReference(ConcreteReference.withClazz(TypeNode.class))) + .build(); + + variableExpr.accept(writerVisitor); + assertEquals( + writerVisitor.write(), + String.format( + createLines(2), + "import com.google.api.generator.engine.ast.AssignmentExpr;\n", + "import com.google.api.generator.engine.ast.TypeNode;\n\n")); + } + @Test public void writeVariableExprImports_reference() { Variable variable = 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 c46bfd1878..9b38d81392 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 @@ -196,7 +196,7 @@ public void writeValueExpr() { } @Test - public void writeVariableExpr() { + public void writeVariableExpr_basic() { Variable variable = Variable.builder().setName("x").setType(TypeNode.INT).build(); VariableExpr variableExpr = VariableExpr.builder().setVariable(variable).build(); @@ -204,6 +204,19 @@ public void writeVariableExpr() { assertEquals(writerVisitor.write(), "x"); } + @Test + public void writeVariableExpr_staticReference() { + VariableExpr variableExpr = + VariableExpr.builder() + .setVariable( + Variable.builder().setType(TypeNode.INT_OBJECT).setName("MAX_VALUE").build()) + .setStaticReferenceType(TypeNode.INT_OBJECT) + .build(); + + variableExpr.accept(writerVisitor); + assertEquals(writerVisitor.write(), "Integer.MAX_VALUE"); + } + @Test public void writeVariableExpr_nonDeclIgnoresModifiers() { Variable variable = Variable.builder().setName("x").setType(TypeNode.BOOLEAN).build(); From f28b8177c421f18789059d27a8d81fa212e14051 Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Fri, 21 Aug 2020 16:26:18 -0700 Subject: [PATCH 09/20] feat: add more default* helpers to ServiceStubSettings codegen --- .../ServiceStubSettingsClassComposer.java | 74 +++++++++++++++++++ .../ServiceStubSettingsClassComposerTest.java | 19 +++++ 2 files changed, 93 insertions(+) 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 315434aab5..4a472db5b5 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 @@ -397,6 +397,80 @@ private static List createDefaultHelperAndGetterMethods( .setReturnExpr(DEFAULT_SERVICE_SCOPES_VAR_EXPR) .build()); + // Create the defaultCredentialsProviderBuilder method. + returnType = + TypeNode.withReference( + ConcreteReference.withClazz(GoogleCredentialsProvider.Builder.class)); + MethodInvocationExpr credsProviderBuilderExpr = + MethodInvocationExpr.builder() + .setStaticReferenceType(STATIC_TYPES.get("GoogleCredentialsProvider")) + .setMethodName("newBuilder") + .build(); + credsProviderBuilderExpr = + MethodInvocationExpr.builder() + .setExprReferenceExpr(credsProviderBuilderExpr) + .setMethodName("setScopesToApply") + .setArguments(DEFAULT_SERVICE_SCOPES_VAR_EXPR) + .setReturnType(returnType) + .build(); + javaMethods.add( + MethodDefinition.builder() + .setScope(ScopeNode.PUBLIC) + .setIsStatic(true) + .setReturnType(returnType) + .setName("defaultCredentialsProviderBuilder") + .setReturnExpr(credsProviderBuilderExpr) + .build()); + + // Create the defaultGrpcTransportProviderBuilder method. + returnType = + TypeNode.withReference( + ConcreteReference.withClazz(InstantiatingGrpcChannelProvider.Builder.class)); + MethodInvocationExpr grpcChannelProviderBuilderExpr = + MethodInvocationExpr.builder() + .setStaticReferenceType(STATIC_TYPES.get("InstantiatingGrpcChannelProvider")) + .setMethodName("newBuilder") + .build(); + grpcChannelProviderBuilderExpr = + MethodInvocationExpr.builder() + .setExprReferenceExpr(grpcChannelProviderBuilderExpr) + .setMethodName("setMaxInboundMessageSize") + .setArguments( + VariableExpr.builder() + .setVariable( + Variable.builder().setType(TypeNode.INT).setName("MAX_VALUE").build()) + .setStaticReferenceType(TypeNode.INT_OBJECT) + .build()) + .setReturnType(returnType) + .build(); + javaMethods.add( + MethodDefinition.builder() + .setScope(ScopeNode.PUBLIC) + .setIsStatic(true) + .setReturnType(returnType) + .setName("defaultGrpcTransportProviderBuilder") + .setReturnExpr(grpcChannelProviderBuilderExpr) + .build()); + + // Create the defaultTransportChannelProvider method. + returnType = STATIC_TYPES.get("TransportChannelProvider"); + MethodInvocationExpr transportProviderBuilderExpr = + MethodInvocationExpr.builder().setMethodName("defaultGrpcTransportProviderBuilder").build(); + transportProviderBuilderExpr = + MethodInvocationExpr.builder() + .setExprReferenceExpr(transportProviderBuilderExpr) + .setMethodName("build") + .setReturnType(returnType) + .build(); + javaMethods.add( + MethodDefinition.builder() + .setScope(ScopeNode.PUBLIC) + .setIsStatic(true) + .setReturnType(returnType) + .setName("defaultTransportChannelProvider") + .setReturnExpr(transportProviderBuilderExpr) + .build()); + return javaMethods; } diff --git a/src/test/java/com/google/api/generator/gapic/composer/ServiceStubSettingsClassComposerTest.java b/src/test/java/com/google/api/generator/gapic/composer/ServiceStubSettingsClassComposerTest.java index 2d960ffa38..97e27f579b 100644 --- a/src/test/java/com/google/api/generator/gapic/composer/ServiceStubSettingsClassComposerTest.java +++ b/src/test/java/com/google/api/generator/gapic/composer/ServiceStubSettingsClassComposerTest.java @@ -83,13 +83,16 @@ public void generateServiceClasses() { + "import static com.google.showcase.v1beta1.EchoClient.PagedExpandPagedResponse;\n" + "\n" + "import com.google.api.core.BetaApi;\n" + + "import com.google.api.gax.core.GoogleCredentialsProvider;\n" + "import com.google.api.gax.core.InstantiatingExecutorProvider;\n" + "import com.google.api.gax.grpc.GrpcTransportChannel;\n" + + "import com.google.api.gax.grpc.InstantiatingGrpcChannelProvider;\n" + "import com.google.api.gax.rpc.OperationCallSettings;\n" + "import com.google.api.gax.rpc.PagedCallSettings;\n" + "import com.google.api.gax.rpc.ServerStreamingCallSettings;\n" + "import com.google.api.gax.rpc.StreamingCallSettings;\n" + "import com.google.api.gax.rpc.StubSettings;\n" + + "import com.google.api.gax.rpc.TransportChannelProvider;\n" + "import com.google.api.gax.rpc.UnaryCallSettings;\n" + "import com.google.common.collect.ImmutableList;\n" + "import com.google.longrunning.Operation;\n" @@ -194,6 +197,22 @@ public void generateServiceClasses() { + " return DEFAULT_SERVICE_SCOPES;\n" + " }\n" + "\n" + + " public static GoogleCredentialsProvider.Builder defaultCredentialsProviderBuilder()" + + " {\n" + + " return" + + " GoogleCredentialsProvider.newBuilder().setScopesToApply(DEFAULT_SERVICE_SCOPES);\n" + + " }\n" + + "\n" + + " public static InstantiatingGrpcChannelProvider.Builder" + + " defaultGrpcTransportProviderBuilder() {\n" + + " return InstantiatingGrpcChannelProvider.newBuilder()\n" + + " .setMaxInboundMessageSize(Integer.MAX_VALUE);\n" + + " }\n" + + "\n" + + " public static TransportChannelProvider defaultTransportChannelProvider() {\n" + + " return defaultGrpcTransportProviderBuilder().build();\n" + + " }\n" + + "\n" + " public static class Builder {}\n" + "}\n"; } From 98d303c8c69dc1b068bd0f354048570de508f07f Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Fri, 21 Aug 2020 17:33:33 -0700 Subject: [PATCH 10/20] feat: support Foo.class field refs in VariableExpr --- .../generator/engine/ast/IdentifierNode.java | 23 ++++++- .../api/generator/engine/ast/Variable.java | 3 +- .../generator/engine/ast/VariableExpr.java | 12 ++++ .../api/generator/engine/lexicon/Keyword.java | 8 ++- .../ServiceStubSettingsClassComposer.java | 55 +++++++++++++++++ .../engine/ast/VariableExprTest.java | 61 +++++++++++++++++++ .../generator/engine/lexicon/KeywordTest.java | 5 ++ 7 files changed, 162 insertions(+), 5 deletions(-) diff --git a/src/main/java/com/google/api/generator/engine/ast/IdentifierNode.java b/src/main/java/com/google/api/generator/engine/ast/IdentifierNode.java index 287dace51d..5719c9bc0e 100644 --- a/src/main/java/com/google/api/generator/engine/ast/IdentifierNode.java +++ b/src/main/java/com/google/api/generator/engine/ast/IdentifierNode.java @@ -59,7 +59,16 @@ public abstract static class Builder { abstract IdentifierNode autoBuild(); + public IdentifierNode buildVariableIdentifier() throws InvalidIdentifierException { + return build(/* isField= */ true); + } + public IdentifierNode build() throws InvalidIdentifierException { + return build(/* isField= */ false); + } + + // Private. + IdentifierNode build(boolean isField) throws InvalidIdentifierException { IdentifierNode identifier = autoBuild(); String identifierName = identifier.name(); Preconditions.checkNotNull(identifierName); @@ -84,9 +93,17 @@ public IdentifierNode build() throws InvalidIdentifierException { String.format("Name %s cannot contain non-alphanumeric characters", identifierName)); } - if (Keyword.isKeyword(identifierName)) { - throw new InvalidIdentifierException( - String.format("Name %s cannot be a keyword.", identifierName)); + if (isField) { + if (Keyword.isInvalidFieldName(identifierName)) { + throw new InvalidIdentifierException( + String.format("Name %s cannot be a keyword.", identifierName)); + } + + } else { + if (Keyword.isKeyword(identifierName)) { + throw new InvalidIdentifierException( + String.format("Name %s cannot be a keyword.", identifierName)); + } } return identifier; diff --git a/src/main/java/com/google/api/generator/engine/ast/Variable.java b/src/main/java/com/google/api/generator/engine/ast/Variable.java index 6cb97710db..9549390f5b 100644 --- a/src/main/java/com/google/api/generator/engine/ast/Variable.java +++ b/src/main/java/com/google/api/generator/engine/ast/Variable.java @@ -42,7 +42,8 @@ public abstract static class Builder { abstract Variable autoBuild(); public Variable build() { - IdentifierNode identifier = IdentifierNode.builder().setName(name()).build(); + IdentifierNode identifier = + IdentifierNode.builder().setName(name()).buildVariableIdentifier(); setIdentifier(identifier); Variable variable = autoBuild(); diff --git a/src/main/java/com/google/api/generator/engine/ast/VariableExpr.java b/src/main/java/com/google/api/generator/engine/ast/VariableExpr.java index d7ce91e98c..51f2e8a52d 100644 --- a/src/main/java/com/google/api/generator/engine/ast/VariableExpr.java +++ b/src/main/java/com/google/api/generator/engine/ast/VariableExpr.java @@ -14,6 +14,7 @@ package com.google.api.generator.engine.ast; +import com.google.api.generator.engine.lexicon.Keyword; import com.google.auto.value.AutoValue; import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableList; @@ -150,6 +151,17 @@ public VariableExpr build() { variableExpr.staticReferenceType())); } + // A variable name of "class" is valid only when it's a static reference. + String varName = variableExpr.variable().identifier().name(); + if (Keyword.isKeyword(varName)) { + Preconditions.checkState( + variableExpr.staticReferenceType() != null + || (variableExpr.exprReferenceExpr() != null + && TypeNode.isReferenceType(variableExpr.exprReferenceExpr().type())), + String.format( + "Variable field name %s is invalid on non-static or non-reference types", varName)); + } + return variableExpr; } } diff --git a/src/main/java/com/google/api/generator/engine/lexicon/Keyword.java b/src/main/java/com/google/api/generator/engine/lexicon/Keyword.java index f1751dd093..765fe2eaf3 100644 --- a/src/main/java/com/google/api/generator/engine/lexicon/Keyword.java +++ b/src/main/java/com/google/api/generator/engine/lexicon/Keyword.java @@ -17,6 +17,9 @@ import com.google.common.collect.ImmutableList; public class Keyword { + // This is a valid field for all objects, so handle particular keyword differently. + private static final String CLASS_KEYWORD = "class"; + private static final ImmutableList KEYWORDS = ImmutableList.of( "abstract", @@ -59,7 +62,6 @@ public class Keyword { "interface", "static", "void", - "class", "finally", "long", "strictfp", @@ -71,6 +73,10 @@ public class Keyword { "while"); public static boolean isKeyword(String s) { + return s.equals(CLASS_KEYWORD) || KEYWORDS.contains(s); + } + + public static boolean isInvalidFieldName(String s) { return KEYWORDS.contains(s); } } 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 4a472db5b5..0850be191c 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 @@ -471,6 +471,61 @@ private static List createDefaultHelperAndGetterMethods( .setReturnExpr(transportProviderBuilderExpr) .build()); + // Create the defaultApiClientHeaderProviderBuilder method. + returnType = + TypeNode.withReference(ConcreteReference.withClazz(ApiClientHeaderProvider.Builder.class)); + MethodInvocationExpr returnExpr = + MethodInvocationExpr.builder() + .setStaticReferenceType(STATIC_TYPES.get("ApiClientHeaderProvider")) + .setMethodName("newBuilder") + .build(); + + MethodInvocationExpr versionArgExpr = + MethodInvocationExpr.builder() + .setStaticReferenceType(STATIC_TYPES.get("GaxProperties")) + .setMethodName("getLibraryVersion") + .setArguments( + VariableExpr.builder() + .setVariable( + Variable.builder() + .setType( + TypeNode.withReference(ConcreteReference.withClazz(Class.class))) + .setName("class") + .build()) + .setStaticReferenceType(types.get(getThisClassName(service.name()))) + .build()) + .build(); + + returnExpr = + MethodInvocationExpr.builder() + .setExprReferenceExpr(returnExpr) + .setMethodName("setGeneratedLibToken") + .setArguments(ValueExpr.withValue(StringObjectValue.withValue("gapic")), versionArgExpr) + .build(); + returnExpr = + MethodInvocationExpr.builder() + .setExprReferenceExpr(returnExpr) + .setMethodName("setTransportToken") + .setArguments( + MethodInvocationExpr.builder() + .setStaticReferenceType(STATIC_TYPES.get("GaxGrpcProperties")) + .setMethodName("getGrpcTokenName") + .build(), + MethodInvocationExpr.builder() + .setStaticReferenceType(STATIC_TYPES.get("GaxGrpcProperties")) + .setMethodName("getGrpcVersion") + .build()) + .setReturnType(returnType) + .build(); + javaMethods.add( + MethodDefinition.builder() + .setScope(ScopeNode.PUBLIC) + .setIsStatic(true) + .setReturnType(returnType) + .setName("defaultApiClientHeaderProviderBuilder") + .setReturnExpr(returnExpr) + .build()); + return javaMethods; } diff --git a/src/test/java/com/google/api/generator/engine/ast/VariableExprTest.java b/src/test/java/com/google/api/generator/engine/ast/VariableExprTest.java index 75c8241ed8..9e8b87e353 100644 --- a/src/test/java/com/google/api/generator/engine/ast/VariableExprTest.java +++ b/src/test/java/com/google/api/generator/engine/ast/VariableExprTest.java @@ -42,6 +42,34 @@ public void validVariableExpr_staticReference() { .build(); } + @Test + public void validVariableExpr_classFieldOnStaticReference() { + VariableExpr.builder() + .setVariable( + Variable.builder() + .setType(TypeNode.withReference(ConcreteReference.withClazz(Class.class))) + .setName("class") + .build()) + .setStaticReferenceType(TypeNode.INT_OBJECT) + .build(); + } + + @Test + public void validVariableExpr_classFieldOnExprReference() { + VariableExpr.builder() + .setVariable( + Variable.builder() + .setType(TypeNode.withReference(ConcreteReference.withClazz(Class.class))) + .setName("class") + .build()) + .setExprReferenceExpr( + MethodInvocationExpr.builder() + .setMethodName("foobar") + .setReturnType(TypeNode.INT_OBJECT) + .build()) + .build(); + } + @Test public void validVariableExpr_withFields() { Variable variable = Variable.builder().setName("x").setType(TypeNode.STRING).build(); @@ -227,4 +255,37 @@ public void invalidVariableExpr_primitiveStaticReference() { .setStaticReferenceType(TypeNode.INT) .build()); } + + @Test + public void invalidVariableExpr_standaloneClassField() { + assertThrows( + IllegalStateException.class, + () -> + VariableExpr.builder() + .setVariable( + Variable.builder() + .setType(TypeNode.withReference(ConcreteReference.withClazz(Class.class))) + .setName("class") + .build()) + .build()); + } + + @Test + public void invalidVariableExpr_classFieldOnPrimitiveType() { + assertThrows( + IllegalStateException.class, + () -> + VariableExpr.builder() + .setVariable( + Variable.builder() + .setType(TypeNode.withReference(ConcreteReference.withClazz(Class.class))) + .setName("class") + .build()) + .setExprReferenceExpr( + MethodInvocationExpr.builder() + .setMethodName("foobar") + .setReturnType(TypeNode.INT) + .build()) + .build()); + } } diff --git a/src/test/java/com/google/api/generator/engine/lexicon/KeywordTest.java b/src/test/java/com/google/api/generator/engine/lexicon/KeywordTest.java index 448a7e297b..17d51461ed 100644 --- a/src/test/java/com/google/api/generator/engine/lexicon/KeywordTest.java +++ b/src/test/java/com/google/api/generator/engine/lexicon/KeywordTest.java @@ -39,5 +39,10 @@ public void keywordDetected() { assertThat(Keyword.isKeyword("null")).isFalse(); assertThat(Keyword.isKeyword("asdf")).isFalse(); assertThat(Keyword.isKeyword("12345")).isFalse(); + + // Keywords. + assertThat(Keyword.isKeyword("while")).isTrue(); + assertThat(Keyword.isKeyword("class")).isTrue(); + assertThat(Keyword.isInvalidFieldName("class")).isFalse(); } } From d8f6143b116a30c15eea7b865115caa50f956947 Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Fri, 21 Aug 2020 17:36:43 -0700 Subject: [PATCH 11/20] feat: add apiDefaultHelper to ServiceStubSettings codegen --- .../ServiceStubSettingsClassComposer.java | 9 +++++++++ .../ServiceStubSettingsClassComposerTest.java | 15 +++++++++++++++ 2 files changed, 24 insertions(+) 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 0850be191c..9cc76479d4 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 @@ -517,8 +517,17 @@ private static List createDefaultHelperAndGetterMethods( .build()) .setReturnType(returnType) .build(); + + AnnotationNode annotation = + AnnotationNode.builder() + .setType(STATIC_TYPES.get("BetaApi")) + .setDescription( + "The surface for customizing headers is not stable yet and may change in the" + + " future.") + .build(); javaMethods.add( MethodDefinition.builder() + .setAnnotations(Arrays.asList(annotation)) .setScope(ScopeNode.PUBLIC) .setIsStatic(true) .setReturnType(returnType) diff --git a/src/test/java/com/google/api/generator/gapic/composer/ServiceStubSettingsClassComposerTest.java b/src/test/java/com/google/api/generator/gapic/composer/ServiceStubSettingsClassComposerTest.java index 97e27f579b..446689332a 100644 --- a/src/test/java/com/google/api/generator/gapic/composer/ServiceStubSettingsClassComposerTest.java +++ b/src/test/java/com/google/api/generator/gapic/composer/ServiceStubSettingsClassComposerTest.java @@ -83,10 +83,13 @@ public void generateServiceClasses() { + "import static com.google.showcase.v1beta1.EchoClient.PagedExpandPagedResponse;\n" + "\n" + "import com.google.api.core.BetaApi;\n" + + "import com.google.api.gax.core.GaxProperties;\n" + "import com.google.api.gax.core.GoogleCredentialsProvider;\n" + "import com.google.api.gax.core.InstantiatingExecutorProvider;\n" + + "import com.google.api.gax.grpc.GaxGrpcProperties;\n" + "import com.google.api.gax.grpc.GrpcTransportChannel;\n" + "import com.google.api.gax.grpc.InstantiatingGrpcChannelProvider;\n" + + "import com.google.api.gax.rpc.ApiClientHeaderProvider;\n" + "import com.google.api.gax.rpc.OperationCallSettings;\n" + "import com.google.api.gax.rpc.PagedCallSettings;\n" + "import com.google.api.gax.rpc.ServerStreamingCallSettings;\n" @@ -213,6 +216,18 @@ public void generateServiceClasses() { + " return defaultGrpcTransportProviderBuilder().build();\n" + " }\n" + "\n" + + " @BetaApi(\"The surface for customizing headers is not stable yet and may change in" + + " the future.\")\n" + + " public static ApiClientHeaderProvider.Builder" + + " defaultApiClientHeaderProviderBuilder() {\n" + + " return ApiClientHeaderProvider.newBuilder()\n" + + " .setGeneratedLibToken(\"gapic\"," + + " GaxProperties.getLibraryVersion(EchoStubSettings.class))\n" + + " .setTransportToken(\n" + + " GaxGrpcProperties.getGrpcTokenName()," + + " GaxGrpcProperties.getGrpcVersion());\n" + + " }\n" + + "\n" + " public static class Builder {}\n" + "}\n"; } From 34c25c9812e57ccb846dceaf89e07ead0167b9b4 Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Fri, 21 Aug 2020 17:50:04 -0700 Subject: [PATCH 12/20] feat: add builder helpers to ServiceStubSettings codegen --- .../ServiceStubSettingsClassComposer.java | 60 ++++++++++++++++++- .../ServiceStubSettingsClassComposerTest.java | 13 ++++ 2 files changed, 71 insertions(+), 2 deletions(-) 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 9cc76479d4..6d010b7577 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 @@ -60,6 +60,7 @@ import com.google.api.generator.engine.ast.IfStatement; import com.google.api.generator.engine.ast.MethodDefinition; import com.google.api.generator.engine.ast.MethodInvocationExpr; +import com.google.api.generator.engine.ast.NewObjectExpr; import com.google.api.generator.engine.ast.Reference; import com.google.api.generator.engine.ast.ReturnExpr; import com.google.api.generator.engine.ast.ScopeNode; @@ -101,6 +102,7 @@ public class ServiceStubSettingsClassComposer { private static final String CLASS_NAME_PATTERN = "%sStubSettings"; private static final String PAGED_RESPONSE_TYPE_NAME_PATTERN = "%sPagedResponse"; + private static final String NESTED_BUILDER_CLASS_NAME = "Builder"; private static final String GRPC_SERVICE_STUB_PATTERN = "Grpc%sStub"; private static final String STUB_PATTERN = "%sStub"; @@ -255,6 +257,7 @@ private static List createClassMethods( javaMethods.addAll(createMethodSettingsGetterMethods(methodSettingsMemberVarExprs)); javaMethods.add(createCreateStubMethod(service, types)); javaMethods.addAll(createDefaultHelperAndGetterMethods(service, types)); + javaMethods.addAll(createBuilderHelperMethods(service, types)); // TODO(miraleung): Fill this out. return javaMethods; } @@ -538,6 +541,59 @@ private static List createDefaultHelperAndGetterMethods( return javaMethods; } + private static List createBuilderHelperMethods( + Service service, Map types) { + List javaMethods = new ArrayList<>(); + // Create the newBuilder() method. + final TypeNode builderReturnType = types.get(NESTED_BUILDER_CLASS_NAME); + javaMethods.add( + MethodDefinition.builder() + .setScope(ScopeNode.PUBLIC) + .setIsStatic(true) + .setReturnType(builderReturnType) + .setName("newBuilder") + .setReturnExpr( + MethodInvocationExpr.builder() + .setStaticReferenceType(builderReturnType) + .setMethodName("createDefault") + .setReturnType(builderReturnType) + .build()) + .build()); + + // Create the newBuilder(ClientContext) method. + Function newBuilderFn = + argExpr -> NewObjectExpr.builder().setType(builderReturnType).setArguments(argExpr).build(); + VariableExpr clientContextVarExpr = + VariableExpr.withVariable( + Variable.builder() + .setType(STATIC_TYPES.get("ClientContext")) + .setName("clientContext") + .build()); + javaMethods.add( + MethodDefinition.builder() + .setScope(ScopeNode.PUBLIC) + .setIsStatic(true) + .setReturnType(builderReturnType) + .setName("newBuilder") + .setArguments(clientContextVarExpr.toBuilder().setIsDecl(true).build()) + .setReturnExpr(newBuilderFn.apply(clientContextVarExpr)) + .build()); + + // Create the toBuilder method. + javaMethods.add( + MethodDefinition.builder() + .setScope(ScopeNode.PUBLIC) + .setReturnType(builderReturnType) + .setName("toBuilder") + .setReturnExpr( + newBuilderFn.apply( + ValueExpr.withValue( + ThisObjectValue.withType(types.get(getThisClassName(service.name())))))) + .build()); + + return javaMethods; + } + private static ClassDefinition createNestedBuilderClass( Service service, Map types) { String thisClassName = getThisClassName(service.name()); @@ -623,10 +679,10 @@ private static Map createDynamicTypes(Service service, String // Nested builder class. dynamicTypes.put( - "Builder", + NESTED_BUILDER_CLASS_NAME, TypeNode.withReference( VaporReference.builder() - .setName("Builder") + .setName(NESTED_BUILDER_CLASS_NAME) .setPakkage(pakkage) .setEnclosingClassName(thisClassName) .setIsStaticImport(true) diff --git a/src/test/java/com/google/api/generator/gapic/composer/ServiceStubSettingsClassComposerTest.java b/src/test/java/com/google/api/generator/gapic/composer/ServiceStubSettingsClassComposerTest.java index 446689332a..d45f3ef0b1 100644 --- a/src/test/java/com/google/api/generator/gapic/composer/ServiceStubSettingsClassComposerTest.java +++ b/src/test/java/com/google/api/generator/gapic/composer/ServiceStubSettingsClassComposerTest.java @@ -90,6 +90,7 @@ public void generateServiceClasses() { + "import com.google.api.gax.grpc.GrpcTransportChannel;\n" + "import com.google.api.gax.grpc.InstantiatingGrpcChannelProvider;\n" + "import com.google.api.gax.rpc.ApiClientHeaderProvider;\n" + + "import com.google.api.gax.rpc.ClientContext;\n" + "import com.google.api.gax.rpc.OperationCallSettings;\n" + "import com.google.api.gax.rpc.PagedCallSettings;\n" + "import com.google.api.gax.rpc.ServerStreamingCallSettings;\n" @@ -228,6 +229,18 @@ public void generateServiceClasses() { + " GaxGrpcProperties.getGrpcVersion());\n" + " }\n" + "\n" + + " public static Builder newBuilder() {\n" + + " return Builder.createDefault();\n" + + " }\n" + + "\n" + + " public static Builder newBuilder(ClientContext clientContext) {\n" + + " return new Builder(clientContext);\n" + + " }\n" + + "\n" + + " public Builder toBuilder() {\n" + + " return new Builder(this);\n" + + " }\n" + + "\n" + " public static class Builder {}\n" + "}\n"; } From 16207d0276b268bc80f3384e101dc374667ef32d Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Fri, 21 Aug 2020 18:08:24 -0700 Subject: [PATCH 13/20] feat: support this/super ctor in ExprStatements --- .../com/google/api/generator/engine/ast/ExprStatement.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/main/java/com/google/api/generator/engine/ast/ExprStatement.java b/src/main/java/com/google/api/generator/engine/ast/ExprStatement.java index a8ea6c55a3..9aeec6003e 100644 --- a/src/main/java/com/google/api/generator/engine/ast/ExprStatement.java +++ b/src/main/java/com/google/api/generator/engine/ast/ExprStatement.java @@ -50,11 +50,12 @@ public ExprStatement build() { } else { Preconditions.checkState( (expr instanceof MethodInvocationExpr) + || (expr instanceof ReferenceConstructorExpr) || (expr instanceof AssignmentExpr) || (expr instanceof ThrowExpr) || (expr instanceof ReturnExpr), - "Expression statements must be either a method invocation, assignment, throw, or" - + " return expression"); + "Expression statements must be either a method invocation, assignment, throw, " + + "this/super constructor, or return expression"); } return exprStatement; } From 9e50411d51dee7f748d0c44d1b2f613e91275034 Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Fri, 21 Aug 2020 18:09:18 -0700 Subject: [PATCH 14/20] fix: clean up error msgs, add varargs in RefCtorExpr --- .../api/generator/engine/ast/ReferenceConstructorExpr.java | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/main/java/com/google/api/generator/engine/ast/ReferenceConstructorExpr.java b/src/main/java/com/google/api/generator/engine/ast/ReferenceConstructorExpr.java index f25cb9a5f0..f3b2d516be 100644 --- a/src/main/java/com/google/api/generator/engine/ast/ReferenceConstructorExpr.java +++ b/src/main/java/com/google/api/generator/engine/ast/ReferenceConstructorExpr.java @@ -17,6 +17,7 @@ import com.google.auto.value.AutoValue; import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableList; +import java.util.Arrays; import java.util.Collections; import java.util.List; @@ -54,6 +55,10 @@ public static Builder superBuilder() { public abstract static class Builder { public abstract Builder setType(TypeNode node); + public Builder setArguments(Expr... arguments) { + return setArguments(Arrays.asList(arguments)); + } + public abstract Builder setArguments(List arguments); // private. @@ -65,7 +70,7 @@ public ReferenceConstructorExpr build() { ReferenceConstructorExpr referenceConstructorExpr = autoBuild(); Preconditions.checkState( referenceConstructorExpr.type().isReferenceType(referenceConstructorExpr.type()), - "ReferenceConstructExpr type must be reference type. "); + "A this/super constructor must have a reference type."); return referenceConstructorExpr; } } From bb04a418dc4de427de9048a6d48f637817ec5813 Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Fri, 21 Aug 2020 18:09:33 -0700 Subject: [PATCH 15/20] feat: add ctor to ServiceStubSettings codegen --- .../ServiceStubSettingsClassComposer.java | 53 +++++++++++++++++++ .../ServiceStubSettingsClassComposerTest.java | 13 +++++ 2 files changed, 66 insertions(+) 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 6d010b7577..9762def49a 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 @@ -62,6 +62,7 @@ import com.google.api.generator.engine.ast.MethodInvocationExpr; import com.google.api.generator.engine.ast.NewObjectExpr; import com.google.api.generator.engine.ast.Reference; +import com.google.api.generator.engine.ast.ReferenceConstructorExpr; import com.google.api.generator.engine.ast.ReturnExpr; import com.google.api.generator.engine.ast.ScopeNode; import com.google.api.generator.engine.ast.Statement; @@ -258,6 +259,7 @@ private static List createClassMethods( javaMethods.add(createCreateStubMethod(service, types)); javaMethods.addAll(createDefaultHelperAndGetterMethods(service, types)); javaMethods.addAll(createBuilderHelperMethods(service, types)); + javaMethods.add(createClassConstructor(service, methodSettingsMemberVarExprs, types)); // TODO(miraleung): Fill this out. return javaMethods; } @@ -594,6 +596,57 @@ private static List createBuilderHelperMethods( return javaMethods; } + private static MethodDefinition createClassConstructor( + Service service, + Map methodSettingsMemberVarExprs, + Map types) { + TypeNode thisType = types.get(getThisClassName(service.name())); + final VariableExpr settingsBuilderVarExpr = + VariableExpr.withVariable( + Variable.builder() + .setType(types.get(NESTED_BUILDER_CLASS_NAME)) + .setName("settingsBuilder") + .build()); + + Expr superCtorExpr = + ReferenceConstructorExpr.superBuilder() + .setType(STATIC_TYPES.get("StubSettings")) + .setArguments(settingsBuilderVarExpr) + .build(); + + List bodyExprs = new ArrayList<>(); + bodyExprs.add(superCtorExpr); + + Function, AssignmentExpr> varInitExprFn = + e -> + AssignmentExpr.builder() + .setVariableExpr(e.getValue()) + .setValueExpr( + MethodInvocationExpr.builder() + .setExprReferenceExpr( + MethodInvocationExpr.builder() + .setExprReferenceExpr(settingsBuilderVarExpr) + .setMethodName(e.getKey()) + .build()) + .setMethodName("build") + .setReturnType(e.getValue().type()) + .build()) + .build(); + bodyExprs.addAll( + methodSettingsMemberVarExprs.entrySet().stream() + .map(e -> varInitExprFn.apply(e)) + .collect(Collectors.toList())); + + return MethodDefinition.constructorBuilder() + .setScope(ScopeNode.PROTECTED) + .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())) + .build(); + } + private static ClassDefinition createNestedBuilderClass( Service service, Map types) { String thisClassName = getThisClassName(service.name()); diff --git a/src/test/java/com/google/api/generator/gapic/composer/ServiceStubSettingsClassComposerTest.java b/src/test/java/com/google/api/generator/gapic/composer/ServiceStubSettingsClassComposerTest.java index d45f3ef0b1..6759638b23 100644 --- a/src/test/java/com/google/api/generator/gapic/composer/ServiceStubSettingsClassComposerTest.java +++ b/src/test/java/com/google/api/generator/gapic/composer/ServiceStubSettingsClassComposerTest.java @@ -241,6 +241,19 @@ public void generateServiceClasses() { + " return new Builder(this);\n" + " }\n" + "\n" + + " protected EchoStubSettings(Builder settingsBuilder) throws IOException {\n" + + " super(settingsBuilder);\n" + + " echoSettings = settingsBuilder.echoSettings().build();\n" + + " expandSettings = settingsBuilder.expandSettings().build();\n" + + " collectSettings = settingsBuilder.collectSettings().build();\n" + + " chatSettings = settingsBuilder.chatSettings().build();\n" + + " chatAgainSettings = settingsBuilder.chatAgainSettings().build();\n" + + " pagedExpandSettings = settingsBuilder.pagedExpandSettings().build();\n" + + " waitSettings = settingsBuilder.waitSettings().build();\n" + + " waitOperationSettings = settingsBuilder.waitOperationSettings().build();\n" + + " blockSettings = settingsBuilder.blockSettings().build();\n" + + " }\n" + + "\n" + " public static class Builder {}\n" + "}\n"; } From c6853c72e612e758c4e65c8abaf8f4ff9eed8e13 Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Fri, 21 Aug 2020 18:18:25 -0700 Subject: [PATCH 16/20] fix: use super in ServiceSettings codegen --- .../ServiceSettingsClassComposer.java | 28 +++++++++---------- .../ServiceSettingsClassComposerTest.java | 10 +++---- 2 files changed, 18 insertions(+), 20 deletions(-) diff --git a/src/main/java/com/google/api/generator/gapic/composer/ServiceSettingsClassComposer.java b/src/main/java/com/google/api/generator/gapic/composer/ServiceSettingsClassComposer.java index 838e468107..b5c32c42af 100644 --- a/src/main/java/com/google/api/generator/gapic/composer/ServiceSettingsClassComposer.java +++ b/src/main/java/com/google/api/generator/gapic/composer/ServiceSettingsClassComposer.java @@ -39,7 +39,9 @@ import com.google.api.generator.engine.ast.MethodInvocationExpr; import com.google.api.generator.engine.ast.NullObjectValue; import com.google.api.generator.engine.ast.Reference; +import com.google.api.generator.engine.ast.ReferenceConstructorExpr; import com.google.api.generator.engine.ast.ScopeNode; +import com.google.api.generator.engine.ast.SuperObjectValue; import com.google.api.generator.engine.ast.TypeNode; import com.google.api.generator.engine.ast.ValueExpr; import com.google.api.generator.engine.ast.VaporReference; @@ -137,7 +139,6 @@ private static MethodDefinition createConstructorMethod( .setType(types.get(BUILDER_CLASS_NAME)) .build()); TypeNode thisClassType = types.get(getThisClassName(service.name())); - // TODO(miraleung): Use super. return MethodDefinition.constructorBuilder() .setScope(ScopeNode.PROTECTED) .setReturnType(thisClassType) @@ -146,10 +147,9 @@ private static MethodDefinition createConstructorMethod( .setBody( Arrays.asList( ExprStatement.withExpr( - MethodInvocationExpr.builder() - .setMethodName("suuper") - .setArguments(Arrays.asList(settingsBuilderVarExpr)) - .setReturnType(thisClassType) + ReferenceConstructorExpr.superBuilder() + .setType(staticTypes.get("ClientSettings")) + .setArguments(settingsBuilderVarExpr) .build()))) .build(); } @@ -391,7 +391,6 @@ private static List createNestedBuilderConstructorMethods( .build()))) .build(); - // TODO(miraleung): Use super. BiFunction ctorMakerFn = (ctorArg, superArg) -> MethodDefinition.constructorBuilder() @@ -401,10 +400,9 @@ private static List createNestedBuilderConstructorMethods( .setBody( Arrays.asList( ExprStatement.withExpr( - MethodInvocationExpr.builder() - .setMethodName("suuper") - .setArguments(Arrays.asList(superArg)) - .setReturnType(builderType) + ReferenceConstructorExpr.superBuilder() + .setType(staticTypes.get("ClientSettings")) + .setArguments(superArg) .build()))) .build(); @@ -497,10 +495,6 @@ private static MethodDefinition createNestedBuilderApplyToAllUnaryMethod( Service service, Map types) { TypeNode builderType = types.get(BUILDER_CLASS_NAME); String javaMethodName = "applyToAllUnaryMethods"; - // TODO(miraleung): Use super. - VariableExpr superExpr = - VariableExpr.withVariable( - Variable.builder().setName("suuper").setType(staticTypes.get("StubSettings")).build()); TypeNode unaryCallSettingsType = TypeNode.withReference( @@ -534,7 +528,11 @@ private static MethodDefinition createNestedBuilderApplyToAllUnaryMethod( MethodInvocationExpr applyMethodExpr = MethodInvocationExpr.builder() - .setExprReferenceExpr(superExpr) + .setExprReferenceExpr( + ValueExpr.withValue( + SuperObjectValue.withType( + TypeNode.withReference( + ConcreteReference.withClazz(ClientSettings.Builder.class))))) .setMethodName(javaMethodName) .setArguments(Arrays.asList(builderMethodExpr, settingsUpdaterVarExpr)) .build(); diff --git a/src/test/java/com/google/api/generator/gapic/composer/ServiceSettingsClassComposerTest.java b/src/test/java/com/google/api/generator/gapic/composer/ServiceSettingsClassComposerTest.java index efd9f30e6f..7e99fbd2d3 100644 --- a/src/test/java/com/google/api/generator/gapic/composer/ServiceSettingsClassComposerTest.java +++ b/src/test/java/com/google/api/generator/gapic/composer/ServiceSettingsClassComposerTest.java @@ -181,7 +181,7 @@ public void generateServiceClasses() { + " }\n" + "\n" + " protected EchoSettings(Builder settingsBuilder) throws IOException {\n" - + " suuper(settingsBuilder);\n" + + " super(settingsBuilder);\n" + " }\n" + "\n" + " public static class Builder extends ClientSettings.Builder" @@ -191,15 +191,15 @@ public void generateServiceClasses() { + " }\n" + "\n" + " protected Builder(ClientContext clientContext) {\n" - + " suuper(EchoStubSettings.newBuilder(clientContext));\n" + + " super(EchoStubSettings.newBuilder(clientContext));\n" + " }\n" + "\n" + " protected Builder(EchoSettings settings) {\n" - + " suuper(settings.getStubSettings().toBuilder());\n" + + " super(settings.getStubSettings().toBuilder());\n" + " }\n" + "\n" + " protected Builder(EchoStubSettings.Builder stubSettings) {\n" - + " suuper(stubSettings);\n" + + " super(stubSettings);\n" + " }\n" + "\n" + " private static Builder createDefault() {\n" @@ -213,7 +213,7 @@ public void generateServiceClasses() { + " public Builder applyToAllUnaryMethods(\n" + " ApiFunction, Void> settingsUpdater) throws" + " Exception {\n" - + " suuper.applyToAllUnaryMethods(\n" + + " super.applyToAllUnaryMethods(\n" + " getStubSettingsBuilder().unaryMethodSettingsBuilders(), settingsUpdater);\n" + " return thiis;\n" + " }\n" From ada6b50bbdfe701ea6cebd29463abb1f4e0d0ac0 Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Fri, 21 Aug 2020 18:38:03 -0700 Subject: [PATCH 17/20] fix: use this and new in ServiceSettings codegen --- .../ServiceSettingsClassComposer.java | 84 ++++++++----------- .../ServiceSettingsClassComposerTest.java | 14 ++-- 2 files changed, 41 insertions(+), 57 deletions(-) diff --git a/src/main/java/com/google/api/generator/gapic/composer/ServiceSettingsClassComposer.java b/src/main/java/com/google/api/generator/gapic/composer/ServiceSettingsClassComposer.java index b5c32c42af..dd5d08cd2f 100644 --- a/src/main/java/com/google/api/generator/gapic/composer/ServiceSettingsClassComposer.java +++ b/src/main/java/com/google/api/generator/gapic/composer/ServiceSettingsClassComposer.java @@ -37,11 +37,13 @@ import com.google.api.generator.engine.ast.ExprStatement; import com.google.api.generator.engine.ast.MethodDefinition; import com.google.api.generator.engine.ast.MethodInvocationExpr; +import com.google.api.generator.engine.ast.NewObjectExpr; import com.google.api.generator.engine.ast.NullObjectValue; import com.google.api.generator.engine.ast.Reference; import com.google.api.generator.engine.ast.ReferenceConstructorExpr; import com.google.api.generator.engine.ast.ScopeNode; import com.google.api.generator.engine.ast.SuperObjectValue; +import com.google.api.generator.engine.ast.ThisObjectValue; import com.google.api.generator.engine.ast.TypeNode; import com.google.api.generator.engine.ast.ValueExpr; import com.google.api.generator.engine.ast.VaporReference; @@ -207,11 +209,18 @@ private static MethodDefinition createCreatorMethod( .setExprReferenceExpr(stubVarExpr) .setMethodName("toBuilder") .build(); - // TODO(miraleung): Actually instantiate the builder instaead of newTodoBuilder. - MethodInvocationExpr returnMethodExpr = - MethodInvocationExpr.builder() - .setMethodName("newTodoBuilder") - .setArguments(Arrays.asList(stubBuilderMethodExpr)) + + TypeNode stubBuilderType = + TypeNode.withReference( + VaporReference.builder() + .setName("Builder") + .setEnclosingClassName(String.format("%sStubSettings", service.name())) + .setPakkage(String.format("%s.stub", service.pakkage())) + .build()); + Expr returnMethodExpr = + NewObjectExpr.builder() + .setType(stubBuilderType) + .setArguments(stubBuilderMethodExpr) .build(); returnMethodExpr = MethodInvocationExpr.builder() @@ -296,39 +305,32 @@ private static List createBuilderHelperMethods( .setName("clientContext") .setType(staticTypes.get("ClientContext")) .build()); - // TODO(miraleung): Actually instantiate thie builder. + MethodDefinition newBuilderMethodTwo = MethodDefinition.builder() .setScope(ScopeNode.PUBLIC) .setIsStatic(true) .setReturnType(builderType) .setName("newBuilder") - .setArguments(Arrays.asList(clientContextVarExpr.toBuilder().setIsDecl(true).build())) + .setArguments(clientContextVarExpr.toBuilder().setIsDecl(true).build()) .setReturnExpr( - MethodInvocationExpr.builder() - .setMethodName("newBuilder") + NewObjectExpr.builder() + .setType(builderType) .setArguments(Arrays.asList(clientContextVarExpr)) - .setReturnType(builderType) .build()) .build(); - // TODO(miraleung): Use this and actually instantiate the builder. MethodDefinition toBuilderMethod = MethodDefinition.builder() .setScope(ScopeNode.PUBLIC) .setReturnType(builderType) .setName("toBuilder") .setReturnExpr( - MethodInvocationExpr.builder() - .setMethodName("newBuilder") + NewObjectExpr.builder() + .setType(builderType) .setArguments( - Arrays.asList( - VariableExpr.withVariable( - Variable.builder() - .setName("thiis") - .setType(types.get(getThisClassName(service.name()))) - .build()))) - .setReturnType(builderType) + ValueExpr.withValue( + ThisObjectValue.withType(types.get(getThisClassName(service.name()))))) .build()) .build(); @@ -370,7 +372,6 @@ private static List createNestedBuilderClassMethods( private static List createNestedBuilderConstructorMethods( Service service, Map types) { TypeNode builderType = types.get(BUILDER_CLASS_NAME); - // TODO(miraleung): Use this. MethodDefinition noArgCtor = MethodDefinition.constructorBuilder() .setScope(ScopeNode.PROTECTED) @@ -379,15 +380,13 @@ private static List createNestedBuilderConstructorMethods( .setBody( Arrays.asList( ExprStatement.withExpr( - MethodInvocationExpr.builder() - .setMethodName("thiis") + ReferenceConstructorExpr.thisBuilder() + .setType(builderType) .setArguments( - Arrays.asList( - CastExpr.builder() - .setType(staticTypes.get("ClientContext")) - .setExpr(ValueExpr.withValue(NullObjectValue.create())) - .build())) - .setReturnType(builderType) + CastExpr.builder() + .setType(staticTypes.get("ClientContext")) + .setExpr(ValueExpr.withValue(NullObjectValue.create())) + .build()) .build()))) .build(); @@ -457,18 +456,12 @@ private static MethodDefinition createNestedBuilderCreatorMethod( .build(); TypeNode builderType = types.get(BUILDER_CLASS_NAME); - // TODO(miraleung): Actually instantiate the Builder instead of newBuilderTodo. return MethodDefinition.builder() .setScope(ScopeNode.PRIVATE) .setIsStatic(true) .setReturnType(builderType) .setName("createDefault") - .setReturnExpr( - MethodInvocationExpr.builder() - .setMethodName("newBuilderTodo") - .setArguments(Arrays.asList(ctorArg)) - .setReturnType(builderType) - .build()) + .setReturnExpr(NewObjectExpr.builder().setType(builderType).setArguments(ctorArg).build()) .build(); } @@ -537,10 +530,6 @@ private static MethodDefinition createNestedBuilderApplyToAllUnaryMethod( .setArguments(Arrays.asList(builderMethodExpr, settingsUpdaterVarExpr)) .build(); - // TODO(miraleung): Use this. - VariableExpr returnExpr = - VariableExpr.withVariable(Variable.builder().setName("thiis").setType(builderType).build()); - return MethodDefinition.builder() .setScope(ScopeNode.PUBLIC) .setReturnType(builderType) @@ -549,7 +538,7 @@ private static MethodDefinition createNestedBuilderApplyToAllUnaryMethod( .setThrowsExceptions( Arrays.asList(TypeNode.withReference(ConcreteReference.withClazz(Exception.class)))) .setBody(Arrays.asList(ExprStatement.withExpr(applyMethodExpr))) - .setReturnExpr(returnExpr) + .setReturnExpr(ValueExpr.withValue(ThisObjectValue.withType(builderType))) .build(); } @@ -590,10 +579,7 @@ private static List createNestedBuilderSettingsGetterMethods( private static MethodDefinition createNestedBuilderClassBuildMethod( Service service, Map types) { - VariableExpr thisExpr = - VariableExpr.withVariable( - // TODO(miraleung): Actually use this. - Variable.builder().setName("thiis").setType(types.get(BUILDER_CLASS_NAME)).build()); + TypeNode builderType = types.get(BUILDER_CLASS_NAME); TypeNode returnType = types.get(getThisClassName(service.name())); return MethodDefinition.builder() .setIsOverride(true) @@ -602,11 +588,9 @@ private static MethodDefinition createNestedBuilderClassBuildMethod( .setName("build") .setThrowsExceptions(Arrays.asList(staticTypes.get("IOException"))) .setReturnExpr( - MethodInvocationExpr.builder() - // TODO(miraleung): Actually instantiate ServiceSettings. - .setMethodName("newServiceSettings") - .setArguments(Arrays.asList(thisExpr)) - .setReturnType(returnType) + NewObjectExpr.builder() + .setType(returnType) + .setArguments(ValueExpr.withValue(ThisObjectValue.withType(builderType))) .build()) .build(); } diff --git a/src/test/java/com/google/api/generator/gapic/composer/ServiceSettingsClassComposerTest.java b/src/test/java/com/google/api/generator/gapic/composer/ServiceSettingsClassComposerTest.java index 7e99fbd2d3..91034d9792 100644 --- a/src/test/java/com/google/api/generator/gapic/composer/ServiceSettingsClassComposerTest.java +++ b/src/test/java/com/google/api/generator/gapic/composer/ServiceSettingsClassComposerTest.java @@ -131,7 +131,7 @@ public void generateServiceClasses() { + "\n" + " public static final EchoSettings create(EchoStubSettings stub) throws IOException" + " {\n" - + " return newTodoBuilder(stub.toBuilder()).build();\n" + + " return new EchoStubSettings.Builder(stub.toBuilder()).build();\n" + " }\n" + "\n" + " public static InstantiatingExecutorProvider.Builder" @@ -173,11 +173,11 @@ public void generateServiceClasses() { + " }\n" + "\n" + " public static Builder newBuilder(ClientContext clientContext) {\n" - + " return newBuilder(clientContext);\n" + + " return new Builder(clientContext);\n" + " }\n" + "\n" + " public Builder toBuilder() {\n" - + " return newBuilder(thiis);\n" + + " return new Builder(this);\n" + " }\n" + "\n" + " protected EchoSettings(Builder settingsBuilder) throws IOException {\n" @@ -187,7 +187,7 @@ public void generateServiceClasses() { + " public static class Builder extends ClientSettings.Builder" + " {\n" + " protected Builder() throws IOException {\n" - + " thiis(((ClientContext) null));\n" + + " this(((ClientContext) null));\n" + " }\n" + "\n" + " protected Builder(ClientContext clientContext) {\n" @@ -203,7 +203,7 @@ public void generateServiceClasses() { + " }\n" + "\n" + " private static Builder createDefault() {\n" - + " return newBuilderTodo(EchoStubSettings.newBuilder());\n" + + " return new Builder(EchoStubSettings.newBuilder());\n" + " }\n" + "\n" + " public EchoStubSettings.Builder getStubSettingsBuilder() {\n" @@ -215,7 +215,7 @@ public void generateServiceClasses() { + " Exception {\n" + " super.applyToAllUnaryMethods(\n" + " getStubSettingsBuilder().unaryMethodSettingsBuilders(), settingsUpdater);\n" - + " return thiis;\n" + + " return this;\n" + " }\n" + "\n" + " public UnaryCallSettings.Builder echoSettings() {\n" @@ -263,7 +263,7 @@ public void generateServiceClasses() { + "\n" + " @Override\n" + " public EchoSettings build() throws IOException {\n" - + " return newServiceSettings(thiis);\n" + + " return new EchoSettings(this);\n" + " }\n" + " }\n" + "}\n"; From 4b80cd07138a093dc1196e63c92a00df5a5d1bb7 Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Fri, 21 Aug 2020 18:41:37 -0700 Subject: [PATCH 18/20] feat: use this ctor in GrpcServiceStub codegen --- .../GrpcServiceStubClassComposer.java | 25 ++++++++----------- .../GrpcServiceStubClassComposerTest.java | 2 +- 2 files changed, 12 insertions(+), 15 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 de48c6713e..5d47bbe9b3 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 @@ -34,6 +34,7 @@ import com.google.api.generator.engine.ast.MethodDefinition; import com.google.api.generator.engine.ast.MethodInvocationExpr; import com.google.api.generator.engine.ast.NewObjectExpr; +import com.google.api.generator.engine.ast.ReferenceConstructorExpr; import com.google.api.generator.engine.ast.ScopeNode; import com.google.api.generator.engine.ast.Statement; import com.google.api.generator.engine.ast.StringObjectValue; @@ -501,21 +502,17 @@ private static List createConstructorMethods( Arrays.asList(settingsVarExpr, clientContextVarExpr), Arrays.asList( ExprStatement.withExpr( - MethodInvocationExpr.builder() - // TODO(miraleung): Actually instantiate this class. - .setMethodName("thiis") + ReferenceConstructorExpr.thisBuilder() + .setType(thisClassType) .setArguments( - Arrays.asList( - settingsVarExpr, - clientContextVarExpr, - NewObjectExpr.builder() - .setType( - types.get( - String.format( - GRPC_SERVICE_CALLABLE_FACTORY_PATTERN, - service.name()))) - .build())) - .setReturnType(thisClassType) + settingsVarExpr, + clientContextVarExpr, + NewObjectExpr.builder() + .setType( + types.get( + String.format( + GRPC_SERVICE_CALLABLE_FACTORY_PATTERN, service.name()))) + .build()) .build()))); Expr thisExpr = diff --git a/src/test/java/com/google/api/generator/gapic/composer/GrpcServiceStubClassComposerTest.java b/src/test/java/com/google/api/generator/gapic/composer/GrpcServiceStubClassComposerTest.java index a80eaa2ed4..81b04fbba5 100644 --- a/src/test/java/com/google/api/generator/gapic/composer/GrpcServiceStubClassComposerTest.java +++ b/src/test/java/com/google/api/generator/gapic/composer/GrpcServiceStubClassComposerTest.java @@ -212,7 +212,7 @@ public void generateServiceClasses() { + "\n" + " protected GrpcEchoStub(EchoStubSettings settings, ClientContext clientContext)\n" + " throws IOException {\n" - + " thiis(settings, clientContext, new GrpcEchoCallableFactory());\n" + + " this(settings, clientContext, new GrpcEchoCallableFactory());\n" + " }\n" + "\n" + " protected GrpcEchoStub(\n" From 9ff208ab59a76c1acbaaef0fb70d900f30d38160 Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Fri, 21 Aug 2020 18:56:37 -0700 Subject: [PATCH 19/20] feat: fill out util methods in ServiceClient codegen --- .../composer/ServiceClientClassComposer.java | 75 ++++++++++++++----- .../ServiceClientClassComposerTest.java | 18 +++-- 2 files changed, 67 insertions(+), 26 deletions(-) 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 e4f1fff522..b9abc006ec 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 @@ -33,7 +33,6 @@ import com.google.api.generator.engine.ast.MethodDefinition; import com.google.api.generator.engine.ast.MethodInvocationExpr; import com.google.api.generator.engine.ast.NullObjectValue; -import com.google.api.generator.engine.ast.PrimitiveValue; import com.google.api.generator.engine.ast.ScopeNode; import com.google.api.generator.engine.ast.Statement; import com.google.api.generator.engine.ast.TernaryExpr; @@ -120,7 +119,7 @@ private static List createClassMethods( // TODO(miraleung): Constructors when "this" and field accessors are checked in. methods.addAll(createGetterMethods(service, types, hasLroClient)); methods.addAll(createServiceMethods(service, messageTypes, types)); - methods.addAll(createBackgroundResourceMethods(types)); + methods.addAll(createBackgroundResourceMethods(service, types)); return methods; } @@ -525,51 +524,75 @@ private static MethodDefinition createCallableMethod( } private static List createBackgroundResourceMethods( - Map types) { + Service service, Map types) { List methods = new ArrayList<>(); - // TODO(miraleung): Fill out the body. + VariableExpr stubVarExpr = + VariableExpr.withVariable( + Variable.builder() + .setType(types.get(String.format("%sStub", service.name()))) + .setName("stub") + .build()); MethodDefinition closeMethod = MethodDefinition.builder() .setIsOverride(true) .setScope(ScopeNode.PUBLIC) + .setIsFinal(true) .setReturnType(TypeNode.VOID) .setName("close") + .setBody( + Arrays.asList( + ExprStatement.withExpr( + MethodInvocationExpr.builder() + .setExprReferenceExpr(stubVarExpr) + .setMethodName("close") + .build()))) .build(); methods.add(closeMethod); - // TODO(miraleung): Fill out the body. MethodDefinition shutdownMethod = MethodDefinition.builder() .setIsOverride(true) .setScope(ScopeNode.PUBLIC) .setReturnType(TypeNode.VOID) .setName("shutdown") + .setBody( + Arrays.asList( + ExprStatement.withExpr( + MethodInvocationExpr.builder() + .setExprReferenceExpr(stubVarExpr) + .setMethodName("shutdown") + .build()))) .build(); methods.add(shutdownMethod); - // TODO(miraleung): Fill out the body. - Expr placeholderReturnExpr = - ValueExpr.withValue( - PrimitiveValue.builder().setType(TypeNode.BOOLEAN).setValue("false").build()); MethodDefinition isShutdownMethod = MethodDefinition.builder() .setIsOverride(true) .setScope(ScopeNode.PUBLIC) .setReturnType(TypeNode.BOOLEAN) .setName("isShutdown") - .setReturnExpr(placeholderReturnExpr) + .setReturnExpr( + MethodInvocationExpr.builder() + .setExprReferenceExpr(stubVarExpr) + .setMethodName("isShutdown") + .setReturnType(TypeNode.BOOLEAN) + .build()) .build(); methods.add(isShutdownMethod); - // TODO(miraleung): Fill out the body. MethodDefinition isTerminatedMethod = MethodDefinition.builder() .setIsOverride(true) .setScope(ScopeNode.PUBLIC) .setReturnType(TypeNode.BOOLEAN) .setName("isTerminated") - .setReturnExpr(placeholderReturnExpr) + .setReturnExpr( + MethodInvocationExpr.builder() + .setExprReferenceExpr(stubVarExpr) + .setMethodName("isTerminated") + .setReturnType(TypeNode.BOOLEAN) + .build()) .build(); methods.add(isTerminatedMethod); @@ -580,19 +603,21 @@ private static List createBackgroundResourceMethods( .setScope(ScopeNode.PUBLIC) .setReturnType(TypeNode.VOID) .setName("shutdownNow") + .setBody( + Arrays.asList( + ExprStatement.withExpr( + MethodInvocationExpr.builder() + .setExprReferenceExpr(stubVarExpr) + .setMethodName("shutdownNow") + .build()))) .build(); methods.add(shutdownNowMethod); - // TODO(miraleung): Fill out the body. List arguments = Arrays.asList( - VariableExpr.builder() - .setVariable(createVariable("duration", TypeNode.LONG)) - .setIsDecl(true) - .build(), + VariableExpr.builder().setVariable(createVariable("duration", TypeNode.LONG)).build(), VariableExpr.builder() .setVariable(createVariable("unit", types.get("TimeUnit"))) - .setIsDecl(true) .build()); MethodDefinition awaitTerminationMethod = @@ -601,9 +626,19 @@ private static List createBackgroundResourceMethods( .setScope(ScopeNode.PUBLIC) .setReturnType(TypeNode.BOOLEAN) .setName("awaitTermination") - .setReturnExpr(placeholderReturnExpr) - .setArguments(arguments) + .setArguments( + arguments.stream() + .map(v -> v.toBuilder().setIsDecl(true).build()) + .collect(Collectors.toList())) .setThrowsExceptions(Arrays.asList(types.get("InterruptedException"))) + .setReturnExpr( + MethodInvocationExpr.builder() + .setExprReferenceExpr(stubVarExpr) + .setMethodName("awaitTermination") + .setArguments( + arguments.stream().map(v -> (Expr) v).collect(Collectors.toList())) + .setReturnType(TypeNode.BOOLEAN) + .build()) .build(); methods.add(awaitTerminationMethod); 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 23bf4f80e4..8fc67e19a7 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 @@ -204,28 +204,34 @@ public void generateServiceClasses() { + " }\n" + "\n" + " @Override\n" - + " public void close() {}\n" + + " public final void close() {\n" + + " stub.close();\n" + + " }\n" + "\n" + " @Override\n" - + " public void shutdown() {}\n" + + " public void shutdown() {\n" + + " stub.shutdown();\n" + + " }\n" + "\n" + " @Override\n" + " public boolean isShutdown() {\n" - + " return false;\n" + + " return stub.isShutdown();\n" + " }\n" + "\n" + " @Override\n" + " public boolean isTerminated() {\n" - + " return false;\n" + + " return stub.isTerminated();\n" + " }\n" + "\n" + " @Override\n" - + " public void shutdownNow() {}\n" + + " public void shutdownNow() {\n" + + " stub.shutdownNow();\n" + + " }\n" + "\n" + " @Override\n" + " public boolean awaitTermination(long duration, TimeUnit unit) throws" + " InterruptedException {\n" - + " return false;\n" + + " return stub.awaitTermination(duration, unit);\n" + " }\n" + "}\n"; } From 6a94f6add4764d7a0a89dff280cce491c95eabeb Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Fri, 21 Aug 2020 19:13:27 -0700 Subject: [PATCH 20/20] feat: add creator methods to ServiceClient codegen --- .../composer/ServiceClientClassComposer.java | 71 ++++++++++++++----- .../ServiceClientClassComposerTest.java | 13 +++- 2 files changed, 67 insertions(+), 17 deletions(-) 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 b9abc006ec..2ac4bd32bf 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 @@ -17,6 +17,7 @@ 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.longrunning.OperationFuture; import com.google.api.gax.rpc.BidiStreamingCallable; @@ -32,6 +33,7 @@ import com.google.api.generator.engine.ast.ExprStatement; import com.google.api.generator.engine.ast.MethodDefinition; import com.google.api.generator.engine.ast.MethodInvocationExpr; +import com.google.api.generator.engine.ast.NewObjectExpr; import com.google.api.generator.engine.ast.NullObjectValue; import com.google.api.generator.engine.ast.ScopeNode; import com.google.api.generator.engine.ast.Statement; @@ -116,7 +118,7 @@ private static List createClassMethods( boolean hasLroClient) { List methods = new ArrayList<>(); methods.addAll(createStaticCreatorMethods(service, types)); - // TODO(miraleung): Constructors when "this" and field accessors are checked in. + // TODO(miraleung): Ctors go here. methods.addAll(createGetterMethods(service, types, hasLroClient)); methods.addAll(createServiceMethods(service, messageTypes, types)); methods.addAll(createBackgroundResourceMethods(service, types)); @@ -164,6 +166,8 @@ private static List createStaticCreatorMethods( List methods = new ArrayList<>(); String thisClientName = String.format("%sClient", service.name()); String settingsName = String.format("%sSettings", service.name()); + TypeNode thisClassType = types.get(thisClientName); + TypeNode exceptionType = types.get("IOException"); TypeNode settingsType = types.get(settingsName); Preconditions.checkNotNull(settingsType, String.format("Type %s not found", settingsName)); @@ -175,7 +179,7 @@ private static List createStaticCreatorMethods( .build(); MethodInvocationExpr buildExpr = MethodInvocationExpr.builder() - .setMethodName("builder") + .setMethodName("build") .setExprReferenceExpr(newBuilderExpr) .build(); MethodInvocationExpr createMethodInvocationExpr = @@ -190,24 +194,59 @@ private static List createStaticCreatorMethods( .setScope(ScopeNode.PUBLIC) .setIsStatic(true) .setIsFinal(true) - .setReturnType(types.get(thisClientName)) + .setReturnType(thisClassType) .setName("create") - .setArguments( - Arrays.asList( - VariableExpr.builder() - .setVariable( - Variable.builder() - .setName("settings") - .setType(types.get(settingsName)) - .build()) - .setIsDecl(true) - .build())) - .setThrowsExceptions(Arrays.asList(types.get("IOException"))) + .setThrowsExceptions(Arrays.asList(exceptionType)) .setReturnExpr(createMethodInvocationExpr) .build(); methods.add(createMethodOne); - // TODO(miraleung): Add the creators that rely on NewObjectExpr when that is checked in. + // Second create(ServiceSettings settings) method. + VariableExpr settingsVarExpr = + VariableExpr.withVariable( + Variable.builder().setName("settings").setType(types.get(settingsName)).build()); + + methods.add( + MethodDefinition.builder() + .setScope(ScopeNode.PUBLIC) + .setIsStatic(true) + .setIsFinal(true) + .setReturnType(thisClassType) + .setName("create") + .setThrowsExceptions(Arrays.asList(exceptionType)) + .setArguments(settingsVarExpr.toBuilder().setIsDecl(true).build()) + .setReturnExpr( + NewObjectExpr.builder() + .setType(thisClassType) + .setArguments(settingsVarExpr) + .build()) + .build()); + + // Third create(ServiceStub stub) method. + VariableExpr stubVarExpr = + VariableExpr.withVariable( + Variable.builder() + .setType(types.get(String.format("%sStub", service.name()))) + .setName("stub") + .build()); + AnnotationNode betaAnnotation = + AnnotationNode.builder() + .setType(types.get("BetaApi")) + .setDescription( + "A restructuring of stub classes is planned, so this may break in the future") + .build(); + methods.add( + MethodDefinition.builder() + .setAnnotations(Arrays.asList(betaAnnotation)) + .setScope(ScopeNode.PUBLIC) + .setIsStatic(true) + .setIsFinal(true) + .setReturnType(thisClassType) + .setName("create") + .setArguments(stubVarExpr.toBuilder().setIsDecl(true).build()) + .setReturnExpr( + NewObjectExpr.builder().setType(thisClassType).setArguments(stubVarExpr).build()) + .build()); return methods; } @@ -596,7 +635,6 @@ private static List createBackgroundResourceMethods( .build(); methods.add(isTerminatedMethod); - // TODO(miraleung): Fill out the body. MethodDefinition shutdownNowMethod = MethodDefinition.builder() .setIsOverride(true) @@ -661,6 +699,7 @@ private static Map createConcreteTypes() { ApiFuture.class, ApiFutures.class, BackgroundResource.class, + BetaApi.class, BidiStreamingCallable.class, ClientStreamingCallable.class, Generated.class, 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 8fc67e19a7..1ab1fd1cee 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 @@ -64,6 +64,7 @@ public void generateServiceClasses() { private static final String EXPECTED_CLASS_STRING = "package com.google.showcase.v1beta1;\n" + "\n" + + "import com.google.api.core.BetaApi;\n" + "import com.google.api.gax.core.BackgroundResource;\n" + "import com.google.api.gax.longrunning.OperationFuture;\n" + "import com.google.api.gax.rpc.BidiStreamingCallable;\n" @@ -87,8 +88,18 @@ public void generateServiceClasses() { + " private final EchoStub stub;\n" + " private final OperationsClient operationsClient;\n" + "\n" + + " public static final EchoClient create() throws IOException {\n" + + " return create(EchoSettings.newBuilder().build());\n" + + " }\n" + + "\n" + " public static final EchoClient create(EchoSettings settings) throws IOException {\n" - + " return create(EchoSettings.newBuilder().builder());\n" + + " return new EchoClient(settings);\n" + + " }\n" + + "\n" + + " @BetaApi(\"A restructuring of stub classes is planned, so this may break in the" + + " future\")\n" + + " public static final EchoClient create(EchoStub stub) {\n" + + " return new EchoClient(stub);\n" + " }\n" + "\n" + " public EchoStub getStub() {\n"