From 596a6657dcb0d1e20044128ad991805f04586276 Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Fri, 20 Nov 2020 16:58:26 -0800 Subject: [PATCH 1/5] fix: bring back gapic.yaml to bazel and Java parsing --- rules_java_gapic/java_gapic.bzl | 4 + run.sh | 9 +- test/integration/BUILD.bazel | 1 + .../logging/LoggingServiceV2StubSettings.java | 97 ++++++++++++++++++- 4 files changed, 105 insertions(+), 6 deletions(-) diff --git a/rules_java_gapic/java_gapic.bzl b/rules_java_gapic/java_gapic.bzl index 26d87cacaf..4a004b57fa 100644 --- a/rules_java_gapic/java_gapic.bzl +++ b/rules_java_gapic/java_gapic.bzl @@ -85,6 +85,7 @@ def java_gapic_library( package = None, service_yaml = None, grpc_service_config = None, + gapic_yaml = None, deps = [], test_deps = [], **kwargs): @@ -93,6 +94,9 @@ def java_gapic_library( if grpc_service_config: file_args_dict[grpc_service_config] = "grpc-service-config" + if gapic_yaml: + file_args_dict[gapic_yaml] = "gapic-config" + # Check the allow-list. if service_yaml: service_yaml_in_allowlist = False diff --git a/run.sh b/run.sh index efe66747a0..5581e0f378 100755 --- a/run.sh +++ b/run.sh @@ -18,6 +18,7 @@ DEFINE_string --alias=s service_config '' 'Path to the JSON service config' # Optional flags. DEFINE_bool --alias=c use_cached false 'If true, does not rebuild the plugin.' DEFINE_string --alias=o out '/tmp/test' 'Output directory' +DEFINE_string gapic_config '' 'Path to the config ending in gapic.yaml. Optional' gbash::init_google "$@" @@ -78,13 +79,19 @@ then SERVICE_CONFIG_OPT="grpc-service-config=$FLAGS_service_config" fi +GAPIC_CONFIG_OPT="" +if [ -n "$FLAGS_gapic_config" ] +then + GAPIC_CONFIG_OPT="gapic-config=$FLAGS_gapic_config" +fi + # Run protoc. protoc -I="${PROTOC_INCLUDE_DIR}" -I="${FLAGS_googleapis}" -I="${FLAGS_protos}" \ -I="${FLAGS_googleapis}/google/longrunning" \ --include_source_info \ --plugin=bazel-bin/protoc-gen-java_gapic ${FLAGS_protos}/*.proto \ --java_gapic_out="${FLAGS_out}" \ - --java_gapic_opt="${SERVICE_CONFIG_OPT}" \ + --java_gapic_opt="${SERVICE_CONFIG_OPT},${GAPIC_CONFIG_OPT}" \ --experimental_allow_proto3_optional echo_success "Output files written to ${FLAGS_out}" diff --git a/test/integration/BUILD.bazel b/test/integration/BUILD.bazel index d4760f7941..162b68f2aa 100644 --- a/test/integration/BUILD.bazel +++ b/test/integration/BUILD.bazel @@ -115,6 +115,7 @@ java_gapic_assembly_gradle_pkg( java_gapic_library( name = "logging_java_gapic", srcs = ["@com_google_googleapis//google/logging/v2:logging_proto_with_info"], + gapic_yaml = "@com_google_googleapis//google/logging/v2:logging_gapic.yaml", grpc_service_config = "@com_google_googleapis//google/logging/v2:logging_grpc_service_config.json", package = "google.logging.v2", test_deps = [ diff --git a/test/integration/goldens/logging/LoggingServiceV2StubSettings.java b/test/integration/goldens/logging/LoggingServiceV2StubSettings.java index ae0e186fd6..62289f4be5 100644 --- a/test/integration/goldens/logging/LoggingServiceV2StubSettings.java +++ b/test/integration/goldens/logging/LoggingServiceV2StubSettings.java @@ -24,6 +24,11 @@ import com.google.api.core.ApiFunction; import com.google.api.core.ApiFuture; import com.google.api.core.BetaApi; +import com.google.api.gax.batching.BatchingSettings; +import com.google.api.gax.batching.FlowControlSettings; +import com.google.api.gax.batching.FlowController; +import com.google.api.gax.batching.PartitionKey; +import com.google.api.gax.batching.RequestBuilder; import com.google.api.gax.core.GaxProperties; import com.google.api.gax.core.GoogleCredentialsProvider; import com.google.api.gax.core.InstantiatingExecutorProvider; @@ -33,6 +38,9 @@ import com.google.api.gax.retrying.RetrySettings; import com.google.api.gax.rpc.ApiCallContext; import com.google.api.gax.rpc.ApiClientHeaderProvider; +import com.google.api.gax.rpc.BatchedRequestIssuer; +import com.google.api.gax.rpc.BatchingCallSettings; +import com.google.api.gax.rpc.BatchingDescriptor; import com.google.api.gax.rpc.ClientContext; import com.google.api.gax.rpc.PageContext; import com.google.api.gax.rpc.PagedCallSettings; @@ -59,6 +67,7 @@ import com.google.logging.v2.WriteLogEntriesResponse; import com.google.protobuf.Empty; import java.io.IOException; +import java.util.Collection; import java.util.List; import java.util.Objects; import javax.annotation.Generated; @@ -110,7 +119,7 @@ public class LoggingServiceV2StubSettings extends StubSettings deleteLogSettings; - private final UnaryCallSettings + private final BatchingCallSettings writeLogEntriesSettings; private final PagedCallSettings< ListLogEntriesRequest, ListLogEntriesResponse, ListLogEntriesPagedResponse> @@ -311,13 +320,73 @@ public ApiFuture getFuturePagedResponse( } }; + private static final BatchingDescriptor + WRITE_LOG_ENTRIES_BATCHING_DESC = + new BatchingDescriptor() { + @Override + public PartitionKey getBatchPartitionKey(WriteLogEntriesRequest request) { + return new PartitionKey( + request.getLogName(), request.getResource(), request.getLabels()); + } + + @Override + public RequestBuilder getRequestBuilder() { + return new RequestBuilder() { + private WriteLogEntriesRequest.Builder builder; + + @Override + public void appendRequest(WriteLogEntriesRequest request) { + if (Objects.isNull(builder)) { + builder = request.toBuilder(); + } else { + builder.addAllEntries(request.getEntriesList()); + } + } + + @Override + public WriteLogEntriesRequest build() { + return builder.build(); + } + }; + } + + @Override + public void splitResponse( + WriteLogEntriesResponse batchResponse, + Collection> batch) { + for (BatchedRequestIssuer responder : batch) { + WriteLogEntriesResponse response = WriteLogEntriesResponse.newBuilder().build(); + responder.setResponse(response); + } + } + + @Override + public void splitException( + Throwable throwable, + Collection> batch) { + for (BatchedRequestIssuer responder : batch) { + responder.setException(throwable); + } + } + + @Override + public long countElements(WriteLogEntriesRequest request) { + return request.getEntriesCount(); + } + + @Override + public long countBytes(WriteLogEntriesRequest request) { + return request.getSerializedSize(); + } + }; + /** Returns the object with the settings used for calls to deleteLog. */ public UnaryCallSettings deleteLogSettings() { return deleteLogSettings; } /** Returns the object with the settings used for calls to writeLogEntries. */ - public UnaryCallSettings + public BatchingCallSettings writeLogEntriesSettings() { return writeLogEntriesSettings; } @@ -425,7 +494,7 @@ protected LoggingServiceV2StubSettings(Builder settingsBuilder) throws IOExcepti public static class Builder extends StubSettings.Builder { private final ImmutableList> unaryMethodSettingsBuilders; private final UnaryCallSettings.Builder deleteLogSettings; - private final UnaryCallSettings.Builder + private final BatchingCallSettings.Builder writeLogEntriesSettings; private final PagedCallSettings.Builder< ListLogEntriesRequest, ListLogEntriesResponse, ListLogEntriesPagedResponse> @@ -481,7 +550,9 @@ protected Builder(ClientContext clientContext) { super(clientContext); deleteLogSettings = UnaryCallSettings.newUnaryCallSettingsBuilder(); - writeLogEntriesSettings = UnaryCallSettings.newUnaryCallSettingsBuilder(); + writeLogEntriesSettings = + BatchingCallSettings.newBuilder(WRITE_LOG_ENTRIES_BATCHING_DESC) + .setBatchingSettings(BatchingSettings.newBuilder().build()); listLogEntriesSettings = PagedCallSettings.newBuilder(LIST_LOG_ENTRIES_PAGE_STR_FACT); listMonitoredResourceDescriptorsSettings = PagedCallSettings.newBuilder(LIST_MONITORED_RESOURCE_DESCRIPTORS_PAGE_STR_FACT); @@ -533,6 +604,22 @@ private static Builder initDefaults(Builder builder) { .setRetryableCodes(RETRYABLE_CODE_DEFINITIONS.get("retry_policy_1_codes")) .setRetrySettings(RETRY_PARAM_DEFINITIONS.get("retry_policy_1_params")); + builder + .writeLogEntriesSettings() + .setBatchingSettings( + BatchingSettings.newBuilder() + .setElementCountThreshold(1000L) + .setRequestByteThreshold(1048576L) + .setDelayThreshold(Duration.ofMillis(50L)) + .setFlowControlSettings( + FlowControlSettings.newBuilder() + .setMaxOutstandingElementCount(100000L) + .setMaxOutstandingRequestBytes(10485760L) + .setLimitExceededBehavior( + FlowController.LimitExceededBehavior.ThrowException) + .build()) + .build()); + builder .writeLogEntriesSettings() .setRetryableCodes(RETRYABLE_CODE_DEFINITIONS.get("retry_policy_1_codes")) @@ -578,7 +665,7 @@ public UnaryCallSettings.Builder deleteLogSettings() { } /** Returns the builder for the settings used for calls to writeLogEntries. */ - public UnaryCallSettings.Builder + public BatchingCallSettings.Builder writeLogEntriesSettings() { return writeLogEntriesSettings; } From 1c11d1e7ba7fb4a2a3dda23d41de2eee2507a4fe Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Fri, 20 Nov 2020 17:50:51 -0800 Subject: [PATCH 2/5] fix: update LRO total_poll_timeout_millis t0 5min --- .../gapic/composer/RetrySettingsComposer.java | 2 +- .../gapic/composer/RetrySettingsComposerTest.java | 2 +- .../gapic/composer/goldens/EchoStubSettings.golden | 2 +- .../goldens/asset/AssetServiceStubSettings.java | 2 +- .../goldens/redis/CloudRedisStubSettings.java | 14 +++++++------- 5 files changed, 11 insertions(+), 11 deletions(-) diff --git a/src/main/java/com/google/api/generator/gapic/composer/RetrySettingsComposer.java b/src/main/java/com/google/api/generator/gapic/composer/RetrySettingsComposer.java index 1f5aa03569..e9785d1b78 100644 --- a/src/main/java/com/google/api/generator/gapic/composer/RetrySettingsComposer.java +++ b/src/main/java/com/google/api/generator/gapic/composer/RetrySettingsComposer.java @@ -67,7 +67,7 @@ public class RetrySettingsComposer { private static final long LRO_DEFAULT_INITIAL_POLL_DELAY_MILLIS = 5000; private static final double LRO_DEFAULT_POLL_DELAY_MULTIPLIER = 1.5; private static final long LRO_DEFAULT_MAX_POLL_DELAY_MILLIS = 45000; - private static final long LRO_DEFAULT_TOTAL_POLL_TIMEOUT_MILLS = 86400000; // 24 hours. + private static final long LRO_DEFAULT_TOTAL_POLL_TIMEOUT_MILLS = 300000; // 5 minutes. private static final double LRO_DEFAULT_MAX_RPC_TIMEOUT = 1.0; public static BlockStatement createRetryParamDefinitionsBlock( diff --git a/src/test/java/com/google/api/generator/gapic/composer/RetrySettingsComposerTest.java b/src/test/java/com/google/api/generator/gapic/composer/RetrySettingsComposerTest.java index 7a1f2ab030..b451dd2579 100644 --- a/src/test/java/com/google/api/generator/gapic/composer/RetrySettingsComposerTest.java +++ b/src/test/java/com/google/api/generator/gapic/composer/RetrySettingsComposerTest.java @@ -375,7 +375,7 @@ public void lroBuilderExpr() { + "RetrySettings.newBuilder().setInitialRetryDelay(Duration.ofMillis(5000L))" + ".setRetryDelayMultiplier(1.5).setMaxRetryDelay(Duration.ofMillis(45000L))" + ".setInitialRpcTimeout(Duration.ZERO).setRpcTimeoutMultiplier(1.0)" - + ".setMaxRpcTimeout(Duration.ZERO).setTotalTimeout(Duration.ofMillis(86400000L))" + + ".setMaxRpcTimeout(Duration.ZERO).setTotalTimeout(Duration.ofMillis(300000L))" + ".build()))"); assertEquals(expected, writerVisitor.write()); } diff --git a/src/test/java/com/google/api/generator/gapic/composer/goldens/EchoStubSettings.golden b/src/test/java/com/google/api/generator/gapic/composer/goldens/EchoStubSettings.golden index cd52c7c439..4c6cb32c83 100644 --- a/src/test/java/com/google/api/generator/gapic/composer/goldens/EchoStubSettings.golden +++ b/src/test/java/com/google/api/generator/gapic/composer/goldens/EchoStubSettings.golden @@ -435,7 +435,7 @@ public class EchoStubSettings extends StubSettings { .setInitialRpcTimeout(Duration.ZERO) .setRpcTimeoutMultiplier(1.0) .setMaxRpcTimeout(Duration.ZERO) - .setTotalTimeout(Duration.ofMillis(86400000L)) + .setTotalTimeout(Duration.ofMillis(300000L)) .build())); return builder; diff --git a/test/integration/goldens/asset/AssetServiceStubSettings.java b/test/integration/goldens/asset/AssetServiceStubSettings.java index be7aac6632..a86ad5dbdc 100644 --- a/test/integration/goldens/asset/AssetServiceStubSettings.java +++ b/test/integration/goldens/asset/AssetServiceStubSettings.java @@ -622,7 +622,7 @@ private static Builder initDefaults(Builder builder) { .setInitialRpcTimeout(Duration.ZERO) .setRpcTimeoutMultiplier(1.0) .setMaxRpcTimeout(Duration.ZERO) - .setTotalTimeout(Duration.ofMillis(86400000L)) + .setTotalTimeout(Duration.ofMillis(300000L)) .build())); return builder; diff --git a/test/integration/goldens/redis/CloudRedisStubSettings.java b/test/integration/goldens/redis/CloudRedisStubSettings.java index c30e46e9d0..74901d60c5 100644 --- a/test/integration/goldens/redis/CloudRedisStubSettings.java +++ b/test/integration/goldens/redis/CloudRedisStubSettings.java @@ -572,7 +572,7 @@ private static Builder initDefaults(Builder builder) { .setInitialRpcTimeout(Duration.ZERO) .setRpcTimeoutMultiplier(1.0) .setMaxRpcTimeout(Duration.ZERO) - .setTotalTimeout(Duration.ofMillis(86400000L)) + .setTotalTimeout(Duration.ofMillis(300000L)) .build())); builder @@ -596,7 +596,7 @@ private static Builder initDefaults(Builder builder) { .setInitialRpcTimeout(Duration.ZERO) .setRpcTimeoutMultiplier(1.0) .setMaxRpcTimeout(Duration.ZERO) - .setTotalTimeout(Duration.ofMillis(86400000L)) + .setTotalTimeout(Duration.ofMillis(300000L)) .build())); builder @@ -620,7 +620,7 @@ private static Builder initDefaults(Builder builder) { .setInitialRpcTimeout(Duration.ZERO) .setRpcTimeoutMultiplier(1.0) .setMaxRpcTimeout(Duration.ZERO) - .setTotalTimeout(Duration.ofMillis(86400000L)) + .setTotalTimeout(Duration.ofMillis(300000L)) .build())); builder @@ -644,7 +644,7 @@ private static Builder initDefaults(Builder builder) { .setInitialRpcTimeout(Duration.ZERO) .setRpcTimeoutMultiplier(1.0) .setMaxRpcTimeout(Duration.ZERO) - .setTotalTimeout(Duration.ofMillis(86400000L)) + .setTotalTimeout(Duration.ofMillis(300000L)) .build())); builder @@ -668,7 +668,7 @@ private static Builder initDefaults(Builder builder) { .setInitialRpcTimeout(Duration.ZERO) .setRpcTimeoutMultiplier(1.0) .setMaxRpcTimeout(Duration.ZERO) - .setTotalTimeout(Duration.ofMillis(86400000L)) + .setTotalTimeout(Duration.ofMillis(300000L)) .build())); builder @@ -692,7 +692,7 @@ private static Builder initDefaults(Builder builder) { .setInitialRpcTimeout(Duration.ZERO) .setRpcTimeoutMultiplier(1.0) .setMaxRpcTimeout(Duration.ZERO) - .setTotalTimeout(Duration.ofMillis(86400000L)) + .setTotalTimeout(Duration.ofMillis(300000L)) .build())); builder @@ -716,7 +716,7 @@ private static Builder initDefaults(Builder builder) { .setInitialRpcTimeout(Duration.ZERO) .setRpcTimeoutMultiplier(1.0) .setMaxRpcTimeout(Duration.ZERO) - .setTotalTimeout(Duration.ofMillis(86400000L)) + .setTotalTimeout(Duration.ofMillis(300000L)) .build())); return builder; From 0db70316520e2b494cc11f814846861e564e6c15 Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Fri, 20 Nov 2020 23:15:55 -0800 Subject: [PATCH 3/5] feat: add GapicLroRetrySettings and parser --- .../gapic/model/GapicLroRetrySettings.java | 58 ++++++++ .../GapicLroRetrySettingsParser.java | 129 ++++++++++++++++++ .../generator/gapic/protoparser/BUILD.bazel | 1 + .../GapicLroRetrySettingsParserTest.java | 84 ++++++++++++ .../gapic/testdata/dataproc_gapic.yaml | 65 +++++++++ 5 files changed, 337 insertions(+) create mode 100644 src/main/java/com/google/api/generator/gapic/model/GapicLroRetrySettings.java create mode 100644 src/main/java/com/google/api/generator/gapic/protoparser/GapicLroRetrySettingsParser.java create mode 100644 src/test/java/com/google/api/generator/gapic/protoparser/GapicLroRetrySettingsParserTest.java create mode 100644 src/test/java/com/google/api/generator/gapic/testdata/dataproc_gapic.yaml diff --git a/src/main/java/com/google/api/generator/gapic/model/GapicLroRetrySettings.java b/src/main/java/com/google/api/generator/gapic/model/GapicLroRetrySettings.java new file mode 100644 index 0000000000..4fd7518916 --- /dev/null +++ b/src/main/java/com/google/api/generator/gapic/model/GapicLroRetrySettings.java @@ -0,0 +1,58 @@ +// 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.model; + +import com.google.auto.value.AutoValue; + +/** Represents the LRO retry settings in a gapic.yaml file. */ +@AutoValue +public abstract class GapicLroRetrySettings { + public abstract String protoPakkage(); + + public abstract String serviceName(); + + public abstract String methodName(); + + public abstract long initialPollDelayMillis(); + + public abstract double pollDelayMultiplier(); + + public abstract long maxPollDelayMillis(); + + public abstract long totalPollTimeoutMillis(); + + public static Builder builder() { + return new AutoValue_GapicLroRetrySettings.Builder(); + } + + @AutoValue.Builder + public abstract static class Builder { + public abstract Builder setProtoPakkage(String protoPakkage); + + public abstract Builder setServiceName(String serviceName); + + public abstract Builder setMethodName(String methodName); + + public abstract Builder setInitialPollDelayMillis(long initialPollDelayMillis); + + public abstract Builder setPollDelayMultiplier(double pollDelayMultiplier); + + public abstract Builder setMaxPollDelayMillis(long maxPollDelayMillis); + + public abstract Builder setTotalPollTimeoutMillis(long totalPollTimeoutMillis); + + public abstract GapicLroRetrySettings build(); + } +} diff --git a/src/main/java/com/google/api/generator/gapic/protoparser/GapicLroRetrySettingsParser.java b/src/main/java/com/google/api/generator/gapic/protoparser/GapicLroRetrySettingsParser.java new file mode 100644 index 0000000000..ff0cf7e462 --- /dev/null +++ b/src/main/java/com/google/api/generator/gapic/protoparser/GapicLroRetrySettingsParser.java @@ -0,0 +1,129 @@ +// 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.api.generator.gapic.model.GapicLroRetrySettings; +import com.google.common.annotations.VisibleForTesting; +import com.google.common.base.Strings; +import java.io.File; +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Paths; +import java.util.ArrayList; +import java.util.List; +import java.util.Map; +import java.util.Optional; +import org.yaml.snakeyaml.Yaml; +import org.yaml.snakeyaml.constructor.SafeConstructor; + +public class GapicLroRetrySettingsParser { + private static final String YAML_KEY_INTERFACES = "interfaces"; + private static final String YAML_KEY_NAME = "name"; + private static final String YAML_KEY_METHODS = "methods"; + private static final String YAML_KEY_LONG_RUNNING = "long_running"; + + private static final String YAML_KEY_INITIAL_POLL_DELAY_MILLIS = "initial_poll_delay_millis"; + private static final String YAML_KEY_POLL_DELAY_MULTIPLIER = "poll_delay_multiplier"; + private static final String YAML_KEY_MAX_POLL_DELAY_MILLIS = "max_poll_delay_millis"; + private static final String YAML_KEY_TOTAL_POLL_TIMEOUT_MILLIS = "total_poll_timeout_millis"; + + public static Optional> parse( + Optional gapicYamlConfigFilePathOpt) { + return gapicYamlConfigFilePathOpt.isPresent() + ? parse(gapicYamlConfigFilePathOpt.get()) + : Optional.empty(); + } + + @VisibleForTesting + static Optional> parse(String gapicYamlConfigFilePath) { + if (Strings.isNullOrEmpty(gapicYamlConfigFilePath) + || !(new File(gapicYamlConfigFilePath)).exists()) { + return Optional.empty(); + } + + String fileContents = null; + + try { + fileContents = new String(Files.readAllBytes(Paths.get(gapicYamlConfigFilePath))); + } catch (IOException e) { + return Optional.empty(); + } + + Yaml yaml = new Yaml(new SafeConstructor()); + Map yamlMap = yaml.load(fileContents); + return parseFromMap(yamlMap); + } + + private static Optional> parseFromMap(Map yamlMap) { + if (!yamlMap.containsKey(YAML_KEY_INTERFACES)) { + return Optional.empty(); + } + + List settings = new ArrayList<>(); + for (Map serviceYamlConfig : + (List>) yamlMap.get(YAML_KEY_INTERFACES)) { + if (!serviceYamlConfig.containsKey(YAML_KEY_METHODS)) { + continue; + } + for (Map methodYamlConfig : + (List>) serviceYamlConfig.get(YAML_KEY_METHODS)) { + if (!methodYamlConfig.containsKey(YAML_KEY_LONG_RUNNING)) { + continue; + } + + Map lroRetrySettingsYamlConfig = + (Map) methodYamlConfig.get(YAML_KEY_LONG_RUNNING); + if (!lroRetrySettingsYamlConfig.containsKey(YAML_KEY_INITIAL_POLL_DELAY_MILLIS) + || !lroRetrySettingsYamlConfig.containsKey(YAML_KEY_POLL_DELAY_MULTIPLIER) + || !lroRetrySettingsYamlConfig.containsKey(YAML_KEY_MAX_POLL_DELAY_MILLIS) + || !lroRetrySettingsYamlConfig.containsKey(YAML_KEY_TOTAL_POLL_TIMEOUT_MILLIS)) { + continue; + } + + String interfaceName = (String) serviceYamlConfig.get(YAML_KEY_NAME); + int lastDotIndex = interfaceName.lastIndexOf("."); + String protoPakkage = interfaceName.substring(0, lastDotIndex); + String serviceName = interfaceName.substring(lastDotIndex + 1); + String methodName = (String) methodYamlConfig.get(YAML_KEY_NAME); + + GapicLroRetrySettings.Builder lroRetrySettingsBuilder = + GapicLroRetrySettings.builder() + .setProtoPakkage(protoPakkage) + .setServiceName(serviceName) + .setMethodName(methodName) + .setInitialPollDelayMillis( + (Integer) lroRetrySettingsYamlConfig.get(YAML_KEY_INITIAL_POLL_DELAY_MILLIS)) + .setMaxPollDelayMillis( + (Integer) lroRetrySettingsYamlConfig.get(YAML_KEY_MAX_POLL_DELAY_MILLIS)) + .setTotalPollTimeoutMillis( + (Integer) lroRetrySettingsYamlConfig.get(YAML_KEY_TOTAL_POLL_TIMEOUT_MILLIS)); + + // Workaround for snakeyaml's automatic type inference. When given a value like "1", + // snakeyaml interprets this as an integer, whereas a value like "1.5" is interpreted as + // a double. + Object pollDelayMultObj = lroRetrySettingsYamlConfig.get(YAML_KEY_POLL_DELAY_MULTIPLIER); + if (pollDelayMultObj instanceof Integer) { + lroRetrySettingsBuilder = + lroRetrySettingsBuilder.setPollDelayMultiplier((Integer) pollDelayMultObj); + } else if (pollDelayMultObj instanceof Double) { + lroRetrySettingsBuilder = + lroRetrySettingsBuilder.setPollDelayMultiplier((Double) pollDelayMultObj); + } + settings.add(lroRetrySettingsBuilder.build()); + } + } + return settings.isEmpty() ? Optional.empty() : Optional.of(settings); + } +} 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 7df82d359f..dda78e43ad 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 @@ -4,6 +4,7 @@ package(default_visibility = ["//visibility:public"]) TESTS = [ "BatchingSettingsConfigParserTest", + "GapicLroRetrySettingsParserTest", "HttpRuleParserTest", "MethodSignatureParserTest", "ParserTest", diff --git a/src/test/java/com/google/api/generator/gapic/protoparser/GapicLroRetrySettingsParserTest.java b/src/test/java/com/google/api/generator/gapic/protoparser/GapicLroRetrySettingsParserTest.java new file mode 100644 index 0000000000..262b266f53 --- /dev/null +++ b/src/test/java/com/google/api/generator/gapic/protoparser/GapicLroRetrySettingsParserTest.java @@ -0,0 +1,84 @@ +// 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 com.google.api.generator.gapic.model.GapicLroRetrySettings; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.util.List; +import java.util.Optional; +import org.junit.Test; + +public class GapicLroRetrySettingsParserTest { + private static final double DELTA = 0.0001; + private static final String YAML_DIRECTORY = + "src/test/java/com/google/api/generator/gapic/testdata/"; + + @Test + public void parseLroRetrySettings_methodsPresentNoLroRetrySettings() { + String filename = "datastore_gapic.yaml"; + Path path = Paths.get(YAML_DIRECTORY, filename); + Optional> settingsOpt = + GapicLroRetrySettingsParser.parse(path.toString()); + assertFalse(settingsOpt.isPresent()); + } + + @Test + public void parseGapicSettings_noMethodSettings() { + String filename = "showcase_gapic.yaml"; + Path path = Paths.get(YAML_DIRECTORY, filename); + Optional> settingsOpt = + GapicLroRetrySettingsParser.parse(path.toString()); + assertFalse(settingsOpt.isPresent()); + } + + @Test + public void parseLroRetrySettings_lroRetrySettingsPresent() { + String filename = "dataproc_gapic.yaml"; + Path path = Paths.get(YAML_DIRECTORY, filename); + Optional> settingsOpt = + GapicLroRetrySettingsParser.parse(path.toString()); + + List lroRetrySettings = settingsOpt.get(); + assertEquals(6, lroRetrySettings.size()); + + // This LRO setting has a max_poll_delay_millis that will be parsed as an integer. + GapicLroRetrySettings setting = lroRetrySettings.get(0); + assertEquals("google.cloud.dataproc.v1", setting.protoPakkage()); + assertEquals("ClusterController", setting.serviceName()); + assertEquals("CreateCluster", setting.methodName()); + + assertEquals(1000, setting.initialPollDelayMillis()); + assertEquals(2, setting.pollDelayMultiplier(), DELTA); + assertEquals(10000, setting.maxPollDelayMillis()); + assertEquals(900000, setting.totalPollTimeoutMillis()); + + // Sanity-check on a different service and method. + // The list element order is coupled to the definition order in the gapic.yaml file. + // This LRO setting has a max_poll_delay_millis that will be parsed as a double. + setting = lroRetrySettings.get(4); + assertEquals("google.cloud.dataproc.v1", setting.protoPakkage()); + assertEquals("WorkflowTemplateService", setting.serviceName()); + assertEquals("InstantiateWorkflowTemplate", setting.methodName()); + + assertEquals(1000, setting.initialPollDelayMillis()); + assertEquals(2.5, setting.pollDelayMultiplier(), DELTA); + assertEquals(10000, setting.maxPollDelayMillis()); + assertEquals(43200000, setting.totalPollTimeoutMillis()); + } +} diff --git a/src/test/java/com/google/api/generator/gapic/testdata/dataproc_gapic.yaml b/src/test/java/com/google/api/generator/gapic/testdata/dataproc_gapic.yaml new file mode 100644 index 0000000000..c8cb4674e0 --- /dev/null +++ b/src/test/java/com/google/api/generator/gapic/testdata/dataproc_gapic.yaml @@ -0,0 +1,65 @@ +type: com.google.api.codegen.ConfigProto +config_schema_version: 2.0.0 +language_settings: + java: + package_name: com.google.cloud.dataproc.v1 + python: + package_name: google.cloud.dataproc_v1.gapic + go: + package_name: cloud.google.com/go/dataproc/apiv1 + csharp: + package_name: Google.Cloud.Dataproc.V1 + ruby: + package_name: Google::Cloud::Dataproc::V1 + php: + package_name: Google\Cloud\Dataproc\V1 + nodejs: + package_name: dataproc.v1 + domain_layer_location: google-cloud +interfaces: +- name: google.cloud.dataproc.v1.ClusterController + smoke_test: + method: ListClusters + init_fields: + - project_id=$PROJECT_ID + - region="global" + methods: + - name: CreateCluster + long_running: + initial_poll_delay_millis: 1000 + poll_delay_multiplier: 2 + max_poll_delay_millis: 10000 + total_poll_timeout_millis: 900000 + - name: UpdateCluster + long_running: + initial_poll_delay_millis: 1000 + poll_delay_multiplier: 2 + max_poll_delay_millis: 10000 + total_poll_timeout_millis: 900000 + - name: DeleteCluster + long_running: + initial_poll_delay_millis: 1000 + poll_delay_multiplier: 2 + max_poll_delay_millis: 10000 + total_poll_timeout_millis: 900000 + - name: DiagnoseCluster + long_running: + initial_poll_delay_millis: 1000 + poll_delay_multiplier: 2 + max_poll_delay_millis: 10000 + total_poll_timeout_millis: 30000 +- name: google.cloud.dataproc.v1.WorkflowTemplateService + methods: + - name: InstantiateWorkflowTemplate + long_running: + initial_poll_delay_millis: 1000 + poll_delay_multiplier: 2.5 + max_poll_delay_millis: 10000 + total_poll_timeout_millis: 43200000 + - name: InstantiateInlineWorkflowTemplate + long_running: + initial_poll_delay_millis: 1000 + poll_delay_multiplier: 2 + max_poll_delay_millis: 10000 + total_poll_timeout_millis: 43200000 + From 7386cfbbba92cf4349bd2d437bca26ff3e0b3510 Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Fri, 20 Nov 2020 23:18:21 -0800 Subject: [PATCH 4/5] feat: integrate GapicLroRetrySettings with ServiceConfig{Parser} --- .../gapic/model/GapicServiceConfig.java | 34 ++++++++++- .../generator/gapic/protoparser/Parser.java | 5 +- .../protoparser/ServiceConfigParser.java | 8 ++- .../BatchingDescriptorComposerTest.java | 4 +- .../composer/RetrySettingsComposerTest.java | 16 ++--- .../ServiceStubSettingsClassComposerTest.java | 6 +- .../gapic/model/GapicServiceConfigTest.java | 60 ++++++++++++++++--- test/integration/BUILD.bazel | 1 + 8 files changed, 109 insertions(+), 25 deletions(-) diff --git a/src/main/java/com/google/api/generator/gapic/model/GapicServiceConfig.java b/src/main/java/com/google/api/generator/gapic/model/GapicServiceConfig.java index d161f44861..4e814b9508 100644 --- a/src/main/java/com/google/api/generator/gapic/model/GapicServiceConfig.java +++ b/src/main/java/com/google/api/generator/gapic/model/GapicServiceConfig.java @@ -42,19 +42,24 @@ public class GapicServiceConfig { private final List methodConfigs; private final Map methodConfigTable; + private final Map lroRetrySettingsTable; private final Map batchingSettingsTable; private GapicServiceConfig( List methodConfigs, Map methodConfigTable, + Map lroRetrySettingsTable, Map batchingSettingsTable) { this.methodConfigs = methodConfigs; this.methodConfigTable = methodConfigTable; + this.lroRetrySettingsTable = lroRetrySettingsTable; this.batchingSettingsTable = batchingSettingsTable; } public static GapicServiceConfig create( - ServiceConfig serviceConfig, Optional> batchingSettingsOpt) { + ServiceConfig serviceConfig, + Optional> lroRetrySettingsOpt, + Optional> batchingSettingsOpt) { // Keep this processing logic out of the constructor. Map methodConfigTable = new HashMap<>(); List methodConfigs = serviceConfig.getMethodConfigList(); @@ -65,6 +70,20 @@ public static GapicServiceConfig create( } } + Map lroRetrySettingsTable = new HashMap<>(); + if (lroRetrySettingsOpt.isPresent()) { + for (GapicLroRetrySettings lroRetrySetting : lroRetrySettingsOpt.get()) { + lroRetrySettingsTable.put( + MethodConfig.Name.newBuilder() + .setService( + String.format( + "%s.%s", lroRetrySetting.protoPakkage(), lroRetrySetting.serviceName())) + .setMethod(lroRetrySetting.methodName()) + .build(), + lroRetrySetting); + } + } + Map batchingSettingsTable = new HashMap<>(); if (batchingSettingsOpt.isPresent()) { for (GapicBatchingSettings batchingSetting : batchingSettingsOpt.get()) { @@ -79,7 +98,8 @@ public static GapicServiceConfig create( } } - return new GapicServiceConfig(methodConfigs, methodConfigTable, batchingSettingsTable); + return new GapicServiceConfig( + methodConfigs, methodConfigTable, lroRetrySettingsTable, batchingSettingsTable); } public Map getAllGapicRetrySettings(Service service) { @@ -118,10 +138,20 @@ public String getRetryParamsName(Service service, Method method) { return NO_RETRY_PARAMS_NAME; } + public boolean hasLroRetrySetting(Service service, Method method) { + return lroRetrySettingsTable.containsKey(toName(service, method)); + } + public boolean hasBatchingSetting(Service service, Method method) { return batchingSettingsTable.containsKey(toName(service, method)); } + public Optional getLroRetrySetting(Service service, Method method) { + return hasLroRetrySetting(service, method) + ? Optional.of(lroRetrySettingsTable.get(toName(service, method))) + : Optional.empty(); + } + public Optional getBatchingSetting(Service service, Method method) { return hasBatchingSetting(service, method) ? Optional.of(batchingSettingsTable.get(toName(service, method))) 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 aebf249d60..192174a66d 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 @@ -21,6 +21,7 @@ import com.google.api.generator.gapic.model.Field; import com.google.api.generator.gapic.model.GapicBatchingSettings; import com.google.api.generator.gapic.model.GapicContext; +import com.google.api.generator.gapic.model.GapicLroRetrySettings; import com.google.api.generator.gapic.model.GapicServiceConfig; import com.google.api.generator.gapic.model.LongrunningOperation; import com.google.api.generator.gapic.model.Message; @@ -84,11 +85,13 @@ public static GapicContext parse(CodeGeneratorRequest request) { PluginArgumentParser.parseGapicYamlConfigPath(request); Optional> batchingSettingsOpt = BatchingSettingsConfigParser.parse(gapicYamlConfigPathOpt); + Optional> lroRetrySettingsOpt = + GapicLroRetrySettingsParser.parse(gapicYamlConfigPathOpt); Optional serviceConfigPathOpt = PluginArgumentParser.parseJsonConfigPath(request); String serviceConfigPath = serviceConfigPathOpt.isPresent() ? serviceConfigPathOpt.get() : null; Optional serviceConfigOpt = - ServiceConfigParser.parse(serviceConfigPath, batchingSettingsOpt); + ServiceConfigParser.parse(serviceConfigPath, lroRetrySettingsOpt, batchingSettingsOpt); Optional serviceYamlConfigPathOpt = PluginArgumentParser.parseServiceYamlConfigPath(request); 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 index dcab2c4ec3..de36581176 100644 --- a/src/main/java/com/google/api/generator/gapic/protoparser/ServiceConfigParser.java +++ b/src/main/java/com/google/api/generator/gapic/protoparser/ServiceConfigParser.java @@ -15,6 +15,7 @@ package com.google.api.generator.gapic.protoparser; import com.google.api.generator.gapic.model.GapicBatchingSettings; +import com.google.api.generator.gapic.model.GapicLroRetrySettings; import com.google.api.generator.gapic.model.GapicServiceConfig; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Strings; @@ -28,12 +29,15 @@ public class ServiceConfigParser { public static Optional parse( - String serviceConfigFilePath, Optional> batchingSettingsOpt) { + String serviceConfigFilePath, + Optional> lroRetrySettingsOpt, + Optional> batchingSettingsOpt) { Optional rawConfigOpt = parseFile(serviceConfigFilePath); if (!rawConfigOpt.isPresent()) { return Optional.empty(); } - return Optional.of(GapicServiceConfig.create(rawConfigOpt.get(), batchingSettingsOpt)); + return Optional.of( + GapicServiceConfig.create(rawConfigOpt.get(), lroRetrySettingsOpt, batchingSettingsOpt)); } @VisibleForTesting diff --git a/src/test/java/com/google/api/generator/gapic/composer/BatchingDescriptorComposerTest.java b/src/test/java/com/google/api/generator/gapic/composer/BatchingDescriptorComposerTest.java index e2d366370e..7f6e682337 100644 --- a/src/test/java/com/google/api/generator/gapic/composer/BatchingDescriptorComposerTest.java +++ b/src/test/java/com/google/api/generator/gapic/composer/BatchingDescriptorComposerTest.java @@ -89,7 +89,7 @@ public void batchingDescriptor_hasSubresponseField() { String jsonFilename = "pubsub_grpc_service_config.json"; Path jsonPath = Paths.get(ComposerConstants.TESTFILES_DIRECTORY, jsonFilename); Optional configOpt = - ServiceConfigParser.parse(jsonPath.toString(), batchingSettingsOpt); + ServiceConfigParser.parse(jsonPath.toString(), Optional.empty(), batchingSettingsOpt); assertTrue(configOpt.isPresent()); GapicServiceConfig config = configOpt.get(); @@ -151,7 +151,7 @@ public void batchingDescriptor_noSubresponseField() { String jsonFilename = "logging_grpc_service_config.json"; Path jsonPath = Paths.get(ComposerConstants.TESTFILES_DIRECTORY, jsonFilename); Optional configOpt = - ServiceConfigParser.parse(jsonPath.toString(), batchingSettingsOpt); + ServiceConfigParser.parse(jsonPath.toString(), Optional.empty(), batchingSettingsOpt); assertTrue(configOpt.isPresent()); GapicServiceConfig config = configOpt.get(); diff --git a/src/test/java/com/google/api/generator/gapic/composer/RetrySettingsComposerTest.java b/src/test/java/com/google/api/generator/gapic/composer/RetrySettingsComposerTest.java index b451dd2579..a93bce5c94 100644 --- a/src/test/java/com/google/api/generator/gapic/composer/RetrySettingsComposerTest.java +++ b/src/test/java/com/google/api/generator/gapic/composer/RetrySettingsComposerTest.java @@ -91,7 +91,7 @@ public void paramDefinitionsBlock_noConfigsFound() { String jsonFilename = "retrying_grpc_service_config.json"; Path jsonPath = Paths.get(ComposerConstants.TESTFILES_DIRECTORY, jsonFilename); Optional serviceConfigOpt = - ServiceConfigParser.parse(jsonPath.toString(), Optional.empty()); + ServiceConfigParser.parse(jsonPath.toString(), Optional.empty(), Optional.empty()); assertTrue(serviceConfigOpt.isPresent()); GapicServiceConfig serviceConfig = serviceConfigOpt.get(); @@ -129,7 +129,7 @@ public void paramDefinitionsBlock_basic() { String jsonFilename = "showcase_grpc_service_config.json"; Path jsonPath = Paths.get(ComposerConstants.TESTFILES_DIRECTORY, jsonFilename); Optional serviceConfigOpt = - ServiceConfigParser.parse(jsonPath.toString(), Optional.empty()); + ServiceConfigParser.parse(jsonPath.toString(), Optional.empty(), Optional.empty()); assertTrue(serviceConfigOpt.isPresent()); GapicServiceConfig serviceConfig = serviceConfigOpt.get(); @@ -181,7 +181,7 @@ public void codesDefinitionsBlock_noConfigsFound() { String jsonFilename = "retrying_grpc_service_config.json"; Path jsonPath = Paths.get(ComposerConstants.TESTFILES_DIRECTORY, jsonFilename); Optional serviceConfigOpt = - ServiceConfigParser.parse(jsonPath.toString(), Optional.empty()); + ServiceConfigParser.parse(jsonPath.toString(), Optional.empty(), Optional.empty()); assertTrue(serviceConfigOpt.isPresent()); GapicServiceConfig serviceConfig = serviceConfigOpt.get(); @@ -219,7 +219,7 @@ public void codesDefinitionsBlock_basic() { String jsonFilename = "showcase_grpc_service_config.json"; Path jsonPath = Paths.get(ComposerConstants.TESTFILES_DIRECTORY, jsonFilename); Optional serviceConfigOpt = - ServiceConfigParser.parse(jsonPath.toString(), Optional.empty()); + ServiceConfigParser.parse(jsonPath.toString(), Optional.empty(), Optional.empty()); assertTrue(serviceConfigOpt.isPresent()); GapicServiceConfig serviceConfig = serviceConfigOpt.get(); @@ -260,7 +260,7 @@ public void simpleBuilderExpr_basic() { String jsonFilename = "showcase_grpc_service_config.json"; Path jsonPath = Paths.get(ComposerConstants.TESTFILES_DIRECTORY, jsonFilename); Optional serviceConfigOpt = - ServiceConfigParser.parse(jsonPath.toString(), Optional.empty()); + ServiceConfigParser.parse(jsonPath.toString(), Optional.empty(), Optional.empty()); assertTrue(serviceConfigOpt.isPresent()); GapicServiceConfig serviceConfig = serviceConfigOpt.get(); @@ -343,7 +343,7 @@ public void lroBuilderExpr() { String jsonFilename = "showcase_grpc_service_config.json"; Path jsonPath = Paths.get(ComposerConstants.TESTFILES_DIRECTORY, jsonFilename); Optional serviceConfigOpt = - ServiceConfigParser.parse(jsonPath.toString(), Optional.empty()); + ServiceConfigParser.parse(jsonPath.toString(), Optional.empty(), Optional.empty()); assertTrue(serviceConfigOpt.isPresent()); GapicServiceConfig serviceConfig = serviceConfigOpt.get(); @@ -411,7 +411,7 @@ public void batchingSettings_minimalFlowControlSettings() { String jsonFilename = "pubsub_grpc_service_config.json"; Path jsonPath = Paths.get(ComposerConstants.TESTFILES_DIRECTORY, jsonFilename); Optional configOpt = - ServiceConfigParser.parse(jsonPath.toString(), batchingSettingsOpt); + ServiceConfigParser.parse(jsonPath.toString(), Optional.empty(), batchingSettingsOpt); assertTrue(configOpt.isPresent()); GapicServiceConfig config = configOpt.get(); @@ -491,7 +491,7 @@ public void batchingSettings_fullFlowControlSettings() { String jsonFilename = "logging_grpc_service_config.json"; Path jsonPath = Paths.get(ComposerConstants.TESTFILES_DIRECTORY, jsonFilename); Optional configOpt = - ServiceConfigParser.parse(jsonPath.toString(), batchingSettingsOpt); + ServiceConfigParser.parse(jsonPath.toString(), Optional.empty(), batchingSettingsOpt); assertTrue(configOpt.isPresent()); GapicServiceConfig config = configOpt.get(); 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 527e513101..ea120724be 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 @@ -84,7 +84,7 @@ public void generateServiceStubSettingsClasses_batchingWithEmptyResponses() thro String jsonFilename = "logging_grpc_service_config.json"; Path jsonPath = Paths.get(ComposerConstants.TESTFILES_DIRECTORY, jsonFilename); Optional configOpt = - ServiceConfigParser.parse(jsonPath.toString(), batchingSettingsOpt); + ServiceConfigParser.parse(jsonPath.toString(), Optional.empty(), batchingSettingsOpt); assertTrue(configOpt.isPresent()); GapicServiceConfig config = configOpt.get(); @@ -127,7 +127,7 @@ public void generateServiceStubSettingsClasses_batchingWithNonemptyResponses() String jsonFilename = "pubsub_grpc_service_config.json"; Path jsonPath = Paths.get(ComposerConstants.TESTFILES_DIRECTORY, jsonFilename); Optional configOpt = - ServiceConfigParser.parse(jsonPath.toString(), batchingSettingsOpt); + ServiceConfigParser.parse(jsonPath.toString(), Optional.empty(), batchingSettingsOpt); assertTrue(configOpt.isPresent()); GapicServiceConfig config = configOpt.get(); @@ -158,7 +158,7 @@ public void generateServiceStubSettingsClasses_basic() throws IOException { String jsonFilename = "showcase_grpc_service_config.json"; Path jsonPath = Paths.get(ComposerConstants.TESTFILES_DIRECTORY, jsonFilename); Optional configOpt = - ServiceConfigParser.parse(jsonPath.toString(), Optional.empty()); + ServiceConfigParser.parse(jsonPath.toString(), Optional.empty(), Optional.empty()); assertTrue(configOpt.isPresent()); GapicServiceConfig config = configOpt.get(); diff --git a/src/test/java/com/google/api/generator/gapic/model/GapicServiceConfigTest.java b/src/test/java/com/google/api/generator/gapic/model/GapicServiceConfigTest.java index 43564350d5..30d5234a8d 100644 --- a/src/test/java/com/google/api/generator/gapic/model/GapicServiceConfigTest.java +++ b/src/test/java/com/google/api/generator/gapic/model/GapicServiceConfigTest.java @@ -39,7 +39,7 @@ public class GapicServiceConfigTest { private static final double EPSILON = 1e-4; - private static final String JSON_DIRECTORY = + private static final String TESTDATA_DIRECTORY = "src/test/java/com/google/api/generator/gapic/testdata/"; @Test @@ -49,10 +49,11 @@ public void serviceConfig_noConfigsFound() { Service service = parseService(echoFileDescriptor); String jsonFilename = "retrying_grpc_service_config.json"; - Path jsonPath = Paths.get(JSON_DIRECTORY, jsonFilename); + Path jsonPath = Paths.get(TESTDATA_DIRECTORY, jsonFilename); + Optional> lroRetrySettingsOpt = Optional.empty(); Optional> batchingSettingsOpt = Optional.empty(); Optional serviceConfigOpt = - ServiceConfigParser.parse(jsonPath.toString(), batchingSettingsOpt); + ServiceConfigParser.parse(jsonPath.toString(), lroRetrySettingsOpt, batchingSettingsOpt); assertTrue(serviceConfigOpt.isPresent()); GapicServiceConfig serviceConfig = serviceConfigOpt.get(); @@ -80,10 +81,11 @@ public void serviceConfig_basic() { Service service = parseService(echoFileDescriptor); String jsonFilename = "showcase_grpc_service_config.json"; - Path jsonPath = Paths.get(JSON_DIRECTORY, jsonFilename); + Path jsonPath = Paths.get(TESTDATA_DIRECTORY, jsonFilename); + Optional> lroRetrySettingsOpt = Optional.empty(); Optional> batchingSettingsOpt = Optional.empty(); Optional serviceConfigOpt = - ServiceConfigParser.parse(jsonPath.toString(), batchingSettingsOpt); + ServiceConfigParser.parse(jsonPath.toString(), lroRetrySettingsOpt, batchingSettingsOpt); assertTrue(serviceConfigOpt.isPresent()); GapicServiceConfig serviceConfig = serviceConfigOpt.get(); @@ -137,7 +139,7 @@ public void serviceConfig_withBatchingSettings() { Service service = parseService(echoFileDescriptor); String jsonFilename = "showcase_grpc_service_config.json"; - Path jsonPath = Paths.get(JSON_DIRECTORY, jsonFilename); + Path jsonPath = Paths.get(TESTDATA_DIRECTORY, jsonFilename); GapicBatchingSettings origBatchingSetting = GapicBatchingSettings.builder() @@ -152,9 +154,10 @@ public void serviceConfig_withBatchingSettings() { .build(); Optional> batchingSettingsOpt = Optional.of(Arrays.asList(origBatchingSetting)); + Optional> lroRetrySettingsOpt = Optional.empty(); Optional serviceConfigOpt = - ServiceConfigParser.parse(jsonPath.toString(), batchingSettingsOpt); + ServiceConfigParser.parse(jsonPath.toString(), lroRetrySettingsOpt, batchingSettingsOpt); assertTrue(serviceConfigOpt.isPresent()); GapicServiceConfig serviceConfig = serviceConfigOpt.get(); @@ -204,6 +207,49 @@ public void serviceConfig_withBatchingSettings() { assertFalse(serviceConfig.hasBatchingSetting(service, method)); } + @Test + public void serviceConfig_withLroRetrySettings() { + FileDescriptor echoFileDescriptor = EchoOuterClass.getDescriptor(); + ServiceDescriptor echoServiceDescriptor = echoFileDescriptor.getServices().get(0); + Service service = parseService(echoFileDescriptor); + + String jsonFilename = "showcase_grpc_service_config.json"; + Path jsonPath = Paths.get(TESTDATA_DIRECTORY, jsonFilename); + Optional> batchingSettingsOpt = Optional.empty(); + + // Construct LRO retry settings. + GapicLroRetrySettings origLroRetrySetting = + GapicLroRetrySettings.builder() + .setProtoPakkage("google.showcase.v1beta1") + .setServiceName("Echo") + .setMethodName("Echo") + .setInitialPollDelayMillis(100) + .setPollDelayMultiplier(1.5) + .setMaxPollDelayMillis(200) + .setTotalPollTimeoutMillis(300) + .build(); + Optional> lroRetrySettingsOpt = + Optional.of(Arrays.asList(origLroRetrySetting)); + + Optional serviceConfigOpt = + ServiceConfigParser.parse(jsonPath.toString(), lroRetrySettingsOpt, batchingSettingsOpt); + assertTrue(serviceConfigOpt.isPresent()); + GapicServiceConfig serviceConfig = serviceConfigOpt.get(); + + // Check LRO retry settings. + Method method = findMethod(service, "Echo"); + assertTrue(serviceConfig.hasLroRetrySetting(service, method)); + Optional retrievedSettingsOpt = + serviceConfig.getLroRetrySetting(service, method); + assertTrue(retrievedSettingsOpt.isPresent()); + GapicLroRetrySettings retrievedSettings = retrievedSettingsOpt.get(); + assertEquals( + origLroRetrySetting.initialPollDelayMillis(), retrievedSettings.initialPollDelayMillis()); + assertEquals(origLroRetrySetting.maxPollDelayMillis(), retrievedSettings.maxPollDelayMillis()); + assertEquals( + origLroRetrySetting.totalPollTimeoutMillis(), retrievedSettings.totalPollTimeoutMillis()); + } + private static Service parseService(FileDescriptor fileDescriptor) { Map messageTypes = Parser.parseMessages(fileDescriptor); Map resourceNames = Parser.parseResourceNames(fileDescriptor); diff --git a/test/integration/BUILD.bazel b/test/integration/BUILD.bazel index 162b68f2aa..072af0f3e5 100644 --- a/test/integration/BUILD.bazel +++ b/test/integration/BUILD.bazel @@ -83,6 +83,7 @@ java_gapic_assembly_gradle_pkg( java_gapic_library( name = "redis_java_gapic", srcs = ["@com_google_googleapis//google/cloud/redis/v1:redis_proto_with_info"], + gapic_yaml = "@com_google_googleapis//google/cloud/redis/v1:redis_gapic.yaml", grpc_service_config = "@com_google_googleapis//google/cloud/redis/v1:redis_grpc_service_config.json", package = "google.cloud.redis.v1", test_deps = [ From 8b08a64446354e25db32d23d14b57e7b1d7b8179 Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Fri, 20 Nov 2020 23:24:15 -0800 Subject: [PATCH 5/5] feat: use gapic.yaml LRO settings if available --- .../gapic/composer/RetrySettingsComposer.java | 35 +++++++++++++----- .../goldens/redis/CloudRedisStubSettings.java | 36 +++++++++---------- 2 files changed, 44 insertions(+), 27 deletions(-) diff --git a/src/main/java/com/google/api/generator/gapic/composer/RetrySettingsComposer.java b/src/main/java/com/google/api/generator/gapic/composer/RetrySettingsComposer.java index e9785d1b78..295dd0d639 100644 --- a/src/main/java/com/google/api/generator/gapic/composer/RetrySettingsComposer.java +++ b/src/main/java/com/google/api/generator/gapic/composer/RetrySettingsComposer.java @@ -38,6 +38,7 @@ import com.google.api.generator.engine.ast.Variable; import com.google.api.generator.engine.ast.VariableExpr; import com.google.api.generator.gapic.model.GapicBatchingSettings; +import com.google.api.generator.gapic.model.GapicLroRetrySettings; import com.google.api.generator.gapic.model.GapicRetrySettings; import com.google.api.generator.gapic.model.GapicServiceConfig; import com.google.api.generator.gapic.model.Method; @@ -55,6 +56,7 @@ import java.util.Arrays; import java.util.List; import java.util.Map; +import java.util.Optional; import java.util.function.Function; import java.util.stream.Collectors; @@ -67,7 +69,7 @@ public class RetrySettingsComposer { private static final long LRO_DEFAULT_INITIAL_POLL_DELAY_MILLIS = 5000; private static final double LRO_DEFAULT_POLL_DELAY_MULTIPLIER = 1.5; private static final long LRO_DEFAULT_MAX_POLL_DELAY_MILLIS = 45000; - private static final long LRO_DEFAULT_TOTAL_POLL_TIMEOUT_MILLS = 300000; // 5 minutes. + private static final long LRO_DEFAULT_TOTAL_POLL_TIMEOUT_MILLIS = 300000; // 5 minutes. private static final double LRO_DEFAULT_MAX_RPC_TIMEOUT = 1.0; public static BlockStatement createRetryParamDefinitionsBlock( @@ -349,7 +351,7 @@ public static Expr createLroSettingsBuilderExpr( .build()) .build(); - Expr lroRetrySettingsExpr = createLroRetrySettingsExpr(); + Expr lroRetrySettingsExpr = createLroRetrySettingsExpr(service, method, serviceConfig); Expr pollAlgoExpr = MethodInvocationExpr.builder() .setStaticReferenceType(STATIC_TYPES.get("OperationTimedPollAlgorithm")) @@ -592,33 +594,49 @@ private static List createRetrySettingsExprs( return Arrays.asList(settingsAssignExpr, definitionsPutExpr); } - private static Expr createLroRetrySettingsExpr() { + private static Expr createLroRetrySettingsExpr( + Service service, Method method, GapicServiceConfig serviceConfig) { Expr lroRetrySettingsExpr = MethodInvocationExpr.builder() .setStaticReferenceType(STATIC_TYPES.get("RetrySettings")) .setMethodName("newBuilder") .build(); + long initialPollDelayMillis = LRO_DEFAULT_INITIAL_POLL_DELAY_MILLIS; + double pollDelayMultiplier = LRO_DEFAULT_POLL_DELAY_MULTIPLIER; + long maxPollDelayMillis = LRO_DEFAULT_MAX_POLL_DELAY_MILLIS; + long totalPollTimeoutMillis = LRO_DEFAULT_TOTAL_POLL_TIMEOUT_MILLIS; + if (serviceConfig.hasLroRetrySetting(service, method)) { + Optional lroRetrySettingsOpt = + serviceConfig.getLroRetrySetting(service, method); + if (lroRetrySettingsOpt.isPresent()) { + GapicLroRetrySettings lroRetrySettings = lroRetrySettingsOpt.get(); + initialPollDelayMillis = lroRetrySettings.initialPollDelayMillis(); + pollDelayMultiplier = lroRetrySettings.pollDelayMultiplier(); + maxPollDelayMillis = lroRetrySettings.maxPollDelayMillis(); + totalPollTimeoutMillis = lroRetrySettings.totalPollTimeoutMillis(); + } + } + lroRetrySettingsExpr = MethodInvocationExpr.builder() .setExprReferenceExpr(lroRetrySettingsExpr) .setMethodName("setInitialRetryDelay") - .setArguments( - createDurationOfMillisExpr(toValExpr(LRO_DEFAULT_INITIAL_POLL_DELAY_MILLIS))) + .setArguments(createDurationOfMillisExpr(toValExpr(initialPollDelayMillis))) .build(); lroRetrySettingsExpr = MethodInvocationExpr.builder() .setExprReferenceExpr(lroRetrySettingsExpr) .setMethodName("setRetryDelayMultiplier") - .setArguments(toValExpr(LRO_DEFAULT_POLL_DELAY_MULTIPLIER)) + .setArguments(toValExpr(pollDelayMultiplier)) .build(); lroRetrySettingsExpr = MethodInvocationExpr.builder() .setExprReferenceExpr(lroRetrySettingsExpr) .setMethodName("setMaxRetryDelay") - .setArguments(createDurationOfMillisExpr(toValExpr(LRO_DEFAULT_MAX_POLL_DELAY_MILLIS))) + .setArguments(createDurationOfMillisExpr(toValExpr(maxPollDelayMillis))) .build(); Expr zeroDurationExpr = @@ -651,8 +669,7 @@ private static Expr createLroRetrySettingsExpr() { MethodInvocationExpr.builder() .setExprReferenceExpr(lroRetrySettingsExpr) .setMethodName("setTotalTimeout") - .setArguments( - createDurationOfMillisExpr(toValExpr(LRO_DEFAULT_TOTAL_POLL_TIMEOUT_MILLS))) + .setArguments(createDurationOfMillisExpr(toValExpr(totalPollTimeoutMillis))) .build(); lroRetrySettingsExpr = diff --git a/test/integration/goldens/redis/CloudRedisStubSettings.java b/test/integration/goldens/redis/CloudRedisStubSettings.java index 74901d60c5..6123d677eb 100644 --- a/test/integration/goldens/redis/CloudRedisStubSettings.java +++ b/test/integration/goldens/redis/CloudRedisStubSettings.java @@ -566,13 +566,13 @@ private static Builder initDefaults(Builder builder) { .setPollingAlgorithm( OperationTimedPollAlgorithm.create( RetrySettings.newBuilder() - .setInitialRetryDelay(Duration.ofMillis(5000L)) + .setInitialRetryDelay(Duration.ofMillis(60000L)) .setRetryDelayMultiplier(1.5) - .setMaxRetryDelay(Duration.ofMillis(45000L)) + .setMaxRetryDelay(Duration.ofMillis(360000L)) .setInitialRpcTimeout(Duration.ZERO) .setRpcTimeoutMultiplier(1.0) .setMaxRpcTimeout(Duration.ZERO) - .setTotalTimeout(Duration.ofMillis(300000L)) + .setTotalTimeout(Duration.ofMillis(7200000L)) .build())); builder @@ -590,13 +590,13 @@ private static Builder initDefaults(Builder builder) { .setPollingAlgorithm( OperationTimedPollAlgorithm.create( RetrySettings.newBuilder() - .setInitialRetryDelay(Duration.ofMillis(5000L)) + .setInitialRetryDelay(Duration.ofMillis(60000L)) .setRetryDelayMultiplier(1.5) - .setMaxRetryDelay(Duration.ofMillis(45000L)) + .setMaxRetryDelay(Duration.ofMillis(360000L)) .setInitialRpcTimeout(Duration.ZERO) .setRpcTimeoutMultiplier(1.0) .setMaxRpcTimeout(Duration.ZERO) - .setTotalTimeout(Duration.ofMillis(300000L)) + .setTotalTimeout(Duration.ofMillis(7200000L)) .build())); builder @@ -638,13 +638,13 @@ private static Builder initDefaults(Builder builder) { .setPollingAlgorithm( OperationTimedPollAlgorithm.create( RetrySettings.newBuilder() - .setInitialRetryDelay(Duration.ofMillis(5000L)) + .setInitialRetryDelay(Duration.ofMillis(60000L)) .setRetryDelayMultiplier(1.5) - .setMaxRetryDelay(Duration.ofMillis(45000L)) + .setMaxRetryDelay(Duration.ofMillis(360000L)) .setInitialRpcTimeout(Duration.ZERO) .setRpcTimeoutMultiplier(1.0) .setMaxRpcTimeout(Duration.ZERO) - .setTotalTimeout(Duration.ofMillis(300000L)) + .setTotalTimeout(Duration.ofMillis(18000000L)) .build())); builder @@ -662,13 +662,13 @@ private static Builder initDefaults(Builder builder) { .setPollingAlgorithm( OperationTimedPollAlgorithm.create( RetrySettings.newBuilder() - .setInitialRetryDelay(Duration.ofMillis(5000L)) + .setInitialRetryDelay(Duration.ofMillis(60000L)) .setRetryDelayMultiplier(1.5) - .setMaxRetryDelay(Duration.ofMillis(45000L)) + .setMaxRetryDelay(Duration.ofMillis(360000L)) .setInitialRpcTimeout(Duration.ZERO) .setRpcTimeoutMultiplier(1.0) .setMaxRpcTimeout(Duration.ZERO) - .setTotalTimeout(Duration.ofMillis(300000L)) + .setTotalTimeout(Duration.ofMillis(18000000L)) .build())); builder @@ -686,13 +686,13 @@ private static Builder initDefaults(Builder builder) { .setPollingAlgorithm( OperationTimedPollAlgorithm.create( RetrySettings.newBuilder() - .setInitialRetryDelay(Duration.ofMillis(5000L)) + .setInitialRetryDelay(Duration.ofMillis(60000L)) .setRetryDelayMultiplier(1.5) - .setMaxRetryDelay(Duration.ofMillis(45000L)) + .setMaxRetryDelay(Duration.ofMillis(360000L)) .setInitialRpcTimeout(Duration.ZERO) .setRpcTimeoutMultiplier(1.0) .setMaxRpcTimeout(Duration.ZERO) - .setTotalTimeout(Duration.ofMillis(300000L)) + .setTotalTimeout(Duration.ofMillis(1200000L)) .build())); builder @@ -710,13 +710,13 @@ private static Builder initDefaults(Builder builder) { .setPollingAlgorithm( OperationTimedPollAlgorithm.create( RetrySettings.newBuilder() - .setInitialRetryDelay(Duration.ofMillis(5000L)) + .setInitialRetryDelay(Duration.ofMillis(60000L)) .setRetryDelayMultiplier(1.5) - .setMaxRetryDelay(Duration.ofMillis(45000L)) + .setMaxRetryDelay(Duration.ofMillis(360000L)) .setInitialRpcTimeout(Duration.ZERO) .setRpcTimeoutMultiplier(1.0) .setMaxRpcTimeout(Duration.ZERO) - .setTotalTimeout(Duration.ofMillis(300000L)) + .setTotalTimeout(Duration.ofMillis(1200000L)) .build())); return builder;