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

.gitignore

@@ -10,4 +10,6 @@ target
 build
 .checkstyle
 .DS_Store
-.factorypath
+.factorypath
+.mvn/
+mvnw

maven-release-manager/src/main/java/org/apache/maven/shared/release/config/ReleaseDescriptorBuilder.java

@@ -34,7 +34,7 @@
  * @since 3.0.0-M5
  */
 public class ReleaseDescriptorBuilder {
-    private static final Pattern PROPERTY_PATTERN = Pattern.compile("\\$\\{[^}]+}");
+    private static final Pattern PROPERTY_PATTERN = Pattern.compile("\\$\\{[^}]+?}");
 
     private final Logger logger;
 

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/AbstractMapVersionsPhase.java

@@ -35,6 +35,7 @@
 import org.apache.maven.shared.release.policy.version.VersionPolicy;
 import org.apache.maven.shared.release.policy.version.VersionPolicyRequest;
 import org.apache.maven.shared.release.scm.ScmRepositoryConfigurator;
+import org.apache.maven.shared.release.util.CiFriendlyVersion;
 import org.apache.maven.shared.release.util.ReleaseUtil;
 import org.apache.maven.shared.release.versions.VersionParseException;
 import org.codehaus.plexus.components.interactivity.Prompter;
@@ -238,12 +239,12 @@ private String resolveNextVersion(
 
                     try {
                         try {
-                            suggestedVersion =
-                                    resolveSuggestedVersion(baseVersion, releaseDescriptor, releaseEnvironment);
+                            suggestedVersion = resolveSuggestedVersion(
+                                    project, baseVersion, releaseDescriptor, releaseEnvironment);
                         } catch (VersionParseException e) {
                             if (releaseDescriptor.isInteractive()) {
                                 suggestedVersion =
-                                        resolveSuggestedVersion("1.0", releaseDescriptor, releaseEnvironment);
+                                        resolveSuggestedVersion(project, "1.0", releaseDescriptor, releaseEnvironment);
                             } else {
                                 throw new ReleaseExecutionException(
                                         "Error parsing version, cannot determine next " + "version: " + e.getMessage(),
@@ -292,7 +293,10 @@ private String getContextString(ReleaseDescriptor releaseDescriptor) {
     }
 
     private String resolveSuggestedVersion(
-            String baseVersion, ReleaseDescriptor releaseDescriptor, ReleaseEnvironment releaseEnvironment)
+            MavenProject project,
+            String baseVersion,
+            ReleaseDescriptor releaseDescriptor,
+            ReleaseEnvironment releaseEnvironment)
             throws PolicyException, VersionParseException {
         String policyId = releaseDescriptor.getProjectVersionPolicyId();
         VersionPolicy policy = versionPolicies.get(policyId);
@@ -307,6 +311,11 @@ private String resolveSuggestedVersion(
         }
         request.setWorkingDirectory(releaseDescriptor.getWorkingDirectory());
 
+        if (CiFriendlyVersion.isCiFriendlyVersion(getOriginalVersion(project))) {
+            String sha1 = CiFriendlyVersion.resolveSha1Property(project.getProperties(), releaseDescriptor);
+            request.setVersion(baseVersion.replace(sha1, ""));
+        }
+
         if (scmRepositoryConfigurator != null && releaseDescriptor.getScmSourceUrl() != null) {
             try {
                 ScmRepository repository = scmRepositoryConfigurator.getConfiguredRepository(
@@ -332,6 +341,15 @@ private String resolveSuggestedVersion(
                 : policy.getReleaseVersion(request).getVersion();
     }
 
+    private static String getOriginalVersion(MavenProject project) {
+        String version = null;
+        while (version == null && project != null) {
+            version = project.getOriginalModel().getVersion();
+            project = project.getParent();
+        }
+        return version;
+    }
+
     private String getDevelopmentVersion(String projectId, ReleaseDescriptor releaseDescriptor) {
         String projectVersion = releaseDescriptor.getProjectDevelopmentVersion(projectId);
 

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,8 @@
 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.MavenExpression;
 import org.apache.maven.shared.release.util.ReleaseUtil;
 import org.codehaus.plexus.util.StringUtils;
 
@@ -92,18 +91,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 +442,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 +464,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 +535,7 @@ private void rewriteArtifactVersions(
                     logInfo(result, "  Updating " + artifactId + " to " + mappedVersion);
                     coordinate.setVersion(mappedVersion);
                 } else {
-                    String property = extractPropertyFromExpression(rawVersion);
+                    String property = MavenExpression.extractPropertyFromExpression(rawVersion);
                     if (property != null) {
                         if (property.startsWith("project.")
                                 || property.startsWith("pom.")
@@ -609,7 +580,9 @@ private void rewriteArtifactVersions(
                                     }
                                 }
                             } else {
-                                if (CI_FRIENDLY_PROPERTIES.contains(property)) {
+                                if (CiFriendlyVersion.isCiFriendlyProperty(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,8 @@
 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.config.ReleaseDescriptor;
+import org.apache.maven.shared.release.util.CiFriendlyVersion;
 import org.jdom2.Document;
 import org.jdom2.Element;
 import org.jdom2.Text;
@@ -47,22 +48,29 @@ public class JDomModel extends Model {
 
     private final JDomModelBase modelBase;
 
+    /**
+     * The ReleaseDescriptor after a commit performed
+     *
+     */
+    private final ReleaseDescriptor releaseDescriptor;
+
     /**
      * <p>Constructor for JDomModel.</p>
      *
      * @param document a {@link org.jdom2.Document} object
      */
-    public JDomModel(Document document) {
-        this(document.getRootElement());
+    public JDomModel(Document document, ReleaseDescriptor releaseDescriptor) {
+        this(document.getRootElement(), releaseDescriptor);
     }
 
     /**
      * <p>Constructor for JDomModel.</p>
      *
      * @param project a {@link org.jdom2.Element} object
      */
-    public JDomModel(Element project) {
+    public JDomModel(Element project, ReleaseDescriptor releaseDescriptor) {
         this.project = project;
+        this.releaseDescriptor = releaseDescriptor;
         this.modelBase = new JDomModelBase(project);
     }
 
@@ -180,7 +188,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 +200,9 @@ 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, getProperties(), releaseDescriptor);
             } else {
                 JDomUtils.rewriteValue(versionElement, version);
             }

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

@@ -159,7 +159,7 @@ public void load(File targetFile) throws ReleaseExecutionException {
 
     @Override
     public Model getModel() {
-        return new JDomModel(document);
+        return new JDomModel(document, releaseDescriptor);
     }
 
     private void normaliseLineEndings(Document document) {

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