From c1c114dbf98f77eefacb5aa6887645e4de9f069c Mon Sep 17 00:00:00 2001 From: Matt Benson Date: Thu, 2 May 2024 08:13:34 -0500 Subject: [PATCH] [MNG-8081] Interpolate available properties during default profile selection (Maven 3.9.x) (#1447) Co-authored-by: Guillaume Nodet --- .../model/building/DefaultModelBuilder.java | 107 ++++++++++++----- .../validation/DefaultModelValidator.java | 112 ++++++++++++++---- .../validation/DefaultModelValidatorTest.java | 75 ++++++++++-- ...tivation-file-with-allowed-expressions.xml | 18 +++ ...tion-property-with-project-expressions.xml | 49 ++++++++ .../repository-with-basedir-expression.xml | 50 ++++++++ .../raw-model/repository-with-expression.xml | 46 +++++++ 7 files changed, 388 insertions(+), 69 deletions(-) create mode 100644 maven-model-builder/src/test/resources/poms/validation/raw-model/profile-activation-property-with-project-expressions.xml create mode 100644 maven-model-builder/src/test/resources/poms/validation/raw-model/repository-with-basedir-expression.xml create mode 100644 maven-model-builder/src/test/resources/poms/validation/raw-model/repository-with-expression.xml diff --git a/maven-model-builder/src/main/java/org/apache/maven/model/building/DefaultModelBuilder.java b/maven-model-builder/src/main/java/org/apache/maven/model/building/DefaultModelBuilder.java index 6ad8b8ee3407..3043a76a0ea6 100644 --- a/maven-model-builder/src/main/java/org/apache/maven/model/building/DefaultModelBuilder.java +++ b/maven-model-builder/src/main/java/org/apache/maven/model/building/DefaultModelBuilder.java @@ -26,23 +26,28 @@ import java.io.IOException; import java.util.ArrayList; import java.util.Collection; +import java.util.Collections; import java.util.HashMap; import java.util.Iterator; import java.util.LinkedHashSet; import java.util.List; import java.util.Map; import java.util.Objects; +import java.util.Optional; import java.util.Properties; +import java.util.Set; +import java.util.function.Consumer; +import java.util.stream.Collectors; import org.apache.maven.artifact.versioning.DefaultArtifactVersion; import org.apache.maven.artifact.versioning.InvalidVersionSpecificationException; import org.apache.maven.artifact.versioning.VersionRange; import org.apache.maven.model.Activation; -import org.apache.maven.model.ActivationFile; import org.apache.maven.model.Build; import org.apache.maven.model.Dependency; import org.apache.maven.model.DependencyManagement; import org.apache.maven.model.InputLocation; +import org.apache.maven.model.InputLocationTracker; import org.apache.maven.model.InputSource; import org.apache.maven.model.Model; import org.apache.maven.model.Parent; @@ -78,6 +83,7 @@ import org.apache.maven.model.validation.ModelValidator; import org.codehaus.plexus.interpolation.InterpolationException; import org.codehaus.plexus.interpolation.MapBasedValueSource; +import org.codehaus.plexus.interpolation.RegexBasedInterpolator; import org.codehaus.plexus.interpolation.StringSearchInterpolator; import org.codehaus.plexus.util.StringUtils; import org.eclipse.sisu.Nullable; @@ -299,14 +305,19 @@ protected ModelBuildingResult build(ModelBuildingRequest request, Collection activePomProfiles = - profileSelector.getActiveProfiles(rawModel.getProfiles(), profileActivationContext, problems); - currentData.setActiveProfiles(activePomProfiles); - Map interpolatedActivations = getInterpolatedActivations(rawModel, profileActivationContext, problems); injectProfileActivations(tmpModel, interpolatedActivations); + List activePomProfiles = + profileSelector.getActiveProfiles(tmpModel.getProfiles(), profileActivationContext, problems); + + Set activeProfileIds = + activePomProfiles.stream().map(Profile::getId).collect(Collectors.toSet()); + currentData.setActiveProfiles(rawModel.getProfiles().stream() + .filter(p -> activeProfileIds.contains(p.getId())) + .collect(Collectors.toList())); + // profile injection for (Profile activeProfile : activePomProfiles) { profileInjector.injectProfile(tmpModel, activeProfile, request, problems); @@ -413,40 +424,72 @@ protected ModelBuildingResult build(ModelBuildingRequest request, Collection getInterpolatedActivations( Model rawModel, DefaultProfileActivationContext context, DefaultModelProblemCollector problems) { Map interpolatedActivations = getProfileActivations(rawModel, true); - for (Activation activation : interpolatedActivations.values()) { - if (activation.getFile() != null) { - replaceWithInterpolatedValue(activation.getFile(), context, problems); - } + + if (interpolatedActivations.isEmpty()) { + return Collections.emptyMap(); } - return interpolatedActivations; - } + RegexBasedInterpolator interpolator = new RegexBasedInterpolator(); - private void replaceWithInterpolatedValue( - ActivationFile activationFile, ProfileActivationContext context, DefaultModelProblemCollector problems) { - try { - if (StringUtils.isNotEmpty(activationFile.getExists())) { - String path = activationFile.getExists(); - String absolutePath = profileActivationFilePathInterpolator.interpolate(path, context); - activationFile.setExists(absolutePath); - } else if (StringUtils.isNotEmpty(activationFile.getMissing())) { - String path = activationFile.getMissing(); - String absolutePath = profileActivationFilePathInterpolator.interpolate(path, context); - activationFile.setMissing(absolutePath); + interpolator.addValueSource(new MapBasedValueSource(context.getProjectProperties())); + interpolator.addValueSource(new MapBasedValueSource(context.getUserProperties())); + interpolator.addValueSource(new MapBasedValueSource(context.getSystemProperties())); + + class Interpolation { + final InputLocationTracker target; + + final InterpolateString impl; + + Interpolation(InputLocationTracker target, InterpolateString impl) { + this.target = target; + this.impl = impl; + } + + void performFor(String value, String locationKey, Consumer mutator) { + if (StringUtils.isEmpty(value)) { + return; + } + try { + mutator.accept(impl.apply(value)); + } catch (InterpolationException e) { + problems.add(new ModelProblemCollectorRequest(Severity.ERROR, Version.BASE) + .setMessage("Failed to interpolate value " + value + ": " + e.getMessage()) + .setLocation(target.getLocation(locationKey)) + .setException(e)); + } } - } catch (InterpolationException e) { - String path = StringUtils.isNotEmpty(activationFile.getExists()) - ? activationFile.getExists() - : activationFile.getMissing(); - - problems.add(new ModelProblemCollectorRequest(Severity.ERROR, Version.BASE) - .setMessage("Failed to interpolate file location " + path + ": " + e.getMessage()) - .setLocation(activationFile.getLocation( - StringUtils.isNotEmpty(activationFile.getExists()) ? "exists" : "missing")) - .setException(e)); } + for (Activation activation : interpolatedActivations.values()) { + Optional a = Optional.of(activation); + a.map(Activation::getFile).ifPresent(fa -> { + Interpolation nt = + new Interpolation(fa, s -> profileActivationFilePathInterpolator.interpolate(s, context)); + nt.performFor(fa.getExists(), "exists", fa::setExists); + nt.performFor(fa.getMissing(), "missing", fa::setMissing); + }); + a.map(Activation::getOs).ifPresent(oa -> { + Interpolation nt = new Interpolation(oa, interpolator::interpolate); + nt.performFor(oa.getArch(), "arch", oa::setArch); + nt.performFor(oa.getFamily(), "family", oa::setFamily); + nt.performFor(oa.getName(), "name", oa::setName); + nt.performFor(oa.getVersion(), "version", oa::setVersion); + }); + a.map(Activation::getProperty).ifPresent(pa -> { + Interpolation nt = new Interpolation(pa, interpolator::interpolate); + nt.performFor(pa.getName(), "name", pa::setName); + nt.performFor(pa.getValue(), "value", pa::setValue); + }); + a.map(Activation::getJdk).ifPresent(ja -> new Interpolation(activation, interpolator::interpolate) + .performFor(ja, "jdk", activation::setJdk)); + } + return interpolatedActivations; } @Override diff --git a/maven-model-builder/src/main/java/org/apache/maven/model/validation/DefaultModelValidator.java b/maven-model-builder/src/main/java/org/apache/maven/model/validation/DefaultModelValidator.java index 47a92ab967e0..dc544d455cc9 100644 --- a/maven-model-builder/src/main/java/org/apache/maven/model/validation/DefaultModelValidator.java +++ b/maven-model-builder/src/main/java/org/apache/maven/model/validation/DefaultModelValidator.java @@ -24,17 +24,24 @@ import java.io.File; import java.util.Arrays; +import java.util.Deque; import java.util.HashMap; import java.util.HashSet; +import java.util.Iterator; +import java.util.LinkedList; import java.util.List; import java.util.Map; import java.util.Objects; +import java.util.Optional; import java.util.Set; +import java.util.function.Consumer; +import java.util.function.Supplier; import java.util.regex.Matcher; import java.util.regex.Pattern; +import java.util.stream.Collectors; +import java.util.stream.StreamSupport; import org.apache.maven.model.Activation; -import org.apache.maven.model.ActivationFile; import org.apache.maven.model.Build; import org.apache.maven.model.BuildBase; import org.apache.maven.model.Dependency; @@ -236,42 +243,97 @@ public void validateRawModel(Model m, ModelBuildingRequest request, ModelProblem } private void validate30RawProfileActivation(ModelProblemCollector problems, Activation activation, String prefix) { - if (activation == null || activation.getFile() == null) { + if (activation == null) { return; } + class ActivationFrame { + String location; + Optional parent; - ActivationFile file = activation.getFile(); - - String path; - String location; - - if (file.getExists() != null && !file.getExists().isEmpty()) { - path = file.getExists(); - location = "exists"; - } else if (file.getMissing() != null && !file.getMissing().isEmpty()) { - path = file.getMissing(); - location = "missing"; - } else { - return; + ActivationFrame(String location, Optional parent) { + this.location = location; + this.parent = parent; + } } - - if (hasProjectExpression(path)) { - Matcher matcher = EXPRESSION_PROJECT_NAME_PATTERN.matcher(path); - while (matcher.find()) { - String propertyName = matcher.group(0); - if (!"${project.basedir}".equals(propertyName)) { + final Deque stk = new LinkedList<>(); + + final Supplier pathSupplier = () -> { + final boolean parallel = false; + return StreamSupport.stream(((Iterable) stk::descendingIterator).spliterator(), parallel) + .map(f -> f.location) + .collect(Collectors.joining(".")); + }; + final Supplier locationSupplier = () -> { + if (stk.size() < 2) { + return null; + } + Iterator f = stk.iterator(); + + String location = f.next().location; + ActivationFrame parent = f.next(); + + return parent.parent.map(p -> p.getLocation(location)).orElse(null); + }; + final Consumer validator = s -> { + if (hasProjectExpression(s)) { + String path = pathSupplier.get(); + Matcher matcher = EXPRESSION_PROJECT_NAME_PATTERN.matcher(s); + while (matcher.find()) { + String propertyName = matcher.group(0); + + if (path.startsWith("activation.file.") && "${project.basedir}".equals(propertyName)) { + continue; + } addViolation( problems, Severity.WARNING, Version.V30, - prefix + "activation.file." + location, + prefix + path, null, - "Failed to interpolate file location " + path + ": " + propertyName + "Failed to interpolate profile activation property " + s + ": " + propertyName + " expressions are not supported during profile activation.", - file.getLocation(location)); + locationSupplier.get()); } } - } + }; + Optional root = Optional.of(activation); + stk.push(new ActivationFrame("activation", root)); + root.map(Activation::getFile).ifPresent(fa -> { + stk.push(new ActivationFrame("file", Optional.of(fa))); + stk.push(new ActivationFrame("exists", Optional.empty())); + validator.accept(fa.getExists()); + stk.peek().location = "missing"; + validator.accept(fa.getMissing()); + stk.pop(); + stk.pop(); + }); + root.map(Activation::getOs).ifPresent(oa -> { + stk.push(new ActivationFrame("os", Optional.of(oa))); + stk.push(new ActivationFrame("arch", Optional.empty())); + validator.accept(oa.getArch()); + stk.peek().location = "family"; + validator.accept(oa.getFamily()); + stk.peek().location = "name"; + validator.accept(oa.getName()); + stk.peek().location = "version"; + validator.accept(oa.getVersion()); + stk.pop(); + stk.pop(); + }); + root.map(Activation::getProperty).ifPresent(pa -> { + stk.push(new ActivationFrame("property", Optional.of(pa))); + stk.push(new ActivationFrame("name", Optional.empty())); + validator.accept(pa.getName()); + stk.peek().location = "value"; + validator.accept(pa.getValue()); + stk.pop(); + stk.pop(); + }); + root.map(Activation::getJdk).ifPresent(jdk -> { + stk.push(new ActivationFrame("jdk", Optional.empty())); + validator.accept(jdk); + stk.pop(); + }); } private void validate20RawPlugins( diff --git a/maven-model-builder/src/test/java/org/apache/maven/model/validation/DefaultModelValidatorTest.java b/maven-model-builder/src/test/java/org/apache/maven/model/validation/DefaultModelValidatorTest.java index 8a7b4fe8b680..22a94dbbe028 100644 --- a/maven-model-builder/src/test/java/org/apache/maven/model/validation/DefaultModelValidatorTest.java +++ b/maven-model-builder/src/test/java/org/apache/maven/model/validation/DefaultModelValidatorTest.java @@ -20,6 +20,8 @@ import java.io.InputStream; import java.util.List; +import java.util.Properties; +import java.util.function.UnaryOperator; import junit.framework.TestCase; import org.apache.maven.model.Model; @@ -44,29 +46,41 @@ private Model read(String pom) throws Exception { } private SimpleProblemCollector validate(String pom) throws Exception { - return validateEffective(pom, ModelBuildingRequest.VALIDATION_LEVEL_STRICT); + return validateEffective(pom, UnaryOperator.identity()); } private SimpleProblemCollector validateRaw(String pom) throws Exception { - return validateRaw(pom, ModelBuildingRequest.VALIDATION_LEVEL_STRICT); + return validateRaw(pom, UnaryOperator.identity()); } private SimpleProblemCollector validateEffective(String pom, int level) throws Exception { - ModelBuildingRequest request = new DefaultModelBuildingRequest().setValidationLevel(level); + return validateEffective(pom, mbr -> mbr.setValidationLevel(level)); + } + + private SimpleProblemCollector validateEffective(String pom, UnaryOperator requestConfigurer) + throws Exception { + Model model = read(pom); - SimpleProblemCollector problems = new SimpleProblemCollector(read(pom)); + SimpleProblemCollector problems = new SimpleProblemCollector(model); - validator.validateEffectiveModel(problems.getModel(), request, problems); + validator.validateEffectiveModel(model, requestConfigurer.apply(new DefaultModelBuildingRequest()), problems); return problems; } private SimpleProblemCollector validateRaw(String pom, int level) throws Exception { - ModelBuildingRequest request = new DefaultModelBuildingRequest().setValidationLevel(level); + return validateRaw(pom, mbr -> mbr.setValidationLevel(level)); + } - SimpleProblemCollector problems = new SimpleProblemCollector(read(pom)); + private SimpleProblemCollector validateRaw(String pom, UnaryOperator requestConfigurer) + throws Exception { + Model model = read(pom); - validator.validateRawModel(problems.getModel(), request, problems); + SimpleProblemCollector problems = new SimpleProblemCollector(model); + + ModelBuildingRequest request = requestConfigurer.apply(new DefaultModelBuildingRequest()); + + validator.validateRawModel(model, request, problems); return problems; } @@ -748,24 +762,61 @@ public void testParentVersionRELEASE() throws Exception { result.getWarnings().get(0)); } + public void testRepositoryWithExpression() throws Exception { + SimpleProblemCollector result = validateRaw("raw-model/repository-with-expression.xml"); + assertViolations(result, 0, 0, 0); + } + + public void testRepositoryWithBasedirExpression() throws Exception { + SimpleProblemCollector result = validateRaw("raw-model/repository-with-basedir-expression.xml"); + assertViolations(result, 0, 0, 0); + } + public void testProfileActivationWithAllowedExpression() throws Exception { - SimpleProblemCollector result = validateRaw("raw-model/profile-activation-file-with-allowed-expressions.xml"); + SimpleProblemCollector result = validateRaw( + "raw-model/profile-activation-file-with-allowed-expressions.xml", + mbr -> mbr.setUserProperties(new Properties() { + private static final long serialVersionUID = 1L; + + { + setProperty("foo", "foo"); + setProperty("bar", "foo"); + } + })); assertViolations(result, 0, 0, 0); } - public void testProfileActivationWithProjectExpression() throws Exception { + public void testProfileActivationFileWithProjectExpression() throws Exception { SimpleProblemCollector result = validateRaw("raw-model/profile-activation-file-with-project-expressions.xml"); assertViolations(result, 0, 0, 2); assertEquals( "'profiles.profile[exists-project-version].activation.file.exists' " - + "Failed to interpolate file location ${project.version}/test.txt: " + + "Failed to interpolate profile activation property ${project.version}/test.txt: " + "${project.version} expressions are not supported during profile activation.", result.getWarnings().get(0)); assertEquals( "'profiles.profile[missing-project-version].activation.file.missing' " - + "Failed to interpolate file location ${project.version}/test.txt: " + + "Failed to interpolate profile activation property ${project.version}/test.txt: " + + "${project.version} expressions are not supported during profile activation.", + result.getWarnings().get(1)); + } + + public void testProfileActivationPropertyWithProjectExpression() throws Exception { + SimpleProblemCollector result = + validateRaw("raw-model/profile-activation-property-with-project-expressions.xml"); + assertViolations(result, 0, 0, 2); + + assertEquals( + "'profiles.profile[property-name-project-version].activation.property.name' " + + "Failed to interpolate profile activation property ${project.version}: " + + "${project.version} expressions are not supported during profile activation.", + result.getWarnings().get(0)); + + assertEquals( + "'profiles.profile[property-value-project-version].activation.property.value' " + + "Failed to interpolate profile activation property ${project.version}: " + "${project.version} expressions are not supported during profile activation.", result.getWarnings().get(1)); } diff --git a/maven-model-builder/src/test/resources/poms/validation/raw-model/profile-activation-file-with-allowed-expressions.xml b/maven-model-builder/src/test/resources/poms/validation/raw-model/profile-activation-file-with-allowed-expressions.xml index a4beb6238f4a..72b6747f1835 100644 --- a/maven-model-builder/src/test/resources/poms/validation/raw-model/profile-activation-file-with-allowed-expressions.xml +++ b/maven-model-builder/src/test/resources/poms/validation/raw-model/profile-activation-file-with-allowed-expressions.xml @@ -60,5 +60,23 @@ under the License. + + dynamic-property-available + + + ${activationProperty} + + + + + + matches-another-property + + + foo + ${bar} + + + diff --git a/maven-model-builder/src/test/resources/poms/validation/raw-model/profile-activation-property-with-project-expressions.xml b/maven-model-builder/src/test/resources/poms/validation/raw-model/profile-activation-property-with-project-expressions.xml new file mode 100644 index 000000000000..8bcf89f66f72 --- /dev/null +++ b/maven-model-builder/src/test/resources/poms/validation/raw-model/profile-activation-property-with-project-expressions.xml @@ -0,0 +1,49 @@ + + + + 4.0.0 + aid + gid + 0.1 + pom + + + + + property-name-project-version + + + ${project.version} + + + + + property-value-project-version + + + project.version + ${project.version} + + + + + + diff --git a/maven-model-builder/src/test/resources/poms/validation/raw-model/repository-with-basedir-expression.xml b/maven-model-builder/src/test/resources/poms/validation/raw-model/repository-with-basedir-expression.xml new file mode 100644 index 000000000000..2dd11eb42ed0 --- /dev/null +++ b/maven-model-builder/src/test/resources/poms/validation/raw-model/repository-with-basedir-expression.xml @@ -0,0 +1,50 @@ + + + + + 4.0.0 + + org.apache.maven.validation + parent + 1 + + + org.apache.maven.validation + project + 1.0.0-SNAPSHOT + + + + repo + file://${basedir}/target/remote-repo + + + repo-project-basedir + file://${project.basedir}/sdk/maven/repo + + + repo-project-baseUri + ${project.baseUri}/sdk/maven/repo + + + + diff --git a/maven-model-builder/src/test/resources/poms/validation/raw-model/repository-with-expression.xml b/maven-model-builder/src/test/resources/poms/validation/raw-model/repository-with-expression.xml new file mode 100644 index 000000000000..fcdd9465d82b --- /dev/null +++ b/maven-model-builder/src/test/resources/poms/validation/raw-model/repository-with-expression.xml @@ -0,0 +1,46 @@ + + + + + 4.0.0 + + org.apache.maven.validation + parent + 1 + + + org.apache.maven.validation + project + 1.0.0-SNAPSHOT + + + just/some/path + + + + + repo + file://${x}/sdk/maven/repo + + + + \ No newline at end of file