[MINVOKER-364] Rename invoker.systemPropertiesFile to invoker.userPropertiesFile

slawekjaranowski · slawekjaranowski · commit a741ea048540 · 2024-05-04T12:48:15.000+02:00
diff --git a/src/it/invocation-multiple/src/it/project/invoker.properties b/src/it/invocation-multiple/src/it/project/invoker.properties
@@ -20,7 +20,7 @@
 #######################################
 invoker.goals = install
 invoker.profiles = plugin,profile0
-invoker.systemPropertiesFile = system1.properties
+invoker.userPropertiesFile = user.properties
 
 #######################################
 # First build
@@ -40,4 +40,4 @@ invoker.profiles.2 = plugin,profile1
 #######################################
 invoker.goals.3 = test:test-maven-plugin:0.1-SNAPSHOT:test
 # profiles should fall back to invoker.profiles
-invoker.systemPropertiesFile.3 = system2.properties
+invoker.userPropertiesFile.3 = user3.properties
diff --git a/src/it/invocation-multiple/src/it/project/user.properties b/src/it/invocation-multiple/src/it/project/user.properties
diff --git a/src/it/invocation-multiple/src/it/project/user3.properties b/src/it/invocation-multiple/src/it/project/user3.properties
diff --git a/src/it/postbuild-executed-only-once/verify.groovy b/src/it/postbuild-executed-only-once/verify.groovy
@@ -17,25 +17,9 @@
  * under the License.
  */
 
-import java.io.*;
-import java.util.*;
-import java.util.regex.*;
+// make sure the Invoker Plugin was indeed run and the build didn't fail somewhere else
+def touchFile = new File(basedir, 'target/it/project/target/touch.txt')
+assert touchFile.exists()
 
-try
-{
-    // make sure the Invoker Plugin was indeed run and the build didn't fail somewhere else
-    File touchFile = new File( basedir, "target/it/project/target/touch.txt" );
-    System.out.println( "Checking for existence of touch file: " + touchFile );
-    if ( !touchFile.exists() )
-    {
-        System.out.println( "FAILED! no touchFile " + touchFile.toString() );
-        return false;
-    }
-}
-catch( Throwable t )
-{
-    t.printStackTrace();
-    return false;
-}
-
-return true;
+def logs = new File(basedir, 'build.log').text
+assert logs.contains('[WARNING] property invoker.systemPropertiesFile is deprecated - please use invoker.userPropertiesFile')
diff --git a/src/main/java/org/apache/maven/plugins/invoker/AbstractInvokerMojo.java b/src/main/java/org/apache/maven/plugins/invoker/AbstractInvokerMojo.java
@@ -526,11 +526,11 @@ public abstract class AbstractInvokerMojo extends AbstractMojo {
      * # can be indexed
      * invoker.offline = true
      *
-     * # The path to the properties file from which to load system properties, defaults to the
+     * # The path to the properties file from which to load user properties, defaults to the
      * # filename given by the plugin parameter testPropertiesFile
      * # Since plugin version 1.4
      * # can be indexed
-     * invoker.systemPropertiesFile = test.properties
+     * invoker.userPropertiesFile = test.properties
      *
      * # An optional human friendly name and description for this build job.
      * # Both name and description have to be set to be included in the build reports.
@@ -1887,9 +1887,9 @@ private boolean runBuild(
                     request.setUserSettingsFile(settingsFile);
                 }
 
-                Properties systemProperties =
-                        getSystemProperties(basedir, invokerProperties.getSystemPropertiesFile(invocationIndex));
-                request.setProperties(systemProperties);
+                Properties userProperties =
+                        getUserProperties(basedir, invokerProperties.getUserPropertiesFile(invocationIndex));
+                request.setProperties(userProperties);
 
                 invokerProperties.configureInvocation(request, invocationIndex);
 
@@ -1999,7 +1999,7 @@ private FileLogger setupBuildLogFile(File basedir) throws MojoExecutionException
      * @return The system properties to use, may be empty but never <code>null</code>.
      * @throws org.apache.maven.plugin.MojoExecutionException If the properties file exists but could not be read.
      */
-    private Properties getSystemProperties(final File basedir, final String filename) throws MojoExecutionException {
+    private Properties getUserProperties(final File basedir, final String filename) throws MojoExecutionException {
         Properties collectedTestProperties = new Properties();
 
         if (properties != null) {
@@ -2014,18 +2014,16 @@ private Properties getSystemProperties(final File basedir, final String filename
         File propertiesFile = null;
         if (filename != null) {
             propertiesFile = new File(basedir, filename);
-        } else if (testPropertiesFile != null) {
-            propertiesFile = new File(basedir, testPropertiesFile);
         }
 
         if (propertiesFile != null && propertiesFile.isFile()) {
 
-            try (InputStream fin = new FileInputStream(propertiesFile)) {
+            try (InputStream fin = Files.newInputStream(propertiesFile.toPath())) {
                 Properties loadedProperties = new Properties();
                 loadedProperties.load(fin);
                 collectedTestProperties.putAll(loadedProperties);
             } catch (IOException e) {
-                throw new MojoExecutionException("Error reading system properties from " + propertiesFile);
+                throw new MojoExecutionException("Error reading user properties from " + propertiesFile);
             }
         }
 
@@ -2369,6 +2367,7 @@ private InvokerProperties getInvokerProperties(final File projectDirectory, Prop
         invokerProperties.setDefaultTimeoutInSeconds(timeoutInSeconds);
         invokerProperties.setDefaultEnvironmentVariables(environmentVariables);
         invokerProperties.setDefaultUpdateSnapshots(updateSnapshots);
+        invokerProperties.setDefaultUserPropertiesFiles(testPropertiesFile);
 
         return invokerProperties;
     }
diff --git a/src/main/java/org/apache/maven/plugins/invoker/InvokerProperties.java b/src/main/java/org/apache/maven/plugins/invoker/InvokerProperties.java
@@ -32,13 +32,18 @@
 import java.util.regex.Pattern;
 
 import org.apache.maven.shared.invoker.InvocationRequest;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 /**
  * Provides a convenient facade around the <code>invoker.properties</code>.
  *
  * @author Benjamin Bentmann
  */
 class InvokerProperties {
+
+    private final Logger logger = LoggerFactory.getLogger(InvokerProperties.class);
+
     private static final String SELECTOR_PREFIX = "selector.";
 
     private static final Pattern ENVIRONMENT_VARIABLES_PATTERN =
@@ -54,6 +59,7 @@ class InvokerProperties {
     private Map<String, String> defaultEnvironmentVariables;
     private File defaultMavenExecutable;
     private Boolean defaultUpdateSnapshots;
+    private String defaultUserPropertiesFiles;
 
     private enum InvocationProperty {
         PROJECT("invoker.project"),
@@ -66,6 +72,7 @@ private enum InvocationProperty {
         NON_RECURSIVE("invoker.nonRecursive"),
         OFFLINE("invoker.offline"),
         SYSTEM_PROPERTIES_FILE("invoker.systemPropertiesFile"),
+        USER_PROPERTIES_FILE("invoker.userPropertiesFile"),
         DEBUG("invoker.debug"),
         QUIET("invoker.quiet"),
         SETTINGS_FILE("invoker.settingsFile"),
@@ -190,6 +197,14 @@ public void setDefaultUpdateSnapshots(boolean defaultUpdateSnapshots) {
         this.defaultUpdateSnapshots = defaultUpdateSnapshots;
     }
 
+    /**
+     * Default value for userPropertiesFile
+     * @param defaultUserPropertiesFiles a default value
+     */
+    public void setDefaultUserPropertiesFiles(String defaultUserPropertiesFiles) {
+        this.defaultUserPropertiesFiles = defaultUserPropertiesFiles;
+    }
+
     /**
      * Gets the invoker properties being wrapped.
      *
@@ -467,13 +482,33 @@ public boolean isExpectedResult(int exitCode, int index) {
     }
 
     /**
-     * Gets the path to the properties file used to set the system properties for the specified invocation.
+     * Gets the path to the properties file used to set the user properties for the specified invocation.
      *
      * @param index The index of the invocation, must not be negative.
      * @return The path to the properties file or <code>null</code> if not set.
      */
-    public String getSystemPropertiesFile(int index) {
-        return get(InvocationProperty.SYSTEM_PROPERTIES_FILE, index).orElse(null);
+    public String getUserPropertiesFile(int index) {
+        Optional<String> userProperties = get(InvocationProperty.USER_PROPERTIES_FILE, index);
+        Optional<String> systemProperties = get(InvocationProperty.SYSTEM_PROPERTIES_FILE, index);
+
+        if (userProperties.isPresent() && systemProperties.isPresent()) {
+            throw new IllegalArgumentException("only one property '" + InvocationProperty.USER_PROPERTIES_FILE
+                    + "' or '" + InvocationProperty.SYSTEM_PROPERTIES_FILE + "' can be used");
+        }
+
+        if (userProperties.isPresent()) {
+            return userProperties.get();
+        }
+
+        if (systemProperties.isPresent()) {
+            logger.warn(
+                    "property {} is deprecated - please use {}",
+                    InvocationProperty.SYSTEM_PROPERTIES_FILE,
+                    InvocationProperty.USER_PROPERTIES_FILE);
+            return systemProperties.get();
+        }
+
+        return defaultUserPropertiesFiles;
     }
 
     /**
diff --git a/src/test/java/org/apache/maven/plugins/invoker/InvokerPropertiesTest.java b/src/test/java/org/apache/maven/plugins/invoker/InvokerPropertiesTest.java
@@ -545,4 +545,43 @@ void testGetToolchainsWithIndex() {
         assertThat(toolchain.getType()).isEqualTo("jdk");
         assertThat(toolchain.getProvides()).containsExactlyEntriesOf(Collections.singletonMap("version", "11"));
     }
+
+    @Test
+    void defaultValueForUserPropertiesFileShouldBeReturned() {
+        InvokerProperties facade = new InvokerProperties(new Properties());
+        facade.setDefaultUserPropertiesFiles("test3.properties");
+
+        assertThat(facade.getUserPropertiesFile(0)).isEqualTo("test3.properties");
+    }
+
+    @Test
+    void userPropertiesFilesShouldBeUsed() {
+        Properties props = new Properties();
+        props.put("invoker.userPropertiesFile", "test1");
+        InvokerProperties facade = new InvokerProperties(props);
+
+        assertThat(facade.getUserPropertiesFile(0)).isEqualTo("test1");
+    }
+
+    @Test
+    void systemPropertiesFilesShouldBeUsed() {
+        Properties props = new Properties();
+        props.put("invoker.systemPropertiesFile", "test1");
+        InvokerProperties facade = new InvokerProperties(props);
+
+        assertThat(facade.getUserPropertiesFile(0)).isEqualTo("test1");
+    }
+
+    @Test
+    void userAndSystemPropertiesFilesShouldThrowException() {
+        Properties props = new Properties();
+        props.put("invoker.systemPropertiesFile", "test1");
+        props.put("invoker.userPropertiesFile", "test2");
+        InvokerProperties facade = new InvokerProperties(props);
+
+        assertThatCode(() -> facade.getUserPropertiesFile(0))
+                .isExactlyInstanceOf(IllegalArgumentException.class)
+                .hasMessage(
+                        "only one property 'invoker.userPropertiesFile' or 'invoker.systemPropertiesFile' can be used");
+    }
 }
