[ggj][codegen] fix: handle singleton resname patterns, name_field, ad…

…d logging test (#250) * feat: add factory var decl in ServiceStubSettings codegen * fix: prevent duplicate MethodDefinition annotations * feat: add descriptor fields to ServiceStubSettings codegen * feat: add starter Builder to ServiceStubSettings codegen * feat: add settings.builder decls to ServiceStubSettings codegen * feat: add first nested ctors to ServiceStubSettings codegen * feat: add GapicServiceConfig DS and processing * feat: integrate GapicServiceConfig into GapicContext, Parser, Composer * feat: initial param block, RetrySettingsComposer, test * fix!: refactor GapicRetrySettings * fix: recognize 1. or .1 double patterns * feat: support BlockStatement in ClassDef stmts * feat: add params block to ServiceStubSettings codegen * feat: add codes def to ServiceStubSettings codegen * feat: add initDefaults() to ServiceStubSettings codegen * feat: add LRO to ServiceStubSettings.Builder.initDefaults * feat: add third ServiceStubSettings.Builder(settings) ctor * feat: add createDefault() to ServiceStubSettings * feat: add ServiceStubSettings.applyToAllUnaryMethods method * feat: add ServiceStubSettings.unaryMethodSettingsBuilders() * feat: add ServiceStubSettings.build() * feat: add settingsBuilder getters in ServiceStubSettings * feat: add gapic.yaml batching parsing * feat: integrate batching with retry settings parsing * fix: remove unused test proto imports * fix: handle singleton resname patterns, name_field, add logging test * fix: pass in logging grpc service config
googleapis · Aug 29, 2020 · 5709ffd · 5709ffd
1 parent 6837708
commit 5709ffd
Show file tree

Hide file tree

Showing 7 changed files with 869 additions and 43 deletions.
diff --git a/src/main/java/com/google/api/generator/gapic/protoparser/BatchingSettingsConfigParser.java b/src/main/java/com/google/api/generator/gapic/protoparser/BatchingSettingsConfigParser.java
@@ -15,6 +15,7 @@
 package com.google.api.generator.gapic.protoparser;
 
 import com.google.api.generator.gapic.model.GapicBatchingSettings;
+import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Preconditions;
 import com.google.common.base.Strings;
 import java.io.File;
@@ -52,6 +53,7 @@ public static Optional<List<GapicBatchingSettings>> parse(
         : Optional.empty();
   }
 
+  @VisibleForTesting
   static Optional<List<GapicBatchingSettings>> parse(String gapicYamlConfigFilePath) {
     if (Strings.isNullOrEmpty(gapicYamlConfigFilePath)
         || !(new File(gapicYamlConfigFilePath)).exists()) {

diff --git a/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java b/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java
@@ -346,10 +346,16 @@ private static Field parseField(FieldDescriptor fieldDescriptor, Descriptor mess
           isChildType
               ? ResourceReference.withChildType(childTypeString)
               : ResourceReference.withType(typeString);
-    } else if (fieldDescriptor.getName().equals(ResourceNameConstants.NAME_FIELD_NAME)
-        && messageOptions.hasExtension(ResourceProto.resource)) {
+    } else if (messageOptions.hasExtension(ResourceProto.resource)) {
       ResourceDescriptor protoResource = messageOptions.getExtension(ResourceProto.resource);
-      resourceReference = ResourceReference.withType(protoResource.getType());
+      // aip.dev/4231.
+      String resourceFieldNameValue = ResourceNameConstants.NAME_FIELD_NAME;
+      if (!Strings.isNullOrEmpty(protoResource.getNameField())) {
+        resourceFieldNameValue = protoResource.getNameField();
+      }
+      if (fieldDescriptor.getName().equals(resourceFieldNameValue)) {
+        resourceReference = ResourceReference.withType(protoResource.getType());
+      }
     }
 
     return Field.builder()

diff --git a/src/main/java/com/google/api/generator/gapic/protoparser/ResourceNameParser.java b/src/main/java/com/google/api/generator/gapic/protoparser/ResourceNameParser.java
@@ -87,17 +87,17 @@ static Optional<ResourceName> parseResourceNameFromMessageType(
       return Optional.empty();
     }
 
+    ResourceDescriptor protoResource = messageOptions.getExtension(ResourceProto.resource);
     // aip.dev/4231.
-    Preconditions.checkNotNull(
-        messageTypeDescriptor.findFieldByName(ResourceNameConstants.NAME_FIELD_NAME),
-        String.format(
-            "Message %s has a resource annotation but no \"name\" field",
-            messageTypeDescriptor.getName()));
+    if (Strings.isNullOrEmpty(protoResource.getNameField())) {
+      Preconditions.checkNotNull(
+          messageTypeDescriptor.findFieldByName(ResourceNameConstants.NAME_FIELD_NAME),
+          String.format(
+              "Message %s has a resource annotation but no \"name\" field",
+              messageTypeDescriptor.getName()));
+    }
 
-    return createResourceName(
-        messageOptions.getExtension(ResourceProto.resource),
-        pakkage,
-        messageTypeDescriptor.getName());
+    return createResourceName(protoResource, pakkage, messageTypeDescriptor.getName());
   }
 
   private static Optional<ResourceName> createResourceName(
@@ -156,15 +156,16 @@ static Optional<String> getVariableNameFromPattern(String pattern) {
     } else if (lastToken.equals(ResourceNameConstants.WILDCARD_PATTERN)) {
       resourceVariableName = null;
     } else {
-      Preconditions.checkState(
-          lastToken.contains("{"),
-          String.format(
-              "Pattern %s must end with a brace-encapsulated variable, e.g. {foobar}", pattern));
-      Set<String> variableNames = PathTemplate.create(pattern).vars();
-      for (String variableName : variableNames) {
-        if (lastToken.contains(variableName)) {
-          resourceVariableName = variableName;
-          break;
+      // Allow singleton patterns like projects/{project}/cmekSettings.
+      if (!lastToken.contains("{")) {
+        resourceVariableName = lastToken;
+      } else {
+        Set<String> variableNames = PathTemplate.create(pattern).vars();
+        for (String variableName : variableNames) {
+          if (lastToken.contains(variableName)) {
+            resourceVariableName = variableName;
+            break;
+          }
         }
       }
     }

diff --git a/src/test/java/com/google/api/generator/gapic/composer/BUILD.bazel b/src/test/java/com/google/api/generator/gapic/composer/BUILD.bazel
@@ -20,14 +20,23 @@ filegroup(
     srcs = ["{0}.java".format(f) for f in TESTS],
 )
 
+java_proto_library(
+    name = "logging_java_proto",
+    deps = [
+        "@com_google_googleapis//google/logging/v2:logging_proto",
+    ],
+)
+
 [java_test(
     name = test_name,
     srcs = ["{0}.java".format(test_name)],
     data = [
+        "//src/test/java/com/google/api/generator/gapic/testdata:gapic_config_files",
         "//src/test/java/com/google/api/generator/gapic/testdata:service_config_files",
     ],
     test_class = "com.google.api.generator.gapic.composer.{0}".format(test_name),
     deps = [
+        ":logging_java_proto",
         "//:rpc_java_proto",
         "//:service_config_java_proto",
         "//src/main/java/com/google/api/generator/engine/ast",