Deprecate <extraDirectory> config in Maven

jib-maven-plugin/src/main/java/com/google/cloud/tools/jib/maven/JibPluginConfiguration.java

@@ -99,7 +99,7 @@ public ToAuthConfiguration() {
     }
   }
 
-  /** Used to configure {@code extraDirectory.permissions} parameter. */
+  /** Used to configure {@code extraDirectories.permissions} parameter. */
   public static class PermissionConfiguration {
 
     @Nullable @Parameter private String file;
@@ -183,29 +183,38 @@ public static class ContainerParameters {
     @Nullable @Parameter private String workingDirectory;
   }
 
+  /** Configuration for the {@code extraDirectories} parameter. */
+  public static class ExtraDirectoriesParameters {
+
+    @Parameter private List<File> paths = Collections.emptyList();
+
+    @Parameter private List<PermissionConfiguration> permissions = Collections.emptyList();
+
+    public List<File> getPaths() {
+      return paths;
+    }
+  }
+
   /** Configuration for the {@code extraDirectory} parameter. */
+  @Deprecated
   public static class ExtraDirectoryParameters {
 
     // retained for backward-compatibility for <extraDirectory><path>...<path></extraDirectory>
     @Deprecated @Nullable @Parameter private File path;
 
-    @Parameter private List<File> paths = Collections.emptyList();
-
-    @Parameter private List<PermissionConfiguration> permissions = Collections.emptyList();
+    @Deprecated @Parameter
+    private List<PermissionConfiguration> permissions = Collections.emptyList();
 
-    /**
-     * Allows users to configure {@code path} using just {@code <extraDirectory>} instead of {@code
-     * <extraDirectory><path>}.
-     *
-     * @param path the value to set {@code path} to
-     */
+    // Allows users to configure a single path using just <extraDirectory> instead of
+    // <extraDirectory><path>.
     @Deprecated
     public void set(File path) {
-      this.paths = Collections.singletonList(path);
+      this.path = path;
     }
 
+    @Deprecated
     public List<File> getPaths() {
-      return path != null ? Collections.singletonList(path) : paths;
+      return path == null ? Collections.emptyList() : Collections.singletonList(path);
     }
   }
 
@@ -224,7 +233,11 @@ public List<File> getPaths() {
   @Parameter private ContainerParameters container = new ContainerParameters();
 
   // this parameter is cloned in FilesMojo
-  @Parameter private ExtraDirectoryParameters extraDirectory = new ExtraDirectoryParameters();
+  @Deprecated @Parameter
+  private ExtraDirectoryParameters extraDirectory = new ExtraDirectoryParameters();
+
+  // this parameter is cloned in FilesMojo
+  @Parameter private ExtraDirectoriesParameters extraDirectories = new ExtraDirectoriesParameters();
 
   @Parameter(
       defaultValue = "false",
@@ -512,12 +525,38 @@ String getFormat() {
    */
   List<Path> getExtraDirectories() {
     // TODO: Should inform user about nonexistent directory if using custom directory.
-    String property = getProperty(PropertyNames.EXTRA_DIRECTORY_PATH);
+    String deprecatedProperty = getProperty(PropertyNames.EXTRA_DIRECTORY_PATH);
+    String newProperty = getProperty(PropertyNames.EXTRA_DIRECTORIES_PATHS);
+
+    List<File> deprecatedPaths = extraDirectory.getPaths();
+    List<File> newPaths = extraDirectories.getPaths();
+
+    if (deprecatedProperty != null) {
+      getLog()
+          .warn(
+              "The property 'jib.extraDirectory.path' is deprecated; "
+                  + "use 'jib.extraDirectories.paths' instead");
+    }
+    if (!deprecatedPaths.isEmpty()) {
+      getLog().warn("<extraDirectory> is deprecated; use <extraDirectories> with <paths><path>");
+    }
+    if (deprecatedProperty != null && newProperty != null) {
+      throw new IllegalArgumentException(
+          "You cannot configure both 'jib.extraDirectory.path' and 'jib.extraDirectories.paths'");
+    }
+    if (!deprecatedPaths.isEmpty() && !newPaths.isEmpty()) {
+      throw new IllegalArgumentException(
+          "You cannot configure both <extraDirectory> and <extraDirectories>");
+    }
+
+    String property = newProperty != null ? newProperty : deprecatedProperty;
     if (property != null) {
       List<String> paths = ConfigurationPropertyValidator.parseListProperty(property);
       return paths.stream().map(Paths::get).collect(Collectors.toList());
     }
-    return extraDirectory.getPaths().stream().map(File::toPath).collect(Collectors.toList());
+
+    List<File> paths = !newPaths.isEmpty() ? newPaths : deprecatedPaths;
+    return paths.stream().map(File::toPath).collect(Collectors.toList());
   }
 
   /**
@@ -526,15 +565,40 @@ List<Path> getExtraDirectories() {
    * @return the configured extra layer file permissions
    */
   List<PermissionConfiguration> getExtraDirectoryPermissions() {
-    String property = getProperty(PropertyNames.EXTRA_DIRECTORY_PERMISSIONS);
+    String deprecatedProperty = getProperty(PropertyNames.EXTRA_DIRECTORY_PERMISSIONS);
+    String newProperty = getProperty(PropertyNames.EXTRA_DIRECTORIES_PERMISSIONS);
+
+    List<PermissionConfiguration> deprecatedPermissions = extraDirectory.permissions;
+    List<PermissionConfiguration> newPermissions = extraDirectories.permissions;
+
+    if (deprecatedProperty != null) {
+      getLog()
+          .warn(
+              "The property 'jib.extraDirectory.permissions' is deprecated; "
+                  + "use 'jib.extraDirectories.permissions' instead");
+    }
+    if (deprecatedProperty != null && newProperty != null) {
+      throw new IllegalArgumentException(
+          "You cannot configure both 'jib.extraDirectory.permissions' and "
+              + "'jib.extraDirectories.permissions'");
+    }
+    if (!deprecatedPermissions.isEmpty() && !newPermissions.isEmpty()) {
+      throw new IllegalArgumentException(
+          "You cannot configure both <extraDirectory> and <extraDirectories>");
+    }
+
+    String property = newProperty != null ? newProperty : deprecatedProperty;
     if (property != null) {
       return ConfigurationPropertyValidator.parseMapProperty(property)
           .entrySet()
           .stream()
           .map(entry -> new PermissionConfiguration(entry.getKey(), entry.getValue()))
           .collect(Collectors.toList());
     }
-    return extraDirectory.permissions;
+
+    return !extraDirectories.getPaths().isEmpty()
+        ? extraDirectories.permissions
+        : extraDirectory.permissions;
   }
 
   boolean getAllowInsecureRegistries() {

jib-maven-plugin/src/main/java/com/google/cloud/tools/jib/maven/skaffold/FilesMojo.java

@@ -16,8 +16,11 @@
 
 package com.google.cloud.tools.jib.maven.skaffold;
 
+import com.google.cloud.tools.jib.maven.JibPluginConfiguration.ExtraDirectoriesParameters;
 import com.google.cloud.tools.jib.maven.JibPluginConfiguration.ExtraDirectoryParameters;
 import com.google.cloud.tools.jib.maven.MavenProjectProperties;
+import com.google.cloud.tools.jib.plugins.common.ConfigurationPropertyValidator;
+import com.google.cloud.tools.jib.plugins.common.PropertyNames;
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Preconditions;
 import com.google.common.collect.ImmutableSet;
@@ -44,6 +47,7 @@
 import org.apache.maven.project.DependencyResolutionResult;
 import org.apache.maven.project.MavenProject;
 import org.apache.maven.project.ProjectDependenciesResolver;
+import org.eclipse.aether.graph.Dependency;
 import org.eclipse.aether.graph.DependencyFilter;
 
 /**
@@ -78,7 +82,11 @@ public class FilesMojo extends AbstractMojo {
   @Nullable @Component private ProjectDependenciesResolver projectDependenciesResolver;
 
   // This parameter is cloned from JibPluginConfiguration
-  @Parameter private ExtraDirectoryParameters extraDirectory = new ExtraDirectoryParameters();
+  @Deprecated @Parameter
+  private ExtraDirectoryParameters extraDirectory = new ExtraDirectoryParameters();
+
+  // This parameter is cloned from JibPluginConfiguration
+  @Parameter private ExtraDirectoriesParameters extraDirectories = new ExtraDirectoriesParameters();
 
   @Override
   public void execute() throws MojoExecutionException, MojoFailureException {
@@ -143,7 +151,7 @@ public void execute() throws MojoExecutionException, MojoFailureException {
       resolutionResult
           .getDependencies()
           .stream()
-          .map(org.eclipse.aether.graph.Dependency::getArtifact)
+          .map(Dependency::getArtifact)
           .filter(org.eclipse.aether.artifact.Artifact::isSnapshot)
           .map(org.eclipse.aether.artifact.Artifact::getFile)
           .forEach(System.out::println);
@@ -153,8 +161,32 @@ public void execute() throws MojoExecutionException, MojoFailureException {
     }
   }
 
-  private List<Path> resolveExtraDirectories() {
-    List<File> paths = extraDirectory.getPaths();
+  private List<Path> resolveExtraDirectories() throws MojoExecutionException {
+    // TODO: Should inform user about nonexistent directory if using custom directory.
+    String deprecatedProperty =
+        MavenProjectProperties.getProperty(PropertyNames.EXTRA_DIRECTORY_PATH, project, session);
+    String newProperty =
+        MavenProjectProperties.getProperty(PropertyNames.EXTRA_DIRECTORIES_PATHS, project, session);
+
+    List<File> deprecatedPaths = extraDirectory.getPaths();
+    List<File> newPaths = extraDirectories.getPaths();
+
+    if (deprecatedProperty != null && newProperty != null) {
+      throw new MojoExecutionException(
+          "You cannot configure both 'jib.extraDirectory.path' and 'jib.extraDirectories.paths'");
+    }
+    if (!deprecatedPaths.isEmpty() && !newPaths.isEmpty()) {
+      throw new MojoExecutionException(
+          "You cannot configure both <extraDirectory> and <extraDirectories>");
+    }
+
+    String property = newProperty != null ? newProperty : deprecatedProperty;
+    if (property != null) {
+      List<String> paths = ConfigurationPropertyValidator.parseListProperty(property);
+      return paths.stream().map(Paths::get).map(Path::toAbsolutePath).collect(Collectors.toList());
+    }
+
+    List<File> paths = !newPaths.isEmpty() ? newPaths : deprecatedPaths;
     if (paths.isEmpty()) {
       Path projectBase =
           Preconditions.checkNotNull(project).getBasedir().getAbsoluteFile().toPath();

jib-maven-plugin/src/main/java/com/google/cloud/tools/jib/maven/skaffold/FilesMojoV2.java

@@ -17,6 +17,7 @@
 package com.google.cloud.tools.jib.maven.skaffold;
 
 import com.google.cloud.tools.jib.maven.MavenProjectProperties;
+import com.google.cloud.tools.jib.plugins.common.ConfigurationPropertyValidator;
 import com.google.cloud.tools.jib.plugins.common.PropertyNames;
 import com.google.cloud.tools.jib.plugins.common.SkaffoldFilesOutput;
 import com.google.common.annotations.VisibleForTesting;
@@ -167,34 +168,54 @@ public void execute() throws MojoExecutionException, MojoFailureException {
     }
   }
 
-  private List<Path> resolveExtraDirectories(MavenProject project) {
+  private List<Path> resolveExtraDirectories(MavenProject project) throws MojoExecutionException {
     // Try getting extra directory from project/session properties
-    String extraDirectoryProperty =
+    String deprecatedProperty =
         MavenProjectProperties.getProperty(PropertyNames.EXTRA_DIRECTORY_PATH, project, session);
-    if (extraDirectoryProperty != null) {
-      return Collections.singletonList(Paths.get(extraDirectoryProperty));
+    String newProperty =
+        MavenProjectProperties.getProperty(PropertyNames.EXTRA_DIRECTORIES_PATHS, project, session);
+
+    if (deprecatedProperty != null && newProperty != null) {
+      throw new MojoExecutionException(
+          "You cannot configure both 'jib.extraDirectory.path' and 'jib.extraDirectories.paths'");
+    }
+
+    String property = newProperty != null ? newProperty : deprecatedProperty;
+    if (property != null) {
+      List<String> paths = ConfigurationPropertyValidator.parseListProperty(property);
+      return paths.stream().map(Paths::get).collect(Collectors.toList());
     }
 
     // Try getting extra directory from project pom
     Plugin jibMavenPlugin = project.getPlugin(MavenProjectProperties.PLUGIN_KEY);
     if (jibMavenPlugin != null) {
       Xpp3Dom pluginConfiguration = (Xpp3Dom) jibMavenPlugin.getConfiguration();
       if (pluginConfiguration != null) {
+
         Xpp3Dom extraDirectoryConfiguration = pluginConfiguration.getChild("extraDirectory");
-        if (extraDirectoryConfiguration != null) {
-          Xpp3Dom pathChild = extraDirectoryConfiguration.getChild("path");
-          if (pathChild != null) {
-            // <extraDirectory><path>...</path></extraDirectory>
-            return Collections.singletonList(Paths.get(pathChild.getValue()));
-          }
-          Xpp3Dom pathsChild = extraDirectoryConfiguration.getChild("paths");
-          if (pathsChild != null) {
-            // <extraDirectory><paths><path>...<path><path>...<path></paths></extraDirectory>
-            return Arrays.stream(pathsChild.getChildren())
+        Xpp3Dom extraDirectoriesConfiguration = pluginConfiguration.getChild("extraDirectories");
+        if (extraDirectoryConfiguration != null && extraDirectoriesConfiguration != null) {
+          throw new MojoExecutionException(
+              "You cannot configure both <extraDirectory> and <extraDirectories>");
+        }
+
+        if (extraDirectoriesConfiguration != null) {
+          Xpp3Dom child = extraDirectoriesConfiguration.getChild("paths");
+          if (child != null) {
+            // <extraDirectories><paths><path>...</path><path>...</path></paths></extraDirectories>
+            return Arrays.stream(child.getChildren())
                 .map(Xpp3Dom::getValue)
                 .map(Paths::get)
                 .collect(Collectors.toList());
           }
+        }
+
+        if (extraDirectoryConfiguration != null) {
+          Xpp3Dom child = extraDirectoryConfiguration.getChild("path");
+          if (child != null) {
+            // <extraDirectory><path>...</path></extraDirectory>
+            return Collections.singletonList(Paths.get(child.getValue()));
+          }
           // <extraDirectory>...</extraDirectory>
           String value = extraDirectoryConfiguration.getValue();
           if (value != null) {

jib-maven-plugin/src/test/java/com/google/cloud/tools/jib/maven/JibPluginConfigurationTest.java

@@ -111,10 +111,10 @@ public void testSystemProperties() {
     sessionProperties.put("jib.container.workingDirectory", "working directory");
     Assert.assertEquals("working directory", testPluginConfiguration.getWorkingDirectory());
 
-    sessionProperties.put("jib.extraDirectory.path", "custom-jib");
+    sessionProperties.put("jib.extraDirectories.paths", "custom-jib");
     Assert.assertEquals(
         Arrays.asList(Paths.get("custom-jib")), testPluginConfiguration.getExtraDirectories());
-    sessionProperties.put("jib.extraDirectory.permissions", "/test/file1=123,/another/file=456");
+    sessionProperties.put("jib.extraDirectories.permissions", "/test/file1=123,/another/file=456");
     List<PermissionConfiguration> permissions =
         testPluginConfiguration.getExtraDirectoryPermissions();
     Assert.assertEquals("/test/file1", permissions.get(0).getFile().get());
@@ -173,12 +173,12 @@ public void testPomProperties() {
     project.getProperties().setProperty("jib.container.workingDirectory", "working directory");
     Assert.assertEquals("working directory", testPluginConfiguration.getWorkingDirectory());
 
-    project.getProperties().setProperty("jib.extraDirectory.path", "custom-jib");
+    project.getProperties().setProperty("jib.extraDirectories.paths", "custom-jib");
     Assert.assertEquals(
         Arrays.asList(Paths.get("custom-jib")), testPluginConfiguration.getExtraDirectories());
     project
         .getProperties()
-        .setProperty("jib.extraDirectory.permissions", "/test/file1=123,/another/file=456");
+        .setProperty("jib.extraDirectories.permissions", "/test/file1=123,/another/file=456");
     List<PermissionConfiguration> permissions =
         testPluginConfiguration.getExtraDirectoryPermissions();
     Assert.assertEquals("/test/file1", permissions.get(0).getFile().get());
@@ -200,4 +200,36 @@ public void testEmptyOrNullTags() {
       Assert.assertEquals("jib.to.tags has empty tag", ex.getMessage());
     }
   }
+
+  @Test
+  public void testDeprecatedSystemProperties() {
+    sessionProperties.put("jib.extraDirectory.path", "custom-jib");
+    Assert.assertEquals(
+        Arrays.asList(Paths.get("custom-jib")), testPluginConfiguration.getExtraDirectories());
+    sessionProperties.put("jib.extraDirectory.permissions", "/test/file13=650,/another/file24=777");
+    List<PermissionConfiguration> permissions =
+        testPluginConfiguration.getExtraDirectoryPermissions();
+    Assert.assertEquals("/test/file13", permissions.get(0).getFile().get());
+    Assert.assertEquals("650", permissions.get(0).getMode().get());
+    Assert.assertEquals("/another/file24", permissions.get(1).getFile().get());
+    Assert.assertEquals("777", permissions.get(1).getMode().get());
+  }
+
+  @Test
+  public void testDeprecatedProperties() {
+    Properties projectProperties = project.getProperties();
+
+    projectProperties.setProperty("jib.extraDirectory.path", "this-is-extra");
+    Assert.assertEquals(
+        Arrays.asList(Paths.get("this-is-extra")), testPluginConfiguration.getExtraDirectories());
+
+    projectProperties.setProperty(
+        "jib.extraDirectory.permissions", "/test/file1=654,/dir/file2=321");
+    List<PermissionConfiguration> permissions =
+        testPluginConfiguration.getExtraDirectoryPermissions();
+    Assert.assertEquals("/test/file1", permissions.get(0).getFile().get());
+    Assert.assertEquals("654", permissions.get(0).getMode().get());
+    Assert.assertEquals("/dir/file2", permissions.get(1).getFile().get());
+    Assert.assertEquals("321", permissions.get(1).getMode().get());
+  }
 }