[MNG-8230] Rewrite CI friendly versions (#1710)

maven-api-impl/src/main/java/org/apache/maven/api/services/model/ModelVersionProcessor.java

@@ -1,44 +0,0 @@
-/*
- * 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.api.services.model;
-
-import java.util.Properties;
-
-import org.apache.maven.api.services.ModelBuilderRequest;
-
-/**
- * Allows a fixed set of properties that are valid inside a version and that could be overwritten for example on the
- * commandline
- */
-public interface ModelVersionProcessor {
-
-    /**
-     * @param property the property to check
-     * @return <code>true</code> if this is a valid property for this processor
-     */
-    boolean isValidProperty(String property);
-
-    /**
-     * This method is responsible for examining the request and possibly overwrite of the valid properties in the model
-     *
-     * @param modelProperties
-     * @param request
-     */
-    void overwriteModelProperties(Properties modelProperties, ModelBuilderRequest request);
-}

maven-api-impl/src/main/java/org/apache/maven/internal/impl/model/DefaultModelBuilder.java

@@ -26,7 +26,6 @@ import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.Paths;
 import java.util.ArrayList;
-import java.util.Arrays;
 import java.util.Collection;
 import java.util.HashMap;
 import java.util.HashSet;
@@ -45,6 +44,8 @@ import java.util.concurrent.Executors;
 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;
 
@@ -233,6 +234,8 @@ public class DefaultModelBuilder implements ModelBuilder {
     }
 
     protected class DefaultModelBuilderSession implements ModelProblemCollector {
+        private static final Pattern REGEX = Pattern.compile("\\$\\{([^}]+)}");
+
         final Session session;
         final ModelBuilderRequest request;
         final DefaultModelBuilderResult result;
@@ -557,58 +560,15 @@ public class DefaultModelBuilder implements ModelBuilder {
         }
 
         //
-        // Transform raw model to build pom
+        // Transform raw model to build pom.
-        //
-        Model transformFileToRaw(Model model) {
-            Model.Builder builder = Model.newBuilder(model);
-            builder = handleParent(model, builder);
-            builder = handleReactorDependencies(model, builder);
-            builder = handleCiFriendlyVersion(model, builder);
-            return builder.build();
-        }
-
-        //
-        // Infer parent information
-        //
-        Model.Builder handleParent(Model model, Model.Builder builder) {
-            Parent parent = model.getParent();
-            if (parent != null) {
-                String version = parent.getVersion();
-                String modVersion = replaceCiFriendlyVersion(version);
-                if (!Objects.equals(version, modVersion)) {
-                    if (builder == null) {
-                        builder = Model.newBuilder(model);
-                    }
-                    builder.parent(parent.withVersion(modVersion));
-                }
-            }
-            return builder;
-        }
-
-        //
-        // CI friendly versions
-        //
-        Model.Builder handleCiFriendlyVersion(Model model, Model.Builder builder) {
-            String version = model.getVersion();
-            String modVersion = replaceCiFriendlyVersion(version);
-            if (!Objects.equals(version, modVersion)) {
-                if (builder == null) {
-                    builder = Model.newBuilder(model);
-                }
-                builder.version(modVersion);
-            }
-            return builder;
-        }
-
-        //
         // Infer inner reactor dependencies version
         //
-        Model.Builder handleReactorDependencies(Model model, Model.Builder builder) {
+        Model transformFileToRaw(Model model) {
             List<Dependency> newDeps = new ArrayList<>();
             boolean modified = false;
             for (Dependency dep : model.getDependencies()) {
-                Dependency.Builder depBuilder = null;
                 if (dep.getVersion() == null) {
+                    Dependency.Builder depBuilder = null;
                     Model depModel = getRawModel(model.getPomFile(), dep.getGroupId(), dep.getArtifactId());
                     if (depModel != null) {
                         String version = depModel.getVersion();
@@ -629,30 +589,35 @@ public class DefaultModelBuilder implements ModelBuilder {
                             depBuilder.groupId(depGroupId).location("groupId", groupIdLocation);
                         }
                     }
-                }
+                    if (depBuilder != null) {
-                if (depBuilder != null) {
+                        newDeps.add(depBuilder.build());
-                    newDeps.add(depBuilder.build());
+                        modified = true;
-                    modified = true;
+                    } else {
-                } else {
+                        newDeps.add(dep);
-                    newDeps.add(dep);
+                    }
                 }
             }
-            if (modified) {
+            return modified ? model.withDependencies(newDeps) : model;
-                if (builder == null) {
-                    builder = Model.newBuilder(model);
-                }
-                builder.dependencies(newDeps);
-            }
-            return builder;
         }
 
-        String replaceCiFriendlyVersion(String version) {
+        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) {
-                for (String key : Arrays.asList("changelist", "revision", "sha1")) {
+                Matcher matcher = REGEX.matcher(version);
-                    String val = request.getUserProperties().get(key);
+                if (matcher.find()) {
-                    if (val != null) {
+                    StringBuilder result = new StringBuilder();
-                        version = version.replace("${" + key + "}", val);
+                    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;
@@ -733,7 +698,6 @@ public class DefaultModelBuilder implements ModelBuilder {
             return Stream.concat(Stream.of(r), r.getChildren().stream().flatMap(this::results));
         }
 
-        @SuppressWarnings("checkstyle:MethodLength")
         private void loadFromRoot(Path root, Path top) {
             try (PhasingExecutor executor = createExecutor()) {
                 DefaultModelBuilderResult r = Objects.equals(top, root) ? result : new DefaultModelBuilderResult();
@@ -1212,16 +1176,18 @@ public class DefaultModelBuilder implements ModelBuilder {
         Model doReadFileModel() throws ModelBuilderException {
             ModelSource modelSource = request.getSource();
             Model model;
+            Path rootDirectory;
             setSource(modelSource.getLocation());
             logger.debug("Reading file model from " + modelSource.getLocation());
             try {
                 boolean strict = request.getRequestType() == ModelBuilderRequest.RequestType.BUILD_POM;
-                // TODO: we do cache, but what if strict does not have the same value?
-                Path rootDirectory;
                 try {
                     rootDirectory = request.getSession().getRootDirectory();
                 } catch (IllegalStateException ignore) {
                     rootDirectory = modelSource.getPath();
+                    while (rootDirectory != null && !Files.isDirectory(rootDirectory)) {
+                        rootDirectory = rootDirectory.getParent();
+                    }
                 }
                 try (InputStream is = modelSource.openStream()) {
                     model = modelProcessor.read(XmlReaderRequest.builder()
@@ -1355,6 +1321,29 @@ public class DefaultModelBuilder implements ModelBuilder {
                         add(Severity.FATAL, Version.V41, "Error discovering subprojects", e);
                     }
                 }
+
+                // CI friendly version
+                // All expressions are interpolated using user properties and properties
+                // defined on the root project.
+                Map<String, String> properties = new HashMap<>();
+                if (!Objects.equals(rootDirectory, model.getProjectDirectory())) {
+                    Model rootModel = derive(ModelSource.fromPath(modelProcessor.locateExistingPom(rootDirectory)))
+                            .readFileModel();
+                    properties.putAll(rootModel.getProperties());
+                } else {
+                    properties.putAll(model.getProperties());
+                }
+                properties.putAll(session.getUserProperties());
+                model = model.with()
+                        .version(replaceCiFriendlyVersion(properties, model.getVersion()))
+                        .parent(
+                                model.getParent() != null
+                                        ? model.getParent()
+                                                .withVersion(replaceCiFriendlyVersion(
+                                                        properties,
+                                                        model.getParent().getVersion()))
+                                        : null)
+                        .build();
             }
 
             for (var transformer : transformers) {

maven-api-impl/src/main/java/org/apache/maven/internal/impl/model/DefaultModelValidator.java

@@ -70,7 +70,6 @@ import org.apache.maven.api.services.ModelProblem;
 import org.apache.maven.api.services.ModelProblem.Version;
 import org.apache.maven.api.services.ModelProblemCollector;
 import org.apache.maven.api.services.model.ModelValidator;
-import org.apache.maven.api.services.model.ModelVersionProcessor;
 import org.apache.maven.model.v4.MavenModelVersion;
 import org.apache.maven.model.v4.MavenTransformer;
 
@@ -288,12 +287,8 @@ public class DefaultModelValidator implements ModelValidator {
 
     private final Set<String> validProfileIds = new HashSet<>();
 
-    private final ModelVersionProcessor versionProcessor;
-
     @Inject
-    public DefaultModelValidator(ModelVersionProcessor versionProcessor) {
+    public DefaultModelValidator() {}
-        this.versionProcessor = versionProcessor;
-    }
 
     @Override
     @SuppressWarnings("checkstyle:MethodLength")
@@ -418,17 +413,42 @@ public class DefaultModelValidator implements ModelValidator {
 
             Severity errOn30 = getSeverity(validationLevel, ModelValidator.VALIDATION_LEVEL_MAVEN_3_0);
 
-            validateStringNoExpression("groupId", problems, Severity.WARNING, Version.V20, m.getGroupId(), m);
+            // The file pom may not contain the modelVersion yet, as it may be set later by the
-            if (parent == null) {
+            // ModelVersionXMLFilter.
-                validateStringNotEmpty("groupId", problems, Severity.FATAL, Version.V20, m.getGroupId(), m);
+            if (m.getModelVersion() != null && !m.getModelVersion().isEmpty()) {
+                validateModelVersion(problems, m.getModelVersion(), m, VALID_MODEL_VERSIONS);
             }
 
-            validateStringNoExpression("artifactId", problems, Severity.WARNING, Version.V20, m.getArtifactId(), m);
+            boolean isModelVersion41OrMore = !Objects.equals(ModelBuilder.MODEL_VERSION_4_0_0, m.getModelVersion());
-            validateStringNotEmpty("artifactId", problems, Severity.FATAL, Version.V20, m.getArtifactId(), m);
+            if (isModelVersion41OrMore) {
+                validateStringNoExpression("groupId", problems, Severity.FATAL, Version.V41, m.getGroupId(), m);
 
-            validateVersionNoExpression("version", problems, Severity.WARNING, Version.V20, m.getVersion(), m);
+                validateStringNotEmpty("artifactId", problems, Severity.FATAL, Version.V20, m.getArtifactId(), m);
-            if (parent == null) {
+                validateStringNoExpression("artifactId", problems, Severity.FATAL, Version.V20, m.getArtifactId(), m);
-                validateStringNotEmpty("version", problems, Severity.FATAL, Version.V20, m.getVersion(), m);
+
+                validateVersionNoExpression("version", problems, Severity.FATAL, Version.V41, m.getVersion(), m);
+
+                if (parent != null) {
+                    validateStringNoExpression(
+                            "groupId", problems, Severity.FATAL, Version.V41, parent.getGroupId(), m);
+                    validateStringNoExpression(
+                            "artifactId", problems, Severity.FATAL, Version.V41, parent.getArtifactId(), m);
+                    validateVersionNoExpression(
+                            "version", problems, Severity.FATAL, Version.V41, parent.getVersion(), m);
+                }
+            } else {
+                validateStringNoExpression("groupId", problems, Severity.WARNING, Version.V20, m.getGroupId(), m);
+                if (parent == null) {
+                    validateStringNotEmpty("groupId", problems, Severity.FATAL, Version.V20, m.getGroupId(), m);
+                }
+
+                validateStringNoExpression("artifactId", problems, Severity.WARNING, Version.V20, m.getArtifactId(), m);
+                validateStringNotEmpty("artifactId", problems, Severity.FATAL, Version.V20, m.getArtifactId(), m);
+
+                validateVersionNoExpression("version", problems, Severity.WARNING, Version.V20, m.getVersion(), m);
+                if (parent == null) {
+                    validateStringNotEmpty("version", problems, Severity.FATAL, Version.V20, m.getVersion(), m);
+                }
             }
 
             validate20RawDependencies(
@@ -1634,19 +1654,14 @@ public class DefaultModelValidator implements ModelValidator {
 
         Matcher m = EXPRESSION_NAME_PATTERN.matcher(string.trim());
         while (m.find()) {
-            String property = m.group(1);
+            addViolation(
-            if (!versionProcessor.isValidProperty(property)) {
+                    problems,
-                addViolation(
+                    severity,
-                        problems,
+                    version,
-                        severity,
+                    fieldName,
-                        version,
+                    null,
-                        fieldName,
+                    "contains an expression but should be a constant.",
-                        null,
+                    tracker);
-                        "contains an expression but should be a constant.",
-                        tracker);
-
-                return false;
-            }
         }
 
         return true;

maven-api-impl/src/main/java/org/apache/maven/internal/impl/model/DefaultModelVersionProcessor.java

@@ -1,63 +0,0 @@
-/*
- * 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.internal.impl.model;
-
-import java.util.Map;
-import java.util.Properties;
-
-import org.apache.maven.api.di.Named;
-import org.apache.maven.api.di.Singleton;
-import org.apache.maven.api.services.ModelBuilderRequest;
-import org.apache.maven.api.services.model.ModelVersionProcessor;
-
-/**
- * Maven default implementation of the {@link ModelVersionProcessor} to support
- * <a href="https://maven.apache.org/maven-ci-friendly.html">CI Friendly Versions</a>
- */
-@Named
-@Singleton
-public class DefaultModelVersionProcessor implements ModelVersionProcessor {
-
-    private static final String SHA1_PROPERTY = "sha1";
-
-    private static final String CHANGELIST_PROPERTY = "changelist";
-
-    private static final String REVISION_PROPERTY = "revision";
-
-    @Override
-    public boolean isValidProperty(String property) {
-        return REVISION_PROPERTY.equals(property)
-                || CHANGELIST_PROPERTY.equals(property)
-                || SHA1_PROPERTY.equals(property);
-    }
-
-    @Override
-    public void overwriteModelProperties(Properties modelProperties, ModelBuilderRequest request) {
-        Map<String, String> props = request.getUserProperties();
-        if (props.containsKey(REVISION_PROPERTY)) {
-            modelProperties.put(REVISION_PROPERTY, props.get(REVISION_PROPERTY));
-        }
-        if (props.containsKey(CHANGELIST_PROPERTY)) {
-            modelProperties.put(CHANGELIST_PROPERTY, props.get(CHANGELIST_PROPERTY));
-        }
-        if (props.containsKey(SHA1_PROPERTY)) {
-            modelProperties.put(SHA1_PROPERTY, props.get(SHA1_PROPERTY));
-        }
-    }
-}

maven-api-impl/src/test/java/org/apache/maven/internal/impl/standalone/RepositorySystemSupplier.java

@@ -42,7 +42,6 @@ import org.apache.maven.internal.impl.model.DefaultModelNormalizer;
 import org.apache.maven.internal.impl.model.DefaultModelPathTranslator;
 import org.apache.maven.internal.impl.model.DefaultModelProcessor;
 import org.apache.maven.internal.impl.model.DefaultModelValidator;
-import org.apache.maven.internal.impl.model.DefaultModelVersionProcessor;
 import org.apache.maven.internal.impl.model.DefaultPathTranslator;
 import org.apache.maven.internal.impl.model.DefaultPluginManagementInjector;
 import org.apache.maven.internal.impl.model.DefaultProfileInjector;
@@ -1041,7 +1040,7 @@ public class RepositorySystemSupplier implements Supplier<RepositorySystem> {
         DefaultModelProcessor modelProcessor = new DefaultModelProcessor(new DefaultModelXmlFactory(), List.of());
         return new DefaultModelBuilder(
                 modelProcessor,
-                new DefaultModelValidator(new DefaultModelVersionProcessor()),
+                new DefaultModelValidator(),
                 new DefaultModelNormalizer(),
                 new DefaultModelInterpolator(
                         new DefaultPathTranslator(), new DefaultUrlNormalizer(), new DefaultRootLocator()),

maven-resolver-provider/src/main/java/org/apache/maven/repository/internal/MavenRepositorySystemSupplier.java

@@ -42,7 +42,6 @@ import org.apache.maven.internal.impl.model.DefaultModelNormalizer;
 import org.apache.maven.internal.impl.model.DefaultModelPathTranslator;
 import org.apache.maven.internal.impl.model.DefaultModelProcessor;
 import org.apache.maven.internal.impl.model.DefaultModelValidator;
-import org.apache.maven.internal.impl.model.DefaultModelVersionProcessor;
 import org.apache.maven.internal.impl.model.DefaultPathTranslator;
 import org.apache.maven.internal.impl.model.DefaultPluginManagementInjector;
 import org.apache.maven.internal.impl.model.DefaultProfileInjector;
@@ -1044,7 +1043,7 @@ public class MavenRepositorySystemSupplier implements Supplier<RepositorySystem>
         DefaultModelProcessor modelProcessor = new DefaultModelProcessor(new DefaultModelXmlFactory(), List.of());
         return new DefaultModelBuilder(
                 modelProcessor,
-                new DefaultModelValidator(new DefaultModelVersionProcessor()),
+                new DefaultModelValidator(),
                 new DefaultModelNormalizer(),
                 new DefaultModelInterpolator(
                         new DefaultPathTranslator(), new DefaultUrlNormalizer(), new DefaultRootLocator()),