[ggj][engx] build: add integration golden tests to CircleCI

.circleci/config.yml

@@ -35,6 +35,14 @@ jobs:
             cd /tmp/gapic-generator-java
             bazel --batch test //src/test/... --noshow_progress
             find . -type f -regex ".*/bazel-testlogs/.*xml" -exec cp {} ${TEST_REPORTS_DIR} \;
+      - run:
+          name: Run integration tests for gapic-generator-java
+          command: |
+            cd /tmp/gapic-generator-java
+            # Check only the goldens, rely on the pre-commits for client library compilation tests.
+            # Otherwise, this would take too long.
+            bazel --batch test //test/integration:asset //test/integration:logging //test/integration:redis --noshow_progress
+            find . -type f -regex ".*/bazel-testlogs/.*xml" -exec cp {} ${TEST_REPORTS_DIR} \;
       - store_test_results:
           path: ~/.cache/bazel
   google-java-format:

.githooks/pre-commit

@@ -63,6 +63,9 @@ fi
 # Check only the staged files.
 NUM_TOTAL_FILES_CHANGED=$(git diff --cached --name-only | wc -l)
 NUM_JAVA_FILES_CHANGED=$(git diff --cached --name-only "*.java" | wc -l)
+NUM_UNIT_GOLDEN_FILES_CHANGED=$(git diff --cached --name-only "src/test/*/*.golden" | wc -l)
+NUM_INTEGRATION_GOLDEN_FILES_CHANGED=$(git diff --cached --name-only "test/integration/goldens/*/*.golden" | wc -l)
+NUM_INTEGRATION_BAZEL_FILES_CHANGED=$(git diff --cached --name-only "test/integration/*/BUILD.bazel" | wc -l)
 NUM_BAZEL_FILES_CHANGED=$(git diff --cached --name-only "*BUILD.bazel" | wc -l)
 
 if [ $NUM_TOTAL_FILES_CHANGED -le 0 ]
@@ -101,10 +104,10 @@ then
   fi
 fi
 
-# Check tests.
-if [ $NUM_JAVA_FILES_CHANGED -gt 0 ]
+# Check unit tests.
+if [ $NUM_JAVA_FILES_CHANGED -gt 0 ] || [ $NUM_UNIT_GOLDEN_FILES_CHANGED -gt 0 ]
 then
-  echo_status "Checking tests..."
+  echo_status "Checking unit tests..."
   bazel --batch test --disk_cache="$BAZEL_CACHE_DIR" //src/test/...
   TEST_STATUS=$?
   if [ $TEST_STATUS != 0 ]
@@ -114,6 +117,21 @@ then
   fi
 fi
 
+# Check integration tests.
+if [ $NUM_JAVA_FILES_CHANGED -gt 0 ] \
+  || [ $NUM_INTEGRATION_GOLDEN_FILES_CHANGED -gt 0 ] \
+  || [ $NUM_INTEGRATION_BAZEL_FILES_CHANGED -gt 0 ]
+then
+  echo_status "Checking integration tests..."
+  bazel --batch test --disk_cache="$BAZEL_CACHE_DIR" //test/integration/...
+  TEST_STATUS=$?
+  if [ $TEST_STATUS != 0 ]
+  then
+    echo_error "Tests failed." "Please fix them and try again."
+    exit 1
+  fi
+fi
+
 # Check and fix Bazel format.
 if [ $NUM_BAZEL_FILES_CHANGED -gt 0 ]
 then

rules_java_gapic/java_gapic.bzl

@@ -31,9 +31,14 @@ def _java_gapic_postprocess_srcjar_impl(ctx):
     unzip -q temp-codegen.srcjar -d {output_dir_path}
     # This may fail if there are spaces and/or too many files (exceed max length of command length).
     {formatter} --replace $(find {output_dir_path} -type f -printf "%p ")
-    zip -r -j {output_srcjar_name}.srcjar {output_dir_path}/src/main/*
-    zip -r -j {output_srcjar_name}-resource-name.srcjar {output_dir_path}/proto/src/main/*
-    zip -r -j {output_srcjar_name}-tests.srcjar {output_dir_path}/src/test/*
+    WORKING_DIR=`pwd`
+    cd {output_dir_path}/src/main/java
+    zip -r $WORKING_DIR/{output_srcjar_name}.srcjar ./
+    cd $WORKING_DIR/{output_dir_path}/proto/src/main/java
+    zip -r $WORKING_DIR/{output_srcjar_name}-resource-name.srcjar ./
+    cd $WORKING_DIR/{output_dir_path}/src/test/java
+    zip -r $WORKING_DIR/{output_srcjar_name}-tests.srcjar ./
+    cd $WORKING_DIR
     mv {output_srcjar_name}.srcjar {output_main}
     mv {output_srcjar_name}-resource-name.srcjar {output_resource_name}
     mv {output_srcjar_name}-tests.srcjar {output_test}

src/main/java/com/google/api/generator/engine/ast/ConcreteReference.java

@@ -89,11 +89,18 @@ public String pakkage() {
   public abstract boolean useFullName();
 
   @Override
-  public String enclosingClassName() {
+  public ImmutableList<String> enclosingClassNames() {
     if (!hasEnclosingClass()) {
-      return null;
+      return ImmutableList.of();
     }
-    return clazz().getEnclosingClass().getSimpleName();
+    // The innermost type will be the last element in the list.
+    ImmutableList.Builder<String> listBuilder = new ImmutableList.Builder<>();
+    Class currentClz = clazz();
+    while (currentClz.getEnclosingClass() != null) {
+      listBuilder.add(currentClz.getEnclosingClass().getSimpleName());
+      currentClz = currentClz.getEnclosingClass();
+    }
+    return listBuilder.build();
   }
 
   @Override

src/main/java/com/google/api/generator/engine/ast/Reference.java

@@ -30,7 +30,7 @@ public interface Reference {
   boolean useFullName();
 
   @Nullable
-  String enclosingClassName();
+  ImmutableList<String> enclosingClassNames();
 
   @Nullable
   Reference wildcardUpperBound();

src/main/java/com/google/api/generator/engine/ast/VaporReference.java

@@ -15,9 +15,9 @@
 package com.google.api.generator.engine.ast;
 
 import com.google.auto.value.AutoValue;
-import com.google.common.base.Strings;
 import com.google.common.collect.ImmutableList;
 import java.util.Arrays;
+import java.util.Collections;
 import java.util.List;
 import java.util.Objects;
 import javax.annotation.Nullable;
@@ -43,7 +43,7 @@ public abstract class VaporReference implements Reference {
 
   @Nullable
   @Override
-  public abstract String enclosingClassName();
+  public abstract ImmutableList<String> enclosingClassNames();
 
   @Nullable
   public abstract Reference supertypeReference();
@@ -56,9 +56,9 @@ public Reference wildcardUpperBound() {
 
   @Override
   public String fullName() {
-    // TODO(unsupported): Nested classes with depth greater than 1.
     if (hasEnclosingClass()) {
-      return String.format("%s.%s.%s", pakkage(), enclosingClassName(), plainName());
+      return String.format(
+          "%s.%s.%s", pakkage(), String.join(DOT, enclosingClassNames()), plainName());
     }
     return String.format("%s.%s", pakkage(), plainName());
   }
@@ -68,7 +68,7 @@ public String fullName() {
 
   @Override
   public boolean hasEnclosingClass() {
-    return !Strings.isNullOrEmpty(enclosingClassName());
+    return !enclosingClassNames().isEmpty();
   }
 
   @Override
@@ -86,7 +86,7 @@ public boolean isSupertypeOrEquals(Reference other) {
     VaporReference ref = (VaporReference) other;
     return pakkage().equals(ref.pakkage())
         && plainName().equals(ref.plainName())
-        && Objects.equals(enclosingClassName(), ref.enclosingClassName());
+        && Objects.equals(enclosingClassNames(), ref.enclosingClassNames());
   }
 
   @Override
@@ -112,14 +112,14 @@ public boolean equals(Object o) {
     return pakkage().equals(ref.pakkage())
         && name().equals(ref.name())
         && generics().equals(ref.generics())
-        && Objects.equals(enclosingClassName(), ref.enclosingClassName());
+        && Objects.equals(enclosingClassNames(), ref.enclosingClassNames());
   }
 
   @Override
   public int hashCode() {
     int hash = 17 * pakkage().hashCode() + 19 * name().hashCode() + 23 * generics().hashCode();
-    if (!Strings.isNullOrEmpty(enclosingClassName())) {
-      hash += 29 * enclosingClassName().hashCode();
+    if (!enclosingClassNames().isEmpty()) {
+      hash += 29 * enclosingClassNames().hashCode();
     }
     return hash;
   }
@@ -133,7 +133,8 @@ public static Builder builder() {
     return new AutoValue_VaporReference.Builder()
         .setUseFullName(false)
         .setGenerics(ImmutableList.of())
-        .setIsStaticImport(false);
+        .setIsStaticImport(false)
+        .setEnclosingClassNames(Collections.emptyList());
   }
 
   // Private.
@@ -153,7 +154,11 @@ public Builder setGenerics(Reference... references) {
 
     public abstract Builder setGenerics(List<Reference> clazzes);
 
-    public abstract Builder setEnclosingClassName(String enclosingClassName);
+    public Builder setEnclosingClassNames(String... enclosingClassNames) {
+      return setEnclosingClassNames(Arrays.asList(enclosingClassNames));
+    }
+
+    public abstract Builder setEnclosingClassNames(List<String> enclosingClassNames);
 
     public abstract Builder setIsStaticImport(boolean isStaticImport);
 
@@ -166,8 +171,7 @@ public Builder setGenerics(Reference... references) {
 
     abstract ImmutableList<Reference> generics();
 
-    @Nullable
-    abstract String enclosingClassName();
+    abstract ImmutableList<String> enclosingClassNames();
 
     abstract boolean isStaticImport();
 
@@ -180,11 +184,11 @@ public VaporReference build() {
 
       setPlainName(name());
 
-      setIsStaticImport(enclosingClassName() != null && isStaticImport());
+      setIsStaticImport(!enclosingClassNames().isEmpty() && isStaticImport());
 
       StringBuilder sb = new StringBuilder();
-      if (enclosingClassName() != null && !isStaticImport()) {
-        sb.append(enclosingClassName());
+      if (!enclosingClassNames().isEmpty() && !isStaticImport()) {
+        sb.append(String.join(DOT, enclosingClassNames()));
         sb.append(DOT);
       }
 

src/main/java/com/google/api/generator/engine/writer/ImportWriterVisitor.java

@@ -68,6 +68,7 @@
 import javax.annotation.Nullable;
 
 public class ImportWriterVisitor implements AstNodeVisitor {
+  private static final String DOT = ".";
   private static final String NEWLINE = "\n";
   private static final String PKG_JAVA_LANG = "java.lang";
 
@@ -423,7 +424,8 @@ private void references(List<Reference> refs) {
 
       if (ref.isStaticImport()
           && !Strings.isNullOrEmpty(currentClassName)
-          && ref.enclosingClassName().equals(currentClassName)) {
+          && !ref.enclosingClassNames().isEmpty()
+          && ref.enclosingClassNames().contains(currentClassName)) {
         continue;
       }
 
@@ -432,7 +434,8 @@ private void references(List<Reference> refs) {
         staticImports.add(ref.fullName());
       } else {
         if (ref.hasEnclosingClass()) {
-          imports.add(String.format("%s.%s", ref.pakkage(), ref.enclosingClassName()));
+          imports.add(
+              String.format("%s.%s", ref.pakkage(), String.join(DOT, ref.enclosingClassNames())));
         } else {
           imports.add(ref.fullName());
         }

src/main/java/com/google/api/generator/gapic/composer/BatchingDescriptorComposer.java

@@ -153,7 +153,7 @@ private static MethodDefinition createGetRequestBuilderMethod(
     TypeNode builderType =
         TypeNode.withReference(
             VaporReference.builder()
-                .setEnclosingClassName(method.inputType().reference().name())
+                .setEnclosingClassNames(method.inputType().reference().name())
                 .setName("Builder")
                 .setPakkage(method.inputType().reference().pakkage())
                 .build());

src/main/java/com/google/api/generator/gapic/composer/DefaultValueComposer.java

@@ -77,10 +77,22 @@ static Expr createDefaultValue(
   }
 
   static Expr createDefaultValue(Field f) {
+    return createDefaultValue(f, false);
+  }
+
+  static Expr createDefaultValue(Field f, boolean useExplicitInitTypeInGenerics) {
     if (f.isRepeated()) {
-      TypeNode newType =
-          TypeNode.withReference(
-              ConcreteReference.withClazz(f.isMap() ? HashMap.class : ArrayList.class));
+      ConcreteReference.Builder refBuilder =
+          ConcreteReference.builder().setClazz(f.isMap() ? HashMap.class : ArrayList.class);
+      if (useExplicitInitTypeInGenerics) {
+        if (f.isMap()) {
+          refBuilder = refBuilder.setGenerics(f.type().reference().generics().subList(0, 2));
+        } else {
+          refBuilder = refBuilder.setGenerics(f.type().reference().generics().get(0));
+        }
+      }
+
+      TypeNode newType = TypeNode.withReference(refBuilder.build());
       return NewObjectExpr.builder().setType(newType).setIsGeneric(true).build();
     }
 
@@ -238,7 +250,7 @@ static Expr createSimpleMessageBuilderExpr(
                 .setReturnType(TypeNode.STRING)
                 .build();
       } else {
-        defaultExpr = createDefaultValue(field);
+        defaultExpr = createDefaultValue(field, true);
       }
       builderExpr =
           MethodInvocationExpr.builder()

src/main/java/com/google/api/generator/gapic/composer/GrpcServiceStubClassComposer.java

@@ -1049,7 +1049,7 @@ private static Map<String, TypeNode> createDynamicTypes(Service service, String
                             VaporReference.builder()
                                 .setName(String.format(PAGED_RESPONSE_TYPE_NAME_PATTERN, m.name()))
                                 .setPakkage(service.pakkage())
-                                .setEnclosingClassName(String.format("%sClient", service.name()))
+                                .setEnclosingClassNames(String.format("%sClient", service.name()))
                                 .setIsStaticImport(true)
                                 .build()))));
     return types;

src/main/java/com/google/api/generator/gapic/composer/ResourceNameHelperClassComposer.java

@@ -1090,21 +1090,25 @@ private static MethodDefinition createToStringMethod(
       List<List<String>> tokenHierarchies) {
     boolean hasVariants = tokenHierarchies.size() > 1;
     if (!hasVariants) {
-      String token = getTokenSet(tokenHierarchies).stream().collect(Collectors.toList()).get(0);
-      String javaTokenVarName = JavaStyle.toLowerCamelCase(token);
-      Preconditions.checkNotNull(
-          patternTokenVarExprs.get(token),
-          String.format(
-              "No expression found for token %s amongst values %s",
-              javaTokenVarName, patternTokenVarExprs.toString()));
+
+      List<Expr> instantiateArgExprs = new ArrayList<>();
+      List<String> tokens = getTokenSet(tokenHierarchies).stream().collect(Collectors.toList());
+      for (int i = 0; i < tokens.size(); i++) {
+        String token = tokens.get(i);
+        Preconditions.checkNotNull(
+            patternTokenVarExprs.get(token),
+            String.format(
+                "No expression found for token %s amongst values %s",
+                token, patternTokenVarExprs.toString()));
+        instantiateArgExprs.add(ValueExpr.withValue(StringObjectValue.withValue(token)));
+        instantiateArgExprs.add(patternTokenVarExprs.get(token));
+      }
 
       MethodInvocationExpr returnInstantiateExpr =
           MethodInvocationExpr.builder()
               .setExprReferenceExpr(templateFinalVarExprs.get(0))
               .setMethodName("instantiate")
-              .setArguments(
-                  ValueExpr.withValue(StringObjectValue.withValue(token)),
-                  patternTokenVarExprs.get(token))
+              .setArguments(instantiateArgExprs)
               .setReturnType(TypeNode.STRING)
               .build();
       return MethodDefinition.builder()
@@ -1575,7 +1579,7 @@ private static Map<String, TypeNode> createDynamicTypes(
             VaporReference.builder()
                 .setName("Builder")
                 .setPakkage(resourceName.pakkage())
-                .setEnclosingClassName(thisClassName)
+                .setEnclosingClassNames(thisClassName)
                 .setIsStaticImport(true)
                 .build()));
 
@@ -1591,7 +1595,7 @@ private static Map<String, TypeNode> createDynamicTypes(
                               VaporReference.builder()
                                   .setName(s)
                                   .setPakkage(resourceName.pakkage())
-                                  .setEnclosingClassName(thisClassName)
+                                  .setEnclosingClassNames(thisClassName)
                                   .setIsStaticImport(true)
                                   .build()))));
     }