Minor cleanups after merging WAR Gradle support contribution (#1072)

jib-core/src/test/java/com/google/cloud/tools/jib/frontend/JavaDockerContextGeneratorTest.java

@@ -167,7 +167,7 @@ public void testMakeDockerfile() throws IOException {
  }
 
  @Test
- public void testMakeDockerfileWithWebapp() throws IOException {
+ public void testMakeDockerfileWithWebApp() throws IOException {
  String expectedBaseImage = "tomcat:8.5-jre8-alpine";
  AbsoluteUnixPath exepectedAppRoot = AbsoluteUnixPath.get("/usr/local/tomcat/webapps/ROOT/");
 
@@ -197,7 +197,7 @@ public void testMakeDockerfileWithWebapp() throws IOException {
  // Need to split/rejoin the string here to avoid cross-platform troubles
  List<String> sampleDockerfile =
  Resources.readLines(
- Resources.getResource("webappSampleDockerfile"), StandardCharsets.UTF_8);
+ Resources.getResource("webAppSampleDockerfile"), StandardCharsets.UTF_8);
  Assert.assertEquals(String.join("\n", sampleDockerfile), dockerfile);
  }
 }

jib-core/src/test/java/com/google/cloud/tools/jib/frontend/JavaLayerConfigurationsTest.java

@@ -161,46 +161,37 @@ public void testAddFile_regularFiles() throws IOException, URISyntaxException {
 
  @Test
  public void testWebApp() throws IOException {
- AbsoluteUnixPath expectedAppRoot = AbsoluteUnixPath.get("/usr/local/tomcat/webapps/ROOT/");
- JavaLayerConfigurations.Builder layerBuilder = JavaLayerConfigurations.builder();
+ AbsoluteUnixPath appRoot = AbsoluteUnixPath.get("/usr/local/tomcat/webapps/ROOT/");
 
  JavaLayerConfigurations configurations =
- layerBuilder
- .addResourceFile(Paths.get("test.jsp"), expectedAppRoot.resolve("test.jsp"))
- .addResourceFile(Paths.get("META-INF/"), expectedAppRoot.resolve("META-INF/"))
- .addResourceFile(
- Paths.get("context.xml"), expectedAppRoot.resolve("WEB-INF/context.xml"))
- .addResourceFile(Paths.get("sub_dir/"), expectedAppRoot.resolve("WEB-INF/sub_dir/"))
- .addDependencyFile(
- Paths.get("myLib.jar"), expectedAppRoot.resolve("WEB-INF/lib/myLib.jar"))
+ JavaLayerConfigurations.builder()
+ .addResourceFile(Paths.get("test.jsp"), appRoot.resolve("test.jsp"))
+ .addResourceFile(Paths.get("META-INF/"), appRoot.resolve("META-INF/"))
+ .addResourceFile(Paths.get("context.xml"), appRoot.resolve("WEB-INF/context.xml"))
+ .addResourceFile(Paths.get("sub_dir/"), appRoot.resolve("WEB-INF/sub_dir/"))
+ .addDependencyFile(Paths.get("myLib.jar"), appRoot.resolve("WEB-INF/lib/myLib.jar"))
  .addSnapshotDependencyFile(
- Paths.get("my-SNAPSHOT.jar"),
- expectedAppRoot.resolve("WEB-INF/lib/my-SNAPSHOT.jar"))
- .addClassFile(
- Paths.get("test.class"), expectedAppRoot.resolve("WEB-INF/classes/test.class"))
+ Paths.get("my-SNAPSHOT.jar"), appRoot.resolve("WEB-INF/lib/my-SNAPSHOT.jar"))
+ .addClassFile(Paths.get("test.class"), appRoot.resolve("WEB-INF/classes/test.class"))
  .addExtraFile(Paths.get("extra.file"), AbsoluteUnixPath.get("/extra.file"))
  .build();
 
  ImmutableList<LayerEntry> expectedDependenciesLayer =
  ImmutableList.of(
- new LayerEntry(
- Paths.get("myLib.jar"), expectedAppRoot.resolve("WEB-INF/lib/myLib.jar")));
+ new LayerEntry(Paths.get("myLib.jar"), appRoot.resolve("WEB-INF/lib/myLib.jar")));
  ImmutableList<LayerEntry> expectedSnapshotDependenciesLayer =
  ImmutableList.of(
  new LayerEntry(
- Paths.get("my-SNAPSHOT.jar"),
- expectedAppRoot.resolve("WEB-INF/lib/my-SNAPSHOT.jar")));
+ Paths.get("my-SNAPSHOT.jar"), appRoot.resolve("WEB-INF/lib/my-SNAPSHOT.jar")));
  ImmutableList<LayerEntry> expectedResourcesLayer =
  ImmutableList.of(
- new LayerEntry(Paths.get("test.jsp"), expectedAppRoot.resolve("test.jsp")),
- new LayerEntry(Paths.get("META-INF"), expectedAppRoot.resolve("META-INF")),
- new LayerEntry(
- Paths.get("context.xml"), expectedAppRoot.resolve("WEB-INF/context.xml")),
- new LayerEntry(Paths.get("sub_dir"), expectedAppRoot.resolve("WEB-INF/sub_dir")));
+ new LayerEntry(Paths.get("test.jsp"), appRoot.resolve("test.jsp")),
+ new LayerEntry(Paths.get("META-INF"), appRoot.resolve("META-INF")),
+ new LayerEntry(Paths.get("context.xml"), appRoot.resolve("WEB-INF/context.xml")),
+ new LayerEntry(Paths.get("sub_dir"), appRoot.resolve("WEB-INF/sub_dir")));
  ImmutableList<LayerEntry> expectedClassesLayer =
  ImmutableList.of(
- new LayerEntry(
- Paths.get("test.class"), expectedAppRoot.resolve("WEB-INF/classes/test.class")));
+ new LayerEntry(Paths.get("test.class"), appRoot.resolve("WEB-INF/classes/test.class")));
  ImmutableList<LayerEntry> expectedExtraLayer =
  ImmutableList.of(
  new LayerEntry(Paths.get("extra.file"), AbsoluteUnixPath.get("/extra.file")));

jib-gradle-plugin/src/main/java/com/google/cloud/tools/jib/gradle/ExplodedWarTask.java

@@ -22,18 +22,19 @@
 import org.gradle.api.tasks.OutputDirectory;
 import org.gradle.api.tasks.Sync;
 
-/** This Gradle Task explodes a War file into a directory */
+/** Gradle task that explodes a WAR file into a directory. */
 public class ExplodedWarTask extends Sync {
+
  @Nullable private File explodedWarDirectory;
 
- public void setWarFile(File warFile) {
+ public void setWarFile(Path warFile) {
  from(getProject().zipTree(warFile));
  }
 
  /**
- * Sets the output directory of Sync Task
+ * Sets the exploded WAR output directory of this {@link Sync} task.
  *
- * @param explodedWarDirectory the directory where to extract the war file
+ * @param explodedWarDirectory the directory where to extract the WAR file
  */
  public void setExplodedWarDirectory(Path explodedWarDirectory) {
  this.explodedWarDirectory = explodedWarDirectory.toFile();

jib-gradle-plugin/src/main/java/com/google/cloud/tools/jib/gradle/GradleLayerConfigurations.java

@@ -23,7 +23,6 @@
 import com.google.cloud.tools.jib.frontend.JavaLayerConfigurations.Builder;
 import java.io.File;
 import java.io.IOException;
-import java.nio.file.FileSystems;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.util.ArrayList;
@@ -42,23 +41,17 @@ class GradleLayerConfigurations {
  /** Name of the `main` {@link SourceSet} to use as source files. */
  private static final String MAIN_SOURCE_SET_NAME = "main";
 
- /** The filename suffix for a maven/gradle snapshot dependency */
+ /** The filename suffix for snapshot dependency JARs. */
  private static final String SNAPSHOT = "SNAPSHOT";
- /** The standard directory name containing libs, classes, web.xml, etc... in a War Project */
- private static final String WEB_INF = "WEB-INF";
- /** The standard directory name containing libs and snapshot-libs in a War Project */
- private static final String WEB_INF_LIB = WEB_INF + "/lib/";
- /** The standard directory name containing classes and some resources in a War Project */
- private static final String WEB_INF_CLASSES = WEB_INF + "/classes/";
 
  /**
  * Resolves the {@link JavaLayerConfigurations} for a Gradle {@link Project}.
  *
  * @param project the Gradle {@link Project}
  * @param logger the logger for providing feedback about the resolution
- * @param extraDirectory path to the directory for the extra files layer
+ * @param extraDirectory path to the source directory for the extra files layer
  * @param appRoot root directory in the image where the app will be placed
- * @return a {@link JavaLayerConfigurations} for the layers for the Gradle {@link Project}
+ * @return {@link JavaLayerConfigurations} for the layers for the Gradle {@link Project}
  * @throws IOException if an I/O exception occurred during resolution
  */
  static JavaLayerConfigurations getForProject(
@@ -77,9 +70,9 @@ static JavaLayerConfigurations getForProject(
  *
  * @param project the Gradle {@link Project}
  * @param logger the logger for providing feedback about the resolution
- * @param extraDirectory path to the directory for the extra files layer
+ * @param extraDirectory path to the source directory for the extra files layer
  * @param appRoot root directory in the image where the app will be placed
- * @return a {@link JavaLayerConfigurations} for the layers for the Gradle {@link Project}
+ * @return {@link JavaLayerConfigurations} for the layers for the Gradle {@link Project}
  * @throws IOException if an I/O exception occurred during resolution
  */
  private static JavaLayerConfigurations getForNonWarProject(
@@ -169,9 +162,9 @@ private static JavaLayerConfigurations getForNonWarProject(
  *
  * @param project the Gradle {@link Project}
  * @param logger the build logger for providing feedback about the resolution
- * @param extraDirectory path to the directory for the extra files layer
+ * @param extraDirectory path to the source directory for the extra files layer
  * @param appRoot root directory in the image where the app will be placed
- * @return a {@link JavaLayerConfigurations} for the layers for the Gradle {@link Project}
+ * @return {@link JavaLayerConfigurations} for the layers for the Gradle {@link Project}
  * @throws IOException if an I/O exception occurred during resolution
  */
  private static JavaLayerConfigurations getForWarProject(
@@ -185,7 +178,7 @@ private static JavaLayerConfigurations getForWarProject(
 
  Path explodedWarPath = GradleProjectProperties.getExplodedWarDirectory(project);
 
- Path libOutputDirectory = explodedWarPath.resolve(WEB_INF_LIB);
+ Path libOutputDirectory = explodedWarPath.resolve("WEB-INF/lib");
  try (Stream<Path> dependencyFileStream = Files.list(libOutputDirectory)) {
  dependencyFileStream.forEach(
  path -> {
@@ -196,13 +189,15 @@ private static JavaLayerConfigurations getForWarProject(
  }
  });
  }
- // All files except classes and libs in resources
+
+ // First, all files except WEB-INF go into the resources layer.
  try (Stream<Path> fileStream = Files.list(explodedWarPath)) {
- fileStream.filter(path -> !path.endsWith(WEB_INF)).forEach(resourcesFiles::add);
+ fileStream.filter(path -> !path.endsWith("WEB-INF")).forEach(resourcesFiles::add);
  }
- // Some files in WEB-INF need to be in resources directory (e.g. web.xml, ...)
- Path webinfOutputDirectory = explodedWarPath.resolve(WEB_INF);
- try (Stream<Path> fileStream = Files.list(webinfOutputDirectory)) {
+ // Some files in WEB-INF/ (e.g. web.xml, ...) need to go into the resources layer. However,
+ // don't add or look into WEB-INF/classes and WEB-INF/lib.
+ Path webInfOutputDirectory = explodedWarPath.resolve("WEB-INF");
+ try (Stream<Path> fileStream = Files.list(webInfOutputDirectory)) {
  fileStream
  .filter(path -> !path.endsWith("classes") && !path.endsWith("lib"))
  .forEach(resourcesWebInfFiles::add);
@@ -216,31 +211,31 @@ private static JavaLayerConfigurations getForWarProject(
  }
 
  Builder layerBuilder = JavaLayerConfigurations.builder();
- AbsoluteUnixPath dependenciesExtractionPath = appRoot.resolve(WEB_INF_LIB);
- AbsoluteUnixPath classesExtractionPath = appRoot.resolve(WEB_INF_CLASSES);
- AbsoluteUnixPath webInfExtractionPath = appRoot.resolve(WEB_INF);
+ AbsoluteUnixPath dependenciesExtractionPath = appRoot.resolve("WEB-INF/lib");
+ AbsoluteUnixPath classesExtractionPath = appRoot.resolve("WEB-INF/classes");
+ AbsoluteUnixPath webInfExtractionPath = appRoot.resolve("WEB-INF");
 
- // For "WEB-INF/classes", *.class in class layer, other files and empty directories in resource
- // layer
- Path srcWebInfClasses = explodedWarPath.resolve(WEB_INF_CLASSES);
- new DirectoryWalker(srcWebInfClasses)
+ // For "WEB-INF/classes", *.class go into the class layer. All other files and empty directories
+ // go into the resource layer.
+ Path webInfClasses = explodedWarPath.resolve("WEB-INF/classes");
+ new DirectoryWalker(webInfClasses)
  .walk(
  path -> {
- if (FileSystems.getDefault().getPathMatcher("glob:**.class").matches(path)) {
- layerBuilder.addClassFile(
- path, classesExtractionPath.resolve(srcWebInfClasses.relativize(path)));
+ AbsoluteUnixPath pathInContainer =
+ classesExtractionPath.resolve(webInfClasses.relativize(path));
+
+ if (path.getFileName().toString().endsWith(".class")) {
+ layerBuilder.addClassFile(path, pathInContainer);
 
  } else if (Files.isDirectory(path)) {
- try (Stream<Path> dirStream = Files.list(path)) {
- if (!dirStream.findAny().isPresent()) {
+ try (Stream<Path> fileStream = Files.list(path)) {
+ if (!fileStream.findAny().isPresent()) {
  // The directory is empty
- layerBuilder.addResourceFile(
- path, classesExtractionPath.resolve(srcWebInfClasses.relativize(path)));
+ layerBuilder.addResourceFile(path, pathInContainer);
  }
  }
  } else {
- layerBuilder.addResourceFile(
- path, classesExtractionPath.resolve(srcWebInfClasses.relativize(path)));
+ layerBuilder.addResourceFile(path, pathInContainer);
  }
  });
 

jib-gradle-plugin/src/main/java/com/google/cloud/tools/jib/gradle/JibExtension.java

@@ -97,6 +97,7 @@ public JibExtension(Project project) {
  allowInsecureRegistries = objectFactory.property(Boolean.class);
  extraDirectory = objectFactory.property(Path.class);
 
+ // Sets defaults.
  jvmFlags.set(Collections.emptyList());
  args.set(Collections.emptyList());
  useOnlyProjectCache.set(DEFAULT_USE_ONLY_PROJECT_CACHE);

jib-gradle-plugin/src/main/java/com/google/cloud/tools/jib/gradle/JibPlugin.java

@@ -16,6 +16,7 @@
 
 package com.google.cloud.tools.jib.gradle;
 
+import com.google.cloud.tools.jib.frontend.JavaLayerConfigurations;
 import com.google.common.annotations.VisibleForTesting;
 import java.util.List;
 import java.util.stream.Collectors;
@@ -44,18 +45,13 @@ public class JibPlugin implements Plugin<Project> {
  @VisibleForTesting static final String EXPLODED_WAR_TASK_NAME = "jibExplodedWar";
 
  @VisibleForTesting static final String DEFAULT_FROM_IMAGE = "gcr.io/distroless/java";
- @VisibleForTesting static final String DEFAULT_WEBAPP_FROM_IMAGE = "gcr.io/distroless/java/jetty";
- /**
- * The default app root in the image. For example, if this is set to {@code "/app"}, dependency
- * JARs will be in {@code "/app/libs"}.
- */
- @VisibleForTesting static final String DEFAULT_APP_ROOT = "/app";
+ @VisibleForTesting static final String DEFAULT_WAR_FROM_IMAGE = "gcr.io/distroless/java/jetty";
 
  /**
- * The default webapp root in the image. For example, if this is set to {@code
- * "/jetty/webapps/ROOT"}, dependency JARs will be in {@code "/jetty/webapps/ROOT/WEB-INF/lib"}.
+ * The default app root in the image for WAR. For example, if this is set to {@code
+ * /jetty/webapps/ROOT}, dependency JARs will be in {@code /jetty/webapps/ROOT/WEB-INF/lib}.
  */
- @VisibleForTesting static final String DEFAULT_WEBAPP_ROOT = "/jetty/webapps/ROOT";
+ @VisibleForTesting static final String DEFAULT_WEB_APP_ROOT = "/jetty/webapps/ROOT";
 
  /**
  * Collects all project dependencies of the style "compile project(':mylib')" for any kind of
@@ -124,36 +120,36 @@ public void apply(Project project) {
 
  project.afterEvaluate(
  projectAfterEvaluation -> {
- // TODO move this to a seperate place
+ // TODO move this to a separate place
  try {
  War warTask = GradleProjectProperties.getWarTask(project);
- final Task dependsOnTask;
+ Task dependsOnTask;
  if (warTask != null) {
  if (jibExtension.getFrom().getImage() == null) {
- jibExtension.getFrom().setImage(DEFAULT_WEBAPP_FROM_IMAGE);
+ jibExtension.getFrom().setImage(DEFAULT_WAR_FROM_IMAGE);
  }
  if (jibExtension.getContainer().getAppRoot().isEmpty()) {
- jibExtension.getContainer().setAppRoot(DEFAULT_WEBAPP_ROOT);
+ jibExtension.getContainer().setAppRoot(DEFAULT_WEB_APP_ROOT);
  }
- // Has all tasks depend on the 'exploded war' task.
  ExplodedWarTask explodedWarTask =
  (ExplodedWarTask)
  project
  .getTasks()
  .create(EXPLODED_WAR_TASK_NAME, ExplodedWarTask.class)
  .dependsOn(warTask);
- explodedWarTask.setWarFile(warTask.getArchivePath());
+ explodedWarTask.setWarFile(warTask.getArchivePath().toPath());
  explodedWarTask.setExplodedWarDirectory(
  GradleProjectProperties.getExplodedWarDirectory(projectAfterEvaluation));
+ // Have all tasks depend on the 'jibExplodedWar' task.
  dependsOnTask = explodedWarTask;
  } else {
  if (jibExtension.getFrom().getImage() == null) {
  jibExtension.getFrom().setImage(DEFAULT_FROM_IMAGE);
  }
  if (jibExtension.getContainer().getAppRoot().isEmpty()) {
- jibExtension.getContainer().setAppRoot(DEFAULT_APP_ROOT);
+ jibExtension.getContainer().setAppRoot(JavaLayerConfigurations.DEFAULT_APP_ROOT);
  }
- // Has all tasks depend on the 'classes' task.
+ // Have all tasks depend on the 'classes' task.
  dependsOnTask = projectAfterEvaluation.getTasks().getByPath("classes");
  }
  buildImageTask.dependsOn(dependsOnTask);