Skip to content

Commit

Permalink
fix: Use parent type instead of child_type in method doc sample (#862)
Browse files Browse the repository at this point in the history 
Fixes: #852.
  • Loading branch information
@meltsufin
meltsufin committed Nov 3, 2021
1 parent 64d8374 commit 6a39c7f
Show file tree
Hide file tree
Showing 20 changed files with 2,114 additions and 133 deletions.
39 changes: 36 additions & 3 deletions src/main/java/com/google/api/generator/gapic/composer/defaultvalue/DefaultValueComposer.java
View file
Expand Up @@ -37,6 +37,7 @@
import com.google.api.generator.gapic.model.ResourceName;
import com.google.api.generator.gapic.utils.JavaStyle;
import com.google.api.generator.gapic.utils.ResourceNameConstants;
import com.google.api.generator.gapic.utils.ResourceReferenceUtils;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Preconditions;
import com.google.longrunning.Operation;
Expand All @@ -48,6 +49,7 @@
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.stream.Collectors;

public class DefaultValueComposer {
Expand Down Expand Up @@ -76,6 +78,7 @@ public static Expr createDefaultValue(
Expr defValue =
createDefaultValue(
resourceName,
methodArg.field().resourceReference().isChildType(),
resourceNames.values().stream().collect(Collectors.toList()),
methodArg.field().name());

Expand Down Expand Up @@ -175,17 +178,46 @@ static Expr createDefaultValue(Field f, boolean useExplicitInitTypeInGenerics) {
}

public static Expr createDefaultValue(
ResourceName resourceName, List<ResourceName> resnames, String fieldOrMessageName) {
return createDefaultValueResourceHelper(resourceName, resnames, fieldOrMessageName, true);
ResourceName resourceName,
boolean isChildType,
List<ResourceName> resnames,
String fieldOrMessageName) {
return createDefaultValueResourceHelper(
resourceName, isChildType, resnames, fieldOrMessageName, true);
}

private static Optional<ResourceName> findParentResource(
ResourceName childResource, List<ResourceName> resourceNames) {
Map<String, ResourceName> patternToResourceName = new HashMap<>();

for (ResourceName resourceName : resourceNames) {
for (String parentPattern : resourceName.patterns()) {
patternToResourceName.put(parentPattern, resourceName);
}
}

for (String childPattern : childResource.patterns()) {
Optional<String> parentPattern = ResourceReferenceUtils.parseParentPattern(childPattern);
if (parentPattern.isPresent() && patternToResourceName.containsKey(parentPattern.get())) {
return Optional.of(patternToResourceName.get(parentPattern.get()));
}
}

return Optional.empty();
}

@VisibleForTesting
static Expr createDefaultValueResourceHelper(
ResourceName resourceName,
boolean isChildType,
List<ResourceName> resnames,
String fieldOrMessageName,
boolean allowAnonResourceNameClass) {

if (isChildType) {
resourceName = findParentResource(resourceName, resnames).orElse(resourceName);
}

boolean hasOnePattern = resourceName.patterns().size() == 1;
if (resourceName.isOnlyWildcard()) {
List<ResourceName> unexaminedResnames = new ArrayList<>(resnames);
Expand All @@ -195,7 +227,7 @@ static Expr createDefaultValueResourceHelper(
continue;
}
unexaminedResnames.remove(resname);
return createDefaultValue(resname, unexaminedResnames, fieldOrMessageName);
return createDefaultValue(resname, false, unexaminedResnames, fieldOrMessageName);
}

if (unexaminedResnames.isEmpty()) {
Expand Down Expand Up @@ -283,6 +315,7 @@ public static Expr createSimpleMessageBuilderExpr(
defaultExpr =
createDefaultValueResourceHelper(
resourceNames.get(field.resourceReference().resourceTypeString()),
field.resourceReference().isChildType(),
resourceNames.values().stream().collect(Collectors.toList()),
message.name(),
/* allowAnonResourceNameClass = */ false);
Expand Down
1 change: 1 addition & 0 deletions ...a/com/google/api/generator/gapic/composer/samplecode/ServiceClientSampleCodeComposer.java
View file
Expand Up @@ -1293,6 +1293,7 @@ private static List<Expr> createRpcMethodArgumentDefaultValueExprs(
.setExprReferenceExpr(
DefaultValueComposer.createDefaultValue(
resourceNames.get(arg.field().resourceReference().resourceTypeString()),
arg.field().resourceReference().isChildType(),
resourceNameList,
arg.field().name()))
.setMethodName("toString")
Expand Down
4 changes: 3 additions & 1 deletion src/main/java/com/google/api/generator/gapic/protoparser/Parser.java
View file
Expand Up @@ -881,7 +881,9 @@ static String parsePageSizeFieldName(
// with monolith-gnerated libraries.
String pagedFieldName = null;

if (inputMessage.fieldMap().containsKey("page_token")
if (inputMessage != null
&& inputMessage.fieldMap().containsKey("page_token")
&& outputMessage != null
&& outputMessage.fieldMap().containsKey("next_page_token")) {
// List of potential field names representing page size.
// page_size gets priority over max_results if both are present
Expand Down
29 changes: 2 additions & 27 deletions src/main/java/com/google/api/generator/gapic/protoparser/ResourceReferenceParser.java
View file
Expand Up @@ -17,7 +17,7 @@
import com.google.api.generator.gapic.model.ResourceName;
import com.google.api.generator.gapic.model.ResourceReference;
import com.google.api.generator.gapic.utils.JavaStyle;
import com.google.api.generator.gapic.utils.ResourceNameConstants;
import com.google.api.generator.gapic.utils.ResourceReferenceUtils;
import com.google.api.pathtemplate.PathTemplate;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Preconditions;
Expand Down Expand Up @@ -108,7 +108,7 @@ static Optional<ResourceName> parseParentResourceName(
String resourceTypeString,
@Nullable String description,
Map<String, ResourceName> patternsToResourceNames) {
Optional<String> parentPatternOpt = parseParentPattern(pattern);
Optional<String> parentPatternOpt = ResourceReferenceUtils.parseParentPattern(pattern);
if (!parentPatternOpt.isPresent()) {
return Optional.empty();
}
Expand Down Expand Up @@ -178,31 +178,6 @@ static Optional<ResourceName> parseParentResourceName(
return Optional.of(parentResourceName);
}

@VisibleForTesting
static Optional<String> parseParentPattern(String pattern) {
String[] tokens = pattern.split(SLASH);
String lastToken = tokens[tokens.length - 1];
if (lastToken.equals(ResourceNameConstants.DELETED_TOPIC_LITERAL)
|| lastToken.equals(ResourceNameConstants.WILDCARD_PATTERN)) {
return Optional.empty();
}

int lastTokenIndex = tokens.length - 2;
int minLengthWithParent = 4;
// Singleton patterns, e.g. projects/{project}/agent.
if (!lastToken.contains("{")) {
minLengthWithParent = 3;
lastTokenIndex = tokens.length - 1;
}

// No fully-formed parent. Expected: ancestors/{ancestor}/childNodes/{child_node}.
if (tokens.length < minLengthWithParent) {
return Optional.empty();
}

return Optional.of(String.join(SLASH, Arrays.asList(tokens).subList(0, lastTokenIndex)));
}

@VisibleForTesting
static String resolvePackages(String resourceNamePackage, String servicePackage) {
return resourceNamePackage.contains(servicePackage) ? resourceNamePackage : servicePackage;
Expand Down
49 changes: 49 additions & 0 deletions src/main/java/com/google/api/generator/gapic/utils/ResourceReferenceUtils.java
View file
@@ -0,0 +1,49 @@
// Copyright 2021 Google LLC
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package com.google.api.generator.gapic.utils;

import java.util.Arrays;
import java.util.Optional;

public final class ResourceReferenceUtils {

private static final String SLASH = "/";

private ResourceReferenceUtils() {}

public static Optional<String> parseParentPattern(String pattern) {
String[] tokens = pattern.split(SLASH);
String lastToken = tokens[tokens.length - 1];
if (lastToken.equals(ResourceNameConstants.DELETED_TOPIC_LITERAL)
|| lastToken.equals(ResourceNameConstants.WILDCARD_PATTERN)) {
return Optional.empty();
}

int lastTokenIndex = tokens.length - 2;
int minLengthWithParent = 4;
// Singleton patterns, e.g. projects/{project}/agent.
if (!lastToken.contains("{")) {
minLengthWithParent = 3;
lastTokenIndex = tokens.length - 1;
}

// No fully-formed parent. Expected: ancestors/{ancestor}/childNodes/{child_node}.
if (tokens.length < minLengthWithParent) {
return Optional.empty();
}

return Optional.of(String.join(SLASH, Arrays.asList(tokens).subList(0, lastTokenIndex)));
}
}
23 changes: 23 additions & 0 deletions src/test/java/com/google/api/generator/gapic/composer/common/TestProtoLoader.java
View file
Expand Up @@ -37,6 +37,7 @@
import com.google.pubsub.v1.PubsubProto;
import com.google.showcase.v1beta1.EchoOuterClass;
import com.google.showcase.v1beta1.IdentityOuterClass;
import com.google.showcase.v1beta1.MessagingOuterClass;
import com.google.showcase.v1beta1.TestingOuterClass;
import com.google.testdata.v1.DeprecatedServiceOuterClass;
import google.cloud.CommonResources;
Expand All @@ -51,6 +52,7 @@
import java.util.Set;

public class TestProtoLoader {

private static final TestProtoLoader INSTANCE =
new TestProtoLoader(Transport.GRPC, "src/test/java/com/google/api/generator/gapic/testdata/");
private final String testFilesDirectory;
Expand Down Expand Up @@ -170,6 +172,27 @@ public GapicContext parseShowcaseIdentity() {
.build();
}

public GapicContext parseShowcaseMessaging() {
FileDescriptor fileDescriptor = MessagingOuterClass.getDescriptor();
ServiceDescriptor messagingService = fileDescriptor.getServices().get(0);
assertEquals(messagingService.getName(), "Messaging");

Map<String, Message> messageTypes = Parser.parseMessages(fileDescriptor);
Map<String, ResourceName> resourceNames = Parser.parseResourceNames(fileDescriptor);
Set<ResourceName> outputResourceNames = new HashSet<>();
List<Service> services =
Parser.parseService(
fileDescriptor, messageTypes, resourceNames, Optional.empty(), outputResourceNames);

return GapicContext.builder()
.setMessages(messageTypes)
.setResourceNames(resourceNames)
.setServices(services)
.setHelperResourceNames(outputResourceNames)
.setTransport(transport)
.build();
}

public GapicContext parseShowcaseTesting() {
FileDescriptor testingFileDescriptor = TestingOuterClass.getDescriptor();
ServiceDescriptor testingService = testingFileDescriptor.getServices().get(0);
Expand Down
7 changes: 6 additions & 1 deletion ...t/java/com/google/api/generator/gapic/composer/defaultvalue/DefaultValueComposerTest.java
View file
Expand Up @@ -161,6 +161,7 @@ public void defaultValue_resourceNameWithOnePattern() {
Expr expr =
DefaultValueComposer.createDefaultValue(
resourceName,
false,
typeStringsToResourceNames.values().stream().collect(Collectors.toList()),
"ignored");
expr.accept(writerVisitor);
Expand All @@ -177,6 +178,7 @@ public void defaultValue_resourceNameWithMultiplePatterns() {
Expr expr =
DefaultValueComposer.createDefaultValue(
resourceName,
false,
typeStringsToResourceNames.values().stream().collect(Collectors.toList()),
"ignored");
expr.accept(writerVisitor);
Expand All @@ -194,6 +196,7 @@ public void defaultValue_resourceNameWithWildcardPattern() {
Expr expr =
DefaultValueComposer.createDefaultValue(
resourceName,
false,
typeStringsToResourceNames.values().stream().collect(Collectors.toList()),
"ignored");
expr.accept(writerVisitor);
Expand All @@ -216,6 +219,7 @@ public void defaultValue_wildcardResourceNameWithOnlyDeletedTopic() {
Expr expr =
DefaultValueComposer.createDefaultValue(
resourceName,
false,
typeStringsToResourceNames.values().stream().collect(Collectors.toList()),
"ignored");
expr.accept(writerVisitor);
Expand All @@ -236,6 +240,7 @@ public void defaultValue_resourceNameWithOnlyWildcards_valueOnly() {
Expr expr =
DefaultValueComposer.createDefaultValueResourceHelper(
resourceName,
false,
Collections.emptyList(),
fallbackField,
/* allowAnonResourceNameClass = */ false);
Expand All @@ -257,7 +262,7 @@ public void defaultValue_resourceNameWithOnlyWildcards_allowAnonResourceNameClas
String fallbackField = "foobar";
Expr expr =
DefaultValueComposer.createDefaultValue(
resourceName, Collections.emptyList(), fallbackField);
resourceName, false, Collections.emptyList(), fallbackField);
expr.accept(writerVisitor);
String expected =
LineFormatter.lines(
Expand Down
13 changes: 13 additions & 0 deletions ...est/java/com/google/api/generator/gapic/composer/grpc/ServiceClientClassComposerTest.java
View file
Expand Up @@ -78,4 +78,17 @@ public void generateServiceClasses_bookshopNameConflicts() {
Path goldenFilePath = Paths.get(Utils.getGoldenDir(this.getClass()), "BookshopClient.golden");
assertCodeEquals(goldenFilePath, visitor.write());
}

@Test
public void generateServiceClasses_childTypeParentInJavadoc() {
GapicContext context = GrpcTestProtoLoader.instance().parseShowcaseMessaging();
Service protoService = context.services().get(0);
GapicClass clazz = ServiceClientClassComposer.instance().generate(context, protoService);

JavaWriterVisitor visitor = new JavaWriterVisitor();
clazz.classDefinition().accept(visitor);
Utils.saveCodegenToFile(this.getClass(), "MessagingClient.golden", visitor.write());
Path goldenFilePath = Paths.get(Utils.getGoldenDir(this.getClass()), "MessagingClient.golden");
assertCodeEquals(goldenFilePath, visitor.write());
}
}

0 comments on commit 6a39c7f

Please sign in to comment.