refactor: expose parsed api short name and version as fields in Service (#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.

Author: emmileaf

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<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) {
@@ -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(
@@ -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) {

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<AnnotationNode> createClassAnnotations(Service service, TypeStore typeStore) {

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<CommentStatement> createClassHeaderComments(

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() {

src/main/java/com/google/api/generator/gapic/model/GapicClass.java

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

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;
+  }
 }

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<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() {
@@ -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;
@@ -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());
@@ -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());
 
@@ -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());
 
@@ -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;
+  }
 }