Skip to content

Commit

Permalink
[ggj][resnames][codegen] fix: add default value fallback for wildcard…
Browse files Browse the repository at this point in the history
…-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
  • Loading branch information
miraleung authored Nov 24, 2020
1 parent fa6b5e8 commit 109bc93
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 19 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,9 @@ static Expr createDefaultValue(
"No resource name found for reference %s",
methodArg.field().resourceReference().resourceTypeString()));
return createDefaultValue(
resourceName, resourceNames.values().stream().collect(Collectors.toList()));
resourceName,
resourceNames.values().stream().collect(Collectors.toList()),
methodArg.field().name());
}

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

static Expr createDefaultValue(ResourceName resourceName, List<ResourceName> resnames) {
static Expr createDefaultValue(
ResourceName resourceName, List<ResourceName> resnames, String fieldOrMessageName) {
boolean hasOnePattern = resourceName.patterns().size() == 1;
if (resourceName.isOnlyWildcard()) {
List<ResourceName> unexaminedResnames = new ArrayList<>(resnames);
Expand All @@ -160,13 +163,14 @@ static Expr createDefaultValue(ResourceName resourceName, List<ResourceName> res
continue;
}
unexaminedResnames.remove(resname);
return createDefaultValue(resname, unexaminedResnames);
return createDefaultValue(resname, unexaminedResnames, fieldOrMessageName);
}

if (unexaminedResnames.isEmpty()) {
return ValueExpr.withValue(
StringObjectValue.withValue(
String.format("%s%s", fieldOrMessageName, fieldOrMessageName.hashCode())));
}
// Should not get here.
Preconditions.checkState(
!unexaminedResnames.isEmpty(),
String.format(
"No default resource name found for wildcard %s", resourceName.resourceTypeString()));
}

// The cost tradeoffs of new ctors versus distinct() don't really matter here, since this list
Expand Down Expand Up @@ -242,7 +246,8 @@ static Expr createSimpleMessageBuilderExpr(
defaultExpr =
createDefaultValue(
resourceNames.get(field.resourceReference().resourceTypeString()),
resourceNames.values().stream().collect(Collectors.toList()));
resourceNames.values().stream().collect(Collectors.toList()),
message.name());
defaultExpr =
MethodInvocationExpr.builder()
.setExprReferenceExpr(defaultExpr)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
package com.google.api.generator.gapic.composer;

import static junit.framework.Assert.assertEquals;
import static org.junit.Assert.assertThrows;

import com.google.api.generator.engine.ast.ConcreteReference;
import com.google.api.generator.engine.ast.Expr;
Expand Down Expand Up @@ -153,7 +152,8 @@ public void defaultValue_resourceNameWithOnePattern() {
Expr expr =
DefaultValueComposer.createDefaultValue(
resourceName,
typeStringsToResourceNames.values().stream().collect(Collectors.toList()));
typeStringsToResourceNames.values().stream().collect(Collectors.toList()),
"ignored");
expr.accept(writerVisitor);
assertEquals("BillingAccountName.of(\"[BILLING_ACCOUNT]\")", writerVisitor.write());
}
Expand All @@ -168,7 +168,8 @@ public void defaultValue_resourceNameWithMultiplePatterns() {
Expr expr =
DefaultValueComposer.createDefaultValue(
resourceName,
typeStringsToResourceNames.values().stream().collect(Collectors.toList()));
typeStringsToResourceNames.values().stream().collect(Collectors.toList()),
"ignored");
expr.accept(writerVisitor);
assertEquals(
"FolderName.ofProjectFolderName(\"[PROJECT]\", \"[FOLDER]\")", writerVisitor.write());
Expand All @@ -184,13 +185,14 @@ public void defaultValue_resourceNameWithWildcardPattern() {
Expr expr =
DefaultValueComposer.createDefaultValue(
resourceName,
typeStringsToResourceNames.values().stream().collect(Collectors.toList()));
typeStringsToResourceNames.values().stream().collect(Collectors.toList()),
"ignored");
expr.accept(writerVisitor);
assertEquals("DocumentName.ofDocumentName(\"[DOCUMENT]\")", writerVisitor.write());
}

@Test
public void invalidDefaultValue_wildcardResourceNameWithOnlyDeletedTopic() {
public void defaultValue_wildcardResourceNameWithOnlyDeletedTopic() {
// Edge case that should never happen in practice.
// Wildcard, but the resource names map has only other names that contain only the deleted-topic
// constant.
Expand All @@ -205,13 +207,14 @@ public void invalidDefaultValue_wildcardResourceNameWithOnlyDeletedTopic() {
Expr expr =
DefaultValueComposer.createDefaultValue(
resourceName,
typeStringsToResourceNames.values().stream().collect(Collectors.toList()));
typeStringsToResourceNames.values().stream().collect(Collectors.toList()),
"ignored");
expr.accept(writerVisitor);
assertEquals("TopicName.ofDeletedTopic()", writerVisitor.write());
}

@Test
public void invalidDefaultValue_resourceNameWithOnlyWildcards() {
public void defaultValue_resourceNameWithOnlyWildcards() {
// Edge case that should never happen in practice.
// Wildcard, but the resource names map has only other names that contain only the deleted-topic
// constant.
Expand All @@ -220,9 +223,13 @@ public void invalidDefaultValue_resourceNameWithOnlyWildcards() {
Parser.parseResourceNames(lockerServiceFileDescriptor);
ResourceName resourceName =
typeStringsToResourceNames.get("cloudresourcemanager.googleapis.com/Anything");
assertThrows(
IllegalStateException.class,
() -> DefaultValueComposer.createDefaultValue(resourceName, Collections.emptyList()));
String fallbackField = "foobar";
Expr expr =
DefaultValueComposer.createDefaultValue(
resourceName, Collections.emptyList(), fallbackField);
expr.accept(writerVisitor);
assertEquals(
String.format("\"%s%s\"", fallbackField, fallbackField.hashCode()), writerVisitor.write());
}

@Test
Expand Down

0 comments on commit 109bc93

Please sign in to comment.