Skip to content

Commit

Permalink
Revert "fix: update sample region tag to parse host instead of proto …
Browse files Browse the repository at this point in the history
…package (#1040)"

This reverts commit eb94f30.
  • Loading branch information
emmileaf committed Nov 3, 2022
1 parent 4f0756f commit ee1fc62
Show file tree
Hide file tree
Showing 233 changed files with 510 additions and 650 deletions.
60 changes: 18 additions & 42 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,7 +15,6 @@
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 @@ -37,8 +36,6 @@
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 @@ -194,57 +191,36 @@ public static List<GapicClass> generateTestClasses(GapicContext context) {

@VisibleForTesting
static List<GapicClass> prepareExecutableSamples(List<GapicClass> clazzes, String protoPackage) {
// parse protoPackage for apiVersion
// parse protoPackage for apiVersion and apiShortName
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
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;
}

// 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
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;
// 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());
}

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

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

private static List<AnnotationNode> createClassAnnotations(Service service, TypeStore typeStore) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -126,8 +126,7 @@ 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))
.withDefaultHost(service.defaultHost());
return GapicClass.create(kind, classDef, SampleComposerUtil.handleDuplicateSamples(samples));
}

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

protected MethodDefinition createDefaultCredentialsProviderBuilderMethod() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,6 @@ 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,10 +35,6 @@ 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 @@ -49,9 +45,7 @@ public static GapicClass create(
}

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

abstract Builder toBuilder();
Expand All @@ -60,10 +54,6 @@ 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 @@ -72,8 +62,6 @@ 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 @@ -20,7 +20,6 @@
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,7 +15,6 @@
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 @@ -26,11 +25,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 @@ -43,13 +42,8 @@ public class ComposerTest {
private final List<GapicClass> clazzes =
Arrays.asList(
GrpcServiceCallableFactoryClassComposer.instance().generate(context, echoProtoService));
private final Sample sample =
Sample.builder()
.setRegionTag(
RegionTag.builder().setServiceName("serviceName").setRpcName("rpcName").build())
.build();
private List<Sample> ListofSamples = Arrays.asList(new Sample[] {sample});
private final String protoPackage = echoProtoService.protoPakkage();
private final String protoPackage = context.gapicMetadata().getProtoPackage();
private final List<Sample> samples = clazzes.get(0).samples();

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

@Test
public void composeSamples_showcase() {
GapicClass testClass = clazzes.get(0).withSamples(ListofSamples);
List<GapicClass> testClassList = Arrays.asList(new GapicClass[] {testClass});
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(), "");
}

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

assertFalse(composedSamples.isEmpty());
for (Sample sample : composedSamples) {
assertEquals(
"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());
"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");
}
}

@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(testClassList, protoPack).get(0).samples();

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

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";
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());

composedSamples = Composer.prepareExecutableSamples(clazzes, protoPack).get(0).samples();
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");
assertEquals("ApiVersion should be v1p1beta1", sample.regionTag().apiVersion(), "v1p1beta1");
}

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

composedSamples = Composer.prepareExecutableSamples(clazzes, protoPack).get(0).samples();
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

0 comments on commit ee1fc62

Please sign in to comment.