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 all 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
60 changes: 42 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,57 @@ 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 for the RegionTag. 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
alicejli marked this conversation as resolved.
Show resolved Hide resolved
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 @@ -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
alicejli marked this conversation as resolved.
Show resolved Hide resolved
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);
blakeli0 marked this conversation as resolved.
Show resolved Hide resolved

abstract GapicClass build();
}
}
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 @@ -15,6 +15,7 @@
package com.google.api.generator.gapic.composer;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;

import com.google.api.generator.engine.ast.ClassDefinition;
import com.google.api.generator.engine.ast.ScopeNode;
Expand All @@ -25,11 +26,11 @@
import com.google.api.generator.gapic.model.GapicClass;
import com.google.api.generator.gapic.model.GapicClass.Kind;
import com.google.api.generator.gapic.model.GapicContext;
import com.google.api.generator.gapic.model.RegionTag;
import com.google.api.generator.gapic.model.Sample;
import com.google.api.generator.gapic.model.Service;
import com.google.api.generator.test.framework.Assert;
import com.google.api.generator.test.framework.Utils;
import com.google.common.collect.ImmutableList;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.Arrays;
Expand All @@ -42,8 +43,13 @@ public class ComposerTest {
private final List<GapicClass> clazzes =
Arrays.asList(
GrpcServiceCallableFactoryClassComposer.instance().generate(context, echoProtoService));
private final String protoPackage = context.gapicMetadata().getProtoPackage();
private final List<Sample> samples = clazzes.get(0).samples();
private final Sample sample =
Sample.builder()
.setRegionTag(
RegionTag.builder().setServiceName("serviceName").setRpcName("rpcName").build())
.build();
private List<Sample> ListofSamples = Arrays.asList(new Sample[] {sample});
alicejli marked this conversation as resolved.
Show resolved Hide resolved
private final String protoPackage = echoProtoService.protoPakkage();

@Test
public void gapicClass_addApacheLicense() {
Expand All @@ -65,58 +71,102 @@ public void gapicClass_addApacheLicense() {

@Test
public void composeSamples_showcase() {
for (Sample sample : samples) {
assertEquals(
"File header will be empty before composing samples",
sample.fileHeader(),
ImmutableList.of());
assertEquals(
"ApiShortName will be empty before composing samples",
sample.regionTag().apiShortName(),
"");
assertEquals(
"ApiVersion will be empty before composing samples", sample.regionTag().apiVersion(), "");
}
GapicClass testClass = clazzes.get(0).withSamples(ListofSamples);
List<GapicClass> testClassList = Arrays.asList(new GapicClass[] {testClass});

List<Sample> composedSamples =
Composer.prepareExecutableSamples(clazzes, protoPackage).get(0).samples();
Composer.prepareExecutableSamples(testClassList, protoPackage).get(0).samples();

assertFalse(composedSamples.isEmpty());
for (Sample sample : composedSamples) {
assertEquals(
"File header should be apache",
sample.fileHeader(),
Arrays.asList(CommentComposer.APACHE_LICENSE_COMMENT));
assertEquals(
"ApiShortName should be showcase", sample.regionTag().apiShortName(), "showcase");
assertEquals("ApiVersion should be v1beta1", sample.regionTag().apiVersion(), "v1beta1");
"File header should be APACHE",
Arrays.asList(CommentComposer.APACHE_LICENSE_COMMENT),
sample.fileHeader());
assertEquals("ApiShortName should be empty", "", sample.regionTag().apiShortName());
assertEquals("ApiVersion should be V1beta1", "V1Beta1", sample.regionTag().apiVersion());
}
}

@Test
public void parseDefaultHost_shouldReturnApiShortNameIfHostContainsRegionalEndpoint() {
String defaultHost = "us-east1-pubsub.googleapis.com";
String apiShortName = Composer.parseDefaultHost(defaultHost);
assertEquals("pubsub", apiShortName);
}

@Test
public void parseDefaultHost_shouldReturnApiShortName() {
String defaultHost = "logging.googleapis.com";
String apiShortName = Composer.parseDefaultHost(defaultHost);
assertEquals("logging", apiShortName);
}

@Test
public void parseDefaultHost_shouldReturnApiShortNameForIam() {
String defaultHost = "iam-meta-api.googleapis.com";
String apiShortName = Composer.parseDefaultHost(defaultHost);
assertEquals("iam", apiShortName);
}

@Test
public void parseDefaultHost_shouldReturnHostIfNoPeriods() {
String defaultHost = "logging:7469";
String apiShortName = Composer.parseDefaultHost(defaultHost);
assertEquals("logging:7469", apiShortName);
}

@Test
public void gapicClass_addRegionTagAndHeaderToSample() {
Sample testSample;
testSample = Composer.addRegionTagAndHeaderToSample(sample, "showcase", "v1");
assertEquals("Showcase", testSample.regionTag().apiShortName());
assertEquals("V1", testSample.regionTag().apiVersion());
assertEquals(Arrays.asList(CommentComposer.APACHE_LICENSE_COMMENT), testSample.fileHeader());
}

@Test
public void composeSamples_parseProtoPackage() {

String defaultHost = "accessapproval.googleapis.com:443";
GapicClass testClass = clazzes.get(0).withSamples(ListofSamples).withDefaultHost(defaultHost);
List<GapicClass> testClassList = Arrays.asList(new GapicClass[] {testClass});
String protoPack = "google.cloud.accessapproval.v1";

List<Sample> composedSamples =
Composer.prepareExecutableSamples(clazzes, protoPack).get(0).samples();
Composer.prepareExecutableSamples(testClassList, protoPack).get(0).samples();

// If samples is empty, the test automatically passes without checking.
assertFalse(composedSamples.isEmpty());

for (Sample sample : composedSamples) {
assertEquals(
"ApiShortName should be accessapproval",
"ApiShortName should be Accessapproval",
sample.regionTag().apiShortName(),
"accessapproval");
assertEquals("ApiVersion should be v1", sample.regionTag().apiVersion(), "v1");
"Accessapproval");
assertEquals("ApiVersion should be V1", sample.regionTag().apiVersion(), "V1");
}

protoPack = "google.cloud.vision.v1p1beta1";
composedSamples = Composer.prepareExecutableSamples(clazzes, protoPack).get(0).samples();
defaultHost = "vision.googleapis.com";
testClass = clazzes.get(0).withSamples(ListofSamples).withDefaultHost(defaultHost);
testClassList = Arrays.asList(new GapicClass[] {testClass});
composedSamples = Composer.prepareExecutableSamples(testClassList, protoPack).get(0).samples();
// If samples is empty, the test automatically passes without checking.
assertFalse(composedSamples.isEmpty());

for (Sample sample : composedSamples) {
assertEquals("ApiShortName should be vision", sample.regionTag().apiShortName(), "vision");
assertEquals("ApiVersion should be v1p1beta1", sample.regionTag().apiVersion(), "v1p1beta1");
assertEquals("ApiShortName should be Vision", sample.regionTag().apiShortName(), "Vision");
alicejli marked this conversation as resolved.
Show resolved Hide resolved
assertEquals("ApiVersion should be V1P1Beta1", sample.regionTag().apiVersion(), "V1P1Beta1");
}

protoPack = "google.cloud.vision";
composedSamples = Composer.prepareExecutableSamples(clazzes, protoPack).get(0).samples();
composedSamples = Composer.prepareExecutableSamples(testClassList, protoPack).get(0).samples();
// If samples is empty, the test automatically passes without checking.
assertFalse(composedSamples.isEmpty());

for (Sample sample : composedSamples) {
assertEquals("ApiShortName should be vision", sample.regionTag().apiShortName(), "vision");
assertEquals("ApiShortName should be Vision", sample.regionTag().apiShortName(), "Vision");
assertEquals("ApiVersion should be empty", sample.regionTag().apiVersion(), "");
}
}
Expand Down
Loading