[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 <[email protected]>
Co-authored-by: Guillaume Nodet <[email protected]>

Author: 3 people

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");

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>

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 + "}");

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 {

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;