Skip to content

Commit 109bc93

Browse files
authored
[ggj][resnames][codegen] fix: add default value fallback for wildcard-only resnames (#562)
* feat: add parsing for language_settings in gapic.yaml * fix: restructure GapicServiceConfig table-processing into ctor * fix: refactor {LRO,Batching} settings out of GapicServiceConfig's required creation params * build: support file deletion in integration test update Bazel rules * feat: support gapic.yaml Java name overrides for Service{Client,Settings} * fix: use gapic.yaml override names in comments * fix: Use the right file * fix: add default value fallback for wildcard-only resnames
1 parent fa6b5e8 commit 109bc93

File tree

2 files changed

+31
-19
lines changed

2 files changed

+31
-19
lines changed

src/main/java/com/google/api/generator/gapic/composer/DefaultValueComposer.java

+14-9
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,9 @@ static Expr createDefaultValue(
6565
"No resource name found for reference %s",
6666
methodArg.field().resourceReference().resourceTypeString()));
6767
return createDefaultValue(
68-
resourceName, resourceNames.values().stream().collect(Collectors.toList()));
68+
resourceName,
69+
resourceNames.values().stream().collect(Collectors.toList()),
70+
methodArg.field().name());
6971
}
7072

7173
if (methodArg.type().equals(methodArg.field().type())) {
@@ -150,7 +152,8 @@ static Expr createDefaultValue(Field f, boolean useExplicitInitTypeInGenerics) {
150152
"Default value for field %s with type %s not implemented yet.", f.name(), f.type()));
151153
}
152154

153-
static Expr createDefaultValue(ResourceName resourceName, List<ResourceName> resnames) {
155+
static Expr createDefaultValue(
156+
ResourceName resourceName, List<ResourceName> resnames, String fieldOrMessageName) {
154157
boolean hasOnePattern = resourceName.patterns().size() == 1;
155158
if (resourceName.isOnlyWildcard()) {
156159
List<ResourceName> unexaminedResnames = new ArrayList<>(resnames);
@@ -160,13 +163,14 @@ static Expr createDefaultValue(ResourceName resourceName, List<ResourceName> res
160163
continue;
161164
}
162165
unexaminedResnames.remove(resname);
163-
return createDefaultValue(resname, unexaminedResnames);
166+
return createDefaultValue(resname, unexaminedResnames, fieldOrMessageName);
167+
}
168+
169+
if (unexaminedResnames.isEmpty()) {
170+
return ValueExpr.withValue(
171+
StringObjectValue.withValue(
172+
String.format("%s%s", fieldOrMessageName, fieldOrMessageName.hashCode())));
164173
}
165-
// Should not get here.
166-
Preconditions.checkState(
167-
!unexaminedResnames.isEmpty(),
168-
String.format(
169-
"No default resource name found for wildcard %s", resourceName.resourceTypeString()));
170174
}
171175

172176
// The cost tradeoffs of new ctors versus distinct() don't really matter here, since this list
@@ -242,7 +246,8 @@ static Expr createSimpleMessageBuilderExpr(
242246
defaultExpr =
243247
createDefaultValue(
244248
resourceNames.get(field.resourceReference().resourceTypeString()),
245-
resourceNames.values().stream().collect(Collectors.toList()));
249+
resourceNames.values().stream().collect(Collectors.toList()),
250+
message.name());
246251
defaultExpr =
247252
MethodInvocationExpr.builder()
248253
.setExprReferenceExpr(defaultExpr)

src/test/java/com/google/api/generator/gapic/composer/DefaultValueComposerTest.java

+17-10
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@
1515
package com.google.api.generator.gapic.composer;
1616

1717
import static junit.framework.Assert.assertEquals;
18-
import static org.junit.Assert.assertThrows;
1918

2019
import com.google.api.generator.engine.ast.ConcreteReference;
2120
import com.google.api.generator.engine.ast.Expr;
@@ -153,7 +152,8 @@ public void defaultValue_resourceNameWithOnePattern() {
153152
Expr expr =
154153
DefaultValueComposer.createDefaultValue(
155154
resourceName,
156-
typeStringsToResourceNames.values().stream().collect(Collectors.toList()));
155+
typeStringsToResourceNames.values().stream().collect(Collectors.toList()),
156+
"ignored");
157157
expr.accept(writerVisitor);
158158
assertEquals("BillingAccountName.of(\"[BILLING_ACCOUNT]\")", writerVisitor.write());
159159
}
@@ -168,7 +168,8 @@ public void defaultValue_resourceNameWithMultiplePatterns() {
168168
Expr expr =
169169
DefaultValueComposer.createDefaultValue(
170170
resourceName,
171-
typeStringsToResourceNames.values().stream().collect(Collectors.toList()));
171+
typeStringsToResourceNames.values().stream().collect(Collectors.toList()),
172+
"ignored");
172173
expr.accept(writerVisitor);
173174
assertEquals(
174175
"FolderName.ofProjectFolderName(\"[PROJECT]\", \"[FOLDER]\")", writerVisitor.write());
@@ -184,13 +185,14 @@ public void defaultValue_resourceNameWithWildcardPattern() {
184185
Expr expr =
185186
DefaultValueComposer.createDefaultValue(
186187
resourceName,
187-
typeStringsToResourceNames.values().stream().collect(Collectors.toList()));
188+
typeStringsToResourceNames.values().stream().collect(Collectors.toList()),
189+
"ignored");
188190
expr.accept(writerVisitor);
189191
assertEquals("DocumentName.ofDocumentName(\"[DOCUMENT]\")", writerVisitor.write());
190192
}
191193

192194
@Test
193-
public void invalidDefaultValue_wildcardResourceNameWithOnlyDeletedTopic() {
195+
public void defaultValue_wildcardResourceNameWithOnlyDeletedTopic() {
194196
// Edge case that should never happen in practice.
195197
// Wildcard, but the resource names map has only other names that contain only the deleted-topic
196198
// constant.
@@ -205,13 +207,14 @@ public void invalidDefaultValue_wildcardResourceNameWithOnlyDeletedTopic() {
205207
Expr expr =
206208
DefaultValueComposer.createDefaultValue(
207209
resourceName,
208-
typeStringsToResourceNames.values().stream().collect(Collectors.toList()));
210+
typeStringsToResourceNames.values().stream().collect(Collectors.toList()),
211+
"ignored");
209212
expr.accept(writerVisitor);
210213
assertEquals("TopicName.ofDeletedTopic()", writerVisitor.write());
211214
}
212215

213216
@Test
214-
public void invalidDefaultValue_resourceNameWithOnlyWildcards() {
217+
public void defaultValue_resourceNameWithOnlyWildcards() {
215218
// Edge case that should never happen in practice.
216219
// Wildcard, but the resource names map has only other names that contain only the deleted-topic
217220
// constant.
@@ -220,9 +223,13 @@ public void invalidDefaultValue_resourceNameWithOnlyWildcards() {
220223
Parser.parseResourceNames(lockerServiceFileDescriptor);
221224
ResourceName resourceName =
222225
typeStringsToResourceNames.get("cloudresourcemanager.googleapis.com/Anything");
223-
assertThrows(
224-
IllegalStateException.class,
225-
() -> DefaultValueComposer.createDefaultValue(resourceName, Collections.emptyList()));
226+
String fallbackField = "foobar";
227+
Expr expr =
228+
DefaultValueComposer.createDefaultValue(
229+
resourceName, Collections.emptyList(), fallbackField);
230+
expr.accept(writerVisitor);
231+
assertEquals(
232+
String.format("\"%s%s\"", fallbackField, fallbackField.hashCode()), writerVisitor.write());
226233
}
227234

228235
@Test

0 commit comments

Comments
 (0)