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.
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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.