From eea3e567be2ddeb7ec42d8cdfd185993ec11374a Mon Sep 17 00:00:00 2001 From: Tamas Cservenak Date: Fri, 13 Jun 2025 20:20:28 +0200 Subject: [PATCH 1/5] User properties are not interpolated for paths --- .../java/org/apache/maven/cling/invoker/BaseParser.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/BaseParser.java b/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/BaseParser.java index d4b99f2ec17a..4e9724b59a46 100644 --- a/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/BaseParser.java +++ b/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/BaseParser.java @@ -56,6 +56,7 @@ import org.apache.maven.properties.internal.SystemProperties; import static java.util.Objects.requireNonNull; +import static org.apache.maven.cling.invoker.CliUtils.createInterpolator; import static org.apache.maven.cling.invoker.CliUtils.getCanonicalPath; import static org.apache.maven.cling.invoker.CliUtils.or; import static org.apache.maven.cling.invoker.CliUtils.prefix; @@ -399,6 +400,7 @@ protected Map populateSystemProperties(LocalContext context) { protected Map populateUserProperties(LocalContext context) { Properties userProperties = new Properties(); + Map paths = context.extraInterpolationSource(); // ---------------------------------------------------------------------- // Options that are set on the command line become system properties @@ -407,12 +409,12 @@ protected Map populateUserProperties(LocalContext context) { // ---------------------------------------------------------------------- Map userSpecifiedProperties = - context.options.userProperties().orElse(new HashMap<>()); + new HashMap<>(context.options.userProperties().orElse(new HashMap<>())); + createInterpolator().interpolate(userSpecifiedProperties, paths::get); // ---------------------------------------------------------------------- // Load config files // ---------------------------------------------------------------------- - Map paths = context.extraInterpolationSource(); UnaryOperator callback = or(paths::get, prefix("cli.", userSpecifiedProperties::get), context.systemProperties::get); From c538ec4751161a7ccbc8f4a650a81ba4c4be5c3c Mon Sep 17 00:00:00 2001 From: Tamas Cservenak Date: Fri, 13 Jun 2025 20:35:40 +0200 Subject: [PATCH 2/5] Add test --- .../cling/invoker/CommonsCliOptions.java | 4 + .../maven/cling/invoker/BaseParserTest.java | 78 +++++++++++++++++++ 2 files changed, 82 insertions(+) create mode 100644 impl/maven-cli/src/test/java/org/apache/maven/cling/invoker/BaseParserTest.java diff --git a/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/CommonsCliOptions.java b/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/CommonsCliOptions.java index 028e3b54cf4d..f78bcb0f5216 100644 --- a/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/CommonsCliOptions.java +++ b/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/CommonsCliOptions.java @@ -46,6 +46,10 @@ import static org.apache.maven.cling.invoker.CliUtils.toMap; public class CommonsCliOptions implements Options { + public static CommonsCliOptions parse(String source, String[] args) throws ParseException { + CLIManager cliManager = new CLIManager(); + return new CommonsCliOptions(source, cliManager, cliManager.parse(args)); + } protected final String source; protected final CLIManager cliManager; diff --git a/impl/maven-cli/src/test/java/org/apache/maven/cling/invoker/BaseParserTest.java b/impl/maven-cli/src/test/java/org/apache/maven/cling/invoker/BaseParserTest.java new file mode 100644 index 000000000000..39b8195e423c --- /dev/null +++ b/impl/maven-cli/src/test/java/org/apache/maven/cling/invoker/BaseParserTest.java @@ -0,0 +1,78 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.maven.cling.invoker; + +import java.util.Arrays; + +import org.apache.commons.cli.ParseException; +import org.apache.maven.api.cli.InvokerRequest; +import org.apache.maven.api.cli.Options; +import org.apache.maven.api.cli.ParserRequest; +import org.apache.maven.api.services.MessageBuilderFactory; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; + +import static org.mockito.Mockito.mock; + +public class BaseParserTest { + private final BaseParser subject = new BaseParser() { + @Override + protected Options parseCliOptions(LocalContext context) { + try { + return CommonsCliOptions.parse( + "test", context.parserRequest.args().toArray(new String[0])); + } catch (ParseException e) { + throw new IllegalArgumentException(e); + } + } + }; + + @Test + void happy() { + InvokerRequest invokerRequest = + subject.parseInvocation(ParserRequest.mvn(Arrays.asList("-v", "-X"), mock(MessageBuilderFactory.class)) + .build()); + + Assertions.assertTrue(invokerRequest.options().isPresent()); + Options options = invokerRequest.options().orElseThrow(); + Assertions.assertTrue(options.showVersionAndExit().orElse(false)); + Assertions.assertTrue(options.verbose().orElse(false)); + } + + @Test + void notHappy() { + InvokerRequest invokerRequest = subject.parseInvocation( + ParserRequest.mvn(Arrays.asList("--what-is-this-option", "-X"), mock(MessageBuilderFactory.class)) + .build()); + + Assertions.assertFalse(invokerRequest.options().isPresent()); + Assertions.assertTrue(invokerRequest.parsingFailed()); + } + + @Test + void specials() { + InvokerRequest invokerRequest = subject.parseInvocation(ParserRequest.mvn( + Arrays.asList("-Dfoo=${session.rootDirectory}", "-X"), mock(MessageBuilderFactory.class)) + .build()); + + Assertions.assertTrue(invokerRequest.options().isPresent()); + Assertions.assertNotEquals( + "${session.rootDirectory}", invokerRequest.userProperties().get("foo")); + } +} From 0b099499d18d9a39bde9ff636cc58e6014a62008 Mon Sep 17 00:00:00 2001 From: Tamas Cservenak Date: Fri, 13 Jun 2025 20:37:03 +0200 Subject: [PATCH 3/5] Add more --- .../test/java/org/apache/maven/cling/invoker/BaseParserTest.java | 1 + 1 file changed, 1 insertion(+) diff --git a/impl/maven-cli/src/test/java/org/apache/maven/cling/invoker/BaseParserTest.java b/impl/maven-cli/src/test/java/org/apache/maven/cling/invoker/BaseParserTest.java index 39b8195e423c..694f4494e4c4 100644 --- a/impl/maven-cli/src/test/java/org/apache/maven/cling/invoker/BaseParserTest.java +++ b/impl/maven-cli/src/test/java/org/apache/maven/cling/invoker/BaseParserTest.java @@ -51,6 +51,7 @@ void happy() { Assertions.assertTrue(invokerRequest.options().isPresent()); Options options = invokerRequest.options().orElseThrow(); + Assertions.assertFalse(options.showVersion().orElse(false)); Assertions.assertTrue(options.showVersionAndExit().orElse(false)); Assertions.assertTrue(options.verbose().orElse(false)); } From 6e8781229e5a52f833cc6e3211334e7dccd3bf7c Mon Sep 17 00:00:00 2001 From: Tamas Cservenak Date: Fri, 13 Jun 2025 20:54:58 +0200 Subject: [PATCH 4/5] Add more test --- impl/maven-cli/pom.xml | 2 ++ .../maven/cling/invoker/BaseParserTest.java | 20 +++++++++++++++++-- .../resources/mavenHome/conf/maven.properties | 1 + .../resources/userHome/.m2/maven.properties | 1 + .../test/resources/userHome/.m2/settings.xml | 3 +++ 5 files changed, 25 insertions(+), 2 deletions(-) create mode 100644 impl/maven-cli/src/test/resources/userHome/.m2/maven.properties create mode 100644 impl/maven-cli/src/test/resources/userHome/.m2/settings.xml diff --git a/impl/maven-cli/pom.xml b/impl/maven-cli/pom.xml index ac80ed94b77d..15e4ce5837f7 100644 --- a/impl/maven-cli/pom.xml +++ b/impl/maven-cli/pom.xml @@ -310,6 +310,8 @@ under the License. false ${basedir}/src/test/resources/mavenHome + ${basedir}/src/test/resources/userHome + ${basedir}/src/test/resources/userDir diff --git a/impl/maven-cli/src/test/java/org/apache/maven/cling/invoker/BaseParserTest.java b/impl/maven-cli/src/test/java/org/apache/maven/cling/invoker/BaseParserTest.java index 694f4494e4c4..31fef48d447c 100644 --- a/impl/maven-cli/src/test/java/org/apache/maven/cling/invoker/BaseParserTest.java +++ b/impl/maven-cli/src/test/java/org/apache/maven/cling/invoker/BaseParserTest.java @@ -18,6 +18,7 @@ */ package org.apache.maven.cling.invoker; +import java.nio.file.Path; import java.util.Arrays; import org.apache.commons.cli.ParseException; @@ -46,20 +47,33 @@ protected Options parseCliOptions(LocalContext context) { @Test void happy() { InvokerRequest invokerRequest = - subject.parseInvocation(ParserRequest.mvn(Arrays.asList("-v", "-X"), mock(MessageBuilderFactory.class)) + subject.parseInvocation(ParserRequest.mvn(Arrays.asList("-e", "-X"), mock(MessageBuilderFactory.class)) + .cwd(Path.of(System.getProperty("userDir"))) + .userHome(Path.of(System.getProperty("userHome"))) .build()); Assertions.assertTrue(invokerRequest.options().isPresent()); Options options = invokerRequest.options().orElseThrow(); Assertions.assertFalse(options.showVersion().orElse(false)); - Assertions.assertTrue(options.showVersionAndExit().orElse(false)); + Assertions.assertFalse(options.showVersionAndExit().orElse(false)); + Assertions.assertTrue(options.showErrors().orElse(false)); Assertions.assertTrue(options.verbose().orElse(false)); + + // user home + Assertions.assertTrue(invokerRequest.userProperties().containsKey("user.property")); + Assertions.assertEquals("yes it is", invokerRequest.userProperties().get("user.property")); + + // maven installation + Assertions.assertTrue(invokerRequest.userProperties().containsKey("maven.property")); + Assertions.assertEquals("yes it is", invokerRequest.userProperties().get("maven.property")); } @Test void notHappy() { InvokerRequest invokerRequest = subject.parseInvocation( ParserRequest.mvn(Arrays.asList("--what-is-this-option", "-X"), mock(MessageBuilderFactory.class)) + .cwd(Path.of(System.getProperty("userDir"))) + .userHome(Path.of(System.getProperty("userHome"))) .build()); Assertions.assertFalse(invokerRequest.options().isPresent()); @@ -70,6 +84,8 @@ void notHappy() { void specials() { InvokerRequest invokerRequest = subject.parseInvocation(ParserRequest.mvn( Arrays.asList("-Dfoo=${session.rootDirectory}", "-X"), mock(MessageBuilderFactory.class)) + .cwd(Path.of(System.getProperty("userDir"))) + .userHome(Path.of(System.getProperty("userHome"))) .build()); Assertions.assertTrue(invokerRequest.options().isPresent()); diff --git a/impl/maven-cli/src/test/resources/mavenHome/conf/maven.properties b/impl/maven-cli/src/test/resources/mavenHome/conf/maven.properties index 1e53fa5df399..7660a67d3102 100644 --- a/impl/maven-cli/src/test/resources/mavenHome/conf/maven.properties +++ b/impl/maven-cli/src/test/resources/mavenHome/conf/maven.properties @@ -23,6 +23,7 @@ # The properties defined in this file will be made available through # user properties at the very beginning of Maven's boot process. # +maven.property = yes it is maven.installation.conf = ${maven.home}/conf maven.user.conf = ${user.home}/.m2 diff --git a/impl/maven-cli/src/test/resources/userHome/.m2/maven.properties b/impl/maven-cli/src/test/resources/userHome/.m2/maven.properties new file mode 100644 index 000000000000..388eea918068 --- /dev/null +++ b/impl/maven-cli/src/test/resources/userHome/.m2/maven.properties @@ -0,0 +1 @@ +user.property=yes it is \ No newline at end of file diff --git a/impl/maven-cli/src/test/resources/userHome/.m2/settings.xml b/impl/maven-cli/src/test/resources/userHome/.m2/settings.xml new file mode 100644 index 000000000000..0ead13db8d6f --- /dev/null +++ b/impl/maven-cli/src/test/resources/userHome/.m2/settings.xml @@ -0,0 +1,3 @@ + + + \ No newline at end of file From 9039022fae63f348b605bab0c453eb543712730e Mon Sep 17 00:00:00 2001 From: Tamas Cservenak Date: Fri, 13 Jun 2025 21:04:28 +0200 Subject: [PATCH 5/5] Better assertion --- .../java/org/apache/maven/cling/invoker/BaseParserTest.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/impl/maven-cli/src/test/java/org/apache/maven/cling/invoker/BaseParserTest.java b/impl/maven-cli/src/test/java/org/apache/maven/cling/invoker/BaseParserTest.java index 31fef48d447c..08546b194a9a 100644 --- a/impl/maven-cli/src/test/java/org/apache/maven/cling/invoker/BaseParserTest.java +++ b/impl/maven-cli/src/test/java/org/apache/maven/cling/invoker/BaseParserTest.java @@ -89,7 +89,9 @@ void specials() { .build()); Assertions.assertTrue(invokerRequest.options().isPresent()); + Assertions.assertTrue(invokerRequest.userProperties().containsKey("foo")); Assertions.assertNotEquals( "${session.rootDirectory}", invokerRequest.userProperties().get("foo")); + Assertions.assertFalse(invokerRequest.userProperties().get("foo").trim().isEmpty()); } }