Skip to content

Commit

Permalink
refactor: expose parsed api short name and version as fields in Servi…
Browse files Browse the repository at this point in the history 
…ce (#1075)

* Exposes parsed apiShortName and apiVersion as fields in Service.java, since the source fields (defaultHost and protoPakkage) are both defined per-service. 

* Adds these two fields correspondingly to GapicClass so that it can be composed from Service given these two upstream fields, then replaces withDefaultHost (for building GapicClass) withApiShortName and withApiVersion. (Composers and tests where this is currently used for sample generation are updated)

* This change will enable Spring Codegen (when eventually split out from this repo) to reuse this parsing logic in descriptive comments and metadata. It also moves the parsing logic to earlier in the parse-compose process.
  • Loading branch information
@emmileaf
emmileaf committed Nov 3, 2022
1 parent 6774245 commit 2ebe948
Show file tree
Hide file tree
Showing 12 changed files with 238 additions and 106 deletions.
37 changes: 3 additions & 34 deletions src/main/java/com/google/api/generator/gapic/composer/Composer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,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 All @@ -50,8 +48,7 @@ public static List<GapicClass> composeServiceClasses(GapicContext context) {
clazzes.addAll(generateServiceClasses(context));
clazzes.addAll(generateMockClasses(context, context.mixinServices()));
clazzes.addAll(generateResourceNameHelperClasses(context));
return addApacheLicense(
prepareExecutableSamples(clazzes, context.gapicMetadata().getProtoPackage()));
return addApacheLicense(prepareExecutableSamples(clazzes));
}

public static GapicPackageInfo composePackageInfo(GapicContext context) {
Expand Down Expand Up @@ -193,16 +190,7 @@ public static List<GapicClass> generateTestClasses(GapicContext context) {
}

@VisibleForTesting
static List<GapicClass> prepareExecutableSamples(List<GapicClass> clazzes, String protoPackage) {
// parse protoPackage for apiVersion
String[] pakkage = protoPackage.split("\\.");
String apiVersion;
// e.g. v1, v2, v1beta1
if (pakkage[pakkage.length - 1].matches("v[0-9].*")) {
apiVersion = pakkage[pakkage.length - 1];
} else {
apiVersion = "";
}
static List<GapicClass> prepareExecutableSamples(List<GapicClass> clazzes) {
// Include license header, apiShortName, and apiVersion
List<GapicClass> clazzesWithSamples = new ArrayList<>();
clazzes.forEach(
Expand All @@ -214,31 +202,12 @@ static List<GapicClass> prepareExecutableSamples(List<GapicClass> clazzes, Strin
sample ->
samples.add(
addRegionTagAndHeaderToSample(
sample, parseDefaultHost(gapicClass.defaultHost()), apiVersion)));
sample, gapicClass.apiShortName(), gapicClass.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;
}

@VisibleForTesting
protected static Sample addRegionTagAndHeaderToSample(
Sample sample, String apiShortName, String apiVersion) {
Expand Down
3 changes: 2 additions & 1 deletion ...va/com/google/api/generator/gapic/composer/common/AbstractServiceClientClassComposer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,8 @@ public GapicClass generate(GapicContext context, Service service) {

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

private static List<AnnotationNode> createClassAnnotations(Service service, TypeStore typeStore) {
Expand Down
3 changes: 2 additions & 1 deletion .../com/google/api/generator/gapic/composer/common/AbstractServiceSettingsClassComposer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,8 @@ public GapicClass generate(GapicContext context, Service service) {
.setNestedClasses(Arrays.asList(createNestedBuilderClass(service, typeStore)))
.build();
return GapicClass.create(kind, classDef, SampleComposerUtil.handleDuplicateSamples(samples))
.withDefaultHost(service.defaultHost());
.withApiShortName(service.apiShortName())
.withApiVersion(service.apiVersion());
}

private static List<CommentStatement> createClassHeaderComments(
Expand Down
3 changes: 2 additions & 1 deletion .../google/api/generator/gapic/composer/common/AbstractServiceStubSettingsClassComposer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,8 @@ public GapicClass generate(GapicContext context, Service service) {
.build();
return GapicClass.create(
GapicClass.Kind.STUB, classDef, SampleComposerUtil.handleDuplicateSamples(samples))
.withDefaultHost(service.defaultHost());
.withApiShortName(service.apiShortName())
.withApiVersion(service.apiVersion());
}

protected MethodDefinition createDefaultCredentialsProviderBuilderMethod() {
Expand Down
23 changes: 16 additions & 7 deletions src/main/java/com/google/api/generator/gapic/model/GapicClass.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,11 @@ 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();
// Only used for generating the region tag for samples; therefore only used in select Composers.
public abstract String apiShortName();

// Only used for generating the region tag for samples; therefore only used in select Composers.
public abstract String apiVersion();

public static GapicClass create(Kind kind, ClassDefinition classDefinition) {
return builder().setKind(kind).setClassDefinition(classDefinition).build();
Expand All @@ -51,7 +53,8 @@ public static GapicClass create(
static Builder builder() {
return new AutoValue_GapicClass.Builder()
.setSamples(Collections.emptyList())
.setDefaultHost("");
.setApiShortName("")
.setApiVersion("");
}

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

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

public final GapicClass withApiVersion(String apiVersion) {
return toBuilder().setApiVersion(apiVersion).build();
}

@AutoValue.Builder
Expand All @@ -72,7 +79,9 @@ abstract static class Builder {

abstract Builder setSamples(List<Sample> samples);

abstract Builder setDefaultHost(String defaultHost);
abstract Builder setApiShortName(String apiShortName);

abstract Builder setApiVersion(String apiVersion);

abstract GapicClass build();
}
Expand Down
47 changes: 47 additions & 0 deletions src/main/java/com/google/api/generator/gapic/model/Service.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,10 @@

import com.google.api.generator.engine.ast.TypeNode;
import com.google.auto.value.AutoValue;
import com.google.common.base.Splitter;
import com.google.common.base.Strings;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.Iterables;
import java.util.List;
import javax.annotation.Nullable;

Expand Down Expand Up @@ -50,6 +52,20 @@ public boolean hasDescription() {
return !Strings.isNullOrEmpty(description());
}

public String apiShortName() {
if (!Strings.isNullOrEmpty(defaultHost())) {
return parseApiShortName(defaultHost());
}
return "";
}

public String apiVersion() {
if (!Strings.isNullOrEmpty(protoPakkage())) {
return parseApiVersion(protoPakkage());
}
return "";
}

public Method operationPollingMethod() {
for (Method method : methods()) {
if (method.isOperationPollingMethod()) {
Expand Down Expand Up @@ -127,4 +143,35 @@ public abstract static class Builder {

public abstract Service build();
}

private static String parseApiVersion(String protoPackage) {
// parse protoPackage for apiVersion
String[] pakkage = protoPackage.split("\\.");
String apiVersion;
// e.g. v1, v2, v1beta1
if (pakkage[pakkage.length - 1].matches("v[0-9].*")) {
apiVersion = pakkage[pakkage.length - 1];
} else {
apiVersion = "";
}
return apiVersion;
}

// Parse defaultHost for apiShortName for the RegionTag. Need to account for regional default
// endpoints like
// "us-east1-pubsub.googleapis.com".
private static String parseApiShortName(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;
}
}
74 changes: 33 additions & 41 deletions src/test/java/com/google/api/generator/gapic/composer/ComposerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,14 +42,16 @@ public class ComposerTest {
private final Service echoProtoService = context.services().get(0);
private final List<GapicClass> clazzes =
Arrays.asList(
GrpcServiceCallableFactoryClassComposer.instance().generate(context, echoProtoService));
GrpcServiceCallableFactoryClassComposer.instance()
.generate(context, echoProtoService)
.withApiShortName(echoProtoService.apiShortName())
.withApiVersion(echoProtoService.apiVersion()));
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();

@Test
public void gapicClass_addApacheLicense() {
Expand All @@ -75,47 +77,22 @@ public void composeSamples_showcase() {
List<GapicClass> testClassList = Arrays.asList(new GapicClass[] {testClass});

List<Sample> composedSamples =
Composer.prepareExecutableSamples(testClassList, protoPackage).get(0).samples();
Composer.prepareExecutableSamples(testClassList).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());
assertEquals(
"ApiShortName should be Localhost7469",
"Localhost7469",
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;
Expand All @@ -129,12 +106,12 @@ public void gapicClass_addRegionTagAndHeaderToSample() {
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";

Service testService =
echoProtoService.toBuilder().setDefaultHost(defaultHost).setProtoPakkage(protoPack).build();
List<GapicClass> testClassList = getTestClassListFromService(testService);
List<Sample> composedSamples =
Composer.prepareExecutableSamples(testClassList, protoPack).get(0).samples();
Composer.prepareExecutableSamples(testClassList).get(0).samples();

// If samples is empty, the test automatically passes without checking.
assertFalse(composedSamples.isEmpty());
Expand All @@ -149,9 +126,10 @@ public void composeSamples_parseProtoPackage() {

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();
testService =
testService.toBuilder().setDefaultHost(defaultHost).setProtoPakkage(protoPack).build();
testClassList = getTestClassListFromService(testService);
composedSamples = Composer.prepareExecutableSamples(testClassList).get(0).samples();
// If samples is empty, the test automatically passes without checking.
assertFalse(composedSamples.isEmpty());

Expand All @@ -161,7 +139,10 @@ public void composeSamples_parseProtoPackage() {
}

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

Expand All @@ -170,4 +151,15 @@ public void composeSamples_parseProtoPackage() {
assertEquals("ApiVersion should be empty", sample.regionTag().apiVersion(), "");
}
}

private List<GapicClass> getTestClassListFromService(Service testService) {
GapicClass testClass =
GrpcServiceCallableFactoryClassComposer.instance()
.generate(context, testService)
.withSamples(ListofSamples)
.withApiShortName(testService.apiShortName())
.withApiVersion(testService.apiVersion());
List<GapicClass> testClassList = Arrays.asList(new GapicClass[] {testClass});
return testClassList;
}
}
Loading

0 comments on commit 2ebe948

Please sign in to comment.