Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: update sample region tag to parse host instead of proto package #1040

Merged
merged 23 commits into from
Oct 13, 2022
Merged
Show file tree
Hide file tree
Changes from 19 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
238ad0a
fix: update sample region tag to parse host instead of proto package
alicejli Sep 15, 2022
d58b78d
update goldens
alicejli Sep 15, 2022
be78a89
update to lowerCamelCase
alicejli Sep 16, 2022
4007abd
use explicit imports
alicejli Sep 16, 2022
037440c
remove setting default host where it's not needed for samples and ref…
alicejli Sep 19, 2022
e21ec74
fix(deps): update dependency com.google.cloud:google-cloud-shared-dep…
renovate-bot Sep 15, 2022
7f4b9b4
chore(deps): update dependency org.apache.maven.plugins:maven-shade-p…
renovate-bot Sep 20, 2022
1439fd8
update iam oneoff
alicejli Sep 20, 2022
88aa879
linter
alicejli Sep 20, 2022
66eab8c
fix(deps): update dependency org.yaml:snakeyaml to v1.33 (#1043)
renovate-bot Sep 26, 2022
ddbc67b
refactor prepareExecutableSamples
alicejli Sep 27, 2022
cc3f4a0
fix imports
alicejli Sep 27, 2022
dc9258a
update name of unit test
alicejli Sep 27, 2022
e50dc03
update comment
alicejli Sep 27, 2022
26776a5
Merge branch 'googleapis:main' into parseHostName
alicejli Sep 28, 2022
6603605
fix(deps): update dependency com.google.cloud:google-cloud-shared-dep…
renovate-bot Oct 3, 2022
8f583d1
fix: Get numeric value for Enum fields if it is configured as query p…
blakeli0 Oct 3, 2022
59d9ffd
refactor unit tests
alicejli Oct 10, 2022
70c84b4
refactor composers
alicejli Oct 10, 2022
fcc1cf3
chore(deps): update dependency com.google.auto.value:auto-value to v1…
renovate-bot Oct 10, 2022
c9578fe
update comment
alicejli Oct 12, 2022
9d8e2b9
fix lint
alicejli Oct 12, 2022
bce120f
Merge branch 'googleapis:main' into parseHostName
alicejli Oct 13, 2022
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
<dependency>
<groupId>com.google.cloud</groupId>
<artifactId>google-cloud-shared-dependencies</artifactId>
<version>3.0.3</version>
<version>3.0.4</version>
<type>pom</type>
<scope>import</scope>
</dependency>
Expand Down
59 changes: 41 additions & 18 deletions src/main/java/com/google/api/generator/gapic/composer/Composer.java
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
package com.google.api.generator.gapic.composer;

import com.google.api.generator.engine.ast.ClassDefinition;
import com.google.api.generator.engine.ast.CommentStatement;
import com.google.api.generator.gapic.composer.comment.CommentComposer;
import com.google.api.generator.gapic.composer.grpc.GrpcServiceCallableFactoryClassComposer;
import com.google.api.generator.gapic.composer.grpc.GrpcServiceStubClassComposer;
Expand All @@ -36,6 +37,8 @@
import com.google.api.generator.gapic.model.Service;
import com.google.api.generator.gapic.model.Transport;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Splitter;
import com.google.common.collect.Iterables;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
Expand Down Expand Up @@ -191,36 +194,56 @@ public static List<GapicClass> generateTestClasses(GapicContext context) {

@VisibleForTesting
static List<GapicClass> prepareExecutableSamples(List<GapicClass> clazzes, String protoPackage) {
// parse protoPackage for apiVersion and apiShortName
// parse protoPackage for apiVersion
String[] pakkage = protoPackage.split("\\.");
String apiVersion;
String apiShortName;
// e.g. v1, v2, v1beta1
if (pakkage[pakkage.length - 1].matches("v[0-9].*")) {
apiVersion = pakkage[pakkage.length - 1];
apiShortName = pakkage[pakkage.length - 2];
} else {
apiVersion = "";
apiShortName = pakkage[pakkage.length - 1];
}
// Include license header, apiShortName, and apiVersion
return clazzes.stream()
.map(
gapicClass -> {
List<Sample> samples =
gapicClass.samples().stream()
.map(
sample -> addRegionTagAndHeaderToSample(sample, apiShortName, apiVersion))
.collect(Collectors.toList());
return gapicClass.withSamples(samples);
})
.collect(Collectors.toList());
// Include license header, apiShortName, and apiVersion
List<GapicClass> clazzesWithSamples = new ArrayList<>();
clazzes.forEach(
gapicClass -> {
List<Sample> samples = new ArrayList<>();
gapicClass
.samples()
.forEach(
sample ->
samples.add(
addRegionTagAndHeaderToSample(
sample, parseDefaultHost(gapicClass.defaultHost()), apiVersion)));
clazzesWithSamples.add(gapicClass.withSamples(samples));
});
return clazzesWithSamples;
}

private static Sample addRegionTagAndHeaderToSample(
// Parse defaultHost for apiShortName. Need to account for regional default endpoints like
// "us-east1-pubsub.googleapis.com".
@VisibleForTesting
protected static String parseDefaultHost(String defaultHost) {
// If the defaultHost is of the format "**.googleapis.com", take the name before the first
// period.
String apiShortName = Iterables.getFirst(Splitter.on(".").split(defaultHost), defaultHost);
// If the defaultHost is of the format "**-**-**.googleapis.com", take the section before the
// first period and after the last dash to follow CSharp's implementation here:
// https://github.com/googleapis/gapic-generator-csharp/blob/main/Google.Api.Generator/Generation/ServiceDetails.cs#L70
apiShortName = Iterables.getLast(Splitter.on("-").split(apiShortName), defaultHost);
// `iam-meta-api` service is an exceptional case and is handled as a one-off
if (defaultHost.contains("iam-meta-api")) {
apiShortName = "iam";
}
return apiShortName;
}

@VisibleForTesting
protected static Sample addRegionTagAndHeaderToSample(
Sample sample, String apiShortName, String apiVersion) {
final List<CommentStatement> header = Arrays.asList(CommentComposer.APACHE_LICENSE_COMMENT);
return sample
.withHeader(Arrays.asList(CommentComposer.APACHE_LICENSE_COMMENT))
.withHeader(header)
.withRegionTag(
sample.regionTag().withApiVersion(apiVersion).withApiShortName(apiShortName));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,8 @@ public GapicClass generate(GapicContext context, Service service) {
.build();

updateGapicMetadata(context, service, className, grpcRpcsToJavaMethodNames);
return GapicClass.create(kind, classDef, SampleComposerUtil.handleDuplicateSamples(samples));
return GapicClass.create(kind, classDef, SampleComposerUtil.handleDuplicateSamples(samples))
.withDefaultHost(service.defaultHost());
}

private static List<AnnotationNode> createClassAnnotations(Service service, TypeStore typeStore) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,8 @@ public GapicClass generate(GapicContext context, Service service) {
.setMethods(createClassMethods(service, typeStore))
.setNestedClasses(Arrays.asList(createNestedBuilderClass(service, typeStore)))
.build();
return GapicClass.create(kind, classDef, SampleComposerUtil.handleDuplicateSamples(samples));
return GapicClass.create(kind, classDef, SampleComposerUtil.handleDuplicateSamples(samples))
.withDefaultHost(service.defaultHost());
}

private static List<CommentStatement> createClassHeaderComments(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,8 @@ public GapicClass generate(GapicContext context, Service service) {
Arrays.asList(createNestedBuilderClass(service, serviceConfig, typeStore)))
.build();
return GapicClass.create(
GapicClass.Kind.STUB, classDef, SampleComposerUtil.handleDuplicateSamples(samples));
GapicClass.Kind.STUB, classDef, SampleComposerUtil.handleDuplicateSamples(samples))
.withDefaultHost(service.defaultHost());
}

protected MethodDefinition createDefaultCredentialsProviderBuilderMethod() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@
import com.google.api.generator.gapic.model.OperationResponse;
import com.google.api.generator.gapic.model.Service;
import com.google.api.generator.gapic.utils.JavaStyle;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.collect.BiMap;
import com.google.common.collect.ImmutableList;
import com.google.protobuf.TypeRegistry;
Expand All @@ -74,6 +75,7 @@
import java.util.stream.Collectors;

public class HttpJsonServiceStubClassComposer extends AbstractTransportServiceStubClassComposer {

private static final HttpJsonServiceStubClassComposer INSTANCE =
new HttpJsonServiceStubClassComposer();

Expand Down Expand Up @@ -940,9 +942,11 @@ private Expr createFieldsExtractorClassInstance(
for (int i = 0; i < descendantFields.length; i++) {
String currFieldName = descendantFields[i];
String bindingFieldMethodName =
(i < descendantFields.length - 1 || !httpBindingFieldName.isRepeated())
? String.format("get%s", JavaStyle.toUpperCamelCase(currFieldName))
: String.format("get%sList", JavaStyle.toUpperCamelCase(currFieldName));
getBindingFieldMethodName(
httpBindingFieldName,
descendantFields.length,
i,
JavaStyle.toUpperCamelCase(currFieldName));
requestFieldGetterExprBuilder =
requestFieldGetterExprBuilder.setMethodName(bindingFieldMethodName);

Expand Down Expand Up @@ -997,6 +1001,7 @@ private Expr createFieldsExtractorClassInstance(
}
}

// Add a fixed query param for numeric enum, see b/232457244 for details
if (restNumericEnumsEnabled && serializerMethodName.equals("putQueryParam")) {
ImmutableList.Builder<Expr> paramsPutArgs = ImmutableList.builder();

Expand All @@ -1023,6 +1028,20 @@ private Expr createFieldsExtractorClassInstance(
.build();
}

@VisibleForTesting
String getBindingFieldMethodName(
HttpBinding httpBindingField, int descendantFieldsLengths, int index, String currFieldName) {
if (index == descendantFieldsLengths - 1) {
if (httpBindingField.isRepeated()) {
return String.format("get%sList", currFieldName);
}
if (httpBindingField.isEnum()) {
return String.format("get%sValue", currFieldName);
}
}
return String.format("get%s", currFieldName);
}

private List<Expr> getHttpMethodTypeExpr(Method protoMethod) {
return Collections.singletonList(
ValueExpr.withValue(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,7 @@ public static Optional<Sample> composeSettingsSample(
.map(e -> ExprStatement.withExpr(e))
.collect(Collectors.toList());

// TODO: alicejli edit RegionTag to match other languages
RegionTag regionTag =
RegionTag.builder()
.setServiceName(classType.reference().name())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,10 @@ public enum Kind {

public abstract List<Sample> samples();

// Represents the host URL for the service. May or may not contain a regional endpoint. Only used
// for generating the region tag for samples; therefore only used in select Composers.
public abstract String defaultHost();

public static GapicClass create(Kind kind, ClassDefinition classDefinition) {
return builder().setKind(kind).setClassDefinition(classDefinition).build();
}
Expand All @@ -45,7 +49,9 @@ public static GapicClass create(
}

static Builder builder() {
return new AutoValue_GapicClass.Builder().setSamples(Collections.emptyList());
return new AutoValue_GapicClass.Builder()
.setSamples(Collections.emptyList())
.setDefaultHost("");
}

abstract Builder toBuilder();
Expand All @@ -54,6 +60,10 @@ public final GapicClass withSamples(List<Sample> samples) {
return toBuilder().setSamples(samples).build();
}

public final GapicClass withDefaultHost(String defaultHost) {
return toBuilder().setDefaultHost(defaultHost).build();
}

@AutoValue.Builder
abstract static class Builder {
abstract Builder setKind(Kind kind);
Expand All @@ -62,6 +72,8 @@ abstract static class Builder {

abstract Builder setSamples(List<Sample> samples);

abstract Builder setDefaultHost(String defaultHost);

abstract GapicClass build();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -36,21 +36,54 @@ public enum HttpVerb {

@AutoValue
public abstract static class HttpBinding implements Comparable<HttpBinding> {

// The fully qualified name of the field. e.g. request.complex_object.another_object.name
public abstract String name();

abstract String lowerCamelName();

public abstract boolean isOptional();
// An object that contains all info of the leaf level field
@Nullable
public abstract Field field();

public abstract boolean isRepeated();
public boolean isOptional() {
return field() != null && field().isProto3Optional();
}

public boolean isRepeated() {
return field() != null && field().isRepeated();
}

public boolean isEnum() {
return field() != null && field().isEnum();
}

@Nullable
public abstract String valuePattern();

public static HttpBinding create(
String name, boolean isOptional, boolean isRepeated, String valuePattern) {
return new AutoValue_HttpBindings_HttpBinding(
name, JavaStyle.toLowerCamelCase(name), isOptional, isRepeated, valuePattern);
public static HttpBindings.HttpBinding.Builder builder() {
return new AutoValue_HttpBindings_HttpBinding.Builder();
}

@AutoValue.Builder
public abstract static class Builder {

public abstract HttpBindings.HttpBinding.Builder setName(String name);

public abstract HttpBindings.HttpBinding.Builder setField(Field field);

abstract HttpBindings.HttpBinding.Builder setLowerCamelName(String lowerCamelName);

public abstract HttpBindings.HttpBinding.Builder setValuePattern(String valuePattern);

abstract String name();

abstract HttpBindings.HttpBinding autoBuild();

public HttpBindings.HttpBinding build() {
setLowerCamelName(JavaStyle.toLowerCamelCase(name()));
return autoBuild();
}
}

// Do not forget to keep it in sync with equals() implementation.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import com.google.auto.value.AutoValue;
import com.google.common.base.Preconditions;

// TODO: alicejli edit RegionTag to match other languages
/**
* This model represents a code sample region tag. Matching region start and end region tag comments
* are used to determine the boundaries of code snippets to be used in documentation.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,11 @@ private static HttpBindings parseHttpRuleHelper(
Map<String, String> patternSampleValues = constructPathValuePatterns(pattern);

// TODO: support nested message fields bindings
// Nested message fields bindings for query params are already supported as part of
// https://github.com/googleapis/gax-java/pull/1784,
// however we need to excludes fields that are already configured for path params and body, see
// https://github.com/googleapis/googleapis/blob/532289228eaebe77c42438f74b8a5afa85fee1b6/google/api/http.proto#L208 for details,
// the current logic does not exclude fields that are more than one level deep.
String body = httpRule.getBody();
Set<String> bodyParamNames;
Set<String> queryParamNames;
Expand Down Expand Up @@ -133,8 +138,9 @@ private static Set<HttpBinding> validateAndConstructHttpBindings(
String patternSampleValue =
patternSampleValues != null ? patternSampleValues.get(paramName) : null;
String[] subFields = paramName.split("\\.");
HttpBinding.Builder httpBindingBuilder = HttpBinding.builder().setName(paramName);
if (inputMessage == null) {
httpBindings.add(HttpBinding.create(paramName, false, false, patternSampleValue));
httpBindings.add(httpBindingBuilder.setValuePattern(patternSampleValue).build());
continue;
}
Message nestedMessage = inputMessage;
Expand All @@ -156,8 +162,7 @@ private static Set<HttpBinding> validateAndConstructHttpBindings(
}
Field field = nestedMessage.fieldMap().get(subFieldName);
httpBindings.add(
HttpBinding.create(
paramName, field.isProto3Optional(), field.isRepeated(), patternSampleValue));
httpBindingBuilder.setValuePattern(patternSampleValue).setField(field).build());
}
}
}
Expand Down
Loading