Skip to content

Commit e654bfb

Browse files
miraleungmiraleung
authored
fix(resnames): fallback to fully-qualified Object name upon proto typing conflicts [ggj] (#803)
* chore: add baseline storage golden files * fix(resnames): fallback to fully-qualified Object name upon proto type conflict
1 parent d94a905 commit e654bfb

10 files changed

Lines changed: 49 additions & 25 deletions

File tree

‎src/main/java/com/google/api/generator/gapic/composer/Composer.java‎

Lines changed: 5 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -33,25 +33,19 @@
3333
import com.google.api.generator.gapic.model.GapicClass.Kind;
3434
import com.google.api.generator.gapic.model.GapicContext;
3535
import com.google.api.generator.gapic.model.GapicPackageInfo;
36-
import com.google.api.generator.gapic.model.ResourceName;
3736
import com.google.api.generator.gapic.model.Service;
3837
import com.google.api.generator.gapic.model.Transport;
3938
import com.google.common.annotations.VisibleForTesting;
4039
import java.util.ArrayList;
4140
import java.util.List;
42-
import java.util.Set;
4341
import java.util.stream.Collectors;
4442

4543
public class Composer {
4644
public static List<GapicClass> composeServiceClasses(GapicContext context) {
4745
List<GapicClass> clazzes = new ArrayList<>();
4846
clazzes.addAll(generateServiceClasses(context));
4947
clazzes.addAll(generateMockClasses(context, context.mixinServices()));
50-
clazzes.addAll(
51-
generateResourceNameHelperClasses(
52-
context.helperResourceNames().values().stream()
53-
.map(r -> r)
54-
.collect(Collectors.toSet())));
48+
clazzes.addAll(generateResourceNameHelperClasses(context));
5549
return addApacheLicense(clazzes);
5650
}
5751

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

71-
public static List<GapicClass> generateResourceNameHelperClasses(
72-
Set<ResourceName> resourceNames) {
73-
return resourceNames.stream()
65+
public static List<GapicClass> generateResourceNameHelperClasses(GapicContext context) {
66+
return context.helperResourceNames().values().stream()
67+
.distinct()
7468
.filter(r -> !r.isOnlyWildcard())
75-
.map(r -> ResourceNameHelperClassComposer.instance().generate(r))
69+
.map(r -> ResourceNameHelperClassComposer.instance().generate(r, context))
7670
.collect(Collectors.toList());
7771
}
7872

‎src/main/java/com/google/api/generator/gapic/composer/resourcename/ResourceNameHelperClassComposer.java‎

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@
4949
import com.google.api.generator.gapic.composer.comment.CommentComposer;
5050
import com.google.api.generator.gapic.composer.store.TypeStore;
5151
import com.google.api.generator.gapic.model.GapicClass;
52+
import com.google.api.generator.gapic.model.GapicContext;
5253
import com.google.api.generator.gapic.model.ResourceName;
5354
import com.google.api.generator.gapic.utils.JavaStyle;
5455
import com.google.api.generator.gapic.utils.ResourceNameConstants;
@@ -79,23 +80,36 @@ public class ResourceNameHelperClassComposer {
7980
new ResourceNameHelperClassComposer();
8081

8182
private static final TypeStore FIXED_TYPESTORE = createStaticTypes();
83+
private static final Map<String, VariableExpr> FIXED_CLASS_VARS =
84+
createFixedClassMemberVariables();
8285

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

8588
private ResourceNameHelperClassComposer() {}
8689

8790
public static ResourceNameHelperClassComposer instance() {
8891
return INSTANCE;
8992
}
9093

91-
public GapicClass generate(ResourceName resourceName) {
94+
public GapicClass generate(ResourceName resourceName, GapicContext context) {
95+
// Set up types.
9296
List<List<String>> tokenHierarchies =
9397
ResourceNameTokenizer.parseTokenHierarchy(resourceName.patterns());
9498
TypeStore typeStore = createDynamicTypes(resourceName, tokenHierarchies);
99+
// Use the full name java.lang.Object if there is a proto message that is also named "Object".
100+
// Affects GCS.
101+
if (context.messages().keySet().stream()
102+
.anyMatch(s -> s.equals("Object") || s.endsWith(".Object"))) {
103+
javaObjectReference =
104+
ConcreteReference.builder().setClazz(Object.class).setUseFullName(true).build();
105+
}
106+
107+
// Set up variables.
95108
List<VariableExpr> templateFinalVarExprs = createTemplateClassMembers(tokenHierarchies);
96109
Map<String, VariableExpr> patternTokenVarExprs =
97110
createPatternTokenClassMembers(tokenHierarchies);
98111

112+
// Check invariants.
99113
Preconditions.checkState(
100114
patternTokenVarExprs.size() > 0,
101115
String.format("No patterns found for resource name %s", resourceName.resourceTypeString()));
@@ -1292,7 +1306,11 @@ private static MethodDefinition createToStringMethod(
12921306
private static MethodDefinition createEqualsMethod(
12931307
ResourceName resourceName, List<List<String>> tokenHierarchies, TypeStore typeStore) {
12941308
// Create method definition variables.
1295-
Variable oVariable = Variable.builder().setType(TypeNode.OBJECT).setName("o").build();
1309+
Variable oVariable =
1310+
Variable.builder()
1311+
.setType(TypeNode.withReference(javaObjectReference))
1312+
.setName("o")
1313+
.build();
12961314
VariableExpr argVarExpr =
12971315
VariableExpr.builder().setIsDecl(false).setVariable(oVariable).build();
12981316
TypeNode thisClassType = typeStore.get(getThisClassName(resourceName));

‎src/test/java/com/google/api/generator/gapic/composer/resourcename/BUILD.bazel‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ TEST_DEPS = [
2323
"//src/main/java/com/google/api/generator/gapic/composer/resourcename",
2424
"//src/main/java/com/google/api/generator/gapic/model",
2525
"//src/main/java/com/google/api/generator/gapic/protoparser",
26+
"//src/test/java/com/google/api/generator/gapic/composer/common",
2627
"//src/test/java/com/google/api/generator/gapic/testdata:showcase_java_proto",
2728
"//src/test/java/com/google/api/generator/test/framework:asserts",
2829
"//src/test/java/com/google/api/generator/test/framework:utils",

‎src/test/java/com/google/api/generator/gapic/composer/resourcename/ResourceNameHelperClassComposerTest.java‎

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,9 @@
1818
import static junit.framework.Assert.assertEquals;
1919

2020
import com.google.api.generator.engine.writer.JavaWriterVisitor;
21+
import com.google.api.generator.gapic.composer.common.TestProtoLoader;
2122
import com.google.api.generator.gapic.model.GapicClass;
23+
import com.google.api.generator.gapic.model.GapicContext;
2224
import com.google.api.generator.gapic.model.Message;
2325
import com.google.api.generator.gapic.model.ResourceName;
2426
import com.google.api.generator.gapic.model.Service;
@@ -101,7 +103,9 @@ public void generateResourceNameClass_echoFoobarMultiplePatterns() {
101103
ResourceName foobarResname = resourceNames.get("showcase.googleapis.com/Foobar");
102104
assertThat(outputResourceNames).contains(foobarResname);
103105

104-
GapicClass clazz = ResourceNameHelperClassComposer.instance().generate(foobarResname);
106+
GapicClass clazz =
107+
ResourceNameHelperClassComposer.instance()
108+
.generate(foobarResname, TestProtoLoader.instance().parseShowcaseEcho());
105109

106110
JavaWriterVisitor visitor = new JavaWriterVisitor();
107111
clazz.classDefinition().accept(visitor);
@@ -149,7 +153,8 @@ public void generateResourceNameClass_loggingOnePatternMultipleVariables() {
149153
assertThat(outputResourceNames).contains(billingAccountLocationResname);
150154

151155
GapicClass clazz =
152-
ResourceNameHelperClassComposer.instance().generate(billingAccountLocationResname);
156+
ResourceNameHelperClassComposer.instance()
157+
.generate(billingAccountLocationResname, TestProtoLoader.instance().parseLogging());
153158

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

182-
GapicClass clazz = ResourceNameHelperClassComposer.instance().generate(sessionResname);
187+
GapicClass clazz =
188+
ResourceNameHelperClassComposer.instance()
189+
.generate(sessionResname, TestProtoLoader.instance().parseShowcaseTesting());
183190

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

211-
GapicClass clazz = ResourceNameHelperClassComposer.instance().generate(testResname);
218+
GapicClass clazz =
219+
ResourceNameHelperClassComposer.instance()
220+
.generate(testResname, TestProtoLoader.instance().parseShowcaseTesting());
212221

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

234-
GapicClass clazz = ResourceNameHelperClassComposer.instance().generate(agentResname);
243+
GapicContext irrelevantContext = TestProtoLoader.instance().parseShowcaseEcho();
244+
GapicClass clazz =
245+
ResourceNameHelperClassComposer.instance().generate(agentResname, irrelevantContext);
235246
JavaWriterVisitor visitor = new JavaWriterVisitor();
236247
clazz.classDefinition().accept(visitor);
237248
Utils.saveCodegenToFile(this.getClass(), "AgentName.golden", visitor.write());

‎src/test/java/com/google/api/generator/gapic/composer/resourcename/goldens/AgentName.golden‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -164,7 +164,7 @@ public class AgentName implements ResourceName {
164164
}
165165

166166
@Override
167-
public boolean equals(Object o) {
167+
public boolean equals(java.lang.Object o) {
168168
if (o == this) {
169169
return true;
170170
}

‎src/test/java/com/google/api/generator/gapic/composer/resourcename/goldens/BillingAccountLocationName.golden‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,7 @@ public class BillingAccountLocationName implements ResourceName {
120120
}
121121

122122
@Override
123-
public boolean equals(Object o) {
123+
public boolean equals(java.lang.Object o) {
124124
if (o == this) {
125125
return true;
126126
}

‎src/test/java/com/google/api/generator/gapic/composer/resourcename/goldens/FoobarName.golden‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -258,7 +258,7 @@ public class FoobarName implements ResourceName {
258258
}
259259

260260
@Override
261-
public boolean equals(Object o) {
261+
public boolean equals(java.lang.Object o) {
262262
if (o == this) {
263263
return true;
264264
}

‎test/integration/goldens/storage/com/google/storage/v2/BucketName.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,7 @@ public String toString() {
133133
}
134134

135135
@Override
136-
public boolean equals(Object o) {
136+
public boolean equals(java.lang.Object o) {
137137
if (o == this) {
138138
return true;
139139
}

‎test/integration/goldens/storage/com/google/storage/v2/CryptoKeyName.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -171,7 +171,7 @@ public String toString() {
171171
}
172172

173173
@Override
174-
public boolean equals(Object o) {
174+
public boolean equals(java.lang.Object o) {
175175
if (o == this) {
176176
return true;
177177
}

‎test/integration/goldens/storage/com/google/storage/v2/ProjectName.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,7 @@ public String toString() {
123123
}
124124

125125
@Override
126-
public boolean equals(Object o) {
126+
public boolean equals(java.lang.Object o) {
127127
if (o == this) {
128128
return true;
129129
}

0 commit comments

Comments
 (0)