This is an automated email from the ASF dual-hosted git repository. gnodet pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/maven.git
The following commit(s) were added to refs/heads/master by this push: new fffdf0ed2a [MNG-8694] Fix version interpolation and ternary operator (#2272) fffdf0ed2a is described below commit fffdf0ed2a4250325d8b2b5c242dad2e22fa0edd Author: Kamil Krzywanski <61804231+kamilkrzywan...@users.noreply.github.com> AuthorDate: Wed Apr 30 13:44:46 2025 +0200 [MNG-8694] Fix version interpolation and ternary operator (#2272) Use the interpolator service instead of regexp for version interpolation and fix the interpolator to properly support both `:+` and `:-` in the same expression to support a ternary operator. When processing ternary expressions like ${release:+${foo}:-${bar}} or ${versionCore}${release:+-DEVELOPER}, the DefaultInterpolator was incorrectly continuing to process subsequent operators after a decision had been made on the first operator. This fix adds a break statement after each successful operator evaluation to stop processing any remaining operators once a decision has been made, ensuring that: 1. When 'release' is true and 'foo' is empty, correctly evaluates to an empty string 2. When 'release' is true, correctly evaluates to just Added test cases to verify both scenarios. --------- Co-authored-by: kkrzywanski <k.krzywan...@aasys.pl> Co-authored-by: Guillaume Nodet <gno...@gmail.com> --- .../project/DefaultMavenProjectBuilderTest.java | 16 ++++++++ .../pom.xml | 17 ++++++++ .../maven/impl/model/DefaultInterpolator.java | 4 ++ .../maven/impl/model/DefaultModelBuilder.java | 25 +----------- .../maven/impl/model/DefaultInterpolatorTest.java | 46 ++++++++++++++++++++++ 5 files changed, 84 insertions(+), 24 deletions(-) diff --git a/impl/maven-core/src/test/java/org/apache/maven/project/DefaultMavenProjectBuilderTest.java b/impl/maven-core/src/test/java/org/apache/maven/project/DefaultMavenProjectBuilderTest.java index 9f7cc8ebe0..3c5bcdd0ac 100644 --- a/impl/maven-core/src/test/java/org/apache/maven/project/DefaultMavenProjectBuilderTest.java +++ b/impl/maven-core/src/test/java/org/apache/maven/project/DefaultMavenProjectBuilderTest.java @@ -28,6 +28,7 @@ import org.apache.maven.api.model.InputLocation; import org.apache.maven.api.model.InputSource; import org.apache.maven.artifact.Artifact; +import org.apache.maven.execution.MavenSession; import org.apache.maven.impl.InternalSession; import org.apache.maven.internal.impl.DefaultProject; import org.apache.maven.internal.impl.InternalMavenSession; @@ -511,6 +512,21 @@ public void testBuildParentVersionRangeExternallyWithChildRevisionExpression() t assertEquals("1.0-SNAPSHOT", mp.getVersion()); } + @Test + public void testParentVersionResolvedFromNestedProperties() throws Exception { + File f1 = getTestFile("src/test/resources/projects/pom-parent-version-from-nested-properties/pom.xml"); + ProjectBuildingRequest request = newBuildingRequest(); + MavenSession session = + InternalMavenSession.from(request.getRepositorySession()).getMavenSession(); + + MavenProject mp = projectBuilder.build(f1, request).getProject(); + assertEquals("0.1.0-DEVELOPER", mp.getVersion()); + + session.getUserProperties().put("release", "true"); + mp = projectBuilder.build(f1, request).getProject(); + assertEquals("0.1.0", mp.getVersion()); + } + @Test public void testSubprojectDiscovery() throws Exception { File pom = getTestFile("src/test/resources/projects/subprojects-discover/pom.xml"); diff --git a/impl/maven-core/src/test/resources/projects/pom-parent-version-from-nested-properties/pom.xml b/impl/maven-core/src/test/resources/projects/pom-parent-version-from-nested-properties/pom.xml new file mode 100644 index 0000000000..dc232b74fa --- /dev/null +++ b/impl/maven-core/src/test/resources/projects/pom-parent-version-from-nested-properties/pom.xml @@ -0,0 +1,17 @@ +<project xmlns="http://maven.apache.org/POM/4.1.0" + xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" + xsi:schemaLocation="http://maven.apache.org/POM/4.1.0 http://maven.apache.org/xsd/maven-4.1.0.xsd" + root="true"> + <modelVersion>4.1.0</modelVersion> + + <groupId>gid</groupId> + <artifactId>aid</artifactId> + <version>${revision}</version> + <packaging>pom</packaging> + + <properties> + <versionCore>0.1.0</versionCore> + <revision>${versionCore}${release:+:--DEVELOPER}</revision> + </properties> + +</project> diff --git a/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultInterpolator.java b/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultInterpolator.java index 41554274cd..0d969222d4 100644 --- a/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultInterpolator.java +++ b/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultInterpolator.java @@ -334,10 +334,14 @@ private static String processSubstitution( if (":+".equals(op)) { if (substValue != null && !substValue.isEmpty()) { substValue = processedOpValue; + // Skip any remaining operators since we've made a decision + break; } } else if (":-".equals(op)) { if (substValue == null || substValue.isEmpty()) { substValue = processedOpValue; + // Skip any remaining operators since we've made a decision + break; } } else { throw new InterpolatorException("Bad substitution operator in: ${" + org + "}"); diff --git a/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelBuilder.java b/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelBuilder.java index e918edf2a4..82ebd5a770 100644 --- a/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelBuilder.java +++ b/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelBuilder.java @@ -43,8 +43,6 @@ import java.util.concurrent.atomic.AtomicReference; import java.util.function.Supplier; import java.util.function.UnaryOperator; -import java.util.regex.Matcher; -import java.util.regex.Pattern; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -247,8 +245,6 @@ public ModelBuilderResult build(ModelBuilderRequest request) throws ModelBuilder } protected class ModelBuilderSessionState implements ModelProblemCollector { - private static final Pattern REGEX = Pattern.compile("\\$\\{([^}]+)}"); - final Session session; final ModelBuilderRequest request; final DefaultModelBuilderResult result; @@ -585,26 +581,7 @@ private Dependency inferDependencyVersion(Model model, Dependency dep) { } String replaceCiFriendlyVersion(Map<String, String> properties, String version) { - // TODO: we're using a simple regex here, but we should probably use - // a proper interpolation service to do the replacements - // once one is available in maven-api-impl - // https://issues.apache.org/jira/browse/MNG-8262 - if (version != null) { - Matcher matcher = REGEX.matcher(version); - if (matcher.find()) { - StringBuilder result = new StringBuilder(); - do { - // extract the key inside ${} - String key = matcher.group(1); - // get replacement from the map, or use the original ${xy} if not found - String replacement = properties.getOrDefault(key, "\\" + matcher.group(0)); - matcher.appendReplacement(result, replacement); - } while (matcher.find()); - matcher.appendTail(result); // Append the remaining part of the string - return result.toString(); - } - } - return version; + return version != null ? interpolator.interpolate(version, properties::get) : null; } private void buildBuildPom() throws ModelBuilderException { diff --git a/impl/maven-impl/src/test/java/org/apache/maven/impl/model/DefaultInterpolatorTest.java b/impl/maven-impl/src/test/java/org/apache/maven/impl/model/DefaultInterpolatorTest.java index c0c06a271f..9d332e9a51 100644 --- a/impl/maven-impl/src/test/java/org/apache/maven/impl/model/DefaultInterpolatorTest.java +++ b/impl/maven-impl/src/test/java/org/apache/maven/impl/model/DefaultInterpolatorTest.java @@ -169,6 +169,52 @@ void testExpansion() { assertEquals("", props.get("c_cp")); } + @Test + void testTernary() { + Map<String, String> props; + + props = new LinkedHashMap<>(); + props.put("foo", "-FOO"); + props.put("bar", "-BAR"); + props.put("version", "1.0${release:+${foo}:-${bar}}"); + performSubstitution(props); + assertEquals("1.0-BAR", props.get("version")); + + props = new LinkedHashMap<>(); + props.put("release", "true"); + props.put("foo", "-FOO"); + props.put("bar", "-BAR"); + props.put("version", "1.0${release:+${foo}:-${bar}}"); + performSubstitution(props); + assertEquals("1.0-FOO", props.get("version")); + + props = new LinkedHashMap<>(); + props.put("foo", ""); + props.put("bar", "-BAR"); + props.put("version", "1.0${release:+${foo}:-${bar}}"); + performSubstitution(props); + assertEquals("1.0-BAR", props.get("version")); + + props = new LinkedHashMap<>(); + props.put("release", "true"); + props.put("foo", ""); + props.put("bar", "-BAR"); + props.put("version", "1.0${release:+${foo}:-${bar}}"); + performSubstitution(props); + assertEquals("1.0", props.get("version")); + + props = new LinkedHashMap<>(); + props.put("version", "1.0${release:+:--BAR}"); + performSubstitution(props); + assertEquals("1.0-BAR", props.get("version")); + + props = new LinkedHashMap<>(); + props.put("release", "true"); + props.put("version", "1.0${release:+:--BAR}"); + performSubstitution(props); + assertEquals("1.0", props.get("version")); + } + @Test void testXdg() { Map<String, String> props;