From 53235e2f21e8013bc356626668325eff6d432d92 Mon Sep 17 00:00:00 2001 From: Slawomir Jaranowski Date: Fri, 3 May 2024 12:18:10 +0200 Subject: [PATCH] [MINVOKER-364] Rename invoker.systemPropertiesFile to invoker.userPropertiesFile --- .../src/it/project/invoker.properties | 4 +- .../{system1.properties => user.properties} | 0 .../{system2.properties => user3.properties} | 0 .../{verify.bsh => verify.groovy} | 26 +++--------- .../plugins/invoker/AbstractInvokerMojo.java | 19 ++++----- .../plugins/invoker/InvokerProperties.java | 41 +++++++++++++++++-- .../invoker/InvokerPropertiesTest.java | 39 ++++++++++++++++++ 7 files changed, 93 insertions(+), 36 deletions(-) rename src/it/invocation-multiple/src/it/project/{system1.properties => user.properties} (100%) rename src/it/invocation-multiple/src/it/project/{system2.properties => user3.properties} (100%) rename src/it/postbuild-executed-only-once/{verify.bsh => verify.groovy} (59%) diff --git a/src/it/invocation-multiple/src/it/project/invoker.properties b/src/it/invocation-multiple/src/it/project/invoker.properties index 89ca0884..3d0e1384 100644 --- 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/system1.properties b/src/it/invocation-multiple/src/it/project/user.properties similarity index 100% rename from src/it/invocation-multiple/src/it/project/system1.properties rename to src/it/invocation-multiple/src/it/project/user.properties diff --git a/src/it/invocation-multiple/src/it/project/system2.properties b/src/it/invocation-multiple/src/it/project/user3.properties similarity index 100% rename from src/it/invocation-multiple/src/it/project/system2.properties rename to src/it/invocation-multiple/src/it/project/user3.properties diff --git a/src/it/postbuild-executed-only-once/verify.bsh b/src/it/postbuild-executed-only-once/verify.groovy similarity index 59% rename from src/it/postbuild-executed-only-once/verify.bsh rename to src/it/postbuild-executed-only-once/verify.groovy index e05fbff3..8ee560ad 100644 --- a/src/it/postbuild-executed-only-once/verify.bsh +++ 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 index 3c454b29..09987f4b 100644 --- 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 null. * @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 index 15e246d0..4bf09e71 100644 --- a/src/main/java/org/apache/maven/plugins/invoker/InvokerProperties.java +++ b/src/main/java/org/apache/maven/plugins/invoker/InvokerProperties.java @@ -32,6 +32,8 @@ 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 invoker.properties. @@ -39,6 +41,9 @@ * @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 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 null if not set. */ - public String getSystemPropertiesFile(int index) { - return get(InvocationProperty.SYSTEM_PROPERTIES_FILE, index).orElse(null); + public String getUserPropertiesFile(int index) { + Optional userProperties = get(InvocationProperty.USER_PROPERTIES_FILE, index); + Optional 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 index 26a706ec..58285f86 100644 --- 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"); + } }