fix: resource name class deduplication (#1854)

diegomarquezp · web-flow · commit 08eca7de7e80 · 2023-07-18T11:34:43.000-04:00
* chore: add resource name golden from collisions.proto

* chore: improve deduplication logic

* chore: finish test case

* chore: unnecessary proto definitions

* chore: format

* chore: remove commented line in collisions.proto

* chore: simplify deduplication logic

* chore: revert changes in TypeStore.java

* chore: improve javadoc of `createImplementsTypes`
diff --git a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/composer/resourcename/ResourceNameHelperClassComposer.java b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/composer/resourcename/ResourceNameHelperClassComposer.java
@@ -131,7 +131,7 @@ public GapicClass generate(ResourceName resourceName, GapicContext context) {
             .setAnnotations(createClassAnnotations())
             .setScope(ScopeNode.PUBLIC)
             .setName(className)
-            .setImplementsTypes(createImplementsTypes())
+            .setImplementsTypes(createImplementsTypes(className))
             .setStatements(
                 createClassStatements(
                     templateFinalVarExprs,
@@ -160,8 +160,27 @@ private static List<AnnotationNode> createClassAnnotations() {
             .build());
   }
 
-  private static List<TypeNode> createImplementsTypes() {
-    return Arrays.asList(FIXED_TYPESTORE.get("ResourceName"));
+  /**
+   * Returns a singleton list with {@code ResourceName} as its only member. Checks for collisions
+   *
+   * @param implementingClassName class that is implementing the resulting list
+   */
+  private static List<TypeNode> createImplementsTypes(String implementingClassName) {
+    // the original resource name reference has useFullName == false
+    TypeNode originalResourceName = FIXED_TYPESTORE.get("ResourceName");
+    if (implementingClassName.equals(originalResourceName.reference().name())) {
+      // we create a copy with useFullName == true
+      return Arrays.asList(
+          TypeNode.withReference(
+              ConcreteReference.builder()
+                  .setUseFullName(true)
+                  .setClazz(com.google.api.resourcenames.ResourceName.class)
+                  .setGenerics(originalResourceName.reference().generics())
+                  .setIsStaticImport(originalResourceName.reference().isStaticImport())
+                  .setWildcardUpperBound(originalResourceName.reference().wildcardUpperBound())
+                  .build()));
+    }
+    return Arrays.asList(originalResourceName);
   }
 
   private static List<VariableExpr> createTemplateClassMembers(
diff --git a/gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/resourcename/ResourceNameHelperClassComposerTest.java b/gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/resourcename/ResourceNameHelperClassComposerTest.java
@@ -16,6 +16,7 @@
 
 import static com.google.common.truth.Truth.assertThat;
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
 
 import com.google.api.generator.engine.writer.JavaWriterVisitor;
 import com.google.api.generator.gapic.model.GapicClass;
@@ -34,6 +35,7 @@
 import com.google.protobuf.Descriptors.ServiceDescriptor;
 import com.google.showcase.v1beta1.EchoOuterClass;
 import com.google.showcase.v1beta1.TestingOuterClass;
+import com.google.test.collisions.CollisionsOuterClass;
 import google.cloud.CommonResources;
 import java.nio.file.Path;
 import java.nio.file.Paths;
@@ -50,6 +52,9 @@
 import org.junit.Test;
 
 public class ResourceNameHelperClassComposerTest {
+
+  private final String COLLIDING_RESOURCE_NAME_KEY = "config.googleapis.com/Resource";
+
   private ServiceDescriptor echoService;
   private FileDescriptor echoFileDescriptor;
 
@@ -238,4 +243,29 @@ public void generateResourceNameClass_childSingleton() {
         Paths.get(GoldenFileWriter.getGoldenDir(this.getClass()), "AgentName.golden");
     Assert.assertCodeEquals(goldenFilePath, visitor.write());
   }
+
+  @Test
+  public void generateResourceNameClass_resourceNameCollisionIsAvoided() {
+    ResourceName collidingResourceName =
+        Parser.parseResourceNames(CollisionsOuterClass.getDescriptor())
+            .get(COLLIDING_RESOURCE_NAME_KEY);
+
+    GapicContext irrelevantContext = TestProtoLoader.instance().parseShowcaseEcho();
+    GapicClass clazz =
+        ResourceNameHelperClassComposer.instance()
+            .generate(collidingResourceName, irrelevantContext);
+    JavaWriterVisitor visitor = new JavaWriterVisitor();
+    clazz.classDefinition().accept(visitor);
+    GoldenFileWriter.saveCodegenToFile(
+        this.getClass(), "CollisionResourceName.golden", visitor.write());
+    Path goldenFilePath =
+        Paths.get(GoldenFileWriter.getGoldenDir(this.getClass()), "CollisionResourceName.golden");
+    Assert.assertCodeEquals(goldenFilePath, visitor.write());
+
+    assertEquals(1, clazz.classDefinition().implementsTypes().size());
+    assertTrue(clazz.classDefinition().implementsTypes().get(0).reference().useFullName());
+    assertEquals(
+        clazz.classDefinition().classIdentifier().name(),
+        clazz.classDefinition().implementsTypes().get(0).reference().name());
+  }
 }
diff --git a/gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/resourcename/goldens/CollisionResourceName.golden b/gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/resourcename/goldens/CollisionResourceName.golden
@@ -0,0 +1,281 @@
+package com.google.test.collisions;
+
+import com.google.api.pathtemplate.PathTemplate;
+import com.google.common.base.Preconditions;
+import com.google.common.collect.ImmutableMap;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+import javax.annotation.Generated;
+
+// AUTO-GENERATED DOCUMENTATION AND CLASS.
+@Generated("by gapic-generator-java")
+public class ResourceName implements com.google.api.resourcenames.ResourceName {
+  private static final PathTemplate PROJECT_LOCATION_DEPLOYMENT_REVISION_RESOURCE =
+      PathTemplate.createWithoutUrlEncoding(
+          "projects/{project}/locations/{location}/deployments/{deployment}/revisions/{revision}/resources/{resource}");
+  private volatile Map<String, String> fieldValuesMap;
+  private final String project;
+  private final String location;
+  private final String deployment;
+  private final String revision;
+  private final String resource;
+
+  @Deprecated
+  protected ResourceName() {
+    project = null;
+    location = null;
+    deployment = null;
+    revision = null;
+    resource = null;
+  }
+
+  private ResourceName(Builder builder) {
+    project = Preconditions.checkNotNull(builder.getProject());
+    location = Preconditions.checkNotNull(builder.getLocation());
+    deployment = Preconditions.checkNotNull(builder.getDeployment());
+    revision = Preconditions.checkNotNull(builder.getRevision());
+    resource = Preconditions.checkNotNull(builder.getResource());
+  }
+
+  public String getProject() {
+    return project;
+  }
+
+  public String getLocation() {
+    return location;
+  }
+
+  public String getDeployment() {
+    return deployment;
+  }
+
+  public String getRevision() {
+    return revision;
+  }
+
+  public String getResource() {
+    return resource;
+  }
+
+  public static Builder newBuilder() {
+    return new Builder();
+  }
+
+  public Builder toBuilder() {
+    return new Builder(this);
+  }
+
+  public static ResourceName of(
+      String project, String location, String deployment, String revision, String resource) {
+    return newBuilder()
+        .setProject(project)
+        .setLocation(location)
+        .setDeployment(deployment)
+        .setRevision(revision)
+        .setResource(resource)
+        .build();
+  }
+
+  public static String format(
+      String project, String location, String deployment, String revision, String resource) {
+    return newBuilder()
+        .setProject(project)
+        .setLocation(location)
+        .setDeployment(deployment)
+        .setRevision(revision)
+        .setResource(resource)
+        .build()
+        .toString();
+  }
+
+  public static ResourceName parse(String formattedString) {
+    if (formattedString.isEmpty()) {
+      return null;
+    }
+    Map<String, String> matchMap =
+        PROJECT_LOCATION_DEPLOYMENT_REVISION_RESOURCE.validatedMatch(
+            formattedString, "ResourceName.parse: formattedString not in valid format");
+    return of(
+        matchMap.get("project"),
+        matchMap.get("location"),
+        matchMap.get("deployment"),
+        matchMap.get("revision"),
+        matchMap.get("resource"));
+  }
+
+  public static List<ResourceName> parseList(List<String> formattedStrings) {
+    List<ResourceName> list = new ArrayList<>(formattedStrings.size());
+    for (String formattedString : formattedStrings) {
+      list.add(parse(formattedString));
+    }
+    return list;
+  }
+
+  public static List<String> toStringList(List<ResourceName> values) {
+    List<String> list = new ArrayList<>(values.size());
+    for (ResourceName value : values) {
+      if (value == null) {
+        list.add("");
+      } else {
+        list.add(value.toString());
+      }
+    }
+    return list;
+  }
+
+  public static boolean isParsableFrom(String formattedString) {
+    return PROJECT_LOCATION_DEPLOYMENT_REVISION_RESOURCE.matches(formattedString);
+  }
+
+  @Override
+  public Map<String, String> getFieldValuesMap() {
+    if (fieldValuesMap == null) {
+      synchronized (this) {
+        if (fieldValuesMap == null) {
+          ImmutableMap.Builder<String, String> fieldMapBuilder = ImmutableMap.builder();
+          if (project != null) {
+            fieldMapBuilder.put("project", project);
+          }
+          if (location != null) {
+            fieldMapBuilder.put("location", location);
+          }
+          if (deployment != null) {
+            fieldMapBuilder.put("deployment", deployment);
+          }
+          if (revision != null) {
+            fieldMapBuilder.put("revision", revision);
+          }
+          if (resource != null) {
+            fieldMapBuilder.put("resource", resource);
+          }
+          fieldValuesMap = fieldMapBuilder.build();
+        }
+      }
+    }
+    return fieldValuesMap;
+  }
+
+  public String getFieldValue(String fieldName) {
+    return getFieldValuesMap().get(fieldName);
+  }
+
+  @Override
+  public String toString() {
+    return PROJECT_LOCATION_DEPLOYMENT_REVISION_RESOURCE.instantiate(
+        "project",
+        project,
+        "location",
+        location,
+        "deployment",
+        deployment,
+        "revision",
+        revision,
+        "resource",
+        resource);
+  }
+
+  @Override
+  public boolean equals(java.lang.Object o) {
+    if (o == this) {
+      return true;
+    }
+    if (o != null || getClass() == o.getClass()) {
+      ResourceName that = ((ResourceName) o);
+      return Objects.equals(this.project, that.project)
+          && Objects.equals(this.location, that.location)
+          && Objects.equals(this.deployment, that.deployment)
+          && Objects.equals(this.revision, that.revision)
+          && Objects.equals(this.resource, that.resource);
+    }
+    return false;
+  }
+
+  @Override
+  public int hashCode() {
+    int h = 1;
+    h *= 1000003;
+    h ^= Objects.hashCode(project);
+    h *= 1000003;
+    h ^= Objects.hashCode(location);
+    h *= 1000003;
+    h ^= Objects.hashCode(deployment);
+    h *= 1000003;
+    h ^= Objects.hashCode(revision);
+    h *= 1000003;
+    h ^= Objects.hashCode(resource);
+    return h;
+  }
+
+  /**
+   * Builder for
+   * projects/{project}/locations/{location}/deployments/{deployment}/revisions/{revision}/resources/{resource}.
+   */
+  public static class Builder {
+    private String project;
+    private String location;
+    private String deployment;
+    private String revision;
+    private String resource;
+
+    protected Builder() {}
+
+    public String getProject() {
+      return project;
+    }
+
+    public String getLocation() {
+      return location;
+    }
+
+    public String getDeployment() {
+      return deployment;
+    }
+
+    public String getRevision() {
+      return revision;
+    }
+
+    public String getResource() {
+      return resource;
+    }
+
+    public Builder setProject(String project) {
+      this.project = project;
+      return this;
+    }
+
+    public Builder setLocation(String location) {
+      this.location = location;
+      return this;
+    }
+
+    public Builder setDeployment(String deployment) {
+      this.deployment = deployment;
+      return this;
+    }
+
+    public Builder setRevision(String revision) {
+      this.revision = revision;
+      return this;
+    }
+
+    public Builder setResource(String resource) {
+      this.resource = resource;
+      return this;
+    }
+
+    private Builder(ResourceName resourceName) {
+      this.project = resourceName.project;
+      this.location = resourceName.location;
+      this.deployment = resourceName.deployment;
+      this.revision = resourceName.revision;
+      this.resource = resourceName.resource;
+    }
+
+    public ResourceName build() {
+      return new ResourceName(this);
+    }
+  }
+}
diff --git a/gapic-generator-java/src/test/proto/collisions.proto b/gapic-generator-java/src/test/proto/collisions.proto
