Skip to content

Commit

Permalink
feat: Parser to consume the api-versioning value from proto (#2630)
Browse files Browse the repository at this point in the history
This is part one to resolve b/330980553, this pr allows read and parse
api versioning from proto to service model.

Changes included in this pr:
- rename current apiVersion to packageVersion. This packageVersion is
parsed from package name and exposed in #1075. It was intended to be
used in Spring codegen, but not actually used. It is used in generating
sample region tags (see details
[9ec3531](9ec3531))
- exposing apiVersion from Service model.
- Parser.java to parse apiVersion from `ClientProto.apiVersion` to
service.
  • Loading branch information
zhumin8 committed Apr 24, 2024
1 parent 463801c commit 40711fd
Show file tree
Hide file tree
Showing 15 changed files with 421 additions and 39 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,7 @@ static List<GapicClass> prepareExecutableSamples(List<GapicClass> clazzes) {
sample ->
samples.add(
addRegionTagAndHeaderToSample(
sample, gapicClass.apiShortName(), gapicClass.apiVersion())));
sample, gapicClass.apiShortName(), gapicClass.packageVersion())));
clazzesWithSamples.add(gapicClass.withSamples(samples));
});
return clazzesWithSamples;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ public GapicClass generate(GapicContext context, Service service) {
updateGapicMetadata(context, service, className, grpcRpcsToJavaMethodNames);
return GapicClass.create(kind, classDef, SampleComposerUtil.handleDuplicateSamples(samples))
.withApiShortName(service.apiShortName())
.withApiVersion(service.apiVersion());
.withPackageVersion(service.packageVersion());
}

private static List<AnnotationNode> createClassAnnotations(Service service, TypeStore typeStore) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ public GapicClass generate(GapicContext context, Service service) {
.build();
return GapicClass.create(kind, classDef, SampleComposerUtil.handleDuplicateSamples(samples))
.withApiShortName(service.apiShortName())
.withApiVersion(service.apiVersion());
.withPackageVersion(service.packageVersion());
}

private static List<CommentStatement> createClassHeaderComments(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,7 @@ public GapicClass generate(GapicContext context, Service service) {
return GapicClass.create(
GapicClass.Kind.STUB, classDef, SampleComposerUtil.handleDuplicateSamples(samples))
.withApiShortName(service.apiShortName())
.withApiVersion(service.apiVersion());
.withPackageVersion(service.packageVersion());
}

protected MethodDefinition createDefaultCredentialsProviderBuilderMethod() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ public enum Kind {
public abstract String apiShortName();

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

/**
* Create a GapicClass with minimal information. This is intended to be used for GapicClasses that
Expand Down Expand Up @@ -76,7 +76,7 @@ static Builder builder() {
return new AutoValue_GapicClass.Builder()
.setSamples(Collections.emptyList())
.setApiShortName("")
.setApiVersion("");
.setPackageVersion("");
}

abstract Builder toBuilder();
Expand All @@ -89,8 +89,8 @@ public final GapicClass withApiShortName(String apiShortName) {
return toBuilder().setApiShortName(apiShortName).build();
}

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

@AutoValue.Builder
Expand All @@ -103,7 +103,7 @@ abstract static class Builder {

abstract Builder setApiShortName(String apiShortName);

abstract Builder setApiVersion(String apiVersion);
abstract Builder setPackageVersion(String packageVersion);

abstract GapicClass build();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,9 @@
public abstract class Service {
public abstract String name();

@Nullable
public abstract String apiVersion();

public abstract String defaultHost();

public abstract ImmutableList<String> oauthScopes();
Expand All @@ -52,6 +55,10 @@ public boolean hasDescription() {
return !Strings.isNullOrEmpty(description());
}

public boolean hasApiVersion() {
return !Strings.isNullOrEmpty(apiVersion());
}

public String hostServiceName() {
// Host Service Name is guaranteed to exist and be non-null and non-empty
// Parser will fail if the default host is not supplied
Expand All @@ -65,9 +72,9 @@ public String apiShortName() {
return "";
}

public String apiVersion() {
public String packageVersion() {
if (!Strings.isNullOrEmpty(protoPakkage())) {
return parseApiVersion(protoPakkage());
return parsePackageVersion(protoPakkage());
}
return "";
}
Expand Down Expand Up @@ -158,6 +165,8 @@ public abstract static class Builder {

public abstract Builder setOverriddenName(String overriddenName);

public abstract Builder setApiVersion(String apiVersion);

public abstract Builder setDefaultHost(String defaultHost);

public abstract Builder setOauthScopes(List<String> oauthScopes);
Expand All @@ -177,17 +186,17 @@ public abstract static class Builder {
public abstract Service build();
}

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

// Parse the service name from the default host configured in the protos
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -470,6 +470,11 @@ public static List<Service> parseService(
}
}

if (serviceOptions.hasExtension(ClientProto.apiVersion)) {
String apiVersion = serviceOptions.getExtension(ClientProto.apiVersion);
serviceBuilder.setApiVersion(apiVersion);
}

String serviceName = s.getName();
String overriddenServiceName = serviceName;
String pakkage = TypeParser.getPackage(fileDescriptor);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ public class ComposerTest {
GrpcServiceCallableFactoryClassComposer.instance()
.generate(context, echoProtoService)
.withApiShortName(echoProtoService.apiShortName())
.withApiVersion(echoProtoService.apiVersion()));
.withPackageVersion(echoProtoService.packageVersion()));
private final Sample sample =
Sample.builder()
.setRegionTag(
Expand Down Expand Up @@ -160,7 +160,7 @@ private List<GapicClass> getTestClassListFromService(Service testService) {
.generate(context, testService)
.withSamples(ListofSamples)
.withApiShortName(testService.apiShortName())
.withApiVersion(testService.apiVersion());
.withPackageVersion(testService.packageVersion());
List<GapicClass> testClassList = Arrays.asList(new GapicClass[] {testClass});
return testClassList;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ public static Collection<Object[]> data() {
public String apiShortNameExpected;

@Parameterized.Parameter(3)
public String apiVersionExpected;
public String packageVersionExpected;

@Test
public void generateServiceClientClasses() {
Expand All @@ -83,6 +83,6 @@ public void generateServiceClientClasses() {
Assert.assertGoldenSamples(
this.getClass(), name, clazz.classDefinition().packageString(), clazz.samples());
Assert.assertCodeEquals(clazz.apiShortName(), apiShortNameExpected);
Assert.assertCodeEquals(clazz.apiVersion(), apiVersionExpected);
Assert.assertCodeEquals(clazz.packageVersion(), packageVersionExpected);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ public static Collection<Object[]> data() {
public String apiShortNameExpected;

@Parameterized.Parameter(3)
public String apiVersionExpected;
public String packageVersionExpected;

@Test
public void generateServiceSettingsClasses() {
Expand All @@ -69,6 +69,6 @@ public void generateServiceSettingsClasses() {
clazz.classDefinition().packageString(),
clazz.samples());
Assert.assertCodeEquals(clazz.apiShortName(), apiShortNameExpected);
Assert.assertCodeEquals(clazz.apiVersion(), apiVersionExpected);
Assert.assertCodeEquals(clazz.packageVersion(), packageVersionExpected);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ public static Collection<Object[]> data() {
public String apiShortNameExpected;

@Parameterized.Parameter(3)
public String apiVersionExpected;
public String packageVersionExpected;

@Test
public void generateServiceStubClasses() {
Expand All @@ -55,6 +55,6 @@ public void generateServiceStubClasses() {
Assert.assertGoldenClass(this.getClass(), clazz, name + ".golden");
Assert.assertEmptySamples(clazz.samples());
Assert.assertCodeEquals(clazz.apiShortName(), apiShortNameExpected);
Assert.assertCodeEquals(clazz.apiVersion(), apiVersionExpected);
Assert.assertCodeEquals(clazz.packageVersion(), packageVersionExpected);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ public static Collection<Object[]> data() {
public String apiShortNameExpected;

@Parameterized.Parameter(3)
public String apiVersionExpected;
public String packageVersionExpected;

@Test
public void generateServiceStubSettingsClasses() {
Expand All @@ -81,6 +81,6 @@ public void generateServiceStubSettingsClasses() {
clazz.classDefinition().packageString(),
clazz.samples());
Assert.assertCodeEquals(clazz.apiShortName(), apiShortNameExpected);
Assert.assertCodeEquals(clazz.apiVersion(), apiVersionExpected);
Assert.assertCodeEquals(clazz.packageVersion(), packageVersionExpected);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,24 +16,20 @@

import static com.google.common.truth.Truth.assertThat;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertTrue;

import com.google.api.generator.engine.ast.TypeNode;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import java.util.Arrays;
import org.junit.Before;
import org.junit.Test;

public class ServiceTest {
private static final String SHOWCASE_PACKAGE_NAME = "com.google.showcase.v1beta1";
private static final Service.Builder testServiceBuilder =
Service.builder()
.setName("Echo")
.setDefaultHost("localhost:7469")
.setOauthScopes(Arrays.asList("https://www.googleapis.com/auth/cloud-platform"))
.setPakkage(SHOWCASE_PACKAGE_NAME)
.setProtoPakkage(SHOWCASE_PACKAGE_NAME)
.setOriginalJavaPackage(SHOWCASE_PACKAGE_NAME)
.setOverriddenName("Echo");
private Service.Builder testServiceBuilder;

private static final Method.Builder testMethodBuilder =
Method.builder()
Expand All @@ -50,6 +46,19 @@ public class ServiceTest {
.setIsAsteriskBody(false)
.setHttpVerb(HttpBindings.HttpVerb.GET);

@Before
public void init() {
testServiceBuilder =
Service.builder()
.setName("Echo")
.setDefaultHost("localhost:7469")
.setOauthScopes(Arrays.asList("https://www.googleapis.com/auth/cloud-platform"))
.setPakkage(SHOWCASE_PACKAGE_NAME)
.setProtoPakkage(SHOWCASE_PACKAGE_NAME)
.setOriginalJavaPackage(SHOWCASE_PACKAGE_NAME)
.setOverriddenName("Echo");
}

@Test
public void apiShortName_shouldReturnApiShortNameIfHostContainsRegionalEndpoint() {
String defaultHost = "us-east1-pubsub.googleapis.com";
Expand Down Expand Up @@ -79,17 +88,40 @@ public void apiShortName_shouldReturnHostIfNoPeriods() {
}

@Test
public void apiVersion_shouldReturnVersionIfMatch() {
public void packageVersion_shouldReturnVersionIfMatch() {
String protoPackage = "com.google.showcase.v1";
Service testService = testServiceBuilder.setProtoPakkage(protoPackage).build();
assertEquals("v1", testService.apiVersion());
assertEquals("v1", testService.packageVersion());
}

@Test
public void apiVersion_shouldReturnEmptyIfNoMatch() {
public void packageVersion_shouldReturnEmptyIfNoMatch() {
String protoPackage = "com.google.showcase";
Service testService = testServiceBuilder.setProtoPakkage(protoPackage).build();
assertEquals("", testService.packageVersion());
}

@Test
public void apiVersion_shouldReturnApiVersion() {
String apiVersion = "v1_20230601";
Service testService = testServiceBuilder.setApiVersion(apiVersion).build();
assertTrue(testService.hasApiVersion());
assertEquals(apiVersion, testService.apiVersion());
}

@Test
public void apiVersion_shouldReturnNoApiVersionWhenNull() {
Service testService = testServiceBuilder.build();
assertNull(testService.apiVersion());
assertFalse(testService.hasApiVersion());
}

@Test
public void apiVersion_shouldReturnNoApiVersionWhenEmpty() {
String apiVersion = "";
Service testService = testServiceBuilder.setApiVersion(apiVersion).build();
assertEquals("", testService.apiVersion());
assertFalse(testService.hasApiVersion());
}

@Test
Expand Down

0 comments on commit 40711fd

Please sign in to comment.