Less magic pathtemplate strings (#2402)

Partially addresses googleapis/google-cloud-java#3604. Don't force pathtemplate strings to start with a literal string. The status quo, for path templates, is to magically append a literal string to templated paths that begin with a bracketed string, e.g. the path {project}/zones/{zone} is rendered as projects/{project}/zones/{zone}. I don't remember exactly why we decided to do this, though it may have been related to purity of usage of PathTemplates, where all existing Google grpc path templates start with a projects (or possibly organizations) substring. More importantly, this PR allows us to parse a resource string returned from the server (see issue googleapis/google-cloud-java#3604), which often looks like https://www.googleapis.com/compute/v1/projects/{project}/zones/{zone}. The compute.v1.json Discovery API specifies that the baseUrl is exactly https://www.googleapis.com/compute/v1/projects. We can then take a resource string from the server, remove the given baseUrl prefix, and then parse the remaining suffix {project}/zones/{zone} into a ProjectsZoneName. See googleapis/google-cloud-java#3899 and googleapis/discovery-artifact-manager#95 for the resulting surface diffs from this generator change.
googleapis · Dec 11, 2018 · d78306f · d78306f
1 parent 03f61fb
commit d78306f
Show file tree

Hide file tree

Showing 6 changed files with 45 additions and 47 deletions.
diff --git a/src/main/java/com/google/api/codegen/discogapic/transformer/DiscoGapicParser.java b/src/main/java/com/google/api/codegen/discogapic/transformer/DiscoGapicParser.java
@@ -83,16 +83,11 @@ public static Name getRequestName(Method method) {
   }
 
   /**
-   * Get the canonical path for a path, in the form "(%s/\{%s\})+" e.g. for a method path
-   * "{project}/regions/{foo}/addresses", this returns "projects/{project}/regions/{region}".
+   * Get the canonical path for a path, in the form "((\{%s\}|%s)/)+{%s\}" e.g. for a method path
+   * "{project}/regions/{foo}/addresses", this returns "{project}/regions/{region}". The resulting
+   * path will end with a {BINDING}.
    */
   public static String getCanonicalPath(String namePattern) {
-    // Ensure the first path segment is a string literal representing a resource type.
-    if (namePattern.charAt(0) == '{') {
-      String firstResource =
-          Inflector.pluralize(namePattern.substring(1, namePattern.indexOf("}")));
-      namePattern = String.format("%s/%s", firstResource, namePattern);
-    }
     // Remove any trailing non-bracketed substring
     if (!namePattern.endsWith("}") && namePattern.contains("}")) {
       namePattern = namePattern.substring(0, namePattern.lastIndexOf('}') + 1);

diff --git a/src/main/resources/com/google/api/codegen/configgen/gapic_config.snip b/src/main/resources/com/google/api/codegen/configgen/gapic_config.snip
@@ -61,7 +61,7 @@
       @if interface.collections
         collections:
         @join collection : interface.collections
-          - name_pattern: {@collection.namePattern}
+          - name_pattern: '{@collection.namePattern}'
             entity_name: {@collection.entityName}
         @end
       @else

diff --git a/.../java/com/google/api/codegen/discogapic/testdata/java/java_simplecompute.v1.json.baseline b/.../java/com/google/api/codegen/discogapic/testdata/java/java_simplecompute.v1.json.baseline
@@ -37,7 +37,7 @@ public final class ProjectDummyObjectName implements ResourceName {
   private final String dummyObject;
   private final String project;
   private static final PathTemplate PATH_TEMPLATE =
-        PathTemplate.createWithoutUrlEncoding("projects/{project}/dummyObjects/{dummyObject}");
+        PathTemplate.createWithoutUrlEncoding("{project}/dummyObjects/{dummyObject}");
 
   private volatile Map<String, String> fieldValuesMap;
 
@@ -226,7 +226,7 @@ public final class ProjectDummyObjectResourceName implements ResourceName {
   private final String project;
   private final String resource;
   private static final PathTemplate PATH_TEMPLATE =
-        PathTemplate.createWithoutUrlEncoding("projects/{project}/dummyObjects/{resource}");
+        PathTemplate.createWithoutUrlEncoding("{project}/dummyObjects/{resource}");
 
   private volatile Map<String, String> fieldValuesMap;
 
@@ -415,7 +415,7 @@ public final class ProjectGlobalAddressName implements ResourceName {
   private final String address;
   private final String project;
   private static final PathTemplate PATH_TEMPLATE =
-        PathTemplate.createWithoutUrlEncoding("projects/{project}/global/addresses/{address}");
+        PathTemplate.createWithoutUrlEncoding("{project}/global/addresses/{address}");
 
   private volatile Map<String, String> fieldValuesMap;
 
@@ -603,7 +603,7 @@ import javax.annotation.Generated;
 public final class ProjectName implements ResourceName {
   private final String project;
   private static final PathTemplate PATH_TEMPLATE =
-        PathTemplate.createWithoutUrlEncoding("projects/{project}");
+        PathTemplate.createWithoutUrlEncoding("{project}");
 
   private volatile Map<String, String> fieldValuesMap;
 
@@ -770,7 +770,7 @@ public final class ProjectRegionAddressName implements ResourceName {
   private final String project;
   private final String region;
   private static final PathTemplate PATH_TEMPLATE =
-        PathTemplate.createWithoutUrlEncoding("projects/{project}/regions/{region}/addresses/{address}");
+        PathTemplate.createWithoutUrlEncoding("{project}/regions/{region}/addresses/{address}");
 
   private volatile Map<String, String> fieldValuesMap;
 
@@ -982,7 +982,7 @@ public final class ProjectRegionName implements ResourceName {
   private final String project;
   private final String region;
   private static final PathTemplate PATH_TEMPLATE =
-        PathTemplate.createWithoutUrlEncoding("projects/{project}/regions/{region}");
+        PathTemplate.createWithoutUrlEncoding("{project}/regions/{region}");
 
   private volatile Map<String, String> fieldValuesMap;
 

diff --git a/src/test/java/com/google/api/codegen/discogapic/testdata/simplecompute_config.baseline b/src/test/java/com/google/api/codegen/discogapic/testdata/simplecompute_config.baseline
@@ -42,13 +42,13 @@ interfaces:
   # The entity_name is the name to be used as a basis for generated methods and
   # classes.
   collections:
-  - name_pattern: projects/{project}
+  - name_pattern: '{project}'
     entity_name: project
-  - name_pattern: projects/{project}/global/addresses/{address}
+  - name_pattern: '{project}/global/addresses/{address}'
     entity_name: projectGlobalAddress
-  - name_pattern: projects/{project}/regions/{region}
+  - name_pattern: '{project}/regions/{region}'
     entity_name: projectRegion
-  - name_pattern: projects/{project}/regions/{region}/addresses/{address}
+  - name_pattern: '{project}/regions/{region}/addresses/{address}'
     entity_name: projectRegionAddress
   # Definition for retryable codes.
   retry_codes_def:
@@ -284,11 +284,11 @@ interfaces:
   # The entity_name is the name to be used as a basis for generated methods and
   # classes.
   collections:
-  - name_pattern: projects/{project}
+  - name_pattern: '{project}'
     entity_name: project
-  - name_pattern: projects/{project}/dummyObjects/{dummyObject}
+  - name_pattern: '{project}/dummyObjects/{dummyObject}'
     entity_name: projectDummyObject
-  - name_pattern: projects/{project}/dummyObjects/{resource}
+  - name_pattern: '{project}/dummyObjects/{resource}'
     entity_name: projectDummyObjectResource
   # Definition for retryable codes.
   retry_codes_def:

diff --git a/src/test/java/com/google/api/codegen/discogapic/testdata/simplecompute_gapic.yaml b/src/test/java/com/google/api/codegen/discogapic/testdata/simplecompute_gapic.yaml
@@ -37,13 +37,13 @@ interfaces:
   # The entity_name is the name to be used as a basis for generated methods and
   # classes.
   collections:
-  - name_pattern: projects/{project}
+  - name_pattern: '{project}'
     entity_name: project
-  - name_pattern: projects/{project}/global/addresses/{address}
+  - name_pattern: '{project}/global/addresses/{address}'
     entity_name: projectGlobalAddress
-  - name_pattern: projects/{project}/regions/{region}
+  - name_pattern: '{project}/regions/{region}'
     entity_name: projectRegion
-  - name_pattern: projects/{project}/regions/{region}/addresses/{address}
+  - name_pattern: '{project}/regions/{region}/addresses/{address}'
     entity_name: projectRegionAddress
   # Definition for retryable codes.
   retry_codes_def:
@@ -76,9 +76,6 @@ interfaces:
   #       message.
   #   required_fields - Fields that are always required for a request to be
   #       valid.
-  #   request_object_method - Turns on or off the generation of a method whose
-  #       sole parameter is a request object. Not all languages will generate
-  #       this method.
   #   resource_name_treatment - An enum that specifies how to treat the
   #       resource name formats defined in the field_name_patterns
   #       and response_field_name_patterns fields.
@@ -121,7 +118,6 @@ interfaces:
         - project
     required_fields:
     - project
-    request_object_method: true
     resource_name_treatment: STATIC_TYPES
     page_streaming:
       request:
@@ -142,7 +138,6 @@ interfaces:
         - address
     required_fields:
     - address
-    request_object_method: true
     resource_name_treatment: STATIC_TYPES
     retry_codes_name: idempotent
     retry_params_name: default
@@ -156,7 +151,6 @@ interfaces:
         - address
     required_fields:
     - address
-    request_object_method: true
     resource_name_treatment: STATIC_TYPES
     retry_codes_name: idempotent
     retry_params_name: default
@@ -172,7 +166,6 @@ interfaces:
     required_fields:
     - region
     - addressResource
-    request_object_method: true
     resource_name_treatment: STATIC_TYPES
     retry_codes_name: non_idempotent
     retry_params_name: default
@@ -186,7 +179,6 @@ interfaces:
         - region
     required_fields:
     - region
-    request_object_method: true
     resource_name_treatment: STATIC_TYPES
     page_streaming:
       request:
@@ -211,7 +203,6 @@ interfaces:
     - address
     - region
     - addressResource
-    request_object_method: true
     resource_name_treatment: STATIC_TYPES
     retry_codes_name: non_idempotent
     retry_params_name: default
@@ -227,7 +218,6 @@ interfaces:
     required_fields:
     - address
     - addressResource
-    request_object_method: true
     resource_name_treatment: STATIC_TYPES
     retry_codes_name: idempotent
     retry_params_name: default
@@ -236,11 +226,11 @@ interfaces:
     timeout_millis: 60000
 - name: google.simplecompute.v1.DummyObjects
   collections:
-  - name_pattern: projects/{project}
+  - name_pattern: '{project}'
     entity_name: project
-  - name_pattern: projects/{project}/dummyObjects/{dummyObject}
+  - name_pattern: '{project}/dummyObjects/{dummyObject}'
     entity_name: projectDummyObject
-  - name_pattern: projects/{project}/dummyObjects/{resource}
+  - name_pattern: '{project}/dummyObjects/{resource}'
     entity_name: projectDummyObjectResource
   retry_codes_def:
   - name: idempotent
@@ -266,7 +256,6 @@ interfaces:
         - project
     required_fields:
     - project
-    request_object_method: true
     resource_name_treatment: STATIC_TYPES
     page_streaming:
       request:
@@ -288,7 +277,6 @@ interfaces:
         - resource
     required_fields:
     - resource
-    request_object_method: true
     resource_name_treatment: STATIC_TYPES
     retry_codes_name: idempotent
     retry_params_name: default
@@ -302,7 +290,6 @@ interfaces:
         - dummyObject
     required_fields:
     - dummyObject
-    request_object_method: true
     resource_name_treatment: STATIC_TYPES
     retry_codes_name: idempotent
     retry_params_name: default

diff --git a/src/test/java/com/google/api/codegen/discogapic/transformer/DiscoGapicParserTest.java b/src/test/java/com/google/api/codegen/discogapic/transformer/DiscoGapicParserTest.java
@@ -16,23 +16,39 @@
 
 import static com.google.common.truth.Truth.assertThat;
 
+import com.google.api.pathtemplate.PathTemplate;
+import java.util.HashMap;
+import java.util.Map;
 import org.junit.Test;
 
 public class DiscoGapicParserTest {
 
   @Test
   public void testCanonicalPath() {
     // The test inputs to the getCanonicalPath() are examples from the Compute Discovery Doc API.
-
     assertThat(DiscoGapicParser.getCanonicalPath("{project}/backendBuckets/{backendBucket}"))
-        .isEqualTo("projects/{project}/backendBuckets/{backendBucket}");
+        .isEqualTo("{project}/backendBuckets/{backendBucket}");
     assertThat(DiscoGapicParser.getCanonicalPath("{project}/global/backendBuckets/{backendBucket}"))
-        .isEqualTo("projects/{project}/global/backendBuckets/{backendBucket}");
+        .isEqualTo("{project}/global/backendBuckets/{backendBucket}");
     assertThat(
             DiscoGapicParser.getCanonicalPath("{project}/zones/{zone}/disks/{resource}/setLabels"))
-        .isEqualTo("projects/{project}/zones/{zone}/disks/{resource}");
+        .isEqualTo("{project}/zones/{zone}/disks/{resource}");
     assertThat(DiscoGapicParser.getCanonicalPath("{project}/global/images/{resource}/setLabels"))
-        .isEqualTo("projects/{project}/global/images/{resource}");
+        .isEqualTo("{project}/global/images/{resource}");
+  }
+
+  @Test
+  public void testCanonicalPathToTemplate() {
+    // Test that the output of getCanonicalPath() can be instantiated as a working PathTemplate.
+    String rawPath = "{project}/zones/{zone}/disks/{resource}/setLabels";
+    String canonicalPath = DiscoGapicParser.getCanonicalPath(rawPath);
+    PathTemplate pathTemplate = PathTemplate.create(canonicalPath);
+    Map<String, String> keysAndValues = new HashMap<>();
+    keysAndValues.put("project", "ojectpray");
+    keysAndValues.put("zone", "onezay");
+    keysAndValues.put("resource", "esourceray");
+    assertThat(pathTemplate.instantiate(keysAndValues))
+        .isEqualTo("ojectpray/zones/onezay/disks/esourceray");
   }
 
   @Test