From 79b09280a52cb7d8e2dc0ee5a9b3f925e69bda07 Mon Sep 17 00:00:00 2001 From: Emily Wang Date: Wed, 19 Oct 2022 14:58:17 -0400 Subject: [PATCH 1/5] WIP: add parsed apiShortName and apiVersion as fields in Service class --- .../generator/gapic/composer/Composer.java | 37 +---------- .../AbstractServiceClientClassComposer.java | 3 +- .../AbstractServiceSettingsClassComposer.java | 3 +- ...tractServiceStubSettingsClassComposer.java | 3 +- .../api/generator/gapic/model/GapicClass.java | 23 ++++--- .../api/generator/gapic/model/Service.java | 47 ++++++++++++++ .../gapic/composer/ComposerTest.java | 63 ++++++++++++------- .../grpc/ServiceClientClassComposerTest.java | 30 ++------- .../ServiceSettingsClassComposerTest.java | 12 +--- .../grpc/ServiceStubClassComposerTest.java | 8 +-- .../ServiceStubSettingsClassComposerTest.java | 15 +---- 11 files changed, 126 insertions(+), 118 deletions(-) diff --git a/src/main/java/com/google/api/generator/gapic/composer/Composer.java b/src/main/java/com/google/api/generator/gapic/composer/Composer.java index 60d4f5dde2..acd914a163 100644 --- a/src/main/java/com/google/api/generator/gapic/composer/Composer.java +++ b/src/main/java/com/google/api/generator/gapic/composer/Composer.java @@ -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; @@ -50,8 +48,7 @@ public static List 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) { @@ -193,16 +190,7 @@ public static List generateTestClasses(GapicContext context) { } @VisibleForTesting - static List prepareExecutableSamples(List 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 prepareExecutableSamples(List clazzes) { // Include license header, apiShortName, and apiVersion List clazzesWithSamples = new ArrayList<>(); clazzes.forEach( @@ -214,31 +202,12 @@ static List prepareExecutableSamples(List 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) { diff --git a/src/main/java/com/google/api/generator/gapic/composer/common/AbstractServiceClientClassComposer.java b/src/main/java/com/google/api/generator/gapic/composer/common/AbstractServiceClientClassComposer.java index c76f25c9f6..073023cfad 100644 --- a/src/main/java/com/google/api/generator/gapic/composer/common/AbstractServiceClientClassComposer.java +++ b/src/main/java/com/google/api/generator/gapic/composer/common/AbstractServiceClientClassComposer.java @@ -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 createClassAnnotations(Service service, TypeStore typeStore) { diff --git a/src/main/java/com/google/api/generator/gapic/composer/common/AbstractServiceSettingsClassComposer.java b/src/main/java/com/google/api/generator/gapic/composer/common/AbstractServiceSettingsClassComposer.java index 08186862ae..fdfa33f471 100644 --- a/src/main/java/com/google/api/generator/gapic/composer/common/AbstractServiceSettingsClassComposer.java +++ b/src/main/java/com/google/api/generator/gapic/composer/common/AbstractServiceSettingsClassComposer.java @@ -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 createClassHeaderComments( diff --git a/src/main/java/com/google/api/generator/gapic/composer/common/AbstractServiceStubSettingsClassComposer.java b/src/main/java/com/google/api/generator/gapic/composer/common/AbstractServiceStubSettingsClassComposer.java index cea124e96d..fe66a11e77 100644 --- a/src/main/java/com/google/api/generator/gapic/composer/common/AbstractServiceStubSettingsClassComposer.java +++ b/src/main/java/com/google/api/generator/gapic/composer/common/AbstractServiceStubSettingsClassComposer.java @@ -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() { diff --git a/src/main/java/com/google/api/generator/gapic/model/GapicClass.java b/src/main/java/com/google/api/generator/gapic/model/GapicClass.java index 165cb4d94b..f05e308290 100644 --- a/src/main/java/com/google/api/generator/gapic/model/GapicClass.java +++ b/src/main/java/com/google/api/generator/gapic/model/GapicClass.java @@ -35,9 +35,11 @@ public enum Kind { public abstract List 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(); @@ -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(); @@ -60,8 +63,12 @@ public final GapicClass withSamples(List 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 @@ -72,7 +79,9 @@ abstract static class Builder { abstract Builder setSamples(List samples); - abstract Builder setDefaultHost(String defaultHost); + abstract Builder setApiShortName(String apiShortName); + + abstract Builder setApiVersion(String apiVersion); abstract GapicClass build(); } diff --git a/src/main/java/com/google/api/generator/gapic/model/Service.java b/src/main/java/com/google/api/generator/gapic/model/Service.java index 34d3f97b25..0916199fc2 100644 --- a/src/main/java/com/google/api/generator/gapic/model/Service.java +++ b/src/main/java/com/google/api/generator/gapic/model/Service.java @@ -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; @@ -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()) { @@ -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; + } } diff --git a/src/test/java/com/google/api/generator/gapic/composer/ComposerTest.java b/src/test/java/com/google/api/generator/gapic/composer/ComposerTest.java index caf070682c..7d22b38e65 100644 --- a/src/test/java/com/google/api/generator/gapic/composer/ComposerTest.java +++ b/src/test/java/com/google/api/generator/gapic/composer/ComposerTest.java @@ -42,14 +42,16 @@ public class ComposerTest { private final Service echoProtoService = context.services().get(0); private final List 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 ListofSamples = Arrays.asList(new Sample[] {sample}); - private final String protoPackage = echoProtoService.protoPakkage(); @Test public void gapicClass_addApacheLicense() { @@ -75,7 +77,7 @@ public void composeSamples_showcase() { List testClassList = Arrays.asList(new GapicClass[] {testClass}); List composedSamples = - Composer.prepareExecutableSamples(testClassList, protoPackage).get(0).samples(); + Composer.prepareExecutableSamples(testClassList).get(0).samples(); assertFalse(composedSamples.isEmpty()); for (Sample sample : composedSamples) { @@ -83,37 +85,42 @@ public void composeSamples_showcase() { "File header should be APACHE", Arrays.asList(CommentComposer.APACHE_LICENSE_COMMENT), sample.fileHeader()); - assertEquals("ApiShortName should be empty", "", sample.regionTag().apiShortName()); + assertEquals( + "ApiShortName should be Localhost7469", + "Localhost7469", + sample.regionTag().apiShortName()); assertEquals("ApiVersion should be V1beta1", "V1Beta1", sample.regionTag().apiVersion()); } } + // TODO (emmwang): these tests now test the Service class - move to ServiceTest.java? + @Test public void parseDefaultHost_shouldReturnApiShortNameIfHostContainsRegionalEndpoint() { String defaultHost = "us-east1-pubsub.googleapis.com"; - String apiShortName = Composer.parseDefaultHost(defaultHost); - assertEquals("pubsub", apiShortName); + Service testService = echoProtoService.toBuilder().setDefaultHost(defaultHost).build(); + assertEquals("pubsub", testService.apiShortName()); } @Test public void parseDefaultHost_shouldReturnApiShortName() { String defaultHost = "logging.googleapis.com"; - String apiShortName = Composer.parseDefaultHost(defaultHost); - assertEquals("logging", apiShortName); + Service testService = echoProtoService.toBuilder().setDefaultHost(defaultHost).build(); + assertEquals("logging", testService.apiShortName()); } @Test public void parseDefaultHost_shouldReturnApiShortNameForIam() { String defaultHost = "iam-meta-api.googleapis.com"; - String apiShortName = Composer.parseDefaultHost(defaultHost); - assertEquals("iam", apiShortName); + Service testService = echoProtoService.toBuilder().setDefaultHost(defaultHost).build(); + assertEquals("iam", testService.apiShortName()); } @Test public void parseDefaultHost_shouldReturnHostIfNoPeriods() { String defaultHost = "logging:7469"; - String apiShortName = Composer.parseDefaultHost(defaultHost); - assertEquals("logging:7469", apiShortName); + Service testService = echoProtoService.toBuilder().setDefaultHost(defaultHost).build(); + assertEquals("logging:7469", testService.apiShortName()); } @Test @@ -128,13 +135,13 @@ public void gapicClass_addRegionTagAndHeaderToSample() { @Test public void composeSamples_parseProtoPackage() { + // TODO (emmwang): clean up / refactor repeated code in this test? + String defaultHost = "accessapproval.googleapis.com:443"; - GapicClass testClass = clazzes.get(0).withSamples(ListofSamples).withDefaultHost(defaultHost); - List testClassList = Arrays.asList(new GapicClass[] {testClass}); String protoPack = "google.cloud.accessapproval.v1"; - - List composedSamples = - Composer.prepareExecutableSamples(testClassList, protoPack).get(0).samples(); + Service testService = + echoProtoService.toBuilder().setDefaultHost(defaultHost).setProtoPakkage(protoPack).build(); + List composedSamples = composeSamplesFromService(testService); // If samples is empty, the test automatically passes without checking. assertFalse(composedSamples.isEmpty()); @@ -149,9 +156,9 @@ 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 = + echoProtoService.toBuilder().setDefaultHost(defaultHost).setProtoPakkage(protoPack).build(); + composedSamples = composeSamplesFromService(testService); // If samples is empty, the test automatically passes without checking. assertFalse(composedSamples.isEmpty()); @@ -161,7 +168,9 @@ public void composeSamples_parseProtoPackage() { } protoPack = "google.cloud.vision"; - composedSamples = Composer.prepareExecutableSamples(testClassList, protoPack).get(0).samples(); + testService = + echoProtoService.toBuilder().setDefaultHost(defaultHost).setProtoPakkage(protoPack).build(); + composedSamples = composeSamplesFromService(testService); // If samples is empty, the test automatically passes without checking. assertFalse(composedSamples.isEmpty()); @@ -170,4 +179,16 @@ public void composeSamples_parseProtoPackage() { assertEquals("ApiVersion should be empty", sample.regionTag().apiVersion(), ""); } } + + private List composeSamplesFromService(Service testService) { + GapicClass testClass = + GrpcServiceCallableFactoryClassComposer.instance() + .generate(context, testService) + .withSamples(ListofSamples) + .withApiShortName(testService.apiShortName()) + .withApiVersion(testService.apiVersion()); + List testClassList = Arrays.asList(new GapicClass[] {testClass}); + List testSamples = Composer.prepareExecutableSamples(testClassList).get(0).samples(); + return testSamples; + } } diff --git a/src/test/java/com/google/api/generator/gapic/composer/grpc/ServiceClientClassComposerTest.java b/src/test/java/com/google/api/generator/gapic/composer/grpc/ServiceClientClassComposerTest.java index 18450c1ca8..cacb3714c2 100644 --- a/src/test/java/com/google/api/generator/gapic/composer/grpc/ServiceClientClassComposerTest.java +++ b/src/test/java/com/google/api/generator/gapic/composer/grpc/ServiceClientClassComposerTest.java @@ -29,27 +29,11 @@ public class ServiceClientClassComposerTest { public static Collection data() { return Arrays.asList( new Object[][] { - {"EchoClient", GrpcTestProtoLoader.instance().parseShowcaseEcho(), "localhost:7469"}, - { - "DeprecatedServiceClient", - GrpcTestProtoLoader.instance().parseDeprecatedService(), - "localhost:7469" - }, - { - "IdentityClient", - GrpcTestProtoLoader.instance().parseShowcaseIdentity(), - "localhost:7469" - }, - { - "BookshopClient", - GrpcTestProtoLoader.instance().parseBookshopService(), - "localhost:2665" - }, - { - "MessagingClient", - GrpcTestProtoLoader.instance().parseShowcaseMessaging(), - "localhost:7469" - }, + {"EchoClient", GrpcTestProtoLoader.instance().parseShowcaseEcho()}, + {"DeprecatedServiceClient", GrpcTestProtoLoader.instance().parseDeprecatedService()}, + {"IdentityClient", GrpcTestProtoLoader.instance().parseShowcaseIdentity()}, + {"BookshopClient", GrpcTestProtoLoader.instance().parseBookshopService()}, + {"MessagingClient", GrpcTestProtoLoader.instance().parseShowcaseMessaging()}, }); } @@ -58,9 +42,6 @@ public static Collection data() { @Parameterized.Parameter(1) public GapicContext context; - @Parameterized.Parameter(2) - public String defaultHostExpected; - @Test public void generateServiceClientClasses() { Service service = context.services().get(0); @@ -69,6 +50,5 @@ public void generateServiceClientClasses() { Assert.assertGoldenClass(this.getClass(), clazz, name + ".golden"); Assert.assertGoldenSamples( this.getClass(), name, clazz.classDefinition().packageString(), clazz.samples()); - Assert.assertCodeEquals(clazz.defaultHost(), defaultHostExpected); } } diff --git a/src/test/java/com/google/api/generator/gapic/composer/grpc/ServiceSettingsClassComposerTest.java b/src/test/java/com/google/api/generator/gapic/composer/grpc/ServiceSettingsClassComposerTest.java index 7f59ac58d0..5857ef3352 100644 --- a/src/test/java/com/google/api/generator/gapic/composer/grpc/ServiceSettingsClassComposerTest.java +++ b/src/test/java/com/google/api/generator/gapic/composer/grpc/ServiceSettingsClassComposerTest.java @@ -31,12 +31,8 @@ public class ServiceSettingsClassComposerTest { public static Collection data() { return Arrays.asList( new Object[][] { - {"EchoSettings", TestProtoLoader.instance().parseShowcaseEcho(), "localhost:7469"}, - { - "DeprecatedServiceSettings", - TestProtoLoader.instance().parseDeprecatedService(), - "localhost:7469" - } + {"EchoSettings", TestProtoLoader.instance().parseShowcaseEcho()}, + {"DeprecatedServiceSettings", TestProtoLoader.instance().parseDeprecatedService()} }); } @@ -45,9 +41,6 @@ public static Collection data() { @Parameterized.Parameter(1) public GapicContext context; - @Parameterized.Parameter(2) - public String defaultHostExpected; - @Test public void generateServiceSettingsClasses() { Service service = context.services().get(0); @@ -59,6 +52,5 @@ public void generateServiceSettingsClasses() { "servicesettings", clazz.classDefinition().packageString(), clazz.samples()); - Assert.assertCodeEquals(clazz.defaultHost(), defaultHostExpected); } } diff --git a/src/test/java/com/google/api/generator/gapic/composer/grpc/ServiceStubClassComposerTest.java b/src/test/java/com/google/api/generator/gapic/composer/grpc/ServiceStubClassComposerTest.java index 0da8ac4548..bb0619d315 100644 --- a/src/test/java/com/google/api/generator/gapic/composer/grpc/ServiceStubClassComposerTest.java +++ b/src/test/java/com/google/api/generator/gapic/composer/grpc/ServiceStubClassComposerTest.java @@ -31,8 +31,8 @@ public class ServiceStubClassComposerTest { public static Collection data() { return Arrays.asList( new Object[][] { - {"EchoStub", TestProtoLoader.instance().parseShowcaseEcho(), ""}, - {"DeprecatedServiceStub", TestProtoLoader.instance().parseDeprecatedService(), ""} + {"EchoStub", TestProtoLoader.instance().parseShowcaseEcho()}, + {"DeprecatedServiceStub", TestProtoLoader.instance().parseDeprecatedService()} }); } @@ -41,9 +41,6 @@ public static Collection data() { @Parameterized.Parameter(1) public GapicContext context; - @Parameterized.Parameter(2) - public String defaultHostExpected; - @Test public void generateServiceStubClasses() { Service service = context.services().get(0); @@ -51,6 +48,5 @@ public void generateServiceStubClasses() { Assert.assertGoldenClass(this.getClass(), clazz, name + ".golden"); Assert.assertEmptySamples(clazz.samples()); - Assert.assertCodeEquals(clazz.defaultHost(), defaultHostExpected); } } diff --git a/src/test/java/com/google/api/generator/gapic/composer/grpc/ServiceStubSettingsClassComposerTest.java b/src/test/java/com/google/api/generator/gapic/composer/grpc/ServiceStubSettingsClassComposerTest.java index 019252dce3..b40d279351 100644 --- a/src/test/java/com/google/api/generator/gapic/composer/grpc/ServiceStubSettingsClassComposerTest.java +++ b/src/test/java/com/google/api/generator/gapic/composer/grpc/ServiceStubSettingsClassComposerTest.java @@ -31,22 +31,17 @@ public static Collection data() { return Arrays.asList( new Object[][] { { - "LoggingServiceV2StubSettings", - GrpcTestProtoLoader.instance().parseLogging(), - "logging.googleapis.com:443" + "LoggingServiceV2StubSettings", GrpcTestProtoLoader.instance().parseLogging(), }, { - "PublisherStubSettings", - GrpcTestProtoLoader.instance().parsePubSubPublisher(), - "pubsub.googleapis.com:443" + "PublisherStubSettings", GrpcTestProtoLoader.instance().parsePubSubPublisher(), }, { - "EchoStubSettings", GrpcTestProtoLoader.instance().parseShowcaseEcho(), "localhost:7469" + "EchoStubSettings", GrpcTestProtoLoader.instance().parseShowcaseEcho(), }, { "DeprecatedServiceStubSettings", GrpcTestProtoLoader.instance().parseDeprecatedService(), - "localhost:7469" } }); } @@ -56,9 +51,6 @@ public static Collection data() { @Parameterized.Parameter(1) public GapicContext context; - @Parameterized.Parameter(2) - public String defaultHostExpected; - @Test public void generateServiceStubSettingsClasses() { Service service = context.services().get(0); @@ -70,6 +62,5 @@ public void generateServiceStubSettingsClasses() { "servicesettings/stub", clazz.classDefinition().packageString(), clazz.samples()); - Assert.assertCodeEquals(clazz.defaultHost(), defaultHostExpected); } } From e98b405dbb798178160f4983b3bfda4878928aeb Mon Sep 17 00:00:00 2001 From: Emily Wang Date: Mon, 24 Oct 2022 09:17:08 -0400 Subject: [PATCH 2/5] Refactor unit tests --- .../gapic/composer/ComposerTest.java | 51 +++---------- .../generator/gapic/model/ServiceTest.java | 75 +++++++++++++++++++ 2 files changed, 86 insertions(+), 40 deletions(-) create mode 100644 src/test/java/com/google/api/generator/gapic/model/ServiceTest.java diff --git a/src/test/java/com/google/api/generator/gapic/composer/ComposerTest.java b/src/test/java/com/google/api/generator/gapic/composer/ComposerTest.java index 7d22b38e65..1dde91da1f 100644 --- a/src/test/java/com/google/api/generator/gapic/composer/ComposerTest.java +++ b/src/test/java/com/google/api/generator/gapic/composer/ComposerTest.java @@ -93,36 +93,6 @@ public void composeSamples_showcase() { } } - // TODO (emmwang): these tests now test the Service class - move to ServiceTest.java? - - @Test - public void parseDefaultHost_shouldReturnApiShortNameIfHostContainsRegionalEndpoint() { - String defaultHost = "us-east1-pubsub.googleapis.com"; - Service testService = echoProtoService.toBuilder().setDefaultHost(defaultHost).build(); - assertEquals("pubsub", testService.apiShortName()); - } - - @Test - public void parseDefaultHost_shouldReturnApiShortName() { - String defaultHost = "logging.googleapis.com"; - Service testService = echoProtoService.toBuilder().setDefaultHost(defaultHost).build(); - assertEquals("logging", testService.apiShortName()); - } - - @Test - public void parseDefaultHost_shouldReturnApiShortNameForIam() { - String defaultHost = "iam-meta-api.googleapis.com"; - Service testService = echoProtoService.toBuilder().setDefaultHost(defaultHost).build(); - assertEquals("iam", testService.apiShortName()); - } - - @Test - public void parseDefaultHost_shouldReturnHostIfNoPeriods() { - String defaultHost = "logging:7469"; - Service testService = echoProtoService.toBuilder().setDefaultHost(defaultHost).build(); - assertEquals("logging:7469", testService.apiShortName()); - } - @Test public void gapicClass_addRegionTagAndHeaderToSample() { Sample testSample; @@ -135,13 +105,13 @@ public void gapicClass_addRegionTagAndHeaderToSample() { @Test public void composeSamples_parseProtoPackage() { - // TODO (emmwang): clean up / refactor repeated code in this test? - String defaultHost = "accessapproval.googleapis.com:443"; String protoPack = "google.cloud.accessapproval.v1"; Service testService = echoProtoService.toBuilder().setDefaultHost(defaultHost).setProtoPakkage(protoPack).build(); - List composedSamples = composeSamplesFromService(testService); + List testClassList = getTestClassListFromService(testService); + List composedSamples = + Composer.prepareExecutableSamples(testClassList).get(0).samples(); // If samples is empty, the test automatically passes without checking. assertFalse(composedSamples.isEmpty()); @@ -157,8 +127,9 @@ public void composeSamples_parseProtoPackage() { protoPack = "google.cloud.vision.v1p1beta1"; defaultHost = "vision.googleapis.com"; testService = - echoProtoService.toBuilder().setDefaultHost(defaultHost).setProtoPakkage(protoPack).build(); - composedSamples = composeSamplesFromService(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()); @@ -169,8 +140,9 @@ public void composeSamples_parseProtoPackage() { protoPack = "google.cloud.vision"; testService = - echoProtoService.toBuilder().setDefaultHost(defaultHost).setProtoPakkage(protoPack).build(); - composedSamples = composeSamplesFromService(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()); @@ -180,7 +152,7 @@ public void composeSamples_parseProtoPackage() { } } - private List composeSamplesFromService(Service testService) { + private List getTestClassListFromService(Service testService) { GapicClass testClass = GrpcServiceCallableFactoryClassComposer.instance() .generate(context, testService) @@ -188,7 +160,6 @@ private List composeSamplesFromService(Service testService) { .withApiShortName(testService.apiShortName()) .withApiVersion(testService.apiVersion()); List testClassList = Arrays.asList(new GapicClass[] {testClass}); - List testSamples = Composer.prepareExecutableSamples(testClassList).get(0).samples(); - return testSamples; + return testClassList; } } diff --git a/src/test/java/com/google/api/generator/gapic/model/ServiceTest.java b/src/test/java/com/google/api/generator/gapic/model/ServiceTest.java new file mode 100644 index 0000000000..c6f60b831f --- /dev/null +++ b/src/test/java/com/google/api/generator/gapic/model/ServiceTest.java @@ -0,0 +1,75 @@ +// Copyright 2022 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.api.generator.gapic.model; + +import static org.junit.Assert.assertEquals; + +import java.util.Arrays; +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"); + + @Test + public void apiShortName_shouldReturnApiShortNameIfHostContainsRegionalEndpoint() { + String defaultHost = "us-east1-pubsub.googleapis.com"; + Service testService = testServiceBuilder.setDefaultHost(defaultHost).build(); + assertEquals("pubsub", testService.apiShortName()); + } + + @Test + public void apiShortName_shouldReturnApiShortName() { + String defaultHost = "logging.googleapis.com"; + Service testService = testServiceBuilder.setDefaultHost(defaultHost).build(); + assertEquals("logging", testService.apiShortName()); + } + + @Test + public void apiShortName_shouldReturnApiShortNameForIam() { + String defaultHost = "iam-meta-api.googleapis.com"; + Service testService = testServiceBuilder.setDefaultHost(defaultHost).build(); + assertEquals("iam", testService.apiShortName()); + } + + @Test + public void apiShortName_shouldReturnHostIfNoPeriods() { + String defaultHost = "logging:7469"; + Service testService = testServiceBuilder.setDefaultHost(defaultHost).build(); + assertEquals("logging:7469", testService.apiShortName()); + } + + @Test + public void apiVersion_shouldReturnVersionIfMatch() { + String protoPackage = "com.google.showcase.v1"; + Service testService = testServiceBuilder.setProtoPakkage(protoPackage).build(); + assertEquals("v1", testService.apiVersion()); + } + + @Test + public void apiVersion_shouldReturnEmptyIfNoMatch() { + String protoPackage = "com.google.showcase"; + Service testService = testServiceBuilder.setProtoPakkage(protoPackage).build(); + assertEquals("", testService.apiVersion()); + } +} From 5e7f9b38ccb87c54dc595c2b07f727b9a5c56d84 Mon Sep 17 00:00:00 2001 From: Emily Wang Date: Mon, 31 Oct 2022 11:55:42 -0400 Subject: [PATCH 3/5] Update composer tests that included checks for defaultHost and sample generation --- .../grpc/ServiceClientClassComposerTest.java | 43 ++++++++++++++++--- .../ServiceSettingsClassComposerTest.java | 22 +++++++++- .../grpc/ServiceStubClassComposerTest.java | 12 +++++- .../ServiceStubSettingsClassComposerTest.java | 25 +++++++++-- 4 files changed, 90 insertions(+), 12 deletions(-) diff --git a/src/test/java/com/google/api/generator/gapic/composer/grpc/ServiceClientClassComposerTest.java b/src/test/java/com/google/api/generator/gapic/composer/grpc/ServiceClientClassComposerTest.java index cacb3714c2..c169c88eb3 100644 --- a/src/test/java/com/google/api/generator/gapic/composer/grpc/ServiceClientClassComposerTest.java +++ b/src/test/java/com/google/api/generator/gapic/composer/grpc/ServiceClientClassComposerTest.java @@ -29,11 +29,36 @@ public class ServiceClientClassComposerTest { public static Collection data() { return Arrays.asList( new Object[][] { - {"EchoClient", GrpcTestProtoLoader.instance().parseShowcaseEcho()}, - {"DeprecatedServiceClient", GrpcTestProtoLoader.instance().parseDeprecatedService()}, - {"IdentityClient", GrpcTestProtoLoader.instance().parseShowcaseIdentity()}, - {"BookshopClient", GrpcTestProtoLoader.instance().parseBookshopService()}, - {"MessagingClient", GrpcTestProtoLoader.instance().parseShowcaseMessaging()}, + { + "EchoClient", + GrpcTestProtoLoader.instance().parseShowcaseEcho(), + "localhost:7469", + "v1beta1" + }, + { + "DeprecatedServiceClient", + GrpcTestProtoLoader.instance().parseDeprecatedService(), + "localhost:7469", + "v1" + }, + { + "IdentityClient", + GrpcTestProtoLoader.instance().parseShowcaseIdentity(), + "localhost:7469", + "v1beta1" + }, + { + "BookshopClient", + GrpcTestProtoLoader.instance().parseBookshopService(), + "localhost:2665", + "v1beta1" + }, + { + "MessagingClient", + GrpcTestProtoLoader.instance().parseShowcaseMessaging(), + "localhost:7469", + "v1beta1" + }, }); } @@ -42,6 +67,12 @@ public static Collection data() { @Parameterized.Parameter(1) public GapicContext context; + @Parameterized.Parameter(2) + public String apiShortNameExpected; + + @Parameterized.Parameter(3) + public String apiVersionExpected; + @Test public void generateServiceClientClasses() { Service service = context.services().get(0); @@ -50,5 +81,7 @@ public void generateServiceClientClasses() { Assert.assertGoldenClass(this.getClass(), clazz, name + ".golden"); Assert.assertGoldenSamples( this.getClass(), name, clazz.classDefinition().packageString(), clazz.samples()); + Assert.assertCodeEquals(clazz.apiShortName(), apiShortNameExpected); + Assert.assertCodeEquals(clazz.apiVersion(), apiVersionExpected); } } diff --git a/src/test/java/com/google/api/generator/gapic/composer/grpc/ServiceSettingsClassComposerTest.java b/src/test/java/com/google/api/generator/gapic/composer/grpc/ServiceSettingsClassComposerTest.java index 5857ef3352..1d245a7490 100644 --- a/src/test/java/com/google/api/generator/gapic/composer/grpc/ServiceSettingsClassComposerTest.java +++ b/src/test/java/com/google/api/generator/gapic/composer/grpc/ServiceSettingsClassComposerTest.java @@ -31,8 +31,18 @@ public class ServiceSettingsClassComposerTest { public static Collection data() { return Arrays.asList( new Object[][] { - {"EchoSettings", TestProtoLoader.instance().parseShowcaseEcho()}, - {"DeprecatedServiceSettings", TestProtoLoader.instance().parseDeprecatedService()} + { + "EchoSettings", + TestProtoLoader.instance().parseShowcaseEcho(), + "localhost:7469", + "v1beta1" + }, + { + "DeprecatedServiceSettings", + TestProtoLoader.instance().parseDeprecatedService(), + "localhost:7469", + "v1" + } }); } @@ -41,6 +51,12 @@ public static Collection data() { @Parameterized.Parameter(1) public GapicContext context; + @Parameterized.Parameter(2) + public String apiShortNameExpected; + + @Parameterized.Parameter(3) + public String apiVersionExpected; + @Test public void generateServiceSettingsClasses() { Service service = context.services().get(0); @@ -52,5 +68,7 @@ public void generateServiceSettingsClasses() { "servicesettings", clazz.classDefinition().packageString(), clazz.samples()); + Assert.assertCodeEquals(clazz.apiShortName(), apiShortNameExpected); + Assert.assertCodeEquals(clazz.apiVersion(), apiVersionExpected); } } diff --git a/src/test/java/com/google/api/generator/gapic/composer/grpc/ServiceStubClassComposerTest.java b/src/test/java/com/google/api/generator/gapic/composer/grpc/ServiceStubClassComposerTest.java index bb0619d315..730136c083 100644 --- a/src/test/java/com/google/api/generator/gapic/composer/grpc/ServiceStubClassComposerTest.java +++ b/src/test/java/com/google/api/generator/gapic/composer/grpc/ServiceStubClassComposerTest.java @@ -31,8 +31,8 @@ public class ServiceStubClassComposerTest { public static Collection data() { return Arrays.asList( new Object[][] { - {"EchoStub", TestProtoLoader.instance().parseShowcaseEcho()}, - {"DeprecatedServiceStub", TestProtoLoader.instance().parseDeprecatedService()} + {"EchoStub", TestProtoLoader.instance().parseShowcaseEcho(), "", ""}, + {"DeprecatedServiceStub", TestProtoLoader.instance().parseDeprecatedService(), "", ""} }); } @@ -41,6 +41,12 @@ public static Collection data() { @Parameterized.Parameter(1) public GapicContext context; + @Parameterized.Parameter(2) + public String apiShortNameExpected; + + @Parameterized.Parameter(3) + public String apiVersionExpected; + @Test public void generateServiceStubClasses() { Service service = context.services().get(0); @@ -48,5 +54,7 @@ public void generateServiceStubClasses() { Assert.assertGoldenClass(this.getClass(), clazz, name + ".golden"); Assert.assertEmptySamples(clazz.samples()); + Assert.assertCodeEquals(clazz.apiShortName(), apiShortNameExpected); + Assert.assertCodeEquals(clazz.apiVersion(), apiVersionExpected); } } diff --git a/src/test/java/com/google/api/generator/gapic/composer/grpc/ServiceStubSettingsClassComposerTest.java b/src/test/java/com/google/api/generator/gapic/composer/grpc/ServiceStubSettingsClassComposerTest.java index b40d279351..747b19b8e0 100644 --- a/src/test/java/com/google/api/generator/gapic/composer/grpc/ServiceStubSettingsClassComposerTest.java +++ b/src/test/java/com/google/api/generator/gapic/composer/grpc/ServiceStubSettingsClassComposerTest.java @@ -31,17 +31,28 @@ public static Collection data() { return Arrays.asList( new Object[][] { { - "LoggingServiceV2StubSettings", GrpcTestProtoLoader.instance().parseLogging(), + "LoggingServiceV2StubSettings", + GrpcTestProtoLoader.instance().parseLogging(), + "logging", + "v2" }, { - "PublisherStubSettings", GrpcTestProtoLoader.instance().parsePubSubPublisher(), + "PublisherStubSettings", + GrpcTestProtoLoader.instance().parsePubSubPublisher(), + "pubsub", + "v1" }, { - "EchoStubSettings", GrpcTestProtoLoader.instance().parseShowcaseEcho(), + "EchoStubSettings", + GrpcTestProtoLoader.instance().parseShowcaseEcho(), + "localhost:7469", + "v1beta1" }, { "DeprecatedServiceStubSettings", GrpcTestProtoLoader.instance().parseDeprecatedService(), + "localhost:7469", + "v1" } }); } @@ -51,6 +62,12 @@ public static Collection data() { @Parameterized.Parameter(1) public GapicContext context; + @Parameterized.Parameter(2) + public String apiShortNameExpected; + + @Parameterized.Parameter(3) + public String apiVersionExpected; + @Test public void generateServiceStubSettingsClasses() { Service service = context.services().get(0); @@ -62,5 +79,7 @@ public void generateServiceStubSettingsClasses() { "servicesettings/stub", clazz.classDefinition().packageString(), clazz.samples()); + Assert.assertCodeEquals(clazz.apiShortName(), apiShortNameExpected); + Assert.assertCodeEquals(clazz.apiVersion(), apiVersionExpected); } } From d0c02ac80b9480ba615516e8ade53341d72b5365 Mon Sep 17 00:00:00 2001 From: Emily Wang Date: Mon, 31 Oct 2022 16:31:48 -0400 Subject: [PATCH 4/5] Fix assertion message --- .../com/google/api/generator/gapic/composer/ComposerTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/java/com/google/api/generator/gapic/composer/ComposerTest.java b/src/test/java/com/google/api/generator/gapic/composer/ComposerTest.java index 1dde91da1f..d99325fbe8 100644 --- a/src/test/java/com/google/api/generator/gapic/composer/ComposerTest.java +++ b/src/test/java/com/google/api/generator/gapic/composer/ComposerTest.java @@ -89,7 +89,7 @@ public void composeSamples_showcase() { "ApiShortName should be Localhost7469", "Localhost7469", sample.regionTag().apiShortName()); - assertEquals("ApiVersion should be V1beta1", "V1Beta1", sample.regionTag().apiVersion()); + assertEquals("ApiVersion should be V1Beta1", "V1Beta1", sample.regionTag().apiVersion()); } } From 2a1096065fd9860434e09b4cdcf8d7e15ccf7840 Mon Sep 17 00:00:00 2001 From: Emily Wang Date: Mon, 31 Oct 2022 16:38:24 -0400 Subject: [PATCH 5/5] Remove extra semicolons --- .../java/com/google/api/generator/gapic/model/Service.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/com/google/api/generator/gapic/model/Service.java b/src/main/java/com/google/api/generator/gapic/model/Service.java index 0916199fc2..2e3a6be7df 100644 --- a/src/main/java/com/google/api/generator/gapic/model/Service.java +++ b/src/main/java/com/google/api/generator/gapic/model/Service.java @@ -57,14 +57,14 @@ public String apiShortName() { return parseApiShortName(defaultHost()); } return ""; - }; + } public String apiVersion() { if (!Strings.isNullOrEmpty(protoPakkage())) { return parseApiVersion(protoPakkage()); } return ""; - }; + } public Method operationPollingMethod() { for (Method method : methods()) {