diff --git a/src/main/java/com/google/api/generator/gapic/model/GapicBatchingSettings.java b/src/main/java/com/google/api/generator/gapic/model/GapicBatchingSettings.java index 04085ab5f4..349cb479ab 100644 --- a/src/main/java/com/google/api/generator/gapic/model/GapicBatchingSettings.java +++ b/src/main/java/com/google/api/generator/gapic/model/GapicBatchingSettings.java @@ -45,6 +45,12 @@ public enum FlowControlLimitExceededBehavior { public abstract FlowControlLimitExceededBehavior flowControlLimitExceededBehavior(); + public boolean matches(Service service, Method method) { + return protoPakkage().equals(service.protoPakkage()) + && serviceName().equals(service.name()) + && methodName().equals(method.name()); + } + public static Builder builder() { return new AutoValue_GapicBatchingSettings.Builder() .setFlowControlLimitExceededBehavior(FlowControlLimitExceededBehavior.IGNORE); 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 c19e23bfdc..d161f44861 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,14 +42,19 @@ public class GapicServiceConfig { private final List methodConfigs; private final Map methodConfigTable; + private final Map batchingSettingsTable; private GapicServiceConfig( - List methodConfigs, Map methodConfigTable) { + List methodConfigs, + Map methodConfigTable, + Map batchingSettingsTable) { this.methodConfigs = methodConfigs; this.methodConfigTable = methodConfigTable; + this.batchingSettingsTable = batchingSettingsTable; } - public static GapicServiceConfig create(ServiceConfig serviceConfig) { + public static GapicServiceConfig create( + ServiceConfig serviceConfig, Optional> batchingSettingsOpt) { // Keep this processing logic out of the constructor. Map methodConfigTable = new HashMap<>(); List methodConfigs = serviceConfig.getMethodConfigList(); @@ -60,7 +65,21 @@ public static GapicServiceConfig create(ServiceConfig serviceConfig) { } } - return new GapicServiceConfig(methodConfigs, methodConfigTable); + Map batchingSettingsTable = new HashMap<>(); + if (batchingSettingsOpt.isPresent()) { + for (GapicBatchingSettings batchingSetting : batchingSettingsOpt.get()) { + batchingSettingsTable.put( + MethodConfig.Name.newBuilder() + .setService( + String.format( + "%s.%s", batchingSetting.protoPakkage(), batchingSetting.serviceName())) + .setMethod(batchingSetting.methodName()) + .build(), + batchingSetting); + } + } + + return new GapicServiceConfig(methodConfigs, methodConfigTable, batchingSettingsTable); } public Map getAllGapicRetrySettings(Service service) { @@ -68,28 +87,7 @@ public Map getAllGapicRetrySettings(Service service) .collect( Collectors.toMap( m -> getRetryParamsName(service, m), - m -> { - GapicRetrySettings.Kind kind = GapicRetrySettings.Kind.FULL; - Optional retryPolicyIndexOpt = retryPolicyIndexLookup(service, m); - if (!retryPolicyIndexOpt.isPresent()) { - kind = GapicRetrySettings.Kind.NONE; - } else { - MethodConfig methodConfig = methodConfigs.get(retryPolicyIndexOpt.get()); - if (!methodConfig.hasTimeout() && !methodConfig.hasRetryPolicy()) { - kind = GapicRetrySettings.Kind.NONE; - } else { - kind = - methodConfig.hasRetryPolicy() - ? GapicRetrySettings.Kind.FULL - : GapicRetrySettings.Kind.NO_RETRY; - } - } - return GapicRetrySettings.builder() - .setTimeout(timeoutLookup(service, m)) - .setRetryPolicy(retryPolicyLookup(service, m)) - .setKind(kind) - .build(); - }, + m -> toGapicRetrySettings(service, m), (r1, r2) -> r2, LinkedHashMap::new)); } @@ -120,6 +118,40 @@ public String getRetryParamsName(Service service, Method method) { return NO_RETRY_PARAMS_NAME; } + public boolean hasBatchingSetting(Service service, Method method) { + return batchingSettingsTable.containsKey(toName(service, method)); + } + + public Optional getBatchingSetting(Service service, Method method) { + return hasBatchingSetting(service, method) + ? Optional.of(batchingSettingsTable.get(toName(service, method))) + : Optional.empty(); + } + + private GapicRetrySettings toGapicRetrySettings(Service service, Method method) { + GapicRetrySettings.Kind kind = GapicRetrySettings.Kind.FULL; + Optional retryPolicyIndexOpt = retryPolicyIndexLookup(service, method); + if (!retryPolicyIndexOpt.isPresent()) { + kind = GapicRetrySettings.Kind.NONE; + } else { + MethodConfig methodConfig = methodConfigs.get(retryPolicyIndexOpt.get()); + if (!methodConfig.hasTimeout() && !methodConfig.hasRetryPolicy()) { + kind = GapicRetrySettings.Kind.NONE; + } else { + kind = + methodConfig.hasRetryPolicy() + ? GapicRetrySettings.Kind.FULL + : GapicRetrySettings.Kind.NO_RETRY; + } + } + + return GapicRetrySettings.builder() + .setTimeout(timeoutLookup(service, method)) + .setRetryPolicy(retryPolicyLookup(service, method)) + .setKind(kind) + .build(); + } + private List retryCodesLookup(Service service, Method method) { RetryPolicy retryPolicy = retryPolicyLookup(service, method); if (retryPolicy.equals(EMPTY_RETRY_POLICY)) { diff --git a/src/main/java/com/google/api/generator/gapic/protoparser/BatchingSettingsConfigParser.java b/src/main/java/com/google/api/generator/gapic/protoparser/BatchingSettingsConfigParser.java index ce55d25750..71ac168343 100644 --- a/src/main/java/com/google/api/generator/gapic/protoparser/BatchingSettingsConfigParser.java +++ b/src/main/java/com/google/api/generator/gapic/protoparser/BatchingSettingsConfigParser.java @@ -45,7 +45,14 @@ public class BatchingSettingsConfigParser { private static String YAML_KEY_BATCHING_FLOW_CONTROL_LIMIT_EXCEEDED_BEHAVIOR = "flow_control_limit_exceeded_behavior"; - public static Optional> parse(String gapicYamlConfigFilePath) { + public static Optional> parse( + Optional gapicYamlConfigFilePathOpt) { + return gapicYamlConfigFilePathOpt.isPresent() + ? parse(gapicYamlConfigFilePathOpt.get()) + : Optional.empty(); + } + + static Optional> parse(String gapicYamlConfigFilePath) { if (Strings.isNullOrEmpty(gapicYamlConfigFilePath) || !(new File(gapicYamlConfigFilePath)).exists()) { return Optional.empty(); 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 42c64e6d9f..7a6721710e 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 @@ -20,6 +20,7 @@ import com.google.api.generator.engine.ast.TypeNode; import com.google.api.generator.engine.ast.VaporReference; 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.GapicServiceConfig; import com.google.api.generator.gapic.model.LongrunningOperation; @@ -70,15 +71,15 @@ public GapicParserException(String errorMessage) { } public static GapicContext parse(CodeGeneratorRequest request) { - Optional serviceConfigPathOpt = PluginArgumentParser.parseJsonConfigPath(request); - String serviceConfigPath = serviceConfigPathOpt.isPresent() ? serviceConfigPathOpt.get() : null; - Optional serviceConfigOpt = ServiceConfigParser.parse(serviceConfigPath); - - // TODO(miraleung): Actually pars the yaml file. Optional gapicYamlConfigPathOpt = PluginArgumentParser.parseGapicYamlConfigPath(request); - String gapicYamlConfigPath = - gapicYamlConfigPathOpt.isPresent() ? gapicYamlConfigPathOpt.get() : null; + Optional> batchingSettingsOpt = + BatchingSettingsConfigParser.parse(gapicYamlConfigPathOpt); + + Optional serviceConfigPathOpt = PluginArgumentParser.parseJsonConfigPath(request); + String serviceConfigPath = serviceConfigPathOpt.isPresent() ? serviceConfigPathOpt.get() : null; + Optional serviceConfigOpt = + ServiceConfigParser.parse(serviceConfigPath, batchingSettingsOpt); // Keep message and resource name parsing separate for cleaner logic. // While this takes an extra pass through the protobufs, the extra time is relatively trivial 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 24bbe84716..dcab2c4ec3 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 @@ -14,6 +14,7 @@ package com.google.api.generator.gapic.protoparser; +import com.google.api.generator.gapic.model.GapicBatchingSettings; import com.google.api.generator.gapic.model.GapicServiceConfig; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Strings; @@ -22,15 +23,17 @@ import java.io.FileNotFoundException; import java.io.FileReader; import java.io.IOException; +import java.util.List; import java.util.Optional; public class ServiceConfigParser { - public static Optional parse(String serviceConfigFilePath) { + public static Optional parse( + String serviceConfigFilePath, Optional> batchingSettingsOpt) { Optional rawConfigOpt = parseFile(serviceConfigFilePath); if (!rawConfigOpt.isPresent()) { return Optional.empty(); } - return Optional.of(GapicServiceConfig.create(rawConfigOpt.get())); + return Optional.of(GapicServiceConfig.create(rawConfigOpt.get(), batchingSettingsOpt)); } @VisibleForTesting 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 d4aae0abb3..3c4a14e5e4 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 @@ -81,7 +81,8 @@ public void paramDefinitionsBlock_noConfigsFound() { String jsonFilename = "retrying_grpc_service_config.json"; Path jsonPath = Paths.get(JSON_DIRECTORY, jsonFilename); - Optional serviceConfigOpt = ServiceConfigParser.parse(jsonPath.toString()); + Optional serviceConfigOpt = + ServiceConfigParser.parse(jsonPath.toString(), Optional.empty()); assertTrue(serviceConfigOpt.isPresent()); GapicServiceConfig serviceConfig = serviceConfigOpt.get(); @@ -117,7 +118,8 @@ public void paramDefinitionsBlock_basic() { String jsonFilename = "showcase_grpc_service_config.json"; Path jsonPath = Paths.get(JSON_DIRECTORY, jsonFilename); - Optional serviceConfigOpt = ServiceConfigParser.parse(jsonPath.toString()); + Optional serviceConfigOpt = + ServiceConfigParser.parse(jsonPath.toString(), Optional.empty()); assertTrue(serviceConfigOpt.isPresent()); GapicServiceConfig serviceConfig = serviceConfigOpt.get(); @@ -157,7 +159,8 @@ public void codesDefinitionsBlock_noConfigsFound() { String jsonFilename = "retrying_grpc_service_config.json"; Path jsonPath = Paths.get(JSON_DIRECTORY, jsonFilename); - Optional serviceConfigOpt = ServiceConfigParser.parse(jsonPath.toString()); + Optional serviceConfigOpt = + ServiceConfigParser.parse(jsonPath.toString(), Optional.empty()); assertTrue(serviceConfigOpt.isPresent()); GapicServiceConfig serviceConfig = serviceConfigOpt.get(); @@ -193,7 +196,8 @@ public void codesDefinitionsBlock_basic() { String jsonFilename = "showcase_grpc_service_config.json"; Path jsonPath = Paths.get(JSON_DIRECTORY, jsonFilename); - Optional serviceConfigOpt = ServiceConfigParser.parse(jsonPath.toString()); + Optional serviceConfigOpt = + ServiceConfigParser.parse(jsonPath.toString(), Optional.empty()); assertTrue(serviceConfigOpt.isPresent()); GapicServiceConfig serviceConfig = serviceConfigOpt.get(); @@ -232,7 +236,8 @@ public void simpleBuilderExpr_basic() { String jsonFilename = "showcase_grpc_service_config.json"; Path jsonPath = Paths.get(JSON_DIRECTORY, jsonFilename); - Optional serviceConfigOpt = ServiceConfigParser.parse(jsonPath.toString()); + Optional serviceConfigOpt = + ServiceConfigParser.parse(jsonPath.toString(), Optional.empty()); assertTrue(serviceConfigOpt.isPresent()); GapicServiceConfig serviceConfig = serviceConfigOpt.get(); @@ -313,7 +318,8 @@ public void lroBuilderExpr() { String jsonFilename = "showcase_grpc_service_config.json"; Path jsonPath = Paths.get(JSON_DIRECTORY, jsonFilename); - Optional serviceConfigOpt = ServiceConfigParser.parse(jsonPath.toString()); + Optional serviceConfigOpt = + ServiceConfigParser.parse(jsonPath.toString(), Optional.empty()); assertTrue(serviceConfigOpt.isPresent()); GapicServiceConfig serviceConfig = serviceConfigOpt.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 75cb1b3275..4050efe984 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 @@ -62,7 +62,8 @@ public void generateServiceClasses() { String jsonFilename = "showcase_grpc_service_config.json"; Path jsonPath = Paths.get(JSON_DIRECTORY, jsonFilename); - Optional configOpt = ServiceConfigParser.parse(jsonPath.toString()); + Optional configOpt = + ServiceConfigParser.parse(jsonPath.toString(), 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 dbeb6c8e92..6b6618a2dd 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 @@ -16,6 +16,7 @@ import static com.google.common.truth.Truth.assertThat; import static junit.framework.Assert.assertEquals; +import static junit.framework.Assert.assertFalse; import static junit.framework.Assert.assertTrue; import com.google.api.generator.gapic.protoparser.Parser; @@ -28,6 +29,7 @@ import io.grpc.serviceconfig.MethodConfig; import java.nio.file.Path; import java.nio.file.Paths; +import java.util.Arrays; import java.util.HashSet; import java.util.List; import java.util.Map; @@ -48,7 +50,9 @@ public void serviceConfig_noConfigsFound() { String jsonFilename = "retrying_grpc_service_config.json"; Path jsonPath = Paths.get(JSON_DIRECTORY, jsonFilename); - Optional serviceConfigOpt = ServiceConfigParser.parse(jsonPath.toString()); + Optional> batchingSettingsOpt = Optional.empty(); + Optional serviceConfigOpt = + ServiceConfigParser.parse(jsonPath.toString(), batchingSettingsOpt); assertTrue(serviceConfigOpt.isPresent()); GapicServiceConfig serviceConfig = serviceConfigOpt.get(); @@ -77,7 +81,9 @@ public void serviceConfig_basic() { String jsonFilename = "showcase_grpc_service_config.json"; Path jsonPath = Paths.get(JSON_DIRECTORY, jsonFilename); - Optional serviceConfigOpt = ServiceConfigParser.parse(jsonPath.toString()); + Optional> batchingSettingsOpt = Optional.empty(); + Optional serviceConfigOpt = + ServiceConfigParser.parse(jsonPath.toString(), batchingSettingsOpt); assertTrue(serviceConfigOpt.isPresent()); GapicServiceConfig serviceConfig = serviceConfigOpt.get(); @@ -124,6 +130,78 @@ public void serviceConfig_basic() { assertEquals(GapicServiceConfig.EMPTY_RETRY_CODES, retryCodes.get(retryCodeName)); } + @Test + public void serviceConfig_withBatchingSettings() { + 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(JSON_DIRECTORY, jsonFilename); + + GapicBatchingSettings origBatchingSetting = + GapicBatchingSettings.builder() + .setProtoPakkage("google.showcase.v1beta1") + .setServiceName("Echo") + .setMethodName("Echo") + .setElementCountThreshold(1000) + .setRequestByteThreshold(2000) + .setDelayThresholdMillis(3000) + .build(); + Optional> batchingSettingsOpt = + Optional.of(Arrays.asList(origBatchingSetting)); + + Optional serviceConfigOpt = + ServiceConfigParser.parse(jsonPath.toString(), batchingSettingsOpt); + assertTrue(serviceConfigOpt.isPresent()); + GapicServiceConfig serviceConfig = serviceConfigOpt.get(); + + Map retrySettings = serviceConfig.getAllGapicRetrySettings(service); + assertEquals(2, retrySettings.size()); + Map> retryCodes = serviceConfig.getAllRetryCodes(service); + assertEquals(2, retryCodes.size()); + + // Echo method has an explicitly-defined setting. + Method method = findMethod(service, "Echo"); + assertThat(method).isNotNull(); + + // No change to the retry settings. + String retryParamsName = serviceConfig.getRetryParamsName(service, method); + assertEquals("retry_policy_1_params", retryParamsName); + GapicRetrySettings settings = retrySettings.get(retryParamsName); + assertThat(settings).isNotNull(); + assertEquals(10, settings.timeout().getSeconds()); + assertEquals(GapicRetrySettings.Kind.FULL, settings.kind()); + + // No changge to the retry codes. + String retryCodeName = serviceConfig.getRetryCodeName(service, method); + assertEquals("retry_policy_1_codes", retryCodeName); + List retryCode = retryCodes.get(retryCodeName); + assertThat(retryCode).containsExactly(Code.UNAVAILABLE, Code.UNKNOWN); + + // Check batching settings. + assertTrue(serviceConfig.hasBatchingSetting(service, method)); + Optional batchingSettingOpt = + serviceConfig.getBatchingSetting(service, method); + assertTrue(batchingSettingOpt.isPresent()); + GapicBatchingSettings batchingSetting = batchingSettingOpt.get(); + assertEquals( + origBatchingSetting.elementCountThreshold(), batchingSetting.elementCountThreshold()); + assertEquals( + origBatchingSetting.requestByteThreshold(), batchingSetting.requestByteThreshold()); + assertEquals( + origBatchingSetting.delayThresholdMillis(), batchingSetting.delayThresholdMillis()); + + // Chat method defaults to the service-defined setting. + method = findMethod(service, "Chat"); + assertThat(method).isNotNull(); + retryParamsName = serviceConfig.getRetryParamsName(service, method); + assertEquals("no_retry_0_params", retryParamsName); + retryCodeName = serviceConfig.getRetryCodeName(service, method); + assertEquals("no_retry_0_codes", retryCodeName); + assertFalse(serviceConfig.hasBatchingSetting(service, method)); + } + private static Service parseService(FileDescriptor fileDescriptor) { Map messageTypes = Parser.parseMessages(fileDescriptor); Map resourceNames = Parser.parseResourceNames(fileDescriptor);