From 15cce16c47c394b66c75a6b668794892cb3dc120 Mon Sep 17 00:00:00 2001 From: Andrea Lin Date: Mon, 22 Apr 2019 11:00:25 -0700 Subject: [PATCH] Merge in gapic_config_v2 (#2721) * Add Gapic config v2 (#2665) * Whittling down config_v2 (#2666) * Add ConfigV2 Validator (#2672) * AutoValue LongRunningConfig; always use gapic config's polling settings (#2698) * ResourceNameOneofConfig fixes (#2704) * Start parsing GAPIC config v2 (#2703) * Bring back timeout millis in GAPIC config v2 (#2708) * Resource names across different protofiles (#2711) * Fix missing default retries (#2718) * Bug fixes for gapic config v2 parsing (#2717) --- BUILD.bazel | 11 +- .../config/DiscoGapicInterfaceConfig.java | 3 +- .../config/FixedResourceNameConfig.java | 1 + .../codegen/config/GapicInterfaceConfig.java | 39 +- .../codegen/config/GapicProductConfig.java | 253 ++++---- .../api/codegen/config/LongRunningConfig.java | 163 +++-- .../codegen/config/PageStreamingConfig.java | 1 + .../codegen/config/ProtoInterfaceModel.java | 12 +- .../config/ResourceNameOneofConfig.java | 65 +- .../api/codegen/config/RetryCodesConfig.java | 83 +-- .../config/SingleResourceNameConfig.java | 5 +- .../codegen/util/ConfigVersionValidator.java | 69 ++ .../proto/com/google/api/codegen/config.proto | 3 +- .../com/google/api/codegen/v2/config_v2.proto | 605 ++++++++++++++++++ .../config/GapicConfigProducerTest.java | 29 +- .../codegen/config/LongRunningConfigTest.java | 24 +- .../config/PageStreamingConfigTest.java | 23 + .../ResourceNameMessageConfigsTest.java | 6 + .../config/testdata/missing_interface_v1.yaml | 60 ++ .../java/java_multiple_services.baseline | 6 + .../java/java_no_path_templates.baseline | 3 + .../nodejs/nodejs_multiple_services.baseline | 8 + .../nodejs/nodejs_no_path_templates.baseline | 4 + .../php/php_no_path_templates.baseline | 4 + .../py/python_multiple_services.baseline | 8 + .../py/python_no_path_templates.baseline | 4 + .../ruby/ruby_multiple_services.baseline | 8 + .../java_library_no_gapic_config.baseline | 53 +- .../ruby_library_no_gapic_config.baseline | 97 +-- .../RetryDefinitionsTransformerTest.java | 8 +- .../api/codegen/util/ConfigValidatorTest.java | 66 ++ 31 files changed, 1347 insertions(+), 377 deletions(-) create mode 100644 src/main/java/com/google/api/codegen/util/ConfigVersionValidator.java create mode 100644 src/main/proto/com/google/api/codegen/v2/config_v2.proto create mode 100644 src/test/java/com/google/api/codegen/config/PageStreamingConfigTest.java create mode 100644 src/test/java/com/google/api/codegen/config/testdata/missing_interface_v1.yaml create mode 100644 src/test/java/com/google/api/codegen/util/ConfigValidatorTest.java diff --git a/BUILD.bazel b/BUILD.bazel index 7f04e8e422..579ea9915f 100644 --- a/BUILD.bazel +++ b/BUILD.bazel @@ -13,6 +13,7 @@ _COMPILE_DEPS = [ "@com_fasterxml_jackson_core_jackson_databind//jar", "@com_google_api_api_common//jar", "@com_google_api_api_compiler//jar", + "@com_google_api_api_compiler_testing//jar", "@com_google_api_grpc_proto_google_common_protos__com_google_api_codegen//jar", "@com_google_auto_value_auto_value//jar", "@com_google_auto_value_auto_value_annotations//jar", @@ -31,7 +32,6 @@ _COMPILE_DEPS = [ ] _TEST_COMPILE_DEPS = [ - "@com_google_api_api_compiler_testing//jar", "@com_google_truth_truth//jar", "@junit_junit//jar", "@pl_pragmatists_JUnitParams//jar", @@ -60,9 +60,16 @@ proto_library( deps = ["@com_google_protobuf//:wrappers_proto"], ) +proto_library( + name = "config_v2_proto", + srcs = ["src/main/proto/com/google/api/codegen/v2/config_v2.proto"], + deps = ["@com_google_protobuf//:wrappers_proto"], +) + java_proto_library( name = "config_java_proto", - deps = [":config_proto"], + deps = [":config_proto", + ":config_v2_proto"], ) java_binary( diff --git a/src/main/java/com/google/api/codegen/config/DiscoGapicInterfaceConfig.java b/src/main/java/com/google/api/codegen/config/DiscoGapicInterfaceConfig.java index 16044f5167..3e4e5bb180 100644 --- a/src/main/java/com/google/api/codegen/config/DiscoGapicInterfaceConfig.java +++ b/src/main/java/com/google/api/codegen/config/DiscoGapicInterfaceConfig.java @@ -71,8 +71,7 @@ static DiscoGapicInterfaceConfig createInterfaceConfig( ResourceNameMessageConfigs messageConfigs, ImmutableMap resourceNameConfigs) { - RetryCodesConfig retryCodesConfig = - RetryCodesConfig.create(model.getDiagCollector(), interfaceConfigProto); + RetryCodesConfig retryCodesConfig = RetryCodesConfig.create(interfaceConfigProto); ImmutableMap retrySettingsDefinition = RetryDefinitionsTransformer.createRetrySettingsDefinition(interfaceConfigProto); diff --git a/src/main/java/com/google/api/codegen/config/FixedResourceNameConfig.java b/src/main/java/com/google/api/codegen/config/FixedResourceNameConfig.java index bea91f3686..bfe4cff8b7 100644 --- a/src/main/java/com/google/api/codegen/config/FixedResourceNameConfig.java +++ b/src/main/java/com/google/api/codegen/config/FixedResourceNameConfig.java @@ -30,6 +30,7 @@ public abstract class FixedResourceNameConfig implements ResourceNameConfig { public abstract String getFixedValue(); + @Nullable @Override public abstract ProtoFile getAssignedProtoFile(); diff --git a/src/main/java/com/google/api/codegen/config/GapicInterfaceConfig.java b/src/main/java/com/google/api/codegen/config/GapicInterfaceConfig.java index 4b38356b4c..448fe1e4cd 100644 --- a/src/main/java/com/google/api/codegen/config/GapicInterfaceConfig.java +++ b/src/main/java/com/google/api/codegen/config/GapicInterfaceConfig.java @@ -36,6 +36,7 @@ import java.util.List; import java.util.Map; import java.util.Map.Entry; +import java.util.Objects; import javax.annotation.Nullable; /** @@ -118,7 +119,6 @@ static GapicInterfaceConfig createInterfaceConfig( RetryCodesConfig retryCodesConfig = RetryCodesConfig.create( - diagCollector, interfaceConfigProto, new ArrayList<>(interfaceInput.getMethodsToGenerate().keySet()), protoParser); @@ -164,19 +164,32 @@ static GapicInterfaceConfig createInterfaceConfig( } ImmutableList.Builder resourcesBuilder = ImmutableList.builder(); - for (CollectionConfigProto collectionConfigProto : interfaceConfigProto.getCollectionsList()) { - String entityName = collectionConfigProto.getEntityName(); - ResourceNameConfig resourceName = resourceNameConfigs.get(entityName); - if (!(resourceName instanceof SingleResourceNameConfig)) { - diagCollector.addDiag( - Diag.error( - SimpleLocation.TOPLEVEL, - "Inconsistent configuration - single resource name %s specified for interface, " - + " but was not found in GapicProductConfig configuration.", - entityName)); - return null; + if (protoParser.isProtoAnnotationsEnabled()) { + resourceNameConfigs + .values() + .stream() + .filter( + r -> + r.getResourceNameType() == ResourceNameType.SINGLE + && Objects.equals(r.getAssignedProtoFile(), apiInterface.getFile())) + .map(r -> (SingleResourceNameConfig) r) + .forEach(resourcesBuilder::add); + } else { + for (CollectionConfigProto collectionConfigProto : + interfaceConfigProto.getCollectionsList()) { + String entityName = collectionConfigProto.getEntityName(); + ResourceNameConfig resourceName = resourceNameConfigs.get(entityName); + if (!(resourceName instanceof SingleResourceNameConfig)) { + diagCollector.addDiag( + Diag.error( + SimpleLocation.TOPLEVEL, + "Inconsistent configuration - single resource name %s specified for interface, " + + " but was not found in GapicProductConfig configuration.", + entityName)); + return null; + } + resourcesBuilder.add((SingleResourceNameConfig) resourceName); } - resourcesBuilder.add((SingleResourceNameConfig) resourceName); } ImmutableList singleResourceNames = resourcesBuilder.build(); diff --git a/src/main/java/com/google/api/codegen/config/GapicProductConfig.java b/src/main/java/com/google/api/codegen/config/GapicProductConfig.java index beb083ef28..47deed2f50 100644 --- a/src/main/java/com/google/api/codegen/config/GapicProductConfig.java +++ b/src/main/java/com/google/api/codegen/config/GapicProductConfig.java @@ -19,7 +19,6 @@ import com.google.api.codegen.CollectionConfigProto; import com.google.api.codegen.CollectionOneofProto; import com.google.api.codegen.ConfigProto; -import com.google.api.codegen.FixedResourceNameValueProto; import com.google.api.codegen.InterfaceConfigProto; import com.google.api.codegen.LanguageSettingsProto; import com.google.api.codegen.MethodConfigProto; @@ -28,6 +27,7 @@ import com.google.api.codegen.VisibilityProto; import com.google.api.codegen.common.TargetLanguage; import com.google.api.codegen.configgen.mergers.LanguageSettingsMerger; +import com.google.api.codegen.util.ConfigVersionValidator; import com.google.api.codegen.util.LicenseHeaderUtil; import com.google.api.codegen.util.ProtoParser; import com.google.api.tools.framework.model.Diag; @@ -35,6 +35,7 @@ import com.google.api.tools.framework.model.Interface; import com.google.api.tools.framework.model.Method; import com.google.api.tools.framework.model.Model; +import com.google.api.tools.framework.model.ProtoElement; import com.google.api.tools.framework.model.ProtoFile; import com.google.api.tools.framework.model.SimpleLocation; import com.google.api.tools.framework.model.SymbolTable; @@ -47,18 +48,16 @@ import com.google.common.collect.Iterables; import com.google.protobuf.Api; import com.google.protobuf.DescriptorProtos; -import java.util.ArrayList; -import java.util.Collection; import java.util.Comparator; import java.util.HashMap; import java.util.HashSet; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; -import java.util.Map.Entry; import java.util.Objects; import java.util.Optional; import java.util.Set; +import java.util.function.Function; import java.util.stream.Collectors; import javax.annotation.Nullable; import org.apache.commons.lang3.StringUtils; @@ -199,11 +198,14 @@ public static GapicProductConfig create( // Toggle on/off proto annotations parsing. ProtoParser protoParser; - // TODO(andrealin): Expose command-line option for toggling proto annotations parsing. - if (configProto == null) { - // By default, enable proto annotations parsing when no GAPIC config is given. + ConfigVersionValidator versionValidator = new ConfigVersionValidator(); + if (versionValidator.isV2Config(configProto)) { + versionValidator.validateV2Config(configProto); protoParser = new ProtoParser(true); - configProto = ConfigProto.getDefaultInstance(); + + if (configProto == null) { + configProto = ConfigProto.getDefaultInstance(); + } } else { protoParser = new ProtoParser(false); } @@ -236,6 +238,7 @@ public static GapicProductConfig create( if (protoParser.isProtoAnnotationsEnabled()) { resourceNameConfigs = createResourceNameConfigsWithProtoFileAndGapicConfig( + model, diagCollector, configProto, packageProtoFile, @@ -246,7 +249,7 @@ public static GapicProductConfig create( } else { resourceNameConfigs = createResourceNameConfigsForGapicConfigOnly( - diagCollector, configProto, packageProtoFile, language); + model, diagCollector, configProto, packageProtoFile, language); } if (resourceNameConfigs == null) { @@ -276,16 +279,18 @@ public static GapicProductConfig create( getInterfacesFromProtoFile(diagCollector, sourceProtos, symbolTable); ImmutableList interfaceInputs; - if (!configProto.equals(ConfigProto.getDefaultInstance())) { + if (protoParser.isProtoAnnotationsEnabled()) { interfaceInputs = - createInterfaceInputsWithGapicConfig( + createInterfaceInputsWithAnnotationsAndGapicConfig( + diagCollector, configProto.getInterfacesList(), protoInterfaces, language); + } else { + interfaceInputs = + createInterfaceInputsWithGapicConfigOnly( diagCollector, configProto.getInterfacesList(), protoInterfaces, symbolTable, language); - } else { - interfaceInputs = createInterfaceInputsWithoutGapicConfig(protoInterfaces.values()); } if (interfaceInputs == null) { return null; @@ -472,7 +477,22 @@ private static GapicProductConfig createDummyInstance( } /** Return the list of information about clients to be generated. */ - private static ImmutableList createInterfaceInputsWithGapicConfig( + private static ImmutableList + createInterfaceInputsWithAnnotationsAndGapicConfig( + DiagCollector diagCollector, + List interfaceConfigProtosList, + ImmutableMap protoInterfaces, + TargetLanguage language) { + return createGapicInterfaceInputList( + diagCollector, + language, + protoInterfaces.values(), + interfaceConfigProtosList + .stream() + .collect(Collectors.toMap(InterfaceConfigProto::getName, Function.identity()))); + } + + private static ImmutableList createInterfaceInputsWithGapicConfigOnly( DiagCollector diagCollector, List interfaceConfigProtosList, ImmutableMap protoInterfaces, @@ -488,11 +508,27 @@ private static ImmutableList createInterfaceInputsWithGapic // Parse GAPIC config for interfaceConfigProtos. for (InterfaceConfigProto interfaceConfigProto : interfaceConfigProtosList) { Interface apiInterface = symbolTable.lookupInterface(interfaceConfigProto.getName()); - if (apiInterface == null || !apiInterface.isReachable()) { + if (apiInterface == null) { + List interfaces = + symbolTable + .getInterfaces() + .stream() + .map(ProtoElement::getFullName) + .collect(Collectors.toList()); + String interfacesString = String.join(",", interfaces); diagCollector.addDiag( Diag.error( SimpleLocation.TOPLEVEL, - "interface not found: %s", + "interface not found: %s. Interfaces: [%s]", + interfaceConfigProto.getName(), + interfacesString)); + continue; + } + if (!apiInterface.isReachable()) { + diagCollector.addDiag( + Diag.error( + SimpleLocation.TOPLEVEL, + "interface not reachable: %s.", interfaceConfigProto.getName())); continue; } @@ -500,11 +536,20 @@ private static ImmutableList createInterfaceInputsWithGapic interfaceMap.put(interfaceConfigProto.getName(), apiInterface); } + return createGapicInterfaceInputList( + diagCollector, language, interfaceMap.values(), interfaceConfigProtos); + } + + private static ImmutableList createGapicInterfaceInputList( + DiagCollector diagCollector, + TargetLanguage language, + Iterable interfaceList, + Map interfaceConfigProtos) { + // Store info about each Interface in a GapicInterfaceInput object. ImmutableList.Builder interfaceInputs = ImmutableList.builder(); - for (Entry interfaceEntry : interfaceMap.entrySet()) { - String serviceFullName = interfaceEntry.getKey(); - Interface apiInterface = interfaceEntry.getValue(); + for (Interface apiInterface : interfaceList) { + String serviceFullName = apiInterface.getFullName(); GapicInterfaceInput.Builder interfaceInput = GapicInterfaceInput.newBuilder().setInterface(apiInterface); @@ -549,22 +594,6 @@ private static ImmutableMap getInterfacesFromProtoFile( return protoInterfaces.build(); } - /** Return the list of information about clients to be generated. */ - private static ImmutableList createInterfaceInputsWithoutGapicConfig( - Collection protoInterfaces) { - // Store info about each Interface in a GapicInterfaceInput object. - ImmutableList.Builder interfaceInputs = ImmutableList.builder(); - for (Interface apiInterface : protoInterfaces) { - GapicInterfaceInput.Builder interfaceInput = - GapicInterfaceInput.newBuilder() - .setInterface(apiInterface) - .setInterfaceConfigProto(InterfaceConfigProto.getDefaultInstance()) - .setMethodsToGenerate(findMethodsToGenerateWithoutConfigYaml(apiInterface)); - interfaceInputs.add(interfaceInput.build()); - } - return interfaceInputs.build(); - } - /** Find the methods that should be generated on the surface when no GAPIC config was given. */ private static ImmutableMap findMethodsToGenerateWithoutConfigYaml( Interface apiInterface) { @@ -710,7 +739,8 @@ private static ImmutableMap createDiscoGapicInterfaceCo private static ImmutableMap createResourceNameConfigs( DiagCollector diagCollector, ConfigProto configProto, TargetLanguage language) { - return createResourceNameConfigsForGapicConfigOnly(diagCollector, configProto, null, language); + return createResourceNameConfigsForGapicConfigOnly( + null, diagCollector, configProto, null, language); } /** @@ -721,6 +751,7 @@ private static ImmutableMap createResourceNameConfig @Nullable static ImmutableMap createResourceNameConfigsWithProtoFileAndGapicConfig( + Model model, DiagCollector diagCollector, ConfigProto configProto, @Nullable ProtoFile sampleProtoFile, @@ -764,14 +795,6 @@ private static ImmutableMap createResourceNameConfig fullyQualifiedFixedResourceNameConfigsFromProtoFile = ImmutableMap.copyOf(fullyQualifiedFixedResourcesFromProtoFileCollector); - ImmutableMap resourceNameOneofConfigsFromProtoFile = - createResourceNameOneofConfigsFromProtoFile( - diagCollector, - fullyQualifiedSingleResourceNameConfigsFromProtoFile, - fullyQualifiedFixedResourceNameConfigsFromProtoFile, - resourceSetDefs, - protoParser); - // Populate a SingleResourceNameConfigs map, using just the unqualified names. LinkedHashMap singleResourceConfigsFromProtoFile = new LinkedHashMap<>(); @@ -791,29 +814,36 @@ private static ImmutableMap createResourceNameConfig fixedResourceConfigsFromProtoFile.put(fullName.substring(periodIndex + 1), config); } - List allCollectionConfigProtos = - new ArrayList<>(configProto.getCollectionsList()); - configProto - .getInterfacesList() - .stream() - .forEach(i -> allCollectionConfigProtos.addAll(i.getCollectionsList())); + Map allCollectionConfigProtos = + getAllCollectionConfigProtos(model, configProto); ImmutableMap singleResourceNameConfigsFromGapicConfig = createSingleResourceNamesFromGapicConfigOnly( diagCollector, allCollectionConfigProtos, sampleProtoFile, language); ImmutableMap fixedResourceNameConfigsFromGapicConfig = - createFixedResourceNamesFromGapicConfigOnly( - diagCollector, - allCollectionConfigProtos, - configProto.getFixedResourceNameValuesList(), - sampleProtoFile); + createFixedResourceNamesFromGapicConfigOnly(diagCollector, allCollectionConfigProtos); // Combine the ResourceNameConfigs from the GAPIC and protofile. - Map finalSingleResourceNameConfigs = + ImmutableMap finalSingleResourceNameConfigs = mergeSingleResourceNameConfigsFromGapicConfigAndProtoFile( singleResourceNameConfigsFromGapicConfig, singleResourceConfigsFromProtoFile); validateSingleResourceNameConfigs(finalSingleResourceNameConfigs); + // TODO(andrealin): Remove this once explicit fixed resource names are gone-zos. + ImmutableMap finalFixedResourceNameConfigs = + mergeResourceNameConfigs( + diagCollector, + fixedResourceNameConfigsFromGapicConfig, + fixedResourceConfigsFromProtoFile); + + ImmutableMap resourceNameOneofConfigsFromProtoFile = + createResourceNameOneofConfigsFromProtoFile( + diagCollector, + finalSingleResourceNameConfigs, + finalFixedResourceNameConfigs, + resourceSetDefs, + protoParser); + // TODO(andrealin): Remove this once ResourceSets are approved. ImmutableMap resourceNameOneofConfigsFromGapicConfig = createResourceNameOneofConfigs( @@ -826,12 +856,6 @@ private static ImmutableMap createResourceNameConfig return null; } - // TODO(andrealin): Remove this once explicit fixed resource names are gone-zos. - Map finalFixedResourceNameConfigs = - mergeResourceNameConfigs( - diagCollector, - fixedResourceNameConfigsFromGapicConfig, - fixedResourceConfigsFromProtoFile); // TODO(andrealin): Remove this once ResourceSets are approved. Map finalResourceOneofNameConfigs = mergeResourceNameConfigs( @@ -854,27 +878,20 @@ private static ImmutableMap createResourceNameConfig @Nullable private static ImmutableMap createResourceNameConfigsForGapicConfigOnly( + Model model, DiagCollector diagCollector, ConfigProto configProto, - @Nullable ProtoFile file, + @Nullable ProtoFile sampleProtoFile, TargetLanguage language) { - List allCollectionConfigProtos = - new ArrayList<>(configProto.getCollectionsList()); - configProto - .getInterfacesList() - .stream() - .forEach(i -> allCollectionConfigProtos.addAll(i.getCollectionsList())); + Map allCollectionConfigProtos = + getAllCollectionConfigProtos(model, configProto); ImmutableMap singleResourceNameConfigsFromGapicConfig = createSingleResourceNamesFromGapicConfigOnly( - diagCollector, allCollectionConfigProtos, file, language); + diagCollector, allCollectionConfigProtos, sampleProtoFile, language); ImmutableMap fixedResourceNameConfigsFromGapicConfig = - createFixedResourceNamesFromGapicConfigOnly( - diagCollector, - allCollectionConfigProtos, - configProto.getFixedResourceNameValuesList(), - file); + createFixedResourceNamesFromGapicConfigOnly(diagCollector, allCollectionConfigProtos); validateSingleResourceNameConfigs(singleResourceNameConfigsFromGapicConfig); @@ -884,7 +901,7 @@ private static ImmutableMap createResourceNameConfig configProto.getCollectionOneofsList(), singleResourceNameConfigsFromGapicConfig, fixedResourceNameConfigsFromGapicConfig, - file); + sampleProtoFile); if (diagCollector.getErrorCount() > 0) { return null; } @@ -900,15 +917,19 @@ private static ImmutableMap createResourceNameConfig private static ImmutableMap createSingleResourceNamesFromGapicConfigOnly( DiagCollector diagCollector, - List allCollectionConfigs, - ProtoFile protoFile, + Map allCollectionConfigs, + ProtoFile sampleProtoFile, TargetLanguage language) { Map singleResourceNameConfigMap = new LinkedHashMap<>(); - for (CollectionConfigProto c : allCollectionConfigs) { - if (Strings.isNullOrEmpty(c.getNamePattern()) - || !FixedResourceNameConfig.isFixedResourceNameConfig(c.getNamePattern())) { + for (Map.Entry entry : allCollectionConfigs.entrySet()) { + CollectionConfigProto collectionConfigProto = entry.getKey(); + Interface anInterface = entry.getValue(); + ProtoFile protoFile = anInterface == null ? sampleProtoFile : anInterface.getFile(); + if (Strings.isNullOrEmpty(collectionConfigProto.getNamePattern()) + || !FixedResourceNameConfig.isFixedResourceNameConfig( + collectionConfigProto.getNamePattern())) { createSingleResourceNameConfigFromGapicConfig( - diagCollector, c, singleResourceNameConfigMap, protoFile, language); + diagCollector, collectionConfigProto, singleResourceNameConfigMap, protoFile, language); } } return ImmutableMap.copyOf(singleResourceNameConfigMap); @@ -916,27 +937,27 @@ private static ImmutableMap createResourceNameConfig private static ImmutableMap createFixedResourceNamesFromGapicConfigOnly( - DiagCollector diagCollector, - List allCollectionConfigs, - List fixedResourceNameValueProtos, - ProtoFile protoFile) { + DiagCollector diagCollector, Map allCollectionConfigs) { LinkedHashMap fixedResourceNameConfigMap = new LinkedHashMap<>(); - for (CollectionConfigProto c : allCollectionConfigs) { - if (!Strings.isNullOrEmpty(c.getNamePattern()) - && FixedResourceNameConfig.isFixedResourceNameConfig(c.getNamePattern())) { + for (Map.Entry entry : allCollectionConfigs.entrySet()) { + CollectionConfigProto collectionConfigProto = entry.getKey(); + Interface interface_ = entry.getValue(); + ProtoFile protoFile = interface_ == null ? null : interface_.getFile(); + if (!Strings.isNullOrEmpty(collectionConfigProto.getNamePattern()) + && FixedResourceNameConfig.isFixedResourceNameConfig( + collectionConfigProto.getNamePattern())) { FixedResourceNameConfig fixedResourceNameConfig = FixedResourceNameConfig.createFixedResourceNameConfig( - diagCollector, c.getEntityName(), c.getNamePattern(), protoFile); + diagCollector, + collectionConfigProto.getEntityName(), + collectionConfigProto.getNamePattern(), + protoFile); insertFixedResourceNameConfig( diagCollector, fixedResourceNameConfig, "", fixedResourceNameConfigMap); } } - // TODO(andrealin): Remove this once all fixed resource names are removed. - fixedResourceNameConfigMap.putAll( - createFixedResourceNameConfigs(diagCollector, fixedResourceNameValueProtos, protoFile)); - return ImmutableMap.copyOf(fixedResourceNameConfigMap); } @@ -1125,28 +1146,6 @@ private static void insertFixedResourceNameConfig( } } - // TODO(andrealin): Remove this once existing fixed resource names are removed. - private static ImmutableMap createFixedResourceNameConfigs( - DiagCollector diagCollector, - Iterable fixedConfigProtos, - @Nullable ProtoFile file) { - ImmutableMap.Builder fixedConfigBuilder = - ImmutableMap.builder(); - for (FixedResourceNameValueProto fixedConfigProto : fixedConfigProtos) { - FixedResourceNameConfig fixedConfig = - FixedResourceNameConfig.createFixedResourceNameConfig( - diagCollector, - fixedConfigProto.getEntityName(), - fixedConfigProto.getFixedValue(), - file); - if (fixedConfig == null) { - continue; - } - fixedConfigBuilder.put(fixedConfig.getEntityId(), fixedConfig); - } - return fixedConfigBuilder.build(); - } - private static ImmutableMap createResourceNameOneofConfigs( DiagCollector diagCollector, Iterable oneofConfigProtos, @@ -1255,4 +1254,26 @@ public List getAllLongRunningConfigs() { .filter(Objects::nonNull) .collect(Collectors.toList()); } + + private static Map getAllCollectionConfigProtos( + @Nullable Model model, ConfigProto configProto) { + Map allCollectionConfigProtos = + configProto + .getCollectionsList() + .stream() + .collect(HashMap::new, (map, config) -> map.put(config, null), HashMap::putAll); + configProto + .getInterfacesList() + .forEach( + i -> + i.getCollectionsList() + .forEach( + c -> + allCollectionConfigProtos.put( + c, + model == null + ? null + : model.getSymbolTable().lookupInterface(i.getName())))); + return allCollectionConfigProtos; + } } diff --git a/src/main/java/com/google/api/codegen/config/LongRunningConfig.java b/src/main/java/com/google/api/codegen/config/LongRunningConfig.java index fbc17e65ac..8b80a86de7 100644 --- a/src/main/java/com/google/api/codegen/config/LongRunningConfig.java +++ b/src/main/java/com/google/api/codegen/config/LongRunningConfig.java @@ -24,6 +24,7 @@ import com.google.api.tools.framework.model.TypeRef; import com.google.auto.value.AutoValue; import com.google.longrunning.OperationInfo; +import javax.annotation.Nonnull; import javax.annotation.Nullable; import org.threeten.bp.Duration; @@ -32,10 +33,10 @@ public abstract class LongRunningConfig { // Default values for LongRunningConfig fields. - static final int LRO_INITIAL_POLL_DELAY_MILLIS = 500; + static final Duration LRO_INITIAL_POLL_DELAY_MILLIS = Duration.ofMillis(500); static final double LRO_POLL_DELAY_MULTIPLIER = 1.5; - static final int LRO_MAX_POLL_DELAY_MILLIS = 5000; - static final int LRO_TOTAL_POLL_TIMEOUT_MILLS = 300000; + static final Duration LRO_MAX_POLL_DELAY_MILLIS = Duration.ofMillis(5000); + static final Duration LRO_TOTAL_POLL_TIMEOUT_MILLS = Duration.ofMillis(300000); /** Returns the message type returned from a completed operation. */ public abstract TypeModel getReturnType(); @@ -74,7 +75,7 @@ private static String qualifyLroTypeName( static LongRunningConfig createLongRunningConfig( Method method, DiagCollector diagCollector, - LongRunningConfigProto longRunningConfigProto, + @Nonnull LongRunningConfigProto longRunningConfigProto, ProtoParser protoParser) { int preexistingErrors = diagCollector.getErrorCount(); @@ -90,13 +91,6 @@ static LongRunningConfig createLongRunningConfig( String metadataTypeName = qualifyLroTypeName(operationTypes.getMetadataType(), method, protoParser); - if (responseTypeName.equals(longRunningConfigProto.getReturnType()) - && metadataTypeName.equals(longRunningConfigProto.getMetadataType())) { - // GAPIC config refers to the same Long running config; so use its retry settings. - return LongRunningConfig.createLongRunningConfigFromGapicConfigOnly( - method.getModel(), diagCollector, longRunningConfigProto); - } - TypeRef returnType = model.getSymbolTable().lookupType(responseTypeName); TypeRef metadataType = model.getSymbolTable().lookupType(metadataTypeName); @@ -132,18 +126,71 @@ static LongRunningConfig createLongRunningConfig( return null; } - // Use default retry settings. - Duration initialPollDelay = Duration.ofMillis(LRO_INITIAL_POLL_DELAY_MILLIS); - Duration maxPollDelay = Duration.ofMillis(LRO_MAX_POLL_DELAY_MILLIS); - Duration totalPollTimeout = Duration.ofMillis(LRO_TOTAL_POLL_TIMEOUT_MILLS); - - return new AutoValue_LongRunningConfig( - ProtoTypeRef.create(returnType), - ProtoTypeRef.create(metadataType), - initialPollDelay, - LRO_POLL_DELAY_MULTIPLIER, - maxPollDelay, - totalPollTimeout); + LongRunningConfig.Builder builder = + getGapicConfigLroRetrySettingsOrDefault(diagCollector, longRunningConfigProto); + + return builder + .setReturnType(ProtoTypeRef.create(returnType)) + .setMetadataType(ProtoTypeRef.create(metadataType)) + .build(); + } + + /** + * Creates an instance of LongRunningConfig.Builder based on a method's GAPIC config's LRO polling + * settings. + */ + private static LongRunningConfig.Builder getGapicConfigLroRetrySettingsOrDefault( + DiagCollector diagCollector, LongRunningConfigProto longRunningConfigProto) { + if (longRunningConfigProto.equals(LongRunningConfigProto.getDefaultInstance())) { + return newBuilder() + .setInitialPollDelay(LRO_INITIAL_POLL_DELAY_MILLIS) + .setPollDelayMultiplier(LRO_POLL_DELAY_MULTIPLIER) + .setMaxPollDelay(LRO_MAX_POLL_DELAY_MILLIS) + .setTotalPollTimeout(LRO_TOTAL_POLL_TIMEOUT_MILLS); + } + + Duration initialPollDelay = + Duration.ofMillis(longRunningConfigProto.getInitialPollDelayMillis()); + if (initialPollDelay.compareTo(Duration.ZERO) < 0) { + diagCollector.addDiag( + Diag.error( + SimpleLocation.TOPLEVEL, + "Initial poll delay must be provided and set to a positive number: '%s'", + longRunningConfigProto.getInitialPollDelayMillis())); + } + + double pollDelayMultiplier = longRunningConfigProto.getPollDelayMultiplier(); + if (pollDelayMultiplier < 1.0) { + diagCollector.addDiag( + Diag.error( + SimpleLocation.TOPLEVEL, + "Poll delay multiplier must be provided and be greater or equal than 1.0: '%s'", + longRunningConfigProto.getPollDelayMultiplier())); + } + + Duration maxPollDelay = Duration.ofMillis(longRunningConfigProto.getMaxPollDelayMillis()); + if (maxPollDelay.compareTo(initialPollDelay) < 0) { + diagCollector.addDiag( + Diag.error( + SimpleLocation.TOPLEVEL, + "Max poll delay must be provided and set be equal or greater than initial poll delay: '%s'", + longRunningConfigProto.getMaxPollDelayMillis())); + } + + Duration totalPollTimeout = + Duration.ofMillis(longRunningConfigProto.getTotalPollTimeoutMillis()); + if (totalPollTimeout.compareTo(maxPollDelay) < 0) { + diagCollector.addDiag( + Diag.error( + SimpleLocation.TOPLEVEL, + "Total poll timeout must be provided and be be equal or greater than max poll delay: '%s'", + longRunningConfigProto.getTotalPollTimeoutMillis())); + } + return newBuilder() + .setInitialPollDelay(initialPollDelay) + .setPollDelayMultiplier(pollDelayMultiplier) + .setMaxPollDelay(maxPollDelay) + .setTotalPollTimeout(totalPollTimeout); } /** Creates an instance of LongRunningConfig based on LongRunningConfigProto. */ @@ -188,54 +235,38 @@ static LongRunningConfig createLongRunningConfigFromGapicConfigOnly( longRunningConfigProto.getMetadataType())); } - Duration initialPollDelay = - Duration.ofMillis(longRunningConfigProto.getInitialPollDelayMillis()); - if (initialPollDelay.compareTo(Duration.ZERO) < 0) { - diagCollector.addDiag( - Diag.error( - SimpleLocation.TOPLEVEL, - "Initial poll delay must be provided and set to a positive number: '%s'", - longRunningConfigProto.getInitialPollDelayMillis())); + if (diagCollector.getErrorCount() - preexistingErrors > 0) { + return null; } - double pollDelayMultiplier = longRunningConfigProto.getPollDelayMultiplier(); - if (pollDelayMultiplier <= 1.0) { - diagCollector.addDiag( - Diag.error( - SimpleLocation.TOPLEVEL, - "Poll delay multiplier must be provided and be greater or equal than 1.0: '%s'", - longRunningConfigProto.getPollDelayMultiplier())); - } + LongRunningConfig.Builder builder = + getGapicConfigLroRetrySettingsOrDefault(diagCollector, longRunningConfigProto); - Duration maxPollDelay = Duration.ofMillis(longRunningConfigProto.getMaxPollDelayMillis()); - if (maxPollDelay.compareTo(initialPollDelay) < 0) { - diagCollector.addDiag( - Diag.error( - SimpleLocation.TOPLEVEL, - "Max poll delay must be provided and set be equal or greater than initial poll delay: '%s'", - longRunningConfigProto.getMaxPollDelayMillis())); - } + return builder + .setReturnType(ProtoTypeRef.create(returnType)) + .setMetadataType(ProtoTypeRef.create(metadataType)) + .build(); + } - Duration totalPollTimeout = - Duration.ofMillis(longRunningConfigProto.getTotalPollTimeoutMillis()); - if (totalPollTimeout.compareTo(maxPollDelay) < 0) { - diagCollector.addDiag( - Diag.error( - SimpleLocation.TOPLEVEL, - "Total poll timeout must be provided and be be equal or greater than max poll delay: '%s'", - longRunningConfigProto.getTotalPollTimeoutMillis())); - } + private static Builder newBuilder() { + return new AutoValue_LongRunningConfig.Builder(); + } - if (diagCollector.getErrorCount() - preexistingErrors > 0) { - return null; - } + @AutoValue.Builder + abstract static class Builder { + + public abstract Builder setReturnType(TypeModel val); + + public abstract Builder setMetadataType(TypeModel val); + + public abstract Builder setInitialPollDelay(Duration val); + + public abstract Builder setPollDelayMultiplier(double val); + + public abstract Builder setMaxPollDelay(Duration val); + + public abstract Builder setTotalPollTimeout(Duration val); - return new AutoValue_LongRunningConfig( - ProtoTypeRef.create(returnType), - ProtoTypeRef.create(metadataType), - initialPollDelay, - pollDelayMultiplier, - maxPollDelay, - totalPollTimeout); + public abstract LongRunningConfig build(); } } diff --git a/src/main/java/com/google/api/codegen/config/PageStreamingConfig.java b/src/main/java/com/google/api/codegen/config/PageStreamingConfig.java index 9737aa8063..40848bb0b5 100644 --- a/src/main/java/com/google/api/codegen/config/PageStreamingConfig.java +++ b/src/main/java/com/google/api/codegen/config/PageStreamingConfig.java @@ -90,6 +90,7 @@ public String getResourcesFieldName() { return getResourcesField().getSimpleName(); } + // TODO(andrealin): combine this with the protofile one, pass in resourcenametreatment as param /** * Creates an instance of PageStreamingConfig based on PageStreamingConfigProto, linking it up * with the provided method. On errors, null will be returned, and diagnostics are reported to the diff --git a/src/main/java/com/google/api/codegen/config/ProtoInterfaceModel.java b/src/main/java/com/google/api/codegen/config/ProtoInterfaceModel.java index 0205645768..e3a382a73c 100644 --- a/src/main/java/com/google/api/codegen/config/ProtoInterfaceModel.java +++ b/src/main/java/com/google/api/codegen/config/ProtoInterfaceModel.java @@ -18,6 +18,7 @@ import com.google.api.tools.framework.model.Method; import com.google.api.tools.framework.model.SymbolTable; import com.google.common.collect.ImmutableList; +import com.google.protobuf.Api; import com.google.protobuf.Mixin; import java.util.List; @@ -73,10 +74,13 @@ public List getMethods() { methods.add(new ProtoMethodModel(method)); } SymbolTable symbolTable = protoInterface.getModel().getSymbolTable(); - for (Mixin mixin : protoInterface.getConfig().getMixinsList()) { - Interface mixinInterface = symbolTable.lookupInterface(mixin.getName()); - for (Method method : mixinInterface.getMethods()) { - methods.add(new ProtoMethodModel(method)); + Api protoInterfaceConfig = protoInterface.getConfig(); + if (protoInterfaceConfig != null) { + for (Mixin mixin : protoInterface.getConfig().getMixinsList()) { + Interface mixinInterface = symbolTable.lookupInterface(mixin.getName()); + for (Method method : mixinInterface.getMethods()) { + methods.add(new ProtoMethodModel(method)); + } } } return methods.build(); diff --git a/src/main/java/com/google/api/codegen/config/ResourceNameOneofConfig.java b/src/main/java/com/google/api/codegen/config/ResourceNameOneofConfig.java index 14528c6652..4061833f2e 100644 --- a/src/main/java/com/google/api/codegen/config/ResourceNameOneofConfig.java +++ b/src/main/java/com/google/api/codegen/config/ResourceNameOneofConfig.java @@ -30,6 +30,7 @@ import java.util.ArrayList; import java.util.List; import javax.annotation.Nullable; +import org.apache.commons.lang3.StringUtils; /** ResourceNameOneofConfig represents the configuration for a oneof set of resource names. */ @AutoValue @@ -108,13 +109,13 @@ static ResourceNameOneofConfig createResourceNameOneof( DiagCollector diagCollector, ResourceSet resourceSet, String oneOfName, - ImmutableMap fileLevelSingleResourceNameConfigs, - ImmutableMap fileLevelFixedResourceNameConfigs, + ImmutableMap singleResourceNameConfigs, + ImmutableMap fixedResourceNameConfigs, ProtoParser protoParser, ProtoFile file) { - if (fileLevelSingleResourceNameConfigs.containsKey(oneOfName) - || fileLevelFixedResourceNameConfigs.containsKey(oneOfName)) { + if (singleResourceNameConfigs.containsKey(oneOfName) + || fixedResourceNameConfigs.containsKey(oneOfName)) { diagCollector.addDiag( Diag.error( SimpleLocation.TOPLEVEL, @@ -129,34 +130,46 @@ static ResourceNameOneofConfig createResourceNameOneof( List resourceReferences = resourceSet.getResourceReferencesList(); for (Resource resource : resourceDefs) { - SingleResourceNameConfig singleResourceNameConfig = - SingleResourceNameConfig.createSingleResourceName( - resource, resource.getPattern(), file, diagCollector); - configList.add(singleResourceNameConfig); - } - for (String resourceRef : resourceReferences) { - if (Strings.isNullOrEmpty(resourceRef)) { + if (Strings.isNullOrEmpty(resource.getSymbol())) { diagCollector.addDiag( Diag.error( SimpleLocation.TOPLEVEL, - "name is required for Resources in a ResourceSet," - + " but ResourceSet %s has at least one Resource without a name.", - oneOfName)); - return null; + String.format( + "google.api.Resource with pattern \"%s\" " + + "and nested inside ResourceSet \"%s\" was not given a symbol", + resource.getPattern(), resourceSet.getSymbol()))); + continue; } - String qualifiedRef = resourceRef; - if (!qualifiedRef.contains(".")) { - qualifiedRef = protoParser.getProtoPackage(file) + "." + resourceRef; + + SingleResourceNameConfig singleResourceNameConfig = + singleResourceNameConfigs.get(resource.getSymbol()); + if (singleResourceNameConfig != null) { + configList.add(singleResourceNameConfig); + continue; + } + + FixedResourceNameConfig fixedResourceNameConfig = + fixedResourceNameConfigs.get(resource.getSymbol()); + if (fixedResourceNameConfig != null) { + configList.add(fixedResourceNameConfig); + continue; } + + // This shouldn't happen because the resource config maps were + // previously constructed from Resource objects. + throw new IllegalStateException( + String.format( + "Failed to create a ResourceNameConfig for the protofile-defined Resource %s.", + resource.getSymbol())); + } + for (String resourceRef : resourceReferences) { + String resourceName = + StringUtils.removeStart(resourceRef, protoParser.getProtoPackage(file) + "."); ResourceNameConfig resourceNameConfig; - if (fileLevelSingleResourceNameConfigs.containsKey(qualifiedRef)) { - resourceNameConfig = fileLevelSingleResourceNameConfigs.get(qualifiedRef); - } else if (fileLevelSingleResourceNameConfigs.containsKey(resourceRef)) { - resourceNameConfig = fileLevelSingleResourceNameConfigs.get(resourceRef); - } else if (fileLevelFixedResourceNameConfigs.containsKey(qualifiedRef)) { - resourceNameConfig = fileLevelFixedResourceNameConfigs.get(qualifiedRef); - } else if (fileLevelFixedResourceNameConfigs.containsKey(resourceRef)) { - resourceNameConfig = fileLevelFixedResourceNameConfigs.get(resourceRef); + if (singleResourceNameConfigs.containsKey(resourceName)) { + resourceNameConfig = singleResourceNameConfigs.get(resourceName); + } else if (fixedResourceNameConfigs.containsKey(resourceName)) { + resourceNameConfig = fixedResourceNameConfigs.get(resourceName); } else { diagCollector.addDiag( Diag.error( diff --git a/src/main/java/com/google/api/codegen/config/RetryCodesConfig.java b/src/main/java/com/google/api/codegen/config/RetryCodesConfig.java index 123a9e6935..bb6620bbd2 100644 --- a/src/main/java/com/google/api/codegen/config/RetryCodesConfig.java +++ b/src/main/java/com/google/api/codegen/config/RetryCodesConfig.java @@ -23,17 +23,14 @@ import com.google.api.codegen.RetryCodesDefinitionProto; import com.google.api.codegen.util.ProtoParser; import com.google.api.codegen.util.SymbolTable; -import com.google.api.tools.framework.model.Diag; -import com.google.api.tools.framework.model.DiagCollector; import com.google.api.tools.framework.model.Method; -import com.google.api.tools.framework.model.SimpleLocation; import com.google.common.base.Strings; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; +import com.google.common.collect.Sets; import java.util.Collection; import java.util.HashMap; -import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Set; @@ -44,6 +41,8 @@ public class RetryCodesConfig { public static ImmutableList RETRY_CODES_FOR_HTTP_GET = DEFAULT_RETRY_CODES.get(RETRY_CODES_IDEMPOTENT_NAME); + public static ImmutableList RETRY_CODES_FOR_HTTP_NON_GET = + DEFAULT_RETRY_CODES.get(RETRY_CODES_NON_IDEMPOTENT_NAME); private Map> retryCodesDefinition = new HashMap<>(); private Map methodRetryNames = new HashMap<>(); @@ -51,7 +50,6 @@ public class RetryCodesConfig { private ImmutableSet retryCodeDefsFromGapicConfig; private ImmutableMap> finalRetryCodesDefinition; private ImmutableMap finalMethodRetryNames; - private boolean error = false; /** * A map of retry config names to the list of codes to retry on, e.g. { "idempotent" : @@ -75,35 +73,22 @@ public Set getRetryCodeDefsFromGapicConfig() { private RetryCodesConfig() {} - public static RetryCodesConfig create( - DiagCollector diagCollector, InterfaceConfigProto interfaceConfigProto) { + public static RetryCodesConfig create(InterfaceConfigProto interfaceConfigProto) { RetryCodesConfig retryCodesConfig = new RetryCodesConfig(); - retryCodesConfig.populateRetryCodesDefinitionFromConfigProto( - diagCollector, interfaceConfigProto); - if (retryCodesConfig.error) { - return null; - } - retryCodesConfig.finalMethodRetryNames = ImmutableMap.copyOf(retryCodesConfig.methodRetryNames); - retryCodesConfig.finalRetryCodesDefinition = - ImmutableMap.copyOf(retryCodesConfig.retryCodesDefinition); + retryCodesConfig.populateRetryCodesDefinitionFromConfigProto(interfaceConfigProto); + retryCodesConfig.setFinalRetryProperties(); return retryCodesConfig; } public static RetryCodesConfig create( - DiagCollector diagCollector, InterfaceConfigProto interfaceConfigProto, List methodsToGenerate, ProtoParser protoParser) { RetryCodesConfig retryCodesConfig = new RetryCodesConfig(); retryCodesConfig.populateRetryCodesDefinition( - diagCollector, interfaceConfigProto, methodsToGenerate, protoParser); - if (retryCodesConfig.error) { - return null; - } - retryCodesConfig.finalMethodRetryNames = ImmutableMap.copyOf(retryCodesConfig.methodRetryNames); - retryCodesConfig.finalRetryCodesDefinition = - ImmutableMap.copyOf(retryCodesConfig.retryCodesDefinition); + interfaceConfigProto, methodsToGenerate, protoParser); + retryCodesConfig.setFinalRetryProperties(); return retryCodesConfig; } @@ -122,47 +107,32 @@ private static ImmutableMap createMethodRetryNamesFromConfigProt @Nullable private static ImmutableMap> - createRetryCodesDefinitionFromConfigProto( - DiagCollector diagCollector, InterfaceConfigProto interfaceConfigProto) { + createRetryCodesDefinitionFromConfigProto(InterfaceConfigProto interfaceConfigProto) { ImmutableMap.Builder> builder = ImmutableMap.builder(); for (RetryCodesDefinitionProto retryDef : interfaceConfigProto.getRetryCodesDefList()) { // Enforce ordering on set for baseline test consistency. - Set codes = new TreeSet<>(); - for (String codeText : retryDef.getRetryCodesList()) { - try { - codes.add(String.valueOf(codeText)); - } catch (IllegalArgumentException e) { - diagCollector.addDiag( - Diag.error( - SimpleLocation.TOPLEVEL, - "status code not found: '%s' (in interface %s)", - codeText, - interfaceConfigProto.getName())); - } - } + Set codes = new TreeSet<>(retryDef.getRetryCodesList()); builder.put(retryDef.getName(), ImmutableList.copyOf(codes)); } - if (diagCollector.getErrorCount() > 0) { - return null; - } return builder.build(); } + private void setFinalRetryProperties() { + finalMethodRetryNames = ImmutableMap.copyOf(methodRetryNames); + finalRetryCodesDefinition = ImmutableMap.copyOf(retryCodesDefinition); + } + /** * Returns a mapping of a retryCodeDef name to the list of retry codes it contains. Also populates * the @param methodNameToRetryCodeNames with a mapping of a Method name to its retry code * settings name. */ private void populateRetryCodesDefinition( - DiagCollector diagCollector, InterfaceConfigProto interfaceConfigProto, Collection methodsToGenerate, ProtoParser protoParser) { // First create the retry codes definitions from the GAPIC config. - populateRetryCodesDefinitionFromConfigProto(diagCollector, interfaceConfigProto); - if (error) { - return; - } + populateRetryCodesDefinitionFromConfigProto(interfaceConfigProto); // Then create the retry codes defs from the proto annotations, but don't overwrite // existing retry codes defs from the GAPIC config. @@ -175,10 +145,10 @@ private void populateRetryCodesDefinition( * settings name. */ private void populateRetryCodesDefinitionFromConfigProto( - DiagCollector diagCollector, InterfaceConfigProto interfaceConfigProto) { + InterfaceConfigProto interfaceConfigProto) { ImmutableMap> retryCodesDefFromConfigProto = - createRetryCodesDefinitionFromConfigProto(diagCollector, interfaceConfigProto); + createRetryCodesDefinitionFromConfigProto(interfaceConfigProto); if (retryCodesDefFromConfigProto == null) { return; } @@ -205,6 +175,11 @@ private void populateRetryCodesDefinitionWithProtoFile( SymbolTable symbolTable = new SymbolTable(); + // Add retry codes for default cases (idempotent and non_idempotent) if they have not + // already be added previously (in the GAPIC config). + retryCodesDefinition.putIfAbsent(RETRY_CODES_IDEMPOTENT_NAME, RETRY_CODES_FOR_HTTP_GET); + retryCodesDefinition.putIfAbsent(RETRY_CODES_NON_IDEMPOTENT_NAME, RETRY_CODES_FOR_HTTP_NON_GET); + for (String retryCodesName : retryCodesDefinition.keySet()) { // Record all the preexisting retryCodeNames from configProto. symbolTable.getNewSymbol(retryCodesName); @@ -213,11 +188,8 @@ private void populateRetryCodesDefinitionWithProtoFile( // Unite all HTTP GET methods that have no additional retry codes under one retry code name to // reduce duplication. String httpGetRetryName; - Set defaultRetryCodesForHttpGet = new HashSet<>(RETRY_CODES_FOR_HTTP_GET); - Set existingIdempotentRetryCodes = - new HashSet<>( - retryCodesDefinition.getOrDefault(RETRY_CODES_IDEMPOTENT_NAME, ImmutableList.of())); - if (defaultRetryCodesForHttpGet.equals(existingIdempotentRetryCodes)) { + if (Sets.newHashSet(RETRY_CODES_FOR_HTTP_GET) + .containsAll(retryCodesDefinition.get(RETRY_CODES_IDEMPOTENT_NAME))) { // The GAPIC config defined RETRY_CODES_IDEMPOTENT_NAME to have the same retry codes as // this method would have, // so we can just reuse RETRY_CODES_IDEMPOTENT_NAME. @@ -228,9 +200,8 @@ private void populateRetryCodesDefinitionWithProtoFile( // Unite all methods that have no retry codes under one retry code name to reduce duplication. String noRetryName; - if (retryCodesDefinition - .getOrDefault(RETRY_CODES_NON_IDEMPOTENT_NAME, ImmutableList.of()) - .isEmpty()) { + if (Sets.newHashSet(RETRY_CODES_FOR_HTTP_NON_GET) + .containsAll(retryCodesDefinition.get(RETRY_CODES_NON_IDEMPOTENT_NAME))) { noRetryName = RETRY_CODES_NON_IDEMPOTENT_NAME; } else { noRetryName = symbolTable.getNewSymbol(RETRY_CODES_NON_IDEMPOTENT_NAME); diff --git a/src/main/java/com/google/api/codegen/config/SingleResourceNameConfig.java b/src/main/java/com/google/api/codegen/config/SingleResourceNameConfig.java index ff2016c4db..c670c656ac 100644 --- a/src/main/java/com/google/api/codegen/config/SingleResourceNameConfig.java +++ b/src/main/java/com/google/api/codegen/config/SingleResourceNameConfig.java @@ -88,7 +88,10 @@ public static SingleResourceNameConfig createSingleResourceName( * returned, and diagnostics are reported to the diag collector. */ static SingleResourceNameConfig createSingleResourceName( - Resource resource, String pathTemplate, ProtoFile file, DiagCollector diagCollector) { + Resource resource, + String pathTemplate, + @Nullable ProtoFile file, + DiagCollector diagCollector) { PathTemplate nameTemplate; try { nameTemplate = PathTemplate.create(pathTemplate); diff --git a/src/main/java/com/google/api/codegen/util/ConfigVersionValidator.java b/src/main/java/com/google/api/codegen/util/ConfigVersionValidator.java new file mode 100644 index 0000000000..efc427051c --- /dev/null +++ b/src/main/java/com/google/api/codegen/util/ConfigVersionValidator.java @@ -0,0 +1,69 @@ +/* Copyright 2019 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.google.api.codegen.util; + +import com.google.api.tools.framework.model.testing.BaselineDiffer; +import com.google.protobuf.DiscardUnknownFieldsParser; +import com.google.protobuf.InvalidProtocolBufferException; +import com.google.protobuf.Parser; +import java.util.Arrays; + +public class ConfigVersionValidator { + + public static String CONFIG_V2_MAJOR_VERSION = "2"; + public static String CONFIG_V2_VERSION = CONFIG_V2_MAJOR_VERSION + ".0.0"; // "2.0.0" + + /** + * Throw {@link IllegalStateException} iff the given input contains fields unknown to the {@link + * com.google.api.codegen.v2.ConfigProto} schema. Do nothing if input is null. + */ + public void validateV2Config(com.google.api.codegen.ConfigProto configV1Proto) + throws IllegalStateException { + if (!isV2Config(configV1Proto)) { + throw new IllegalStateException( + String.format( + "Provided ConfigProto version is %s but should be >= %s", + configV1Proto.getConfigSchemaVersion(), CONFIG_V2_VERSION)); + } + if (configV1Proto == null) { + return; + } + + try { + // Serialize and deserialize the Config v1 proto under the Config v2 schema to remove fields + // unknown to Config v2 schema. + Parser parser = + DiscardUnknownFieldsParser.wrap(com.google.api.codegen.v2.ConfigProto.parser()); + com.google.api.codegen.v2.ConfigProto configV2 = + parser.parseFrom(configV1Proto.toByteString()); + + // Compare the v1-serialized and v2-serialized strings of the same config proto object. + if (!Arrays.equals(configV2.toByteArray(), configV1Proto.toByteArray())) { + BaselineDiffer differ = new BaselineDiffer(); + throw new IllegalStateException( + String.format( + "Unknown fields to ConfigProto v2 in configProto:\n%s", + differ.diff(configV1Proto.toString(), configV2.toString()))); + } + } catch (InvalidProtocolBufferException e) { + throw new IllegalStateException(e); + } + } + + public boolean isV2Config(com.google.api.codegen.ConfigProto configV1Proto) { + return configV1Proto == null + || configV1Proto.getConfigSchemaVersion().startsWith(CONFIG_V2_MAJOR_VERSION + "."); + } +} diff --git a/src/main/proto/com/google/api/codegen/config.proto b/src/main/proto/com/google/api/codegen/config.proto index dbcba2384f..77f742fb2b 100644 --- a/src/main/proto/com/google/api/codegen/config.proto +++ b/src/main/proto/com/google/api/codegen/config.proto @@ -100,7 +100,8 @@ message ConfigProto { // A required field to specify the version of GAPIC config schema. // It is versioned using Semantic Versioning 2.0.0 (semver) and follows the - // semver specification. Currently, the only valid value is "1.0.0". + // semver specification. Currently, the only valid value is "1.0.0" and "2.0.0" + // Those following "2.0.0" should also follow the config_v2.proto schema. string config_schema_version = 21; } diff --git a/src/main/proto/com/google/api/codegen/v2/config_v2.proto b/src/main/proto/com/google/api/codegen/v2/config_v2.proto new file mode 100644 index 0000000000..d50774009f --- /dev/null +++ b/src/main/proto/com/google/api/codegen/v2/config_v2.proto @@ -0,0 +1,605 @@ +syntax = "proto3"; + +package com.google.api.codegen.v2; + +import "google/protobuf/wrappers.proto"; + +option java_multiple_files = true; +option java_outer_classname = "ConfigProtoDesc"; +option java_package = "com.google.api.codegen.v2"; + +message ConfigProto { + // The settings of generated code in a specific language. + map language_settings = 4; + + reserved 5, 20; + + // The configuration for the license header to put on generated files. + LicenseHeaderProto license_header = 7; + + // A list of API interface configurations. + repeated InterfaceConfigProto interfaces = 10; + + // A list of resource name collection configs. This must be consistent with + // collections defined in the interfaces. + repeated CollectionConfigProto collections = 15; + + // A list of resource name oneof configs. + repeated CollectionOneofProto collection_oneofs = 16; + + repeated FixedResourceNameValueProto fixed_resource_name_values = 17; + + // Set this value to toggle String format functions for resource name + // entities. If this is not set, it will fallback to default behavior. + // This option is exposed for backward compatibility of Java clients. + // TODO(andrealin): Migrate this to a command-line option after migrating + // off artman. + // [DEPRECATED] + .google.protobuf.BoolValue enable_string_format_functions_override = 22; + + // A required field to specify the version of GAPIC config schema. + // It is versioned using Semantic Versioning 2.0.0 (semver) and follows the + // semver specification. Currently, the only valid value is "2.0.0". + string config_schema_version = 21; +} + +message LanguageSettingsProto { + // Package name used for the language. + string package_name = 1; + + // Location of the domain layer, if any, that the user should use + // instead of the generated client. + string domain_layer_location = 2; + + // Set the interface name to be used when generating GAPIC code for + // particular interfaces. Keys are fully qualified interface names; + // values are unqualified interface names (to specify the package/namespace, + // use the package_name setting). + map interface_names = 4; + + // Overrides ConfigProto::license_header. + LicenseHeaderProto license_header_override = 5; + + // The release level of the client in the language + ReleaseLevel release_level = 6; +} + +// ReleaseLevel indicates the stage of development of a piece of code and +// relatedly what the promises are on quality. +// ALPHA: Not feature complete, expect breaking changes +// BETA: Feature complete, minimal breaking changes +// GA: General availability, no breaking changes without major version bump +enum ReleaseLevel { + UNSET_RELEASE_LEVEL = 0; + ALPHA = 1; + BETA = 2; + GA = 3; + DEPRECATED = 4; +} + +message LicenseHeaderProto { + reserved 1; + + // The file containing the raw license header without any copyright line(s). + string license_file = 2; +} + +message InterfaceConfigProto { + // The fully qualified name of the API interface. + string name = 1; + + // Language specific documentation. Injected after proto docs. + map lang_doc = 2; + + // Configuration of smoke test. + SmokeTestConfigProto smoke_test = 5; + + // A list of resource collection configurations. + repeated CollectionConfigProto collections = 10; + + // A list of method configurations. + repeated MethodConfigProto methods = 20; + + // Definition for retryable codes + repeated RetryCodesDefinitionProto retry_codes_def = 30; + + // Definition for retry/backoff parameters + repeated RetryParamsDefinitionProto retry_params_def = 31; + + // Params that are always required to construct an instance of the + // API wrapper class. + repeated string required_constructor_params = 50; +} + +message SmokeTestConfigProto { + // The name of the method used in the smoke test. + // There must be a flattening defined in that method, and that + // flattened method will be used as the one to generate a smoke test on. + string method = 1; + + // A list describing the name and the value of the init fields. + repeated string init_fields = 2; + + reserved 3; +} + +message CollectionConfigProto { + reserved 1; + + // Name to be used as a basis for generated methods and classes. + string entity_name = 2; + + repeated CollectionLanguageOverridesProto language_overrides = 3; +} + +message CollectionLanguageOverridesProto { + // The language of the generated code. + string language = 1; + + // The entity name to override the default with + string entity_name = 2; + + // Optional fully-qualified type-name of a common resource-name + string common_resource_name = 3; +} + +message CollectionOneofProto { + // Name to be used as a basis for generated methods and classes. + string oneof_name = 1; + + // A list of the entity names of the CollectionConfigs to be included in the + // oneof collection. + repeated string collection_names = 2; +} + +message FixedResourceNameValueProto { + // Name to be used as a basis for generated methods and classes. + string entity_name = 1; + + // The fixed, unformatted string value accepted by this configuration. + string fixed_value = 2; +} + +// `MethodConfigProto` describes the generator configuration for a method. +message MethodConfigProto { + // The fully qualified name of the method. + string name = 1; + + reserved 2, 3, 7, 8, 9, 14, 15; + + // Specifies the configuration for gRPC-streaming responses. + // Note that this is for configuring paged gRPC-streaming responses. + // Some method can be gRPC-streaming even if this field does not exist. + PageStreamingConfigProto grpc_streaming = 13; + + // Specifies the configuration for retryable codes. + // The name must be defined in InterfaceConfigProto::retry_codes_def. + string retry_codes_name = 4; + + // Specifies the configuration for retry/backoff parameters. + // The name must be defined in InterfaceConfigProto::retry_params_def. + string retry_params_name = 5; + + // Specifies the default timeout for a non-retrying call. If the call is + // retrying, refer to `retry_params_name` instead. + uint64 timeout_millis = 11; // UNSUPPORTED in Java, C#, PHP, Go. + + // Specifies the configuration for batching. + BatchingConfigProto batching = 6; + + // Specifies complex structure fields that need to be initialized by the sample code for + // the sample to be usable. + repeated string sample_code_init_fields = 10; + + // Specifies sets of parameter values to be used together in a + // sample. If this is not set, no samples are output. + // + // UNDER DEVELOPMENT: Usage of this field for sample generation is + // still being developed. For now, continue using + // `sample_code_init_fields` to generate in-code samples. + repeated SampleValueSet sample_value_sets = 16; + + // Defines combinations of method calling forms (sample code + // structures as output by the client library generator) and + // `sample_value_sets` that should be combined to generate samples. + // + // UNDER DEVELOPMENT: Usage of this field for sample generation is + // still being developed. For now, continue using + // `sample_code_init_fields` to generate in-code samples. + SampleConfiguration samples = 17; + + // Route calls through a different gRPC interface than the one this method + // is contained in. This specifically supports the Pub/Sub IAM hack to route + // IAM methods to IamPolicy for Pub/Sub. + string reroute_to_grpc_interface = 12; + + repeated SurfaceTreatmentProto surface_treatments = 20; + + // Long-running settings. + LongRunningConfigProto long_running = 30; + +} + +// `SampleValueSet` defines a set of parameter values used in +// generating samples. +message SampleValueSet { + // An identifier, unique to a particular API method, for this set of + // values. + string id = 1; + + // A short, user-visible name for this set of values. This name may + // be used in a UI menu, for example. + string title = 2; + + // A longer description for this set of values. This may be + // used in the sample itself or in a UI element such as hoverbox. + string description = 3; + + // How to initialize the request object for the RPC. + SampleParameters parameters = 4; + + // How to process a successful response from the client library + // function being exemplified. + repeated OutputSpec on_success = 5; +} + +// `SampleInitAttribute` controls request object initialization. +message SampleParameters { + // A set of default values to be used together in samples. Each + // member of can be specified in one of two formats: + // * `DOTTED.RESOURCE.NAME=VALUE`, to specify the entire value of + // `NAME`. If `VALUE` is not specified, it is treated as the + // default zero value for `NAME`'s type. + // * `DOTTED.RESOURCE.NAME%FIELD=VALUE`, if `NAME` is required to be + // a specially formatted string using `FIELD` as one of possibly + // several placeholders. + repeated string defaults = 1; + + // Attributes governing how to init request object. + // Each attribute must have unique, non-overlapping, path. + repeated SampleInitAttribute attributes = 2; +} + +// `SampleInitAttribute` controls how a sample initializes the request object. +message SampleInitAttribute { + // Required. The part of the request object this attribute is talking about. + string parameter = 1; + + // In a sample, the call to client lib method is itself in a function. + // If `sample_argument_name` is not empty, the part of the request object specified by `path` + // should be accepted as a parameter, with the given name, to the sample function. + string sample_argument_name = 2; + + // If `read_file` is true, then during sample initialization, the value of this + // attribute gets replaced by the contents of the local file with the given name. + // This is only allowed when this parameter is a bytes field. + // If this parameter is required or optional, the original value of this parameter + // is the file name. + // If this parameter is repeated, the original value of this parameter is a list of + // comma-separated list of file names. + bool read_file = 3; + + // The description of the parameter passed to the sample function. + // Should only be specified when `sample_argument_name` is specified. + string description = 4; +} + +// `SampleConfiguration` defines combinations of "calling forms" +// (sample code structures as output by the client library generator) +// and sample value sets that should be combined to generate samples. +message SampleConfiguration { + + // `SampleTypeConfiguration` defines the samples that will show up + // as a particular sample type. If order matters for this sample + // type, the order of the samples is taken to be primarily the order + // in which the `calling_forms` appear, and secondarily, for each + // such form, the order in which the `value_sets` appear. + message SampleTypeConfiguration { + // One or more expressions matching the method calling forms + // (defined by the client generator) applicable to this + // method. Any expressions that do not match a calling form in a + // particular language are ignored for that language. If not + // specified, samples are generated for all such forms. + // + // An expression matches an ID if the sequence of characters in + // the expression before the first "*", if any, match the initial + // sequence of characters in the ID. This allows for prefix + // matching. In particular, the expression "*" will + // match all IDs. + // + // TODO(vchudnov-g): Point to a reference listing the possible calling forms + // for each method type. + repeated string calling_forms = 1; + + // One or more expressions matching the + // `MethodConfigProto.sample_value_sets.id`s defined for this + // method. Any expressions on this list that do not match a + // `SampleValueSet.id` cause a fatal error during code + // generation. If not specified or if empty, no samples of this + // type will be generated. + // + // An expression matches an ID if the sequence of characters in + // the expression before the first "*", if any, match the initial + // sequence of characters in the ID. This allows for prefix + // matching. In particular, the expression "*" will + // match all IDs. + repeated string value_sets = 2; + + // The region tag value to be used inside each of these particular + // samples to bracket off (in language-specific comments) + // important areas of the sample. A region tag value can only + // consist of word characters (letters, numbers, or underscores) + // as well as the special tokens "%m", "%c", and "%v" (no quotes) + // which get replaced by each sample's method name, calling form, + // and value set, respectively. If not specified, `region_tag` + // defaults to "sample". + string region_tag = 3; + } + + // The configuration for in-code samples, which appear in the format + // and location idiomatic to each generated language. The various + // in-code samples will be presented in the order in which they are + // configured here. If this field is not specified, one in-code + // sample will be produced for this method. + repeated SampleTypeConfiguration in_code = 1; + + // The configuration for stand-alone, runnable samples in each + // generated language. If this field is not specified, a stand-alone + // sample is generated for every combination of calling form and + // `MethodConfigProto.sample_value_sets`, if the latter is defined. + // + // If multiple samples configured here would result in files + // clobbering each other (ie. two files with the same path but not + // with identical calling forms, value sets, and region tags), an + // exception is raised. + repeated SampleTypeConfiguration standalone = 2; + + // The configuration for user-parametrized samples ("API explorer + // samples") that can be displayed in, for example, an interactive + // UI. The various explorer samples will be presented in the order + // in which they are configured here. If this field is not + // specified, an explorer sample is generated for every combination + // of calling form and `MethodConfigProto.sample_value_sets`, if the + // latter is defined. + // + // If multiple samples configured here would result in files + // clobbering each other (ie. two files with the same path but not + // with identical calling forms, value sets, and region tags), an + // exception is raised. + repeated SampleTypeConfiguration api_explorer = 3; +} + +// The ResourceNameTreatment enum can be used to specify how to treat the +// resource name formats defined in the field_name_patterns +// and response_field_name_patterns fields. +// UNSET: default value +// NONE: the collection configs will not be used by the generated code. +// VALIDATE: string fields will be validated by the client against the +// specified resource name formats. +// STATIC_TYPES: the client will use generated types for resource names. +// SAMPLE_ONLY: the generated types for resource names will only be used in samples. +enum ResourceNameTreatment { + UNSET_TREATMENT = 0; + NONE = 1; + VALIDATE = 2; + STATIC_TYPES = 3; + SAMPLE_ONLY = 4; +} + +// `PageStreamingConfigProto` describes information for generating a method which +// transforms a paging list rpc into a stream of resources. +message PageStreamingConfigProto { + reserved 1; + + // Specifies response information of the list method. + PageStreamingResponseProto response = 2; +} + +// `PageStreamingResponseProto` defines which fields match the paging pattern in +// the response. +message PageStreamingResponseProto { + // The name of the field in the response containing the next page + // token. + string token_field = 1; + + // The name of the field in the response containing the list of + // resources belonging to the page. + string resources_field = 2; +} + +// `RetryConfigDefinitionProto` specifies, by name, +// GRPC codes that a method should consider retryable. +message RetryCodesDefinitionProto { + string name = 1; + repeated string retry_codes = 2; +} + +// `RetryParamsDefinitionProto` specifies, by name, +// the backoff behavior of a method when retrying. +message RetryParamsDefinitionProto { + string name = 1; + + uint64 initial_retry_delay_millis = 2; + double retry_delay_multiplier = 3; + uint64 max_retry_delay_millis = 4; + + uint64 initial_rpc_timeout_millis = 5; + double rpc_timeout_multiplier = 6; + uint64 max_rpc_timeout_millis = 7; + + uint64 total_timeout_millis = 8; +} + +// `BatchingConfigProto` defines the batching configuration for an API method. +message BatchingConfigProto { + // The thresholds which trigger a batched request to be sent. + BatchingSettingsProto thresholds = 1; + + // The request and response fields used in batching. + BatchingDescriptorProto batch_descriptor = 2; +} + +// `BatchingSettingsProto` specifies a set of batching thresholds, each of +// which acts as a trigger to send a batch of messages as a request. At least +// one threshold must be positive nonzero. +message BatchingSettingsProto { + // The number of elements of a field collected into a batch which, if + // exceeded, causes the batch to be sent. + uint32 element_count_threshold = 1; + + // The aggregated size of the batched field which, if exceeded, causes the + // batch to be sent. This size is computed by aggregating the sizes of the + // request field to be batched, not of the entire request message. + uint64 request_byte_threshold = 2; + + // The duration, in milliseconds, after which a batch should be sent, + // starting from the addition of the first message to that batch. + uint64 delay_threshold_millis = 3; + + // The maximum number of elements collected in a batch that could be accepted by server. + uint32 element_count_limit = 4; + + // The maximum size of the request that could be accepted by server. + uint32 request_byte_limit = 5; + + uint32 flow_control_element_limit = 6; + + uint32 flow_control_byte_limit = 7; + + FlowControlLimitExceededBehaviorProto flow_control_limit_exceeded_behavior = 8; +} + +enum FlowControlLimitExceededBehaviorProto { + UNSET_BEHAVIOR = 0; + THROW_EXCEPTION = 1; + BLOCK = 2; + IGNORE = 3; +} + +// `BatchingDescriptorProto` specifies the fields of the request message to be +// used for batching, and, optionally, the fields of the response message to be +// used for demultiplexing. +message BatchingDescriptorProto { + // The repeated field in the request message to be aggregated by batching. + string batched_field = 1; + + // A list of the fields in the request message. Two requests will be batched + // together only if the values of every field specified in + // `request_discriminator_fields` is equal between the two requests. + repeated string discriminator_fields = 2; + + // Optional. When present, indicates the field in the response message to be + // used to demultiplex the response into multiple response messages, in + // correspondence with the multiple request messages originally batched + // together. + string subresponse_field = 3; +} + +// SurfaceTreatmentProto describes treatments to the code generation +// that are expected to be different for each language. +message SurfaceTreatmentProto { + // The languages affected by this treatment. + repeated string include_languages = 1; + + // The visibility of the surface. + VisibilityProto visibility = 2; + + // Whether the method is deprecated. + ReleaseLevel release_level = 3; +} + +enum VisibilityProto { + UNSET_VISIBILITY = 0; + PUBLIC = 1; + PACKAGE = 2; + PRIVATE = 3; + DISABLED = 4; +} + +// LongRunningProto describes settings to use when generating API methods +// that use the long-running operation pattern. +message LongRunningConfigProto { + reserved 1, 2; + + // Whether or not the server implements delete. + bool implements_delete = 3; + + // Whether or not the server implements cancel. + bool implements_cancel = 4; + + // Initial delay after which first poll request will be made. + uint64 initial_poll_delay_millis = 5; + + // Multiplier to gradually increase delay between subsequent polls until it + // reaches max_poll_delay_millis. + double poll_delay_multiplier = 6; + + // Maximum time between two subsequent poll requests. + uint64 max_poll_delay_millis = 7; + + // Total polling timeout. + uint64 total_poll_timeout_millis = 8; +} + +message OutputSpec { + message LoopStatement { + // The collection over which to iterate. + // Exactly one of `map` and `collection` should be specified. + string collection = 1; + + // The iteration variable. + // Required and should only be specified if `collection` is specified. + string variable = 2; + + // The contents of the loop. + repeated OutputSpec body = 3; + + // The map over which to iterate. + // Exactly one of `map` and `collection` should be specified. + string map = 4; + + // The iteration variable for key. + // Can only be specified when `map` is specified. + // At least one of `key` and `value` should be specified if `map` is specified. + string key = 5; + + // The iteration variable for value. + // Can only be specified when `map` is specified. + // At least one of `key` and `value` should be specified if `map` is specified. + string value = 6; + } + + // Exactly one of the following should be set. + + // A loop construct. + LoopStatement loop = 1; + + // The elements of `print` consist of a printf-like format string + // followed by variables to be interpolated in the string (for + // example, ["%s=%s", thing.variable, thing.value]). These + // variables must either be `$resp` or have been previously defined + // by `loop` or `define`. + // + // Note that this field name is singular because the array + // represents a single print statement. A newline will be + // automatically appended to the print statement. + repeated string print = 2; + + // A variable definition construct, of the form + // `name=var[. field...]`, where `name` is the new variable being + // defined, `var` is either `$resp` or a variable that was + // previously defined by `loop` or `define`, and everything to the + // right of the period is any valid sequence of subfield or + // bracketed list subscripts or map keys to dereference a descendant + // of `var`. + string define = 3; + + // The first element of `comment` is a printf-like format string, and + // subsequent elements are variable names (not values) to be interpolated in + // the string. The variable names will be converted to a language-idiomatic + // style (snake case or camel case). The interpolated format string will then + // be rendered as a comment in a language-idiomatic style. + // + // Like `print`, a newline will be automatically appended to the comment. + repeated string comment = 4; +} diff --git a/src/test/java/com/google/api/codegen/config/GapicConfigProducerTest.java b/src/test/java/com/google/api/codegen/config/GapicConfigProducerTest.java index 9fc23558bf..d2384a2a58 100644 --- a/src/test/java/com/google/api/codegen/config/GapicConfigProducerTest.java +++ b/src/test/java/com/google/api/codegen/config/GapicConfigProducerTest.java @@ -32,14 +32,11 @@ public class GapicConfigProducerTest { @ClassRule public static TemporaryFolder tempDir = new TemporaryFolder(); - private static Model model; - private static GapicProductConfig productConfig; - @Test public void missingConfigSchemaVersion() { TestDataLocator locator = MixedPathTestDataLocator.create(this.getClass()); locator.addTestDataSource(CodegenTestUtil.class, "testsrc/common"); - model = + Model model = CodegenTestUtil.readModel( locator, tempDir, new String[] {"myproto.proto"}, new String[] {"myproto.yaml"}); @@ -48,11 +45,33 @@ public void missingConfigSchemaVersion() { model.getDiagReporter().getDiagCollector(), locator, new String[] {"missing_config_schema_version.yaml"}); - productConfig = GapicProductConfig.create(model, configProto, null, null, TargetLanguage.JAVA); + GapicProductConfig.create(model, configProto, null, null, TargetLanguage.JAVA); Diag expectedError = Diag.error( SimpleLocation.TOPLEVEL, "config_schema_version field is required in GAPIC yaml."); assertThat(model.getDiagReporter().getDiagCollector().hasErrors()).isTrue(); assertThat(model.getDiagReporter().getDiagCollector().getDiags()).contains(expectedError); } + + @Test + public void missingInterface() { + TestDataLocator locator = MixedPathTestDataLocator.create(this.getClass()); + locator.addTestDataSource(CodegenTestUtil.class, "testsrc/common"); + Model model = + CodegenTestUtil.readModel( + locator, tempDir, new String[] {"myproto.proto"}, new String[] {"myproto.yaml"}); + + ConfigProto configProto = + CodegenTestUtil.readConfig( + model.getDiagReporter().getDiagCollector(), + locator, + new String[] {"missing_interface_v1.yaml"}); + GapicProductConfig.create(model, configProto, null, null, TargetLanguage.JAVA); + Diag expectedError = + Diag.error( + SimpleLocation.TOPLEVEL, + "interface not found: google.example.myproto.v1.MyUnknownProto. Interfaces: [google.example.myproto.v1.MyProto]"); + assertThat(model.getDiagReporter().getDiagCollector().hasErrors()).isTrue(); + assertThat(model.getDiagReporter().getDiagCollector().getDiags()).contains(expectedError); + } } diff --git a/src/test/java/com/google/api/codegen/config/LongRunningConfigTest.java b/src/test/java/com/google/api/codegen/config/LongRunningConfigTest.java index 6478e6dc44..e0d9d4addd 100644 --- a/src/test/java/com/google/api/codegen/config/LongRunningConfigTest.java +++ b/src/test/java/com/google/api/codegen/config/LongRunningConfigTest.java @@ -104,6 +104,7 @@ public static void startUp() { @Test public void testCreateLROWithoutGapicConfig() { + Mockito.when(protoParser.isProtoAnnotationsEnabled()).thenReturn(true); DiagCollector diagCollector = new BoundedDiagCollector(); LongRunningConfig longRunningConfig = LongRunningConfig.createLongRunningConfig( @@ -120,19 +121,20 @@ public void testCreateLROWithoutGapicConfig() { ProtoTypeRef returnTypeModel = (ProtoTypeRef) longRunningConfig.getReturnType(); assertThat(returnTypeModel.getProtoType()).isEqualTo(annotationsReturnType); - assertThat(longRunningConfig.getInitialPollDelay().toMillis()) + assertThat(longRunningConfig.getInitialPollDelay()) .isEqualTo(LongRunningConfig.LRO_INITIAL_POLL_DELAY_MILLIS); - assertThat(longRunningConfig.getMaxPollDelay().toMillis()) + assertThat(longRunningConfig.getMaxPollDelay()) .isEqualTo(LongRunningConfig.LRO_MAX_POLL_DELAY_MILLIS); assertThat(longRunningConfig.getPollDelayMultiplier()) .isEqualTo(LongRunningConfig.LRO_POLL_DELAY_MULTIPLIER); - assertThat(longRunningConfig.getTotalPollTimeout().toMillis()) + assertThat(longRunningConfig.getTotalPollTimeout()) .isEqualTo(LongRunningConfig.LRO_TOTAL_POLL_TIMEOUT_MILLS); } @Test public void testCreateLROWithGapicConfigOnly() { DiagCollector diagCollector = new BoundedDiagCollector(); + Mockito.when(protoParser.isProtoAnnotationsEnabled()).thenReturn(false); // simpleMethod has no LRO proto annotations. // lroConfigProtoWithPollSettings contains LRO settings. @@ -160,6 +162,7 @@ public void testCreateLROWithGapicConfigOnly() { @Test public void testCreateLROWithAnnotationsOverridingGapicConfig() { DiagCollector diagCollector = new BoundedDiagCollector(); + Mockito.when(protoParser.isProtoAnnotationsEnabled()).thenReturn(true); // lroAnnotatedMethod contains different settings than that in lroConfigProtoWithPollSettings. LongRunningConfig longRunningConfig = @@ -169,24 +172,25 @@ public void testCreateLROWithAnnotationsOverridingGapicConfig() { assertThat(diagCollector.getErrorCount()).isEqualTo(0); assertThat(longRunningConfig).isNotNull(); - // Assert that proto annotations settings take precendence over gapic config. + // Assert that proto annotations settings take precendence over gapic config for + // return and metadata types. ProtoTypeRef metadataTypeModel = (ProtoTypeRef) longRunningConfig.getMetadataType(); assertThat(metadataTypeModel.getProtoType()).isEqualTo(annotationsMetadataType); ProtoTypeRef returnTypeModel = (ProtoTypeRef) longRunningConfig.getReturnType(); assertThat(returnTypeModel.getProtoType()).isEqualTo(annotationsReturnType); + // Assert that GAPIC config timeout values are used. assertThat(longRunningConfig.getInitialPollDelay().toMillis()) - .isEqualTo(LongRunningConfig.LRO_INITIAL_POLL_DELAY_MILLIS); - assertThat(longRunningConfig.getMaxPollDelay().toMillis()) - .isEqualTo(LongRunningConfig.LRO_MAX_POLL_DELAY_MILLIS); - assertThat(longRunningConfig.getPollDelayMultiplier()) - .isEqualTo(LongRunningConfig.LRO_POLL_DELAY_MULTIPLIER); + .isEqualTo(TEST_INITIAL_POLL_DELAY); + assertThat(longRunningConfig.getMaxPollDelay().toMillis()).isEqualTo(TEST_MAX_POLL_DELAY); + assertThat(longRunningConfig.getPollDelayMultiplier()).isEqualTo(TEST_POLL_DELAY_MULTIPLIER); assertThat(longRunningConfig.getTotalPollTimeout().toMillis()) - .isEqualTo(LongRunningConfig.LRO_TOTAL_POLL_TIMEOUT_MILLS); + .isEqualTo(TEST_TOTAL_POLL_TIMEOUT); } @Test public void testCreateSameLROFromProtoFileAndGapicConfig() { + Mockito.when(protoParser.isProtoAnnotationsEnabled()).thenReturn(true); // Given a Protobuf LRO method annotated with the same Return and Metadata Type // as in the GAPIC config, use the GAPIC config settings. DiagCollector diagCollector = new BoundedDiagCollector(); diff --git a/src/test/java/com/google/api/codegen/config/PageStreamingConfigTest.java b/src/test/java/com/google/api/codegen/config/PageStreamingConfigTest.java new file mode 100644 index 0000000000..328a31fc6d --- /dev/null +++ b/src/test/java/com/google/api/codegen/config/PageStreamingConfigTest.java @@ -0,0 +1,23 @@ +/* Copyright 2019 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.google.api.codegen.config; + +import org.junit.Test; + +public class PageStreamingConfigTest { + + @Test + public void testMissingPageStreamingParams() {} +} diff --git a/src/test/java/com/google/api/codegen/config/ResourceNameMessageConfigsTest.java b/src/test/java/com/google/api/codegen/config/ResourceNameMessageConfigsTest.java index 1a8bbdd7b0..337bef533a 100644 --- a/src/test/java/com/google/api/codegen/config/ResourceNameMessageConfigsTest.java +++ b/src/test/java/com/google/api/codegen/config/ResourceNameMessageConfigsTest.java @@ -35,6 +35,7 @@ import com.google.api.tools.framework.model.Diag.Kind; import com.google.api.tools.framework.model.DiagCollector; import com.google.api.tools.framework.model.Field; +import com.google.api.tools.framework.model.Interface; import com.google.api.tools.framework.model.MessageType; import com.google.api.tools.framework.model.Method; import com.google.api.tools.framework.model.ProtoFile; @@ -70,6 +71,7 @@ public class ResourceNameMessageConfigsTest { private static final MessageType bookMessage = Mockito.mock(MessageType.class); private static final ProtoFile protoFile = Mockito.mock(ProtoFile.class); private static final ImmutableList sourceProtoFiles = ImmutableList.of(protoFile); + private static final Interface anInterface = Mockito.mock(Interface.class); private static final Method insertBook = Mockito.mock(Method.class); private static final String DEFAULT_PACKAGE = "library"; @@ -172,6 +174,8 @@ public static void startUp() { Mockito.doReturn(bookMessage).when(insertBook).getInputMessage(); Mockito.doReturn(protoFile).when(bookMessage).getParent(); + Mockito.doReturn(ImmutableList.of(anInterface)).when(protoFile).getInterfaces(); + Mockito.doReturn("library.LibraryService").when(anInterface).getFullName(); // Mockito.doReturn("Book").when(protoParser).getResourceReference(bookName); } @@ -271,6 +275,7 @@ public void testCreateResourceNameConfigs() { Map resourceNameConfigs = GapicProductConfig.createResourceNameConfigsWithProtoFileAndGapicConfig( + null, diagCollector, configProto, protoFile, @@ -364,6 +369,7 @@ public void testCreateFlattenings() { protoParser); ImmutableMap resourceNameConfigs = GapicProductConfig.createResourceNameConfigsWithProtoFileAndGapicConfig( + null, diagCollector, extraConfigProto, protoFile, diff --git a/src/test/java/com/google/api/codegen/config/testdata/missing_interface_v1.yaml b/src/test/java/com/google/api/codegen/config/testdata/missing_interface_v1.yaml new file mode 100644 index 0000000000..e98791cb50 --- /dev/null +++ b/src/test/java/com/google/api/codegen/config/testdata/missing_interface_v1.yaml @@ -0,0 +1,60 @@ +type: com.google.api.codegen.ConfigProto +config_schema_version: 1.0.0 +language_settings: + go: + package_name: cloud.google.com/go/myapi/apiv1 +interfaces: +- name: google.example.myproto.v1.MyProto + retry_codes_def: + - name: idempotent + retry_codes: + - DEADLINE_EXCEEDED + - name: non_idempotent + retry_codes: + retry_params_def: + - name: default + initial_retry_delay_millis: 100 + retry_delay_multiplier: 1.2 + max_retry_delay_millis: 1000 + initial_rpc_timeout_millis: 300 + rpc_timeout_multiplier: 1.3 + max_rpc_timeout_millis: 3000 + total_timeout_millis: 30000 + methods: + - name: MyMethod + flattening: + groups: + - parameters: + - myfield + required_fields: + - myfield + retry_codes_name: non_idempotent + retry_params_name: default + timeout_millis: 1000 +- name: google.example.myproto.v1.MyUnknownProto + retry_codes_def: + - name: idempotent + retry_codes: + - DEADLINE_EXCEEDED + - name: non_idempotent + retry_codes: + retry_params_def: + - name: default + initial_retry_delay_millis: 100 + retry_delay_multiplier: 1.2 + max_retry_delay_millis: 1000 + initial_rpc_timeout_millis: 300 + rpc_timeout_multiplier: 1.3 + max_rpc_timeout_millis: 3000 + total_timeout_millis: 30000 + methods: + - name: MyMethod + flattening: + groups: + - parameters: + - myfield + required_fields: + - myfield + retry_codes_name: non_idempotent + retry_params_name: default + timeout_millis: 1000 \ No newline at end of file diff --git a/src/test/java/com/google/api/codegen/gapic/testdata/java/java_multiple_services.baseline b/src/test/java/com/google/api/codegen/gapic/testdata/java/java_multiple_services.baseline index b68c240baf..2aec187372 100644 --- a/src/test/java/com/google/api/codegen/gapic/testdata/java/java_multiple_services.baseline +++ b/src/test/java/com/google/api/codegen/gapic/testdata/java/java_multiple_services.baseline @@ -1086,6 +1086,9 @@ public class DecrementerServiceStubSettings extends StubSettings> definitions = ImmutableMap.builder(); + definitions.put( + "idempotent", + ImmutableSet.copyOf(Lists.newArrayList(StatusCode.Code.DEADLINE_EXCEEDED, StatusCode.Code.UNAVAILABLE))); definitions.put( "non_idempotent", ImmutableSet.copyOf(Lists.newArrayList())); @@ -1877,6 +1880,9 @@ public class IncrementerServiceStubSettings extends StubSettings> definitions = ImmutableMap.builder(); + definitions.put( + "idempotent", + ImmutableSet.copyOf(Lists.newArrayList(StatusCode.Code.DEADLINE_EXCEEDED, StatusCode.Code.UNAVAILABLE))); definitions.put( "non_idempotent", ImmutableSet.copyOf(Lists.newArrayList())); diff --git a/src/test/java/com/google/api/codegen/gapic/testdata/java/java_no_path_templates.baseline b/src/test/java/com/google/api/codegen/gapic/testdata/java/java_no_path_templates.baseline index 4382a05fae..f968cb3ae3 100644 --- a/src/test/java/com/google/api/codegen/gapic/testdata/java/java_no_path_templates.baseline +++ b/src/test/java/com/google/api/codegen/gapic/testdata/java/java_no_path_templates.baseline @@ -959,6 +959,9 @@ public class NoTemplatesApiServiceStubSettings extends StubSettings> definitions = ImmutableMap.builder(); + definitions.put( + "idempotent", + ImmutableSet.copyOf(Lists.newArrayList(StatusCode.Code.DEADLINE_EXCEEDED, StatusCode.Code.UNAVAILABLE))); definitions.put( "non_idempotent", ImmutableSet.copyOf(Lists.newArrayList())); diff --git a/src/test/java/com/google/api/codegen/gapic/testdata/nodejs/nodejs_multiple_services.baseline b/src/test/java/com/google/api/codegen/gapic/testdata/nodejs/nodejs_multiple_services.baseline index 1133c0742b..3acea4211d 100644 --- a/src/test/java/com/google/api/codegen/gapic/testdata/nodejs/nodejs_multiple_services.baseline +++ b/src/test/java/com/google/api/codegen/gapic/testdata/nodejs/nodejs_multiple_services.baseline @@ -392,6 +392,10 @@ module.exports = DecrementerServiceClient; "interfaces": { "google.cloud.example.v1.foo.DecrementerService": { "retry_codes": { + "idempotent": [ + "DEADLINE_EXCEEDED", + "UNAVAILABLE" + ], "non_idempotent": [] }, "retry_params": { @@ -710,6 +714,10 @@ module.exports = IncrementerServiceClient; "interfaces": { "google.cloud.example.v1.foo.IncrementerService": { "retry_codes": { + "idempotent": [ + "DEADLINE_EXCEEDED", + "UNAVAILABLE" + ], "non_idempotent": [] }, "retry_params": { diff --git a/src/test/java/com/google/api/codegen/gapic/testdata/nodejs/nodejs_no_path_templates.baseline b/src/test/java/com/google/api/codegen/gapic/testdata/nodejs/nodejs_no_path_templates.baseline index b8db4d1f05..2362b4f669 100644 --- a/src/test/java/com/google/api/codegen/gapic/testdata/nodejs/nodejs_no_path_templates.baseline +++ b/src/test/java/com/google/api/codegen/gapic/testdata/nodejs/nodejs_no_path_templates.baseline @@ -387,6 +387,10 @@ module.exports = NoTemplatesApiServiceClient; "interfaces": { "google.cloud.example.v1.NoTemplatesAPIService": { "retry_codes": { + "idempotent": [ + "DEADLINE_EXCEEDED", + "UNAVAILABLE" + ], "non_idempotent": [] }, "retry_params": { diff --git a/src/test/java/com/google/api/codegen/gapic/testdata/php/php_no_path_templates.baseline b/src/test/java/com/google/api/codegen/gapic/testdata/php/php_no_path_templates.baseline index dd017c1dcb..027e0b0315 100644 --- a/src/test/java/com/google/api/codegen/gapic/testdata/php/php_no_path_templates.baseline +++ b/src/test/java/com/google/api/codegen/gapic/testdata/php/php_no_path_templates.baseline @@ -282,6 +282,10 @@ class NoTemplatesApiServiceClient extends NoTemplatesApiServiceGapicClient "interfaces": { "google.cloud.example.v1.NoTemplatesAPIService": { "retry_codes": { + "idempotent": [ + "DEADLINE_EXCEEDED", + "UNAVAILABLE" + ], "non_idempotent": [] }, "retry_params": { diff --git a/src/test/java/com/google/api/codegen/gapic/testdata/py/python_multiple_services.baseline b/src/test/java/com/google/api/codegen/gapic/testdata/py/python_multiple_services.baseline index 2a20ce1d9b..c788e116f0 100644 --- a/src/test/java/com/google/api/codegen/gapic/testdata/py/python_multiple_services.baseline +++ b/src/test/java/com/google/api/codegen/gapic/testdata/py/python_multiple_services.baseline @@ -1073,6 +1073,10 @@ config = { "interfaces": { "google.cloud.example.v1.foo.DecrementerService": { "retry_codes": { + "idempotent": [ + "DEADLINE_EXCEEDED", + "UNAVAILABLE" + ], "non_idempotent": [] }, "retry_params": { @@ -1307,6 +1311,10 @@ config = { "interfaces": { "google.cloud.example.v1.foo.IncrementerService": { "retry_codes": { + "idempotent": [ + "DEADLINE_EXCEEDED", + "UNAVAILABLE" + ], "non_idempotent": [] }, "retry_params": { diff --git a/src/test/java/com/google/api/codegen/gapic/testdata/py/python_no_path_templates.baseline b/src/test/java/com/google/api/codegen/gapic/testdata/py/python_no_path_templates.baseline index 008fbb9424..fffe8ee830 100644 --- a/src/test/java/com/google/api/codegen/gapic/testdata/py/python_no_path_templates.baseline +++ b/src/test/java/com/google/api/codegen/gapic/testdata/py/python_no_path_templates.baseline @@ -1012,6 +1012,10 @@ config = { "interfaces": { "google.cloud.example.v1.NoTemplatesAPIService": { "retry_codes": { + "idempotent": [ + "DEADLINE_EXCEEDED", + "UNAVAILABLE" + ], "non_idempotent": [] }, "retry_params": { diff --git a/src/test/java/com/google/api/codegen/gapic/testdata/ruby/ruby_multiple_services.baseline b/src/test/java/com/google/api/codegen/gapic/testdata/ruby/ruby_multiple_services.baseline index 08e1c7674c..69fceee489 100644 --- a/src/test/java/com/google/api/codegen/gapic/testdata/ruby/ruby_multiple_services.baseline +++ b/src/test/java/com/google/api/codegen/gapic/testdata/ruby/ruby_multiple_services.baseline @@ -1146,6 +1146,10 @@ end "interfaces": { "google.cloud.example.v1.foo.DecrementerService": { "retry_codes": { + "idempotent": [ + "DEADLINE_EXCEEDED", + "UNAVAILABLE" + ], "non_idempotent": [] }, "retry_params": { @@ -1425,6 +1429,10 @@ end "interfaces": { "google.cloud.example.v1.foo.IncrementerService": { "retry_codes": { + "idempotent": [ + "DEADLINE_EXCEEDED", + "UNAVAILABLE" + ], "non_idempotent": [] }, "retry_params": { diff --git a/src/test/java/com/google/api/codegen/protoannotations/testdata/java_library_no_gapic_config.baseline b/src/test/java/com/google/api/codegen/protoannotations/testdata/java_library_no_gapic_config.baseline index a888879bf0..2bb1234f42 100644 --- a/src/test/java/com/google/api/codegen/protoannotations/testdata/java_library_no_gapic_config.baseline +++ b/src/test/java/com/google/api/codegen/protoannotations/testdata/java_library_no_gapic_config.baseline @@ -1955,7 +1955,7 @@ public class LibraryServiceClient implements BackgroundResource { * Sample code: * 

    * try (LibraryServiceClient libraryServiceClient = LibraryServiceClient.create()) {
-   *   BookName name = DeletedBookName.of();
+   *   BookName name = BookName.of("[SHELF_ID]", "[BOOK_ID]");
    *   BookName altBookName = BookName.of("[SHELF_ID]", "[BOOK_ID]");
    *   BookFromAnywhere response = libraryServiceClient.getBookFromAnywhere(name, altBookName);
    * }
@@ -1983,7 +1983,7 @@ public class LibraryServiceClient implements BackgroundResource {
    * Sample code:
    * 

    * try (LibraryServiceClient libraryServiceClient = LibraryServiceClient.create()) {
-   *   BookName name = DeletedBookName.of();
+   *   BookName name = BookName.of("[SHELF_ID]", "[BOOK_ID]");
    *   BookName altBookName = BookName.of("[SHELF_ID]", "[BOOK_ID]");
    *   BookFromAnywhere response = libraryServiceClient.getBookFromAnywhere(name.toString(), altBookName.toString());
    * }
@@ -2011,7 +2011,7 @@ public class LibraryServiceClient implements BackgroundResource {
    * Sample code:
    * 

    * try (LibraryServiceClient libraryServiceClient = LibraryServiceClient.create()) {
-   *   BookName name = DeletedBookName.of();
+   *   BookName name = BookName.of("[SHELF_ID]", "[BOOK_ID]");
    *   BookName altBookName = BookName.of("[SHELF_ID]", "[BOOK_ID]");
    *   GetBookFromAnywhereRequest request = GetBookFromAnywhereRequest.newBuilder()
    *     .setName(name.toString())
@@ -2035,7 +2035,7 @@ public class LibraryServiceClient implements BackgroundResource {
    * Sample code:
    * 

    * try (LibraryServiceClient libraryServiceClient = LibraryServiceClient.create()) {
-   *   BookName name = DeletedBookName.of();
+   *   BookName name = BookName.of("[SHELF_ID]", "[BOOK_ID]");
    *   BookName altBookName = BookName.of("[SHELF_ID]", "[BOOK_ID]");
    *   GetBookFromAnywhereRequest request = GetBookFromAnywhereRequest.newBuilder()
    *     .setName(name.toString())
@@ -2058,7 +2058,7 @@ public class LibraryServiceClient implements BackgroundResource {
    * Sample code:
    * 

    * try (LibraryServiceClient libraryServiceClient = LibraryServiceClient.create()) {
-   *   BookName name = DeletedBookName.of();
+   *   BookName name = BookName.of("[SHELF_ID]", "[BOOK_ID]");
    *   BookFromAnywhere response = libraryServiceClient.getBookFromAbsolutelyAnywhere(name);
    * }
    * 

@@ -2082,7 +2082,7 @@ public class LibraryServiceClient implements BackgroundResource {
    * Sample code:
    * 

    * try (LibraryServiceClient libraryServiceClient = LibraryServiceClient.create()) {
-   *   BookName name = DeletedBookName.of();
+   *   BookName name = BookName.of("[SHELF_ID]", "[BOOK_ID]");
    *   BookFromAnywhere response = libraryServiceClient.getBookFromAbsolutelyAnywhere(name.toString());
    * }
    * 

@@ -2106,7 +2106,7 @@ public class LibraryServiceClient implements BackgroundResource {
    * Sample code:
    * 

    * try (LibraryServiceClient libraryServiceClient = LibraryServiceClient.create()) {
-   *   BookName name = DeletedBookName.of();
+   *   BookName name = BookName.of("[SHELF_ID]", "[BOOK_ID]");
    *   GetBookFromAbsolutelyAnywhereRequest request = GetBookFromAbsolutelyAnywhereRequest.newBuilder()
    *     .setName(name.toString())
    *     .build();
@@ -2128,7 +2128,7 @@ public class LibraryServiceClient implements BackgroundResource {
    * Sample code:
    * 

    * try (LibraryServiceClient libraryServiceClient = LibraryServiceClient.create()) {
-   *   BookName name = DeletedBookName.of();
+   *   BookName name = BookName.of("[SHELF_ID]", "[BOOK_ID]");
    *   GetBookFromAbsolutelyAnywhereRequest request = GetBookFromAbsolutelyAnywhereRequest.newBuilder()
    *     .setName(name.toString())
    *     .build();
@@ -2864,7 +2864,7 @@ public class LibraryServiceClient implements BackgroundResource {
    *   ByteString optionalSingularBytes = ByteString.copyFromUtf8("");
    *   TestOptionalRequiredFlatteningParamsRequest.InnerMessage optionalSingularMessage = TestOptionalRequiredFlatteningParamsRequest.InnerMessage.newBuilder().build();
    *   BookName optionalSingularResourceName = BookName.of("[SHELF_ID]", "[BOOK_ID]");
-   *   BookName optionalSingularResourceNameOneof = DeletedBookName.of();
+   *   BookName optionalSingularResourceNameOneof = BookName.of("[SHELF_ID]", "[BOOK_ID]");
    *   ProjectName optionalSingularResourceNameCommon = ProjectName.of("[PROJECT]");
    *   int optionalSingularFixed32 = 0;
    *   long optionalSingularFixed64 = 0L;
@@ -3155,7 +3155,7 @@ public class LibraryServiceClient implements BackgroundResource {
    *   ByteString optionalSingularBytes = ByteString.copyFromUtf8("");
    *   TestOptionalRequiredFlatteningParamsRequest.InnerMessage optionalSingularMessage = TestOptionalRequiredFlatteningParamsRequest.InnerMessage.newBuilder().build();
    *   String formattedOptionalSingularResourceName = BookName.format("[SHELF_ID]", "[BOOK_ID]");
-   *   String formattedOptionalSingularResourceNameOneof = DeletedBookName.format();
+   *   String formattedOptionalSingularResourceNameOneof = BookName.format("[SHELF_ID]", "[BOOK_ID]");
    *   String formattedOptionalSingularResourceNameCommon = ProjectName.format("[PROJECT]");
    *   int optionalSingularFixed32 = 0;
    *   long optionalSingularFixed64 = 0L;
@@ -8278,6 +8278,9 @@ public class MyProtoStubSettings extends StubSettings {
 
     static {
       ImmutableMap.Builder> definitions = ImmutableMap.builder();
+      definitions.put(
+          "idempotent",
+          ImmutableSet.copyOf(Lists.newArrayList(StatusCode.Code.DEADLINE_EXCEEDED, StatusCode.Code.UNAVAILABLE)));
       definitions.put(
           "non_idempotent",
           ImmutableSet.copyOf(Lists.newArrayList()));
@@ -8809,7 +8812,7 @@ public class LibraryServiceClientTest {
   @Test
   @SuppressWarnings("all")
   public void createBookTest() {
-    BookName name2 = DeletedBookName.of();
+    BookName name2 = BookName.of("[SHELF_ID]", "[BOOK_ID]");
     String author = "author-1406328437";
     String title = "title110371416";
     boolean read = true;
@@ -8908,7 +8911,7 @@ public class LibraryServiceClientTest {
   @Test
   @SuppressWarnings("all")
   public void getBookTest() {
-    BookName name2 = DeletedBookName.of();
+    BookName name2 = BookName.of("[SHELF_ID]", "[BOOK_ID]");
     String author = "author-1406328437";
     String title = "title110371416";
     boolean read = true;
@@ -9043,7 +9046,7 @@ public class LibraryServiceClientTest {
   @Test
   @SuppressWarnings("all")
   public void updateBookTest() {
-    BookName name2 = DeletedBookName.of();
+    BookName name2 = BookName.of("[SHELF_ID]", "[BOOK_ID]");
     String author = "author-1406328437";
     String title = "title110371416";
     boolean read = true;
@@ -9094,7 +9097,7 @@ public class LibraryServiceClientTest {
   @Test
   @SuppressWarnings("all")
   public void updateBookTest2() {
-    BookName name2 = DeletedBookName.of();
+    BookName name2 = BookName.of("[SHELF_ID]", "[BOOK_ID]");
     String author = "author-1406328437";
     String title = "title110371416";
     boolean read = true;
@@ -9154,7 +9157,7 @@ public class LibraryServiceClientTest {
   @Test
   @SuppressWarnings("all")
   public void moveBookTest() {
-    BookName name2 = DeletedBookName.of();
+    BookName name2 = BookName.of("[SHELF_ID]", "[BOOK_ID]");
     String author = "author-1406328437";
     String title = "title110371416";
     boolean read = true;
@@ -9394,7 +9397,7 @@ public class LibraryServiceClientTest {
   @Test
   @SuppressWarnings("all")
   public void getBookFromAnywhereTest() {
-    BookName name2 = DeletedBookName.of();
+    BookName name2 = BookName.of("[SHELF_ID]", "[BOOK_ID]");
     String author = "author-1406328437";
     String title = "title110371416";
     boolean read = true;
@@ -9406,7 +9409,7 @@ public class LibraryServiceClientTest {
       .build();
     mockLibraryService.addResponse(expectedResponse);
 
-    BookName name = DeletedBookName.of();
+    BookName name = BookName.of("[SHELF_ID]", "[BOOK_ID]");
     BookName altBookName = BookName.of("[SHELF_ID]", "[BOOK_ID]");
 
     BookFromAnywhere actualResponse =
@@ -9432,7 +9435,7 @@ public class LibraryServiceClientTest {
     mockLibraryService.addException(exception);
 
     try {
-      BookName name = DeletedBookName.of();
+      BookName name = BookName.of("[SHELF_ID]", "[BOOK_ID]");
       BookName altBookName = BookName.of("[SHELF_ID]", "[BOOK_ID]");
 
       client.getBookFromAnywhere(name, altBookName);
@@ -9445,7 +9448,7 @@ public class LibraryServiceClientTest {
   @Test
   @SuppressWarnings("all")
   public void getBookFromAbsolutelyAnywhereTest() {
-    BookName name2 = DeletedBookName.of();
+    BookName name2 = BookName.of("[SHELF_ID]", "[BOOK_ID]");
     String author = "author-1406328437";
     String title = "title110371416";
     boolean read = true;
@@ -9457,7 +9460,7 @@ public class LibraryServiceClientTest {
       .build();
     mockLibraryService.addResponse(expectedResponse);
 
-    BookName name = DeletedBookName.of();
+    BookName name = BookName.of("[SHELF_ID]", "[BOOK_ID]");
 
     BookFromAnywhere actualResponse =
         client.getBookFromAbsolutelyAnywhere(name);
@@ -9481,7 +9484,7 @@ public class LibraryServiceClientTest {
     mockLibraryService.addException(exception);
 
     try {
-      BookName name = DeletedBookName.of();
+      BookName name = BookName.of("[SHELF_ID]", "[BOOK_ID]");
 
       client.getBookFromAbsolutelyAnywhere(name);
       Assert.fail("No exception raised");
@@ -9583,7 +9586,7 @@ public class LibraryServiceClientTest {
   @Test
   @SuppressWarnings("all")
   public void streamBooksTest() throws Exception {
-    BookName name2 = DeletedBookName.of();
+    BookName name2 = BookName.of("[SHELF_ID]", "[BOOK_ID]");
     String author = "author-1406328437";
     String title = "title110371416";
     boolean read = true;
@@ -9752,7 +9755,7 @@ public class LibraryServiceClientTest {
   @Test
   @SuppressWarnings("all")
   public void getBigBookTest() throws Exception {
-    BookName name2 = DeletedBookName.of();
+    BookName name2 = BookName.of("[SHELF_ID]", "[BOOK_ID]");
     String author = "author-1406328437";
     String title = "title110371416";
     boolean read = true;
@@ -9937,7 +9940,7 @@ public class LibraryServiceClientTest {
     ByteString optionalSingularBytes = ByteString.copyFromUtf8("2");
     TestOptionalRequiredFlatteningParamsRequest.InnerMessage optionalSingularMessage = TestOptionalRequiredFlatteningParamsRequest.InnerMessage.newBuilder().build();
     BookName optionalSingularResourceName = BookName.of("[SHELF_ID]", "[BOOK_ID]");
-    BookName optionalSingularResourceNameOneof = DeletedBookName.of();
+    BookName optionalSingularResourceNameOneof = BookName.of("[SHELF_ID]", "[BOOK_ID]");
     ProjectName optionalSingularResourceNameCommon = ProjectName.of("[PROJECT]");
     int optionalSingularFixed32 = 1648847958;
     long optionalSingularFixed64 = 1648847863;
@@ -10139,7 +10142,7 @@ public class LibraryServiceClientTest {
       ByteString optionalSingularBytes = ByteString.copyFromUtf8("2");
       TestOptionalRequiredFlatteningParamsRequest.InnerMessage optionalSingularMessage = TestOptionalRequiredFlatteningParamsRequest.InnerMessage.newBuilder().build();
       BookName optionalSingularResourceName = BookName.of("[SHELF_ID]", "[BOOK_ID]");
-      BookName optionalSingularResourceNameOneof = DeletedBookName.of();
+      BookName optionalSingularResourceNameOneof = BookName.of("[SHELF_ID]", "[BOOK_ID]");
       ProjectName optionalSingularResourceNameCommon = ProjectName.of("[PROJECT]");
       int optionalSingularFixed32 = 1648847958;
       long optionalSingularFixed64 = 1648847863;
diff --git a/src/test/java/com/google/api/codegen/protoannotations/testdata/ruby_library_no_gapic_config.baseline b/src/test/java/com/google/api/codegen/protoannotations/testdata/ruby_library_no_gapic_config.baseline
index e0c378300e..ed1f5768e7 100644
--- a/src/test/java/com/google/api/codegen/protoannotations/testdata/ruby_library_no_gapic_config.baseline
+++ b/src/test/java/com/google/api/codegen/protoannotations/testdata/ruby_library_no_gapic_config.baseline
@@ -2772,17 +2772,11 @@ module Library
         self::GRPC_INTERCEPTORS = LibraryServiceClient::GRPC_INTERCEPTORS
       end
 
-      SHELF_PATH_TEMPLATE = Google::Gax::PathTemplate.new(
-        "shelves/{shelf_id}"
-      )
-
-      private_constant :SHELF_PATH_TEMPLATE
-
-      DELETED_BOOK_PATH_TEMPLATE = Google::Gax::PathTemplate.new(
-        "_deleted-book_"
+      ARCHIVED_BOOK_PATH_TEMPLATE = Google::Gax::PathTemplate.new(
+        "archives/{archive_path}/books/{book_id=**}"
       )
 
-      private_constant :DELETED_BOOK_PATH_TEMPLATE
+      private_constant :ARCHIVED_BOOK_PATH_TEMPLATE
 
       BOOK_PATH_TEMPLATE = Google::Gax::PathTemplate.new(
         "shelves/{shelf_id}/books/{book_id}"
@@ -2790,32 +2784,26 @@ module Library
 
       private_constant :BOOK_PATH_TEMPLATE
 
-      ARCHIVED_BOOK_PATH_TEMPLATE = Google::Gax::PathTemplate.new(
-        "archives/{archive_path}/books/{book_id=**}"
-      )
-
-      private_constant :ARCHIVED_BOOK_PATH_TEMPLATE
-
       PROJECT_PATH_TEMPLATE = Google::Gax::PathTemplate.new(
         "projects/{project}"
       )
 
       private_constant :PROJECT_PATH_TEMPLATE
 
-      # Returns a fully-qualified shelf resource name string.
-      # @param shelf_id [String]
-      # @return [String]
-      def self.shelf_path shelf_id
-        SHELF_PATH_TEMPLATE.render(
-          :"shelf_id" => shelf_id
-        )
-      end
+      SHELF_PATH_TEMPLATE = Google::Gax::PathTemplate.new(
+        "shelves/{shelf_id}"
+      )
 
-      # Returns a fully-qualified deleted_book resource name string.
-      # @return [String]
-      def self.deleted_book_path
-        DELETED_BOOK_PATH_TEMPLATE.render(
+      private_constant :SHELF_PATH_TEMPLATE
 
+      # Returns a fully-qualified archived_book resource name string.
+      # @param archive_path [String]
+      # @param book_id [String]
+      # @return [String]
+      def self.archived_book_path archive_path, book_id
+        ARCHIVED_BOOK_PATH_TEMPLATE.render(
+          :"archive_path" => archive_path,
+          :"book_id" => book_id
         )
       end
 
@@ -2830,17 +2818,6 @@ module Library
         )
       end
 
-      # Returns a fully-qualified archived_book resource name string.
-      # @param archive_path [String]
-      # @param book_id [String]
-      # @return [String]
-      def self.archived_book_path archive_path, book_id
-        ARCHIVED_BOOK_PATH_TEMPLATE.render(
-          :"archive_path" => archive_path,
-          :"book_id" => book_id
-        )
-      end
-
       # Returns a fully-qualified project resource name string.
       # @param project [String]
       # @return [String]
@@ -2850,6 +2827,15 @@ module Library
         )
       end
 
+      # Returns a fully-qualified shelf resource name string.
+      # @param shelf_id [String]
+      # @return [String]
+      def self.shelf_path shelf_id
+        SHELF_PATH_TEMPLATE.render(
+          :"shelf_id" => shelf_id
+        )
+      end
+
       # @param credentials [Google::Auth::Credentials, String, Hash, GRPC::Core::Channel, GRPC::Core::ChannelCredentials, Proc]
       #   Provides the means for authenticating requests made by the client. This parameter can
       #   be many types.
@@ -3795,7 +3781,7 @@ module Library
       #   require "library"
       #
       #   library_client = Library::Library.new(version: :v1)
-      #   formatted_name = Library::V1::LibraryServiceClient.deleted_book_path()
+      #   formatted_name = Library::V1::LibraryServiceClient.book_path("[SHELF_ID]", "[BOOK_ID]")
       #   formatted_alt_book_name = Library::V1::LibraryServiceClient.book_path("[SHELF_ID]", "[BOOK_ID]")
       #   response = library_client.get_book_from_anywhere(formatted_name, formatted_alt_book_name)
 
@@ -3831,7 +3817,7 @@ module Library
       #   require "library"
       #
       #   library_client = Library::Library.new(version: :v1)
-      #   formatted_name = Library::V1::LibraryServiceClient.deleted_book_path()
+      #   formatted_name = Library::V1::LibraryServiceClient.book_path("[SHELF_ID]", "[BOOK_ID]")
       #   response = library_client.get_book_from_absolutely_anywhere(formatted_name)
 
       def get_book_from_absolutely_anywhere \
@@ -4981,6 +4967,25 @@ module Library
       ].freeze
 
 
+      RETURN_PATH_TEMPLATE = Google::Gax::PathTemplate.new(
+        "shelves/{shelf}/books/{book}/returns/{return}"
+      )
+
+      private_constant :RETURN_PATH_TEMPLATE
+
+      # Returns a fully-qualified return resource name string.
+      # @param shelf [String]
+      # @param book [String]
+      # @param return_ [String]
+      # @return [String]
+      def self.return_path shelf, book, return_
+        RETURN_PATH_TEMPLATE.render(
+          :"shelf" => shelf,
+          :"book" => book,
+          :"return" => return_
+        )
+      end
+
       # @param credentials [Google::Auth::Credentials, String, Hash, GRPC::Core::Channel, GRPC::Core::ChannelCredentials, Proc]
       #   Provides the means for authenticating requests made by the client. This parameter can
       #   be many types.
@@ -5138,6 +5143,10 @@ end
   "interfaces": {
     "google.example.library.v1.MyProto": {
       "retry_codes": {
+        "idempotent": [
+          "DEADLINE_EXCEEDED",
+          "UNAVAILABLE"
+        ],
         "non_idempotent": []
       },
       "retry_params": {
@@ -6455,7 +6464,7 @@ describe Library::V1::LibraryServiceClient do
 
     it 'invokes get_book_from_anywhere without error' do
       # Create request parameters
-      formatted_name = Library::V1::LibraryServiceClient.deleted_book_path()
+      formatted_name = Library::V1::LibraryServiceClient.book_path("[SHELF_ID]", "[BOOK_ID]")
       formatted_alt_book_name = Library::V1::LibraryServiceClient.book_path("[SHELF_ID]", "[BOOK_ID]")
 
       # Create expected grpc response
@@ -6505,7 +6514,7 @@ describe Library::V1::LibraryServiceClient do
 
     it 'invokes get_book_from_anywhere with error' do
       # Create request parameters
-      formatted_name = Library::V1::LibraryServiceClient.deleted_book_path()
+      formatted_name = Library::V1::LibraryServiceClient.book_path("[SHELF_ID]", "[BOOK_ID]")
       formatted_alt_book_name = Library::V1::LibraryServiceClient.book_path("[SHELF_ID]", "[BOOK_ID]")
 
       # Mock Grpc layer
@@ -6541,7 +6550,7 @@ describe Library::V1::LibraryServiceClient do
 
     it 'invokes get_book_from_absolutely_anywhere without error' do
       # Create request parameters
-      formatted_name = Library::V1::LibraryServiceClient.deleted_book_path()
+      formatted_name = Library::V1::LibraryServiceClient.book_path("[SHELF_ID]", "[BOOK_ID]")
 
       # Create expected grpc response
       name_2 = "name2-1052831874"
@@ -6589,7 +6598,7 @@ describe Library::V1::LibraryServiceClient do
 
     it 'invokes get_book_from_absolutely_anywhere with error' do
       # Create request parameters
-      formatted_name = Library::V1::LibraryServiceClient.deleted_book_path()
+      formatted_name = Library::V1::LibraryServiceClient.book_path("[SHELF_ID]", "[BOOK_ID]")
 
       # Mock Grpc layer
       mock_method = proc do |request|
diff --git a/src/test/java/com/google/api/codegen/transformer/RetryDefinitionsTransformerTest.java b/src/test/java/com/google/api/codegen/transformer/RetryDefinitionsTransformerTest.java
index 77b711f94d..162328b2d6 100644
--- a/src/test/java/com/google/api/codegen/transformer/RetryDefinitionsTransformerTest.java
+++ b/src/test/java/com/google/api/codegen/transformer/RetryDefinitionsTransformerTest.java
@@ -99,8 +99,7 @@ public void testWithConfigAndInterface() {
     DiagCollector diagCollector = new BoundedDiagCollector();
 
     RetryCodesConfig retryCodesConfig =
-        RetryCodesConfig.create(
-            diagCollector, interfaceConfigProto, apiInterface.getMethods(), protoParser);
+        RetryCodesConfig.create(interfaceConfigProto, apiInterface.getMethods(), protoParser);
 
     Map> retryCodesDef = retryCodesConfig.getRetryCodesDefinition();
     Map retryCodesMap = retryCodesConfig.getMethodRetryNames();
@@ -154,11 +153,8 @@ public void testWithInterfaceOnly() {
             .addMethods(MethodConfigProto.newBuilder().setName(PERMISSION_DENIED_METHOD_NAME))
             .build();
 
-    DiagCollector diagCollector = new BoundedDiagCollector();
-
     RetryCodesConfig retryCodesConfig =
-        RetryCodesConfig.create(
-            diagCollector, bareBonesConfigProto, apiInterface.getMethods(), protoParser);
+        RetryCodesConfig.create(bareBonesConfigProto, apiInterface.getMethods(), protoParser);
 
     Map> retryCodesDef = retryCodesConfig.getRetryCodesDefinition();
     Map retryCodesMap = retryCodesConfig.getMethodRetryNames();
diff --git a/src/test/java/com/google/api/codegen/util/ConfigValidatorTest.java b/src/test/java/com/google/api/codegen/util/ConfigValidatorTest.java
new file mode 100644
index 0000000000..4632449b8a
--- /dev/null
+++ b/src/test/java/com/google/api/codegen/util/ConfigValidatorTest.java
@@ -0,0 +1,66 @@
+/* Copyright 2019 Google LLC
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package com.google.api.codegen.util;
+
+import com.google.api.codegen.ConfigProto;
+import com.google.api.codegen.FlatteningConfigProto;
+import com.google.api.codegen.FlatteningGroupProto;
+import com.google.api.codegen.InterfaceConfigProto;
+import com.google.api.codegen.MethodConfigProto;
+import com.google.protobuf.BoolValue;
+import org.junit.Test;
+
+public class ConfigValidatorTest {
+  private static final ConfigProto validV2Proto =
+      ConfigProto.newBuilder()
+          .setConfigSchemaVersion("2.0.0")
+          // This is a valid field for both Config v1 and Config v2.
+          .setEnableStringFormatFunctionsOverride(BoolValue.of(true))
+          .build();
+
+  private static final ConfigVersionValidator validator = new ConfigVersionValidator();
+
+  @Test(expected = IllegalStateException.class)
+  public void testProtoIsNotConfigNextVersion() {
+    ConfigProto smallProto = validV2Proto.toBuilder().setConfigSchemaVersion("1.0.0").build();
+    validator.validateV2Config(smallProto);
+  }
+
+  @Test(expected = IllegalStateException.class)
+  public void testProtoIsNotValid() {
+    ConfigProto smallProto =
+        validV2Proto
+            .toBuilder()
+            .addInterfaces(
+                InterfaceConfigProto.newBuilder()
+                    .addMethods(
+                        MethodConfigProto.newBuilder()
+                            .setFlattening(
+                                FlatteningConfigProto.newBuilder()
+                                    .addGroups(
+                                        FlatteningGroupProto.newBuilder()
+                                            .addParameters("flattenedParam")
+                                            .build())
+                                    .build()))
+                    .build())
+            .build();
+    validator.validateV2Config(smallProto);
+  }
+
+  @Test
+  public void testProtoIsValid() {
+    validator.validateV2Config(validV2Proto);
+  }
+}