Skip to content

Commit 6a39c7f

Browse files
meltsufinmeltsufin
authored
fix: Use parent type instead of child_type in method doc sample (#862)
Fixes: #852.
1 parent 64d8374 commit 6a39c7f

20 files changed

Lines changed: 2114 additions & 133 deletions

File tree

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

Lines changed: 36 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@
3737
import com.google.api.generator.gapic.model.ResourceName;
3838
import com.google.api.generator.gapic.utils.JavaStyle;
3939
import com.google.api.generator.gapic.utils.ResourceNameConstants;
40+
import com.google.api.generator.gapic.utils.ResourceReferenceUtils;
4041
import com.google.common.annotations.VisibleForTesting;
4142
import com.google.common.base.Preconditions;
4243
import com.google.longrunning.Operation;
@@ -48,6 +49,7 @@
4849
import java.util.HashSet;
4950
import java.util.List;
5051
import java.util.Map;
52+
import java.util.Optional;
5153
import java.util.stream.Collectors;
5254

5355
public class DefaultValueComposer {
@@ -76,6 +78,7 @@ public static Expr createDefaultValue(
7678
Expr defValue =
7779
createDefaultValue(
7880
resourceName,
81+
methodArg.field().resourceReference().isChildType(),
7982
resourceNames.values().stream().collect(Collectors.toList()),
8083
methodArg.field().name());
8184

@@ -175,17 +178,46 @@ static Expr createDefaultValue(Field f, boolean useExplicitInitTypeInGenerics) {
175178
}
176179

177180
public static Expr createDefaultValue(
178-
ResourceName resourceName, List<ResourceName> resnames, String fieldOrMessageName) {
179-
return createDefaultValueResourceHelper(resourceName, resnames, fieldOrMessageName, true);
181+
ResourceName resourceName,
182+
boolean isChildType,
183+
List<ResourceName> resnames,
184+
String fieldOrMessageName) {
185+
return createDefaultValueResourceHelper(
186+
resourceName, isChildType, resnames, fieldOrMessageName, true);
187+
}
188+
189+
private static Optional<ResourceName> findParentResource(
190+
ResourceName childResource, List<ResourceName> resourceNames) {
191+
Map<String, ResourceName> patternToResourceName = new HashMap<>();
192+
193+
for (ResourceName resourceName : resourceNames) {
194+
for (String parentPattern : resourceName.patterns()) {
195+
patternToResourceName.put(parentPattern, resourceName);
196+
}
197+
}
198+
199+
for (String childPattern : childResource.patterns()) {
200+
Optional<String> parentPattern = ResourceReferenceUtils.parseParentPattern(childPattern);
201+
if (parentPattern.isPresent() && patternToResourceName.containsKey(parentPattern.get())) {
202+
return Optional.of(patternToResourceName.get(parentPattern.get()));
203+
}
204+
}
205+
206+
return Optional.empty();
180207
}
181208

182209
@VisibleForTesting
183210
static Expr createDefaultValueResourceHelper(
184211
ResourceName resourceName,
212+
boolean isChildType,
185213
List<ResourceName> resnames,
186214
String fieldOrMessageName,
187215
boolean allowAnonResourceNameClass) {
188216

217+
if (isChildType) {
218+
resourceName = findParentResource(resourceName, resnames).orElse(resourceName);
219+
}
220+
189221
boolean hasOnePattern = resourceName.patterns().size() == 1;
190222
if (resourceName.isOnlyWildcard()) {
191223
List<ResourceName> unexaminedResnames = new ArrayList<>(resnames);
@@ -195,7 +227,7 @@ static Expr createDefaultValueResourceHelper(
195227
continue;
196228
}
197229
unexaminedResnames.remove(resname);
198-
return createDefaultValue(resname, unexaminedResnames, fieldOrMessageName);
230+
return createDefaultValue(resname, false, unexaminedResnames, fieldOrMessageName);
199231
}
200232

201233
if (unexaminedResnames.isEmpty()) {
@@ -283,6 +315,7 @@ public static Expr createSimpleMessageBuilderExpr(
283315
defaultExpr =
284316
createDefaultValueResourceHelper(
285317
resourceNames.get(field.resourceReference().resourceTypeString()),
318+
field.resourceReference().isChildType(),
286319
resourceNames.values().stream().collect(Collectors.toList()),
287320
message.name(),
288321
/* allowAnonResourceNameClass = */ false);

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1293,6 +1293,7 @@ private static List<Expr> createRpcMethodArgumentDefaultValueExprs(
12931293
.setExprReferenceExpr(
12941294
DefaultValueComposer.createDefaultValue(
12951295
resourceNames.get(arg.field().resourceReference().resourceTypeString()),
1296+
arg.field().resourceReference().isChildType(),
12961297
resourceNameList,
12971298
arg.field().name()))
12981299
.setMethodName("toString")

‎src/main/java/com/google/api/generator/gapic/protoparser/Parser.java‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -881,7 +881,9 @@ static String parsePageSizeFieldName(
881881
// with monolith-gnerated libraries.
882882
String pagedFieldName = null;
883883

884-
if (inputMessage.fieldMap().containsKey("page_token")
884+
if (inputMessage != null
885+
&& inputMessage.fieldMap().containsKey("page_token")
886+
&& outputMessage != null
885887
&& outputMessage.fieldMap().containsKey("next_page_token")) {
886888
// List of potential field names representing page size.
887889
// page_size gets priority over max_results if both are present

‎src/main/java/com/google/api/generator/gapic/protoparser/ResourceReferenceParser.java‎

Lines changed: 2 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717
import com.google.api.generator.gapic.model.ResourceName;
1818
import com.google.api.generator.gapic.model.ResourceReference;
1919
import com.google.api.generator.gapic.utils.JavaStyle;
20-
import com.google.api.generator.gapic.utils.ResourceNameConstants;
20+
import com.google.api.generator.gapic.utils.ResourceReferenceUtils;
2121
import com.google.api.pathtemplate.PathTemplate;
2222
import com.google.common.annotations.VisibleForTesting;
2323
import com.google.common.base.Preconditions;
@@ -108,7 +108,7 @@ static Optional<ResourceName> parseParentResourceName(
108108
String resourceTypeString,
109109
@Nullable String description,
110110
Map<String, ResourceName> patternsToResourceNames) {
111-
Optional<String> parentPatternOpt = parseParentPattern(pattern);
111+
Optional<String> parentPatternOpt = ResourceReferenceUtils.parseParentPattern(pattern);
112112
if (!parentPatternOpt.isPresent()) {
113113
return Optional.empty();
114114
}
@@ -178,31 +178,6 @@ static Optional<ResourceName> parseParentResourceName(
178178
return Optional.of(parentResourceName);
179179
}
180180

181-
@VisibleForTesting
182-
static Optional<String> parseParentPattern(String pattern) {
183-
String[] tokens = pattern.split(SLASH);
184-
String lastToken = tokens[tokens.length - 1];
185-
if (lastToken.equals(ResourceNameConstants.DELETED_TOPIC_LITERAL)
186-
|| lastToken.equals(ResourceNameConstants.WILDCARD_PATTERN)) {
187-
return Optional.empty();
188-
}
189-
190-
int lastTokenIndex = tokens.length - 2;
191-
int minLengthWithParent = 4;
192-
// Singleton patterns, e.g. projects/{project}/agent.
193-
if (!lastToken.contains("{")) {
194-
minLengthWithParent = 3;
195-
lastTokenIndex = tokens.length - 1;
196-
}
197-
198-
// No fully-formed parent. Expected: ancestors/{ancestor}/childNodes/{child_node}.
199-
if (tokens.length < minLengthWithParent) {
200-
return Optional.empty();
201-
}
202-
203-
return Optional.of(String.join(SLASH, Arrays.asList(tokens).subList(0, lastTokenIndex)));
204-
}
205-
206181
@VisibleForTesting
207182
static String resolvePackages(String resourceNamePackage, String servicePackage) {
208183
return resourceNamePackage.contains(servicePackage) ? resourceNamePackage : servicePackage;

‎src/main/java/com/google/api/generator/gapic/utils/ResourceReferenceUtils.java‎

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
// Copyright 2021 Google LLC
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
package com.google.api.generator.gapic.utils;
16+
17+
import java.util.Arrays;
18+
import java.util.Optional;
19+
20+
public final class ResourceReferenceUtils {
21+
22+
private static final String SLASH = "/";
23+
24+
private ResourceReferenceUtils() {}
25+
26+
public static Optional<String> parseParentPattern(String pattern) {
27+
String[] tokens = pattern.split(SLASH);
28+
String lastToken = tokens[tokens.length - 1];
29+
if (lastToken.equals(ResourceNameConstants.DELETED_TOPIC_LITERAL)
30+
|| lastToken.equals(ResourceNameConstants.WILDCARD_PATTERN)) {
31+
return Optional.empty();
32+
}
33+
34+
int lastTokenIndex = tokens.length - 2;
35+
int minLengthWithParent = 4;
36+
// Singleton patterns, e.g. projects/{project}/agent.
37+
if (!lastToken.contains("{")) {
38+
minLengthWithParent = 3;
39+
lastTokenIndex = tokens.length - 1;
40+
}
41+
42+
// No fully-formed parent. Expected: ancestors/{ancestor}/childNodes/{child_node}.
43+
if (tokens.length < minLengthWithParent) {
44+
return Optional.empty();
45+
}
46+
47+
return Optional.of(String.join(SLASH, Arrays.asList(tokens).subList(0, lastTokenIndex)));
48+
}
49+
}

‎src/test/java/com/google/api/generator/gapic/composer/common/TestProtoLoader.java‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@
3737
import com.google.pubsub.v1.PubsubProto;
3838
import com.google.showcase.v1beta1.EchoOuterClass;
3939
import com.google.showcase.v1beta1.IdentityOuterClass;
40+
import com.google.showcase.v1beta1.MessagingOuterClass;
4041
import com.google.showcase.v1beta1.TestingOuterClass;
4142
import com.google.testdata.v1.DeprecatedServiceOuterClass;
4243
import google.cloud.CommonResources;
@@ -51,6 +52,7 @@
5152
import java.util.Set;
5253

5354
public class TestProtoLoader {
55+
5456
private static final TestProtoLoader INSTANCE =
5557
new TestProtoLoader(Transport.GRPC, "src/test/java/com/google/api/generator/gapic/testdata/");
5658
private final String testFilesDirectory;
@@ -170,6 +172,27 @@ public GapicContext parseShowcaseIdentity() {
170172
.build();
171173
}
172174

175+
public GapicContext parseShowcaseMessaging() {
176+
FileDescriptor fileDescriptor = MessagingOuterClass.getDescriptor();
177+
ServiceDescriptor messagingService = fileDescriptor.getServices().get(0);
178+
assertEquals(messagingService.getName(), "Messaging");
179+
180+
Map<String, Message> messageTypes = Parser.parseMessages(fileDescriptor);
181+
Map<String, ResourceName> resourceNames = Parser.parseResourceNames(fileDescriptor);
182+
Set<ResourceName> outputResourceNames = new HashSet<>();
183+
List<Service> services =
184+
Parser.parseService(
185+
fileDescriptor, messageTypes, resourceNames, Optional.empty(), outputResourceNames);
186+
187+
return GapicContext.builder()
188+
.setMessages(messageTypes)
189+
.setResourceNames(resourceNames)
190+
.setServices(services)
191+
.setHelperResourceNames(outputResourceNames)
192+
.setTransport(transport)
193+
.build();
194+
}
195+
173196
public GapicContext parseShowcaseTesting() {
174197
FileDescriptor testingFileDescriptor = TestingOuterClass.getDescriptor();
175198
ServiceDescriptor testingService = testingFileDescriptor.getServices().get(0);

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

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -161,6 +161,7 @@ public void defaultValue_resourceNameWithOnePattern() {
161161
Expr expr =
162162
DefaultValueComposer.createDefaultValue(
163163
resourceName,
164+
false,
164165
typeStringsToResourceNames.values().stream().collect(Collectors.toList()),
165166
"ignored");
166167
expr.accept(writerVisitor);
@@ -177,6 +178,7 @@ public void defaultValue_resourceNameWithMultiplePatterns() {
177178
Expr expr =
178179
DefaultValueComposer.createDefaultValue(
179180
resourceName,
181+
false,
180182
typeStringsToResourceNames.values().stream().collect(Collectors.toList()),
181183
"ignored");
182184
expr.accept(writerVisitor);
@@ -194,6 +196,7 @@ public void defaultValue_resourceNameWithWildcardPattern() {
194196
Expr expr =
195197
DefaultValueComposer.createDefaultValue(
196198
resourceName,
199+
false,
197200
typeStringsToResourceNames.values().stream().collect(Collectors.toList()),
198201
"ignored");
199202
expr.accept(writerVisitor);
@@ -216,6 +219,7 @@ public void defaultValue_wildcardResourceNameWithOnlyDeletedTopic() {
216219
Expr expr =
217220
DefaultValueComposer.createDefaultValue(
218221
resourceName,
222+
false,
219223
typeStringsToResourceNames.values().stream().collect(Collectors.toList()),
220224
"ignored");
221225
expr.accept(writerVisitor);
@@ -236,6 +240,7 @@ public void defaultValue_resourceNameWithOnlyWildcards_valueOnly() {
236240
Expr expr =
237241
DefaultValueComposer.createDefaultValueResourceHelper(
238242
resourceName,
243+
false,
239244
Collections.emptyList(),
240245
fallbackField,
241246
/* allowAnonResourceNameClass = */ false);
@@ -257,7 +262,7 @@ public void defaultValue_resourceNameWithOnlyWildcards_allowAnonResourceNameClas
257262
String fallbackField = "foobar";
258263
Expr expr =
259264
DefaultValueComposer.createDefaultValue(
260-
resourceName, Collections.emptyList(), fallbackField);
265+
resourceName, false, Collections.emptyList(), fallbackField);
261266
expr.accept(writerVisitor);
262267
String expected =
263268
LineFormatter.lines(

‎src/test/java/com/google/api/generator/gapic/composer/grpc/ServiceClientClassComposerTest.java‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,4 +78,17 @@ public void generateServiceClasses_bookshopNameConflicts() {
7878
Path goldenFilePath = Paths.get(Utils.getGoldenDir(this.getClass()), "BookshopClient.golden");
7979
assertCodeEquals(goldenFilePath, visitor.write());
8080
}
81+
82+
@Test
83+
public void generateServiceClasses_childTypeParentInJavadoc() {
84+
GapicContext context = GrpcTestProtoLoader.instance().parseShowcaseMessaging();
85+
Service protoService = context.services().get(0);
86+
GapicClass clazz = ServiceClientClassComposer.instance().generate(context, protoService);
87+
88+
JavaWriterVisitor visitor = new JavaWriterVisitor();
89+
clazz.classDefinition().accept(visitor);
90+
Utils.saveCodegenToFile(this.getClass(), "MessagingClient.golden", visitor.write());
91+
Path goldenFilePath = Paths.get(Utils.getGoldenDir(this.getClass()), "MessagingClient.golden");
92+
assertCodeEquals(goldenFilePath, visitor.write());
93+
}
8194
}

0 commit comments

Comments
 (0)