[MRELEASE-1109] Snapshot detection and support for versions like ${revision}${sha1}${changelist}

maven-release-manager/src/main/java/org/apache/maven/shared/release/phase/AbstractCheckPomPhase.java

@@ -31,6 +31,7 @@
 import org.apache.maven.shared.release.env.ReleaseEnvironment;
 import org.apache.maven.shared.release.scm.ReleaseScmRepositoryException;
 import org.apache.maven.shared.release.scm.ScmRepositoryConfigurator;
+import org.apache.maven.shared.release.util.MavenExpression;
 import org.codehaus.plexus.util.StringUtils;
 
 import static java.util.Objects.requireNonNull;
@@ -89,7 +90,8 @@ public ReleaseResult execute(
         boolean containsSnapshotProjects = false;
 
         for (MavenProject project : reactorProjects) {
-            if (ArtifactUtils.isSnapshot(project.getVersion())) {
+            String projectVersion = MavenExpression.evaluate(project.getVersion(), project.getProperties());
+            if (ArtifactUtils.isSnapshot(projectVersion)) {
                 containsSnapshotProjects = true;
 
                 break;

maven-release-manager/src/main/java/org/apache/maven/shared/release/phase/AbstractRewritePomsPhase.java

@@ -23,15 +23,12 @@
 import java.text.DateFormat;
 import java.text.SimpleDateFormat;
 import java.util.ArrayList;
-import java.util.Arrays;
 import java.util.Collection;
 import java.util.Date;
 import java.util.List;
 import java.util.Map;
 import java.util.Properties;
 import java.util.TimeZone;
-import java.util.regex.Matcher;
-import java.util.regex.Pattern;
 
 import org.apache.maven.artifact.Artifact;
 import org.apache.maven.artifact.ArtifactUtils;
@@ -63,6 +60,7 @@
 import org.apache.maven.shared.release.transform.ModelETLFactory;
 import org.apache.maven.shared.release.transform.ModelETLRequest;
 import org.apache.maven.shared.release.transform.jdom2.JDomModelETLFactory;
+import org.apache.maven.shared.release.util.CiFriendlyVersion;
 import org.apache.maven.shared.release.util.ReleaseUtil;
 import org.codehaus.plexus.util.StringUtils;
 
@@ -92,18 +90,6 @@ public abstract class AbstractRewritePomsPhase extends AbstractReleasePhase impl
      */
     private String modelETL = JDomModelETLFactory.NAME;
 
-    /**
-     * Regular expression pattern matching Maven expressions (i.e. references to Maven properties).
-     * The first group selects the property name the expression refers to.
-     */
-    private static final Pattern EXPRESSION_PATTERN = Pattern.compile("\\$\\{(.+?)\\}");
-
-    /**
-     * All Maven properties allowed to be referenced in parent versions via expressions
-     * @see <a href="https://maven.apache.org/maven-ci-friendly.html">CI-Friendly Versions</a>
-     */
-    private static final List<String> CI_FRIENDLY_PROPERTIES = Arrays.asList("revision", "sha1", "changelist");
-
     private long startTime = -1 * 1000;
 
     protected AbstractRewritePomsPhase(
@@ -455,23 +441,6 @@ private void rewriteVersion(
         modelTarget.setVersion(version);
     }
 
-    /**
-     * Extracts the Maven property name from a given expression.
-     * @param expression the expression
-     * @return either {@code null} if value is no expression otherwise the property referenced in the expression
-     */
-    public static String extractPropertyFromExpression(String expression) {
-        Matcher matcher = EXPRESSION_PATTERN.matcher(expression);
-        if (!matcher.find()) {
-            return null;
-        }
-        return matcher.group(1);
-    }
-
-    public static boolean isCiFriendlyVersion(String version) {
-        return CI_FRIENDLY_PROPERTIES.contains(extractPropertyFromExpression(version));
-    }
-
     private String rewriteParent(
             MavenProject project,
             Model targetModel,
@@ -494,7 +463,8 @@ private String rewriteParent(
                     throw new ReleaseFailureException("Version for parent '" + parent.getName() + "' was not mapped");
                 }
             } else {
-                if (!isCiFriendlyVersion(targetModel.getParent().getVersion())) {
+                if (!CiFriendlyVersion.isCiFriendlyVersion(
+                        targetModel.getParent().getVersion())) {
                     targetModel.getParent().setVersion(parentVersion);
                 } else {
                     logInfo(
@@ -564,7 +534,7 @@ private void rewriteArtifactVersions(
                     logInfo(result, "  Updating " + artifactId + " to " + mappedVersion);
                     coordinate.setVersion(mappedVersion);
                 } else {
-                    String property = extractPropertyFromExpression(rawVersion);
+                    String property = CiFriendlyVersion.extractPropertyFromExpression(rawVersion);
                     if (property != null) {
                         if (property.startsWith("project.")
                                 || property.startsWith("pom.")
@@ -609,7 +579,9 @@ private void rewriteArtifactVersions(
                                     }
                                 }
                             } else {
-                                if (CI_FRIENDLY_PROPERTIES.contains(property)) {
+                                if (CiFriendlyVersion.containsCiFriendlyProperties(property)) {
+                                    // the parent's pom revision is set inside
+                                    // org.apache.maven.shared.release.transform.jdom2.JDomModel.setVersion
                                     logInfo(
                                             result,
                                             "  Ignoring artifact version update for CI friendly expression "

maven-release-manager/src/main/java/org/apache/maven/shared/release/transform/jdom2/JDomModel.java

@@ -31,7 +31,7 @@
 import org.apache.maven.model.Profile;
 import org.apache.maven.model.Reporting;
 import org.apache.maven.model.Scm;
-import org.apache.maven.shared.release.phase.AbstractRewritePomsPhase;
+import org.apache.maven.shared.release.util.CiFriendlyVersion;
 import org.jdom2.Document;
 import org.jdom2.Element;
 import org.jdom2.Text;
@@ -180,7 +180,7 @@ public void setVersion(String version) {
 
         if (versionElement == null) {
             // never add version when parent references CI friendly property
-            if (!(parentVersion != null && AbstractRewritePomsPhase.isCiFriendlyVersion(parentVersion))
+            if (!(parentVersion != null && CiFriendlyVersion.isCiFriendlyVersion(parentVersion))
                     && !version.equals(parentVersion)) {
                 // we will add this after artifactId, since it was missing but different from the inherited version
                 Element artifactIdElement = project.getChild("artifactId", project.getNamespace());
@@ -192,14 +192,10 @@ public void setVersion(String version) {
                 project.addContent(index + 2, versionElement);
             }
         } else {
-            if (AbstractRewritePomsPhase.isCiFriendlyVersion(versionElement.getTextNormalize())) {
+            if (CiFriendlyVersion.isCiFriendlyVersion(versionElement.getTextNormalize())) {
                 // try to rewrite property if CI friendly expression is used
-                String ciFriendlyPropertyName =
-                        AbstractRewritePomsPhase.extractPropertyFromExpression(versionElement.getTextNormalize());
-                Properties properties = getProperties();
-                if (properties != null) {
-                    properties.setProperty(ciFriendlyPropertyName, version);
-                }
+                CiFriendlyVersion.rewriteVersionAndProperties(
+                        version, versionElement.getTextNormalize(), getProperties());
             } else {
                 JDomUtils.rewriteValue(versionElement, version);
             }

maven-release-manager/src/main/java/org/apache/maven/shared/release/transform/jdom2/JDomProperties.java

@@ -51,10 +51,13 @@ public JDomProperties(Element properties) {
     }
 
     @Override
-    public synchronized Object setProperty(String key, String value) {
-        Element property = properties.getChild(key, properties.getNamespace());
+    public synchronized Object put(Object key, Object value) {
+        Element property = properties.getChild((String) key, properties.getNamespace());
 
-        JDomUtils.rewriteValue(property, value);
+        if (property == null) {
+            property = new Element((String) key, properties.getNamespace());
+        }
+        JDomUtils.rewriteValue(property, (String) value);
 
         // todo follow specs of Hashtable.put
         return null;
@@ -112,9 +115,20 @@ public String getProperty(String key) {
         }
     }
 
+    @Override
+    public boolean containsKey(Object key) {
+        if (key instanceof String) {
+            Element property = properties.getChild((String) key, properties.getNamespace());
+            return property != null;
+        }
+        return false;
+    }
+
     @Override
     public String getProperty(String key, String defaultValue) {
-        throw new UnsupportedOperationException();
+        String property = getProperty(key);
+
+        return property == null ? defaultValue : property;
     }
 
     @Override

maven-release-manager/src/main/java/org/apache/maven/shared/release/util/CiFriendlyVersion.java

@@ -0,0 +1,96 @@
+/*
+ * 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.shared.release.util;
+
+import java.util.Arrays;
+import java.util.List;
+import java.util.Properties;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+
+import org.apache.maven.artifact.ArtifactUtils;
+
+public class CiFriendlyVersion {
+
+    /**
+     * Regular expression pattern matching Maven expressions (i.e. references to Maven properties).
+     * The first group selects the property name the expression refers to.
+     */
+    private static final Pattern EXPRESSION_PATTERN = Pattern.compile("\\$\\{(.+?)\\}");
+
+    /**
+     * All Maven properties allowed to be referenced in parent versions via expressions
+     * @see <a href="https://maven.apache.org/maven-ci-friendly.html">CI-Friendly Versions</a>
+     */
+    private static final String REVISION = "revision";
+
+    private static final String SHA_1 = "sha1";
+    private static final String CHANGELIST = "changelist";
+    private static final List<String> CI_FRIENDLY_PROPERTIES = Arrays.asList(REVISION, SHA_1, CHANGELIST);
+
+    private static final String SNAPSHOT = "-SNAPSHOT";
+
+    private CiFriendlyVersion() {}
+
+    /**
+     * Extracts the Maven property name from a given expression.
+     * @param expression the expression
+     * @return either {@code null} if value is no expression otherwise the property referenced in the expression
+     */
+    public static String extractPropertyFromExpression(String expression) {
+        Matcher matcher = EXPRESSION_PATTERN.matcher(expression);
+        if (!matcher.find()) {
+            return null;
+        }
+        return matcher.group(1);
+    }
+
+    public static boolean isCiFriendlyVersion(String version) {
+        return containsCiFriendlyProperties(extractPropertyFromExpression(version));
+    }
+
+    public static boolean containsCiFriendlyProperties(String property) {
+        return CI_FRIENDLY_PROPERTIES.contains(property);
+    }
+
+    public static void rewriteVersionAndProperties(String version, String versionElement, Properties properties) {
+        // try to rewrite property if CI friendly expression is used
+        String ciFriendlyPropertyName = extractPropertyFromExpression(versionElement);
+        if (properties != null) {
+            String sha1 = properties.getProperty(SHA_1, System.getProperty(SHA_1, ""));
+            // assume that everybody follows the example and properties are simply chained
+            //  and the changelist can only be '-SNAPSHOT'
+            if (ArtifactUtils.isSnapshot(version)) {
+                if (properties.containsKey(CHANGELIST)) {
+                    properties.setProperty(
+                            ciFriendlyPropertyName, version.replace(sha1, "").replace(SNAPSHOT, ""));
+                    properties.setProperty(CHANGELIST, SNAPSHOT);
+                } else {
+                    properties.setProperty(ciFriendlyPropertyName, version.replace(sha1, ""));
+                }
+            } else {
+                properties.setProperty(
+                        ciFriendlyPropertyName, version.replace(sha1, "").replace(SNAPSHOT, ""));
+                if (properties.containsKey(CHANGELIST)) {
+                    properties.setProperty(CHANGELIST, "");
+                }
+            }
+        }
+    }
+}

maven-release-manager/src/main/java/org/apache/maven/shared/release/util/MavenExpression.java

@@ -0,0 +1,44 @@
+/*
+ * 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.shared.release.util;
+
+import java.util.Map;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+
+/**
+ * @author <a href="mailto:mikhail_kolesnikov@outlook.com">Mikhail Kolesnikov</a>
+ */
+public class MavenExpression {
+    public static final Pattern EXPRESSION_PATTERN = Pattern.compile("\\$\\{(.+?)\\}");
+
+    private MavenExpression() {}
+
+    public static String evaluate(String expression, Map<?, ?> properties) {
+        StringBuilder result = new StringBuilder(expression);
+        Matcher matcher = EXPRESSION_PATTERN.matcher(result);
+        while (matcher.find()) {
+            String propertyName = matcher.group(1);
+            Object propertyValue = properties.get(propertyName);
+            result.replace(matcher.start(), matcher.end(), String.valueOf(propertyValue));
+            matcher.reset();
+        }
+        return result.toString();
+    }
+}

maven-release-manager/src/test/java/org/apache/maven/shared/release/phase/RewritePomsForDevelopmentPhaseTest.java

@@ -398,4 +398,37 @@ public void testRewritePomDependenciesWithoutDependenciesVersionUpdate() throws
 
         assertTrue(comparePomFiles(reactorProjects));
     }
+
+    @Test
+    public void testRewritePomWithCiFriendlyReactor() throws Exception {
+        List<MavenProject> reactorProjects = createReactorProjects("pom-with-parent-and-cifriendly-expressions");
+
+        ReleaseDescriptorBuilder builder =
+                createDescriptorFromProjects(reactorProjects, "pom-with-parent-and-cifriendly-expressions");
+        builder.addReleaseVersion("groupId:artifactId", RELEASE_VERSION);
+        builder.addDevelopmentVersion("groupId:artifactId", NEXT_VERSION);
+        builder.addReleaseVersion("groupId:subproject1", RELEASE_VERSION);
+        builder.addDevelopmentVersion("groupId:subproject1", NEXT_VERSION);
+
+        mapScm(builder);
+
+        phase.execute(ReleaseUtils.buildReleaseDescriptor(builder), new DefaultReleaseEnvironment(), reactorProjects);
+
+        assertTrue(comparePomFiles(reactorProjects));
+    }
+
+    @Test
+    public void testRewritePomWithCiFriendlyReactorWithOnlyRevision() throws Exception {
+        List<MavenProject> reactorProjects = createReactorProjects("pom-with-parent-and-cifriendly-revision");
+
+        ReleaseDescriptorBuilder builder =
+                createDescriptorFromProjects(reactorProjects, "pom-with-parent-and-cifriendly-revision");
+        builder.addReleaseVersion("groupId:artifactId", RELEASE_VERSION);
+        builder.addDevelopmentVersion("groupId:artifactId", NEXT_VERSION);
+        builder.addReleaseVersion("groupId:subproject1", RELEASE_VERSION);
+        builder.addDevelopmentVersion("groupId:subproject1", NEXT_VERSION);
+        phase.execute(ReleaseUtils.buildReleaseDescriptor(builder), new DefaultReleaseEnvironment(), reactorProjects);
+
+        assertTrue(comparePomFiles(reactorProjects));
+    }
 }

maven-release-manager/src/test/java/org/apache/maven/shared/release/phase/RewritePomsForReleasePhaseTest.java

@@ -334,6 +334,19 @@ public void testRewritePomWithCiFriendlyReactor() throws Exception {
         assertTrue(comparePomFiles(reactorProjects));
     }
 
+    @Test
+    public void testRewritePomWithCiFriendlyReactorWithOnlyRevision() throws Exception {
+        List<MavenProject> reactorProjects = createReactorProjects("pom-with-parent-and-cifriendly-revision");
+
+        ReleaseDescriptorBuilder builder =
+                createDescriptorFromProjects(reactorProjects, "pom-with-parent-and-cifriendly-revision");
+        builder.addReleaseVersion("groupId:artifactId", NEXT_VERSION);
+        builder.addReleaseVersion("groupId:subproject1", NEXT_VERSION);
+        phase.execute(ReleaseUtils.buildReleaseDescriptor(builder), new DefaultReleaseEnvironment(), reactorProjects);
+
+        assertTrue(comparePomFiles(reactorProjects));
+    }
+
     // MRELEASE-311
     @Test
     public void testRewritePomWithDependencyPropertyCoordinate() throws Exception {

maven-release-manager/src/test/java/org/apache/maven/shared/release/transform/jdom2/JDomModelTest.java

@@ -69,7 +69,7 @@ public void testSetVersion() throws Exception {
         assertNull(model.getVersion());
 
         // inherit from parent via CI friendly
-        content = "<project><parent><version>${revision}</version></parent></project>";
+        content = "<project><parent><version>${revision}${changelist}</version></parent></project>";
         projectElm = builder.build(new StringReader(content)).getRootElement();
         model = new JDomModel(projectElm);
         assertNull(model.getVersion());