Skip to content

Commit

Permalink
fix(resnames): fallback to fully-qualified Object name upon proto typ…
Browse files Browse the repository at this point in the history
…ing conflicts [ggj] (#803)

* chore: add baseline storage golden files

* fix(resnames): fallback to fully-qualified Object name upon proto type conflict
  • Loading branch information
miraleung committed Jul 30, 2021
1 parent d94a905 commit e654bfb
Show file tree
Hide file tree
Showing 10 changed files with 49 additions and 25 deletions.
Expand Up @@ -33,25 +33,19 @@
import com.google.api.generator.gapic.model.GapicClass.Kind;
import com.google.api.generator.gapic.model.GapicContext;
import com.google.api.generator.gapic.model.GapicPackageInfo;
import com.google.api.generator.gapic.model.ResourceName;
import com.google.api.generator.gapic.model.Service;
import com.google.api.generator.gapic.model.Transport;
import com.google.common.annotations.VisibleForTesting;
import java.util.ArrayList;
import java.util.List;
import java.util.Set;
import java.util.stream.Collectors;

public class Composer {
public static List<GapicClass> composeServiceClasses(GapicContext context) {
List<GapicClass> clazzes = new ArrayList<>();
clazzes.addAll(generateServiceClasses(context));
clazzes.addAll(generateMockClasses(context, context.mixinServices()));
clazzes.addAll(
generateResourceNameHelperClasses(
context.helperResourceNames().values().stream()
.map(r -> r)
.collect(Collectors.toSet())));
clazzes.addAll(generateResourceNameHelperClasses(context));
return addApacheLicense(clazzes);
}

Expand All @@ -68,11 +62,11 @@ public static List<GapicClass> generateServiceClasses(GapicContext context) {
return clazzes;
}

public static List<GapicClass> generateResourceNameHelperClasses(
Set<ResourceName> resourceNames) {
return resourceNames.stream()
public static List<GapicClass> generateResourceNameHelperClasses(GapicContext context) {
return context.helperResourceNames().values().stream()
.distinct()
.filter(r -> !r.isOnlyWildcard())
.map(r -> ResourceNameHelperClassComposer.instance().generate(r))
.map(r -> ResourceNameHelperClassComposer.instance().generate(r, context))
.collect(Collectors.toList());
}

Expand Down
Expand Up @@ -49,6 +49,7 @@
import com.google.api.generator.gapic.composer.comment.CommentComposer;
import com.google.api.generator.gapic.composer.store.TypeStore;
import com.google.api.generator.gapic.model.GapicClass;
import com.google.api.generator.gapic.model.GapicContext;
import com.google.api.generator.gapic.model.ResourceName;
import com.google.api.generator.gapic.utils.JavaStyle;
import com.google.api.generator.gapic.utils.ResourceNameConstants;
Expand Down Expand Up @@ -79,23 +80,36 @@ public class ResourceNameHelperClassComposer {
new ResourceNameHelperClassComposer();

private static final TypeStore FIXED_TYPESTORE = createStaticTypes();
private static final Map<String, VariableExpr> FIXED_CLASS_VARS =
createFixedClassMemberVariables();

private static Map<String, VariableExpr> FIXED_CLASS_VARS = createFixedClassMemberVariables();
private static Reference javaObjectReference = ConcreteReference.withClazz(Object.class);

private ResourceNameHelperClassComposer() {}

public static ResourceNameHelperClassComposer instance() {
return INSTANCE;
}

public GapicClass generate(ResourceName resourceName) {
public GapicClass generate(ResourceName resourceName, GapicContext context) {
// Set up types.
List<List<String>> tokenHierarchies =
ResourceNameTokenizer.parseTokenHierarchy(resourceName.patterns());
TypeStore typeStore = createDynamicTypes(resourceName, tokenHierarchies);
// Use the full name java.lang.Object if there is a proto message that is also named "Object".
// Affects GCS.
if (context.messages().keySet().stream()
.anyMatch(s -> s.equals("Object") || s.endsWith(".Object"))) {
javaObjectReference =
ConcreteReference.builder().setClazz(Object.class).setUseFullName(true).build();
}

// Set up variables.
List<VariableExpr> templateFinalVarExprs = createTemplateClassMembers(tokenHierarchies);
Map<String, VariableExpr> patternTokenVarExprs =
createPatternTokenClassMembers(tokenHierarchies);

// Check invariants.
Preconditions.checkState(
patternTokenVarExprs.size() > 0,
String.format("No patterns found for resource name %s", resourceName.resourceTypeString()));
Expand Down Expand Up @@ -1292,7 +1306,11 @@ private static MethodDefinition createToStringMethod(
private static MethodDefinition createEqualsMethod(
ResourceName resourceName, List<List<String>> tokenHierarchies, TypeStore typeStore) {
// Create method definition variables.
Variable oVariable = Variable.builder().setType(TypeNode.OBJECT).setName("o").build();
Variable oVariable =
Variable.builder()
.setType(TypeNode.withReference(javaObjectReference))
.setName("o")
.build();
VariableExpr argVarExpr =
VariableExpr.builder().setIsDecl(false).setVariable(oVariable).build();
TypeNode thisClassType = typeStore.get(getThisClassName(resourceName));
Expand Down
Expand Up @@ -23,6 +23,7 @@ TEST_DEPS = [
"//src/main/java/com/google/api/generator/gapic/composer/resourcename",
"//src/main/java/com/google/api/generator/gapic/model",
"//src/main/java/com/google/api/generator/gapic/protoparser",
"//src/test/java/com/google/api/generator/gapic/composer/common",
"//src/test/java/com/google/api/generator/gapic/testdata:showcase_java_proto",
"//src/test/java/com/google/api/generator/test/framework:asserts",
"//src/test/java/com/google/api/generator/test/framework:utils",
Expand Down
Expand Up @@ -18,7 +18,9 @@
import static junit.framework.Assert.assertEquals;

import com.google.api.generator.engine.writer.JavaWriterVisitor;
import com.google.api.generator.gapic.composer.common.TestProtoLoader;
import com.google.api.generator.gapic.model.GapicClass;
import com.google.api.generator.gapic.model.GapicContext;
import com.google.api.generator.gapic.model.Message;
import com.google.api.generator.gapic.model.ResourceName;
import com.google.api.generator.gapic.model.Service;
Expand Down Expand Up @@ -101,7 +103,9 @@ public void generateResourceNameClass_echoFoobarMultiplePatterns() {
ResourceName foobarResname = resourceNames.get("showcase.googleapis.com/Foobar");
assertThat(outputResourceNames).contains(foobarResname);

GapicClass clazz = ResourceNameHelperClassComposer.instance().generate(foobarResname);
GapicClass clazz =
ResourceNameHelperClassComposer.instance()
.generate(foobarResname, TestProtoLoader.instance().parseShowcaseEcho());

JavaWriterVisitor visitor = new JavaWriterVisitor();
clazz.classDefinition().accept(visitor);
Expand Down Expand Up @@ -149,7 +153,8 @@ public void generateResourceNameClass_loggingOnePatternMultipleVariables() {
assertThat(outputResourceNames).contains(billingAccountLocationResname);

GapicClass clazz =
ResourceNameHelperClassComposer.instance().generate(billingAccountLocationResname);
ResourceNameHelperClassComposer.instance()
.generate(billingAccountLocationResname, TestProtoLoader.instance().parseLogging());

JavaWriterVisitor visitor = new JavaWriterVisitor();
clazz.classDefinition().accept(visitor);
Expand Down Expand Up @@ -179,7 +184,9 @@ public void generateResourceNameClass_testingSessionOnePattern() {
ResourceName sessionResname = resourceNames.get("showcase.googleapis.com/Session");
assertThat(outputResourceNames).contains(sessionResname);

GapicClass clazz = ResourceNameHelperClassComposer.instance().generate(sessionResname);
GapicClass clazz =
ResourceNameHelperClassComposer.instance()
.generate(sessionResname, TestProtoLoader.instance().parseShowcaseTesting());

JavaWriterVisitor visitor = new JavaWriterVisitor();
clazz.classDefinition().accept(visitor);
Expand Down Expand Up @@ -208,7 +215,9 @@ public void generateResourceNameClass_testingBlueprintPatternWithNonSlashSeparat
ResourceName testResname = resourceNames.get("showcase.googleapis.com/Test");
assertThat(outputResourceNames).contains(testResname);

GapicClass clazz = ResourceNameHelperClassComposer.instance().generate(testResname);
GapicClass clazz =
ResourceNameHelperClassComposer.instance()
.generate(testResname, TestProtoLoader.instance().parseShowcaseTesting());

JavaWriterVisitor visitor = new JavaWriterVisitor();
clazz.classDefinition().accept(visitor);
Expand All @@ -231,7 +240,9 @@ public void generateResourceNameClass_childSingleton() {
.setDescription("This is a description")
.build();

GapicClass clazz = ResourceNameHelperClassComposer.instance().generate(agentResname);
GapicContext irrelevantContext = TestProtoLoader.instance().parseShowcaseEcho();
GapicClass clazz =
ResourceNameHelperClassComposer.instance().generate(agentResname, irrelevantContext);
JavaWriterVisitor visitor = new JavaWriterVisitor();
clazz.classDefinition().accept(visitor);
Utils.saveCodegenToFile(this.getClass(), "AgentName.golden", visitor.write());
Expand Down
Expand Up @@ -164,7 +164,7 @@ public class AgentName implements ResourceName {
}

@Override
public boolean equals(Object o) {
public boolean equals(java.lang.Object o) {
if (o == this) {
return true;
}
Expand Down
Expand Up @@ -120,7 +120,7 @@ public class BillingAccountLocationName implements ResourceName {
}

@Override
public boolean equals(Object o) {
public boolean equals(java.lang.Object o) {
if (o == this) {
return true;
}
Expand Down
Expand Up @@ -258,7 +258,7 @@ public class FoobarName implements ResourceName {
}

@Override
public boolean equals(Object o) {
public boolean equals(java.lang.Object o) {
if (o == this) {
return true;
}
Expand Down
Expand Up @@ -133,7 +133,7 @@ public String toString() {
}

@Override
public boolean equals(Object o) {
public boolean equals(java.lang.Object o) {
if (o == this) {
return true;
}
Expand Down
Expand Up @@ -171,7 +171,7 @@ public String toString() {
}

@Override
public boolean equals(Object o) {
public boolean equals(java.lang.Object o) {
if (o == this) {
return true;
}
Expand Down
Expand Up @@ -123,7 +123,7 @@ public String toString() {
}

@Override
public boolean equals(Object o) {
public boolean equals(java.lang.Object o) {
if (o == this) {
return true;
}
Expand Down

0 comments on commit e654bfb

Please sign in to comment.