From a58551d9ab572b534aea4671d831faf7ea13abe6 Mon Sep 17 00:00:00 2001 From: Guillaume Nodet Date: Thu, 28 Nov 2024 10:45:14 +0100 Subject: [PATCH] [MNG-8391] Wrong effective model when conflicting values come from parents and profiles --- .../model/ProfileActivationContext.java | 105 ++++-- .../impl/model/DefaultModelBuilder.java | 123 ++++--- .../DefaultProfileActivationContext.java | 331 +++++++++++++++++- .../impl/model/DefaultProfileSelector.java | 8 +- ...ProfileActivationFilePathInterpolator.java | 86 ----- .../model/profile/ConditionFunctions.java | 82 +---- .../profile/ConditionProfileActivator.java | 52 +-- .../model/profile/FileProfileActivator.java | 32 +- .../profile/JdkVersionProfileActivator.java | 2 +- .../OperatingSystemProfileActivator.java | 18 +- .../profile/PackagingProfileActivator.java | 2 +- .../profile/PropertyProfileActivator.java | 6 +- .../impl/model/DefaultModelBuilderTest.java | 66 ++++ .../profile/AbstractProfileActivatorTest.java | 5 +- .../model/profile/ConditionParserTest.java | 14 +- .../ConditionProfileActivatorTest.java | 11 +- .../profile/FileProfileActivatorTest.java | 8 +- .../standalone/RepositorySystemSupplier.java | 7 +- .../props-and-profiles-grand-parent.xml | 34 ++ .../factory/props-and-profiles-parent.xml | 28 ++ .../poms/factory/props-and-profiles.xml | 24 ++ 21 files changed, 686 insertions(+), 358 deletions(-) delete mode 100644 impl/maven-impl/src/main/java/org/apache/maven/internal/impl/model/ProfileActivationFilePathInterpolator.java create mode 100644 impl/maven-impl/src/test/java/org/apache/maven/internal/impl/model/DefaultModelBuilderTest.java create mode 100644 impl/maven-impl/src/test/resources/poms/factory/props-and-profiles-grand-parent.xml create mode 100644 impl/maven-impl/src/test/resources/poms/factory/props-and-profiles-parent.xml create mode 100644 impl/maven-impl/src/test/resources/poms/factory/props-and-profiles.xml diff --git a/impl/maven-impl/src/main/java/org/apache/maven/api/services/model/ProfileActivationContext.java b/impl/maven-impl/src/main/java/org/apache/maven/api/services/model/ProfileActivationContext.java index 1cc7f3c46f..b7951ea3e1 100644 --- a/impl/maven-impl/src/main/java/org/apache/maven/api/services/model/ProfileActivationContext.java +++ b/impl/maven-impl/src/main/java/org/apache/maven/api/services/model/ProfileActivationContext.java @@ -18,11 +18,11 @@ */ package org.apache.maven.api.services.model; -import java.util.List; -import java.util.Map; - import org.apache.maven.api.annotations.Nonnull; +import org.apache.maven.api.annotations.Nullable; import org.apache.maven.api.model.Model; +import org.apache.maven.api.services.InterpolatorException; +import org.apache.maven.api.services.ModelBuilderException; /** * Describes the environmental context used to determine the activation status of profiles. @@ -34,45 +34,100 @@ import org.apache.maven.api.model.Model; public interface ProfileActivationContext { /** - * Gets the identifiers of those profiles that should be activated by explicit demand. + * Checks if the specified profile has been explicitly activated. * - * @return The identifiers of those profiles to activate, never {@code null}. + * @param profileId the profile id + * @return whether the profile has been activated */ - @Nonnull - List getActiveProfileIds(); + boolean isProfileActive(@Nonnull String profileId); /** - * Gets the identifiers of those profiles that should be deactivated by explicit demand. + * Checks if the specified profile has been explicitly deactivated. * - * @return The identifiers of those profiles to deactivate, never {@code null}. + * @param profileId the profile id + * @return whether the profile has been deactivated */ - @Nonnull - List getInactiveProfileIds(); + boolean isProfileInactive(@Nonnull String profileId); /** - * Gets the system properties to use for interpolation and profile activation. The system properties are collected + * Gets the system property to use for interpolation and profile activation. The system properties are collected * from the runtime environment like {@link System#getProperties()} and environment variables. * - * @return The execution properties, never {@code null}. + * @param key the name of the system property + * @return the system property for the specified key, or {@code null} */ - @Nonnull - Map getSystemProperties(); + @Nullable + String getSystemProperty(@Nonnull String key); /** - * Gets the user properties to use for interpolation and profile activation. The user properties have been - * configured directly by the user on his discretion, e.g. via the {@code -Dkey=value} parameter on the command - * line. + * Gets the user property to use for interpolation and profile activation. The user properties have been + * configured directly by the user on his discretion, e.g. via the {@code -Dkey=value} parameter on the command line. * - * @return The user properties, never {@code null}. + * @param key the name of the user property + * @return The user property for the specified key, or {@code null}. */ - @Nonnull - Map getUserProperties(); + @Nullable + String getUserProperty(@Nonnull String key); /** - * Gets the model which is being activated. + * Gets the model property to use for interpolation and profile activation. * - * @return The project model, never {@code null}. + * @param key the name of the model property + * @return The model property for the specified key, or {@code null}; */ - @Nonnull - Model getModel(); + @Nullable + String getModelProperty(@Nonnull String key); + + /** + * Gets the artifactId from the current model. + * + * @return The artifactId of the current model, or {@code null} if not set. + */ + @Nullable + String getModelArtifactId(); + + /** + * Gets the packaging type from the current model. + * + * @return The packaging type of the current model, or {@code null} if not set. + */ + @Nullable + String getModelPackaging(); + + /** + * Gets the root directory of the current model. + * + * @return The root directory path of the current model, or {@code null} if not set. + */ + @Nullable + String getModelRootDirectory(); + + /** + * Gets the base directory of the current model. + * + * @return The base directory path of the current model, or {@code null} if not set. + */ + @Nullable + String getModelBaseDirectory(); + + /** + * Interpolates the given path string using the current context's properties. + * + * @param path The path string to interpolate + * @return The interpolated path string + * @throws InterpolatorException if an error occurs during interpolation + */ + @Nullable + String interpolatePath(@Nullable String path); + + /** + * Checks if a file or directory matching the given glob pattern exists at the specified path. + * + * @param path the base path to check + * @param glob whether the path can be a glob expression + * @return {@code true} if a matching file exists, {@code false} otherwise + * @throws ModelBuilderException if an error occurs while checking the path + * @throws InterpolatorException if an error occurs during interpolation + */ + boolean exists(@Nullable String path, boolean glob); } diff --git a/impl/maven-impl/src/main/java/org/apache/maven/internal/impl/model/DefaultModelBuilder.java b/impl/maven-impl/src/main/java/org/apache/maven/internal/impl/model/DefaultModelBuilder.java index 908f2e0e1e..0490de64fb 100644 --- a/impl/maven-impl/src/main/java/org/apache/maven/internal/impl/model/DefaultModelBuilder.java +++ b/impl/maven-impl/src/main/java/org/apache/maven/internal/impl/model/DefaultModelBuilder.java @@ -99,6 +99,7 @@ import org.apache.maven.api.services.model.ModelResolverException; import org.apache.maven.api.services.model.ModelUrlNormalizer; import org.apache.maven.api.services.model.ModelValidator; import org.apache.maven.api.services.model.ModelVersionParser; +import org.apache.maven.api.services.model.PathTranslator; import org.apache.maven.api.services.model.PluginConfigurationExpander; import org.apache.maven.api.services.model.PluginManagementInjector; import org.apache.maven.api.services.model.ProfileInjector; @@ -145,12 +146,13 @@ public class DefaultModelBuilder implements ModelBuilder { private final DependencyManagementInjector dependencyManagementInjector; private final DependencyManagementImporter dependencyManagementImporter; private final PluginConfigurationExpander pluginConfigurationExpander; - private final ProfileActivationFilePathInterpolator profileActivationFilePathInterpolator; private final ModelVersionParser versionParser; private final List transformers; private final ModelCacheFactory modelCacheFactory; private final ModelResolver modelResolver; private final Interpolator interpolator; + private final PathTranslator pathTranslator; + private final RootLocator rootLocator; @SuppressWarnings("checkstyle:ParameterNumber") @Inject @@ -169,12 +171,13 @@ public class DefaultModelBuilder implements ModelBuilder { DependencyManagementInjector dependencyManagementInjector, DependencyManagementImporter dependencyManagementImporter, PluginConfigurationExpander pluginConfigurationExpander, - ProfileActivationFilePathInterpolator profileActivationFilePathInterpolator, ModelVersionParser versionParser, List transformers, ModelCacheFactory modelCacheFactory, ModelResolver modelResolver, - Interpolator interpolator) { + Interpolator interpolator, + PathTranslator pathTranslator, + RootLocator rootLocator) { this.modelProcessor = modelProcessor; this.modelValidator = modelValidator; this.modelNormalizer = modelNormalizer; @@ -189,12 +192,13 @@ public class DefaultModelBuilder implements ModelBuilder { this.dependencyManagementInjector = dependencyManagementInjector; this.dependencyManagementImporter = dependencyManagementImporter; this.pluginConfigurationExpander = pluginConfigurationExpander; - this.profileActivationFilePathInterpolator = profileActivationFilePathInterpolator; this.versionParser = versionParser; this.transformers = transformers; this.modelCacheFactory = modelCacheFactory; this.modelResolver = modelResolver; this.interpolator = interpolator; + this.pathTranslator = pathTranslator; + this.rootLocator = rootLocator; } public ModelBuilderSession newSession() { @@ -862,12 +866,12 @@ public class DefaultModelBuilder implements ModelBuilder { } } - Model readParent(Model childModel) throws ModelBuilderException { + Model readParent(Model childModel, DefaultProfileActivationContext profileActivationContext) { Model parentModel; Parent parent = childModel.getParent(); if (parent != null) { - parentModel = resolveParent(childModel); + parentModel = resolveParent(childModel, profileActivationContext); if (!"pom".equals(parentModel.getPackaging())) { add( @@ -892,18 +896,20 @@ public class DefaultModelBuilder implements ModelBuilder { return parentModel; } - private Model resolveParent(Model childModel) throws ModelBuilderException { + private Model resolveParent(Model childModel, DefaultProfileActivationContext profileActivationContext) + throws ModelBuilderException { Model parentModel = null; if (isBuildRequest()) { - parentModel = readParentLocally(childModel); + parentModel = readParentLocally(childModel, profileActivationContext); } if (parentModel == null) { - parentModel = resolveAndReadParentExternally(childModel); + parentModel = resolveAndReadParentExternally(childModel, profileActivationContext); } return parentModel; } - private Model readParentLocally(Model childModel) throws ModelBuilderException { + private Model readParentLocally(Model childModel, DefaultProfileActivationContext profileActivationContext) + throws ModelBuilderException { ModelSource candidateSource; Parent parent = childModel.getParent(); @@ -938,7 +944,9 @@ public class DefaultModelBuilder implements ModelBuilder { return null; } - Model candidateModel = derive(candidateSource).readAsParentModel(); + ModelBuilderSessionState derived = derive(candidateSource); + Model candidateModel = derived.readAsParentModel(profileActivationContext); + addActivePomProfiles(derived.result.getActivePomProfiles()); String groupId = getGroupId(candidateModel); String artifactId = candidateModel.getArtifactId(); @@ -1020,7 +1028,8 @@ public class DefaultModelBuilder implements ModelBuilder { add(Severity.FATAL, Version.BASE, buffer.toString(), parent.getLocation("")); } - Model resolveAndReadParentExternally(Model childModel) throws ModelBuilderException { + Model resolveAndReadParentExternally(Model childModel, DefaultProfileActivationContext profileActivationContext) + throws ModelBuilderException { ModelBuilderRequest request = this.request; setSource(childModel); @@ -1078,7 +1087,7 @@ public class DefaultModelBuilder implements ModelBuilder { .source(modelSource) .build(); - Model parentModel = derive(lenientRequest).readAsParentModel(); + Model parentModel = derive(lenientRequest).readAsParentModel(profileActivationContext); if (!parent.getVersion().equals(version)) { String rawChildModelVersion = childModel.getVersion(); @@ -1119,7 +1128,7 @@ public class DefaultModelBuilder implements ModelBuilder { for (Profile profile : activeExternalProfiles) { profileProps.putAll(profile.getProperties()); } - profileProps.putAll(profileActivationContext.getUserProperties()); + profileProps.putAll(request.getUserProperties()); profileActivationContext.setUserProperties(profileProps); } @@ -1163,11 +1172,12 @@ public class DefaultModelBuilder implements ModelBuilder { for (Profile profile : activeExternalProfiles) { profileProps.putAll(profile.getProperties()); } - profileProps.putAll(profileActivationContext.getUserProperties()); + profileProps.putAll(request.getUserProperties()); profileActivationContext.setUserProperties(profileProps); } - Model parentModel = readParent(activatedFileModel); + Model parentModel = readParent(activatedFileModel, profileActivationContext); + // Now that we have read the parent, we can set the relative // path correctly if it was not set in the input model if (inputModel.getParent() != null && inputModel.getParent().getRelativePath() == null) { @@ -1186,16 +1196,7 @@ public class DefaultModelBuilder implements ModelBuilder { inputModel = inputModel.withParent(inputModel.getParent().withRelativePath(relPath)); } - List parentInterpolatedProfiles = - interpolateActivations(parentModel.getProfiles(), profileActivationContext, this); - // profile injection - List parentActivePomProfiles = - getActiveProfiles(parentInterpolatedProfiles, profileActivationContext); - Model injectedParentModel = profileInjector - .injectProfiles(parentModel, parentActivePomProfiles, request, this) - .withProfiles(List.of()); - - Model model = inheritanceAssembler.assembleModelInheritance(inputModel, injectedParentModel, request, this); + Model model = inheritanceAssembler.assembleModelInheritance(inputModel, parentModel, request, this); // model normalization model = modelNormalizer.mergeDuplicates(model, request, this); @@ -1211,10 +1212,7 @@ public class DefaultModelBuilder implements ModelBuilder { model = profileInjector.injectProfiles(model, activePomProfiles, request, this); model = profileInjector.injectProfiles(model, activeExternalProfiles, request, this); - List allProfiles = new ArrayList<>(parentActivePomProfiles.size() + activePomProfiles.size()); - allProfiles.addAll(parentActivePomProfiles); - allProfiles.addAll(activePomProfiles); - result.setActivePomProfiles(allProfiles); + addActivePomProfiles(activePomProfiles); // model interpolation Model resultModel = model; @@ -1239,6 +1237,15 @@ public class DefaultModelBuilder implements ModelBuilder { return resultModel; } + private void addActivePomProfiles(List activePomProfiles) { + if (activePomProfiles != null) { + if (result.getActivePomProfiles() == null) { + result.setActivePomProfiles(new ArrayList<>()); + } + result.getActivePomProfiles().addAll(activePomProfiles); + } + } + private List getActiveProfiles( Collection interpolatedProfiles, DefaultProfileActivationContext profileActivationContext) { if (isBuildRequestWithActivation()) { @@ -1491,13 +1498,25 @@ public class DefaultModelBuilder implements ModelBuilder { /** * Reads the request source's parent. */ - Model readAsParentModel() throws ModelBuilderException { - return cache(request.getSource(), PARENT, this::doReadAsParentModel); + Model readAsParentModel(DefaultProfileActivationContext profileActivationContext) throws ModelBuilderException { + Map parentsPerContext = + cache(request.getSource(), PARENT, ConcurrentHashMap::new); + for (Map.Entry e : parentsPerContext.entrySet()) { + if (e.getKey().matches(profileActivationContext)) { + return e.getValue(); + } + } + DefaultProfileActivationContext.Record prev = profileActivationContext.start(); + Model model = doReadAsParentModel(profileActivationContext); + DefaultProfileActivationContext.Record record = profileActivationContext.stop(prev); + parentsPerContext.put(record, model); + return model; } - private Model doReadAsParentModel() throws ModelBuilderException { + private Model doReadAsParentModel(DefaultProfileActivationContext profileActivationContext) + throws ModelBuilderException { Model raw = readRawModel(); - Model parentData = readParent(raw); + Model parentData = readParent(raw, profileActivationContext); Model parent = new DefaultInheritanceAssembler(new DefaultInheritanceAssembler.InheritanceModelMerger() { @Override protected void mergeModel_Modules( @@ -1514,23 +1533,21 @@ public class DefaultModelBuilder implements ModelBuilder { Model source, boolean sourceDominant, Map context) {} - - @SuppressWarnings("deprecation") - @Override - protected void mergeModel_Profiles( - Model.Builder builder, - Model target, - Model source, - boolean sourceDominant, - Map context) { - builder.profiles(Stream.concat(source.getProfiles().stream(), target.getProfiles().stream()) - .map(p -> p.withModules(null).withSubprojects(null)) - .toList()); - } }) .assembleModelInheritance(raw, parentData, request, this); - return parent.withParent(null); + // activate profiles + List parentInterpolatedProfiles = + interpolateActivations(parent.getProfiles(), profileActivationContext, this); + // profile injection + List parentActivePomProfiles = + getActiveProfiles(parentInterpolatedProfiles, profileActivationContext); + Model injectedParentModel = profileInjector + .injectProfiles(parent, parentActivePomProfiles, request, this) + .withProfiles(List.of()); + addActivePomProfiles(parentActivePomProfiles); + + return injectedParentModel.withParent(null); } private Model importDependencyManagement(Model model, Collection importIds) { @@ -1763,9 +1780,8 @@ public class DefaultModelBuilder implements ModelBuilder { ProfileInterpolator() { super(s -> { try { - Map map1 = context.getUserProperties(); - Map map2 = context.getSystemProperties(); - return interpolator.interpolate(s, Interpolator.chain(map1::get, map2::get)); + return interpolator.interpolate( + s, Interpolator.chain(context::getUserProperty, context::getSystemProperty)); } catch (InterpolatorException e) { problems.add(Severity.ERROR, Version.BASE, e.getMessage(), e); } @@ -1809,7 +1825,7 @@ public class DefaultModelBuilder implements ModelBuilder { private String transformPath(String path, ActivationFile target, String locationKey) { try { - return profileActivationFilePathInterpolator.interpolate(path, context); + return context.interpolatePath(path); } catch (InterpolatorException e) { problems.add( Severity.ERROR, @@ -1860,7 +1876,8 @@ public class DefaultModelBuilder implements ModelBuilder { } private DefaultProfileActivationContext getProfileActivationContext(ModelBuilderRequest request, Model model) { - DefaultProfileActivationContext context = new DefaultProfileActivationContext(); + DefaultProfileActivationContext context = + new DefaultProfileActivationContext(pathTranslator, rootLocator, interpolator); context.setActiveProfileIds(request.getActiveProfileIds()); context.setInactiveProfileIds(request.getInactiveProfileIds()); diff --git a/impl/maven-impl/src/main/java/org/apache/maven/internal/impl/model/DefaultProfileActivationContext.java b/impl/maven-impl/src/main/java/org/apache/maven/internal/impl/model/DefaultProfileActivationContext.java index ef04023b21..6735661484 100644 --- a/impl/maven-impl/src/main/java/org/apache/maven/internal/impl/model/DefaultProfileActivationContext.java +++ b/impl/maven-impl/src/main/java/org/apache/maven/internal/impl/model/DefaultProfileActivationContext.java @@ -18,32 +18,168 @@ */ package org.apache.maven.internal.impl.model; +import java.io.File; +import java.io.IOException; +import java.nio.file.FileVisitResult; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.PathMatcher; +import java.nio.file.Paths; +import java.nio.file.SimpleFileVisitor; +import java.nio.file.attribute.BasicFileAttributes; import java.util.Collections; +import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Objects; +import java.util.concurrent.atomic.AtomicBoolean; import org.apache.maven.api.model.Model; +import org.apache.maven.api.services.Interpolator; +import org.apache.maven.api.services.InterpolatorException; +import org.apache.maven.api.services.ModelBuilderException; +import org.apache.maven.api.services.ProjectBuilderException; +import org.apache.maven.api.services.model.PathTranslator; import org.apache.maven.api.services.model.ProfileActivationContext; +import org.apache.maven.api.services.model.RootLocator; /** * Describes the environmental context used to determine the activation status of profiles. - * */ public class DefaultProfileActivationContext implements ProfileActivationContext { + record ExistRequest(String path, boolean enableGlob) {} + + enum ModelInfo { + ArtifactId, + Packaging, + BaseDirectory, + RootDirectory + } + + /** + * This class keeps track of information that are used during profile activation. + * This allows to cache the activated parent and check if the result of the + * activation will be the same by verifying that the used keys are the same. + */ + static class Record { + private final Map usedActiveProfiles = new HashMap<>(); + private final Map usedInactiveProfiles = new HashMap<>(); + private final Map usedSystemProperties = new HashMap<>(); + private final Map usedUserProperties = new HashMap<>(); + private final Map usedModelProperties = new HashMap<>(); + private final Map usedModelInfos = new HashMap<>(); + private final Map usedExists = new HashMap<>(); + + @Override + public boolean equals(Object o) { + if (o instanceof Record record) { + return Objects.equals(usedActiveProfiles, record.usedActiveProfiles) + && Objects.equals(usedInactiveProfiles, record.usedInactiveProfiles) + && Objects.equals(usedSystemProperties, record.usedSystemProperties) + && Objects.equals(usedUserProperties, record.usedUserProperties) + && Objects.equals(usedModelProperties, record.usedModelProperties) + && Objects.equals(usedModelInfos, record.usedModelInfos) + && Objects.equals(usedExists, record.usedExists); + } + return false; + } + + @Override + public int hashCode() { + return Objects.hash( + usedActiveProfiles, + usedInactiveProfiles, + usedSystemProperties, + usedUserProperties, + usedModelProperties, + usedModelInfos, + usedExists); + } + + boolean matches(DefaultProfileActivationContext context) { + return matchesProfiles(usedActiveProfiles, context.activeProfileIds) + && matchesProfiles(usedInactiveProfiles, context.inactiveProfileIds) + && matchesProperties(usedSystemProperties, context.systemProperties) + && matchesProperties(usedUserProperties, context.userProperties) + && matchesProperties(usedModelProperties, context.model.getProperties()) + && matchesModelInfos(usedModelInfos, context) + && matchesExists(usedExists, context); + } + + private boolean matchesProfiles(Map expected, List actual) { + return expected.entrySet().stream() + .allMatch(e -> Objects.equals(e.getValue(), actual.contains(e.getKey()))); + } + + private boolean matchesProperties(Map expected, Map actual) { + return expected.entrySet().stream().allMatch(e -> Objects.equals(e.getValue(), actual.get(e.getKey()))); + } + + private boolean matchesModelInfos(Map infos, DefaultProfileActivationContext context) { + return infos.entrySet().stream() + .allMatch(e -> Objects.equals(e.getValue(), getModelValue(e.getKey(), context))); + } + + private String getModelValue(ModelInfo key, DefaultProfileActivationContext context) { + return switch (key) { + case ArtifactId -> context.model.getArtifactId(); + case Packaging -> context.model.getPackaging(); + case BaseDirectory -> context.doGetModelBaseDirectory(); + case RootDirectory -> context.doGetModelRootDirectory(); + }; + } + + private boolean matchesExists(Map exists, DefaultProfileActivationContext context) { + return exists.entrySet().stream() + .allMatch(e -> Objects.equals( + e.getValue(), + context.doExists(e.getKey().path(), e.getKey().enableGlob()))); + } + } + + private final PathTranslator pathTranslator; + private final RootLocator rootLocator; + private final Interpolator interpolator; + private List activeProfileIds = Collections.emptyList(); - private List inactiveProfileIds = Collections.emptyList(); - private Map systemProperties = Collections.emptyMap(); - private Map userProperties = Collections.emptyMap(); - private Model model; + private final ThreadLocal records = new ThreadLocal<>(); + + public DefaultProfileActivationContext( + PathTranslator pathTranslator, RootLocator rootLocator, Interpolator interpolator) { + this.pathTranslator = pathTranslator; + this.rootLocator = rootLocator; + this.interpolator = interpolator; + } + + Record start() { + Record record = records.get(); + records.set(new Record()); + return record; + } + + Record stop(Record previous) { + Record record = records.get(); + records.set(previous); + // only keep keys for which the value is `true` + record.usedActiveProfiles.values().removeIf(value -> !value); + record.usedInactiveProfiles.values().removeIf(value -> !value); + return record; + } + @Override - public List getActiveProfileIds() { - return activeProfileIds; + public boolean isProfileActive(String profileId) { + Record record = records.get(); + if (record != null) { + return record.usedActiveProfiles.computeIfAbsent(profileId, activeProfileIds::contains); + } else { + return activeProfileIds.contains(profileId); + } } /** @@ -58,8 +194,13 @@ public class DefaultProfileActivationContext implements ProfileActivationContext } @Override - public List getInactiveProfileIds() { - return inactiveProfileIds; + public boolean isProfileInactive(String profileId) { + Record record = records.get(); + if (record != null) { + return record.usedInactiveProfiles.computeIfAbsent(profileId, inactiveProfileIds::contains); + } else { + return inactiveProfileIds.contains(profileId); + } } /** @@ -74,8 +215,13 @@ public class DefaultProfileActivationContext implements ProfileActivationContext } @Override - public Map getSystemProperties() { - return systemProperties; + public String getSystemProperty(String key) { + Record record = records.get(); + if (record != null) { + return record.usedSystemProperties.computeIfAbsent(key, systemProperties::get); + } else { + return systemProperties.get(key); + } } /** @@ -91,8 +237,13 @@ public class DefaultProfileActivationContext implements ProfileActivationContext } @Override - public Map getUserProperties() { - return userProperties; + public String getUserProperty(String key) { + Record record = records.get(); + if (record != null) { + return record.usedUserProperties.computeIfAbsent(key, userProperties::get); + } else { + return userProperties.get(key); + } } /** @@ -109,16 +260,164 @@ public class DefaultProfileActivationContext implements ProfileActivationContext } @Override - public Model getModel() { - return model; + public String getModelArtifactId() { + Record record = records.get(); + if (record != null) { + return record.usedModelInfos.computeIfAbsent(ModelInfo.ArtifactId, k -> model.getArtifactId()); + } else { + return model.getArtifactId(); + } + } + + @Override + public String getModelPackaging() { + Record record = records.get(); + if (record != null) { + return record.usedModelInfos.computeIfAbsent(ModelInfo.Packaging, k -> model.getPackaging()); + } else { + return model.getPackaging(); + } + } + + @Override + public String getModelProperty(String key) { + Record record = records.get(); + if (record != null) { + return record.usedModelProperties.computeIfAbsent( + key, k -> model.getProperties().get(k)); + } else { + return model.getProperties().get(key); + } + } + + @Override + public String getModelBaseDirectory() { + Record record = records.get(); + if (record != null) { + return record.usedModelInfos.computeIfAbsent(ModelInfo.BaseDirectory, k -> doGetModelBaseDirectory()); + } else { + return doGetModelBaseDirectory(); + } + } + + private String doGetModelBaseDirectory() { + Path basedir = model.getProjectDirectory(); + return basedir != null ? basedir.toAbsolutePath().toString() : null; + } + + @Override + public String getModelRootDirectory() { + Record record = records.get(); + if (record != null) { + return record.usedModelInfos.computeIfAbsent(ModelInfo.RootDirectory, k -> doGetModelRootDirectory()); + } else { + return doGetModelRootDirectory(); + } + } + + private String doGetModelRootDirectory() { + Path basedir = model != null ? model.getProjectDirectory() : null; + Path rootdir = rootLocator != null ? rootLocator.findRoot(basedir) : null; + return rootdir != null ? rootdir.toAbsolutePath().toString() : null; } public DefaultProfileActivationContext setModel(Model model) { this.model = model; - return this; } + @Override + public String interpolatePath(String path) throws InterpolatorException { + if (path == null) { + return null; + } + String absolutePath = interpolator.interpolate(path, s -> { + if ("basedir".equals(s) || "project.basedir".equals(s)) { + return getModelBaseDirectory(); + } + if ("project.rootDirectory".equals(s)) { + return getModelRootDirectory(); + } + String r = getModelProperty(s); + if (r == null) { + r = getUserProperty(s); + } + if (r == null) { + r = getSystemProperty(s); + } + return r; + }); + return pathTranslator.alignToBaseDirectory(absolutePath, model.getProjectDirectory()); + } + + @Override + public boolean exists(String path, boolean enableGlob) throws ModelBuilderException { + Record record = records.get(); + if (record != null) { + return record.usedExists.computeIfAbsent( + new ExistRequest(path, enableGlob), r -> doExists(r.path, r.enableGlob)); + } else { + return doExists(path, enableGlob); + } + } + + private boolean doExists(String path, boolean enableGlob) throws ModelBuilderException { + String pattern = interpolatePath(path); + String fixed, glob; + if (enableGlob) { + int asteriskIndex = pattern.indexOf('*'); + int questionMarkIndex = pattern.indexOf('?'); + int firstWildcardIndex = questionMarkIndex < 0 + ? asteriskIndex + : asteriskIndex < 0 ? questionMarkIndex : Math.min(asteriskIndex, questionMarkIndex); + if (firstWildcardIndex < 0) { + fixed = pattern; + glob = ""; + } else { + int lastSep = pattern.substring(0, firstWildcardIndex).lastIndexOf(File.separatorChar); + if (lastSep < 0) { + fixed = ""; + glob = pattern; + } else { + fixed = pattern.substring(0, lastSep); + glob = pattern.substring(lastSep + 1); + } + } + } else { + fixed = pattern; + glob = ""; + } + Path fixedPath = Paths.get(fixed); + return doExists(fixedPath, glob); + } + + private static Boolean doExists(Path fixedPath, String glob) { + if (fixedPath == null || !Files.exists(fixedPath)) { + return false; + } + if (glob != null && !glob.isEmpty()) { + try { + PathMatcher matcher = fixedPath.getFileSystem().getPathMatcher("glob:" + glob); + AtomicBoolean found = new AtomicBoolean(false); + Files.walkFileTree(fixedPath, new SimpleFileVisitor<>() { + @Override + public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) { + if (found.get() || matcher.matches(fixedPath.relativize(file))) { + found.set(true); + return FileVisitResult.TERMINATE; + } + return FileVisitResult.CONTINUE; + } + }); + return found.get(); + } catch (IOException e) { + throw new ProjectBuilderException( + "Unable to verify file existence for '" + glob + "' inside '" + fixedPath + "'", e); + } + } + return true; + } + private static List unmodifiable(List list) { return list != null ? Collections.unmodifiableList(list) : Collections.emptyList(); } diff --git a/impl/maven-impl/src/main/java/org/apache/maven/internal/impl/model/DefaultProfileSelector.java b/impl/maven-impl/src/main/java/org/apache/maven/internal/impl/model/DefaultProfileSelector.java index 36dc4a9fcd..3415de844e 100644 --- a/impl/maven-impl/src/main/java/org/apache/maven/internal/impl/model/DefaultProfileSelector.java +++ b/impl/maven-impl/src/main/java/org/apache/maven/internal/impl/model/DefaultProfileSelector.java @@ -20,7 +20,6 @@ package org.apache.maven.internal.impl.model; import java.util.ArrayList; import java.util.Collection; -import java.util.HashSet; import java.util.List; import org.apache.maven.api.di.Inject; @@ -64,16 +63,13 @@ public class DefaultProfileSelector implements ProfileSelector { @Override public List getActiveProfiles( Collection profiles, ProfileActivationContext context, ModelProblemCollector problems) { - Collection activatedIds = new HashSet<>(context.getActiveProfileIds()); - Collection deactivatedIds = new HashSet<>(context.getInactiveProfileIds()); - List activeProfiles = new ArrayList<>(profiles.size()); List activePomProfilesByDefault = new ArrayList<>(); boolean activatedPomProfileNotByDefault = false; for (Profile profile : profiles) { - if (!deactivatedIds.contains(profile.getId())) { - if (activatedIds.contains(profile.getId()) || isActive(profile, context, problems)) { + if (!context.isProfileInactive(profile.getId())) { + if (context.isProfileActive(profile.getId()) || isActive(profile, context, problems)) { activeProfiles.add(profile); if (Profile.SOURCE_POM.equals(profile.getSource())) { activatedPomProfileNotByDefault = true; diff --git a/impl/maven-impl/src/main/java/org/apache/maven/internal/impl/model/ProfileActivationFilePathInterpolator.java b/impl/maven-impl/src/main/java/org/apache/maven/internal/impl/model/ProfileActivationFilePathInterpolator.java deleted file mode 100644 index d512b727ec..0000000000 --- a/impl/maven-impl/src/main/java/org/apache/maven/internal/impl/model/ProfileActivationFilePathInterpolator.java +++ /dev/null @@ -1,86 +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.nio.file.Path; - -import org.apache.maven.api.di.Inject; -import org.apache.maven.api.di.Named; -import org.apache.maven.api.di.Singleton; -import org.apache.maven.api.model.ActivationFile; -import org.apache.maven.api.services.Interpolator; -import org.apache.maven.api.services.InterpolatorException; -import org.apache.maven.api.services.model.PathTranslator; -import org.apache.maven.api.services.model.ProfileActivationContext; -import org.apache.maven.api.services.model.RootLocator; - -/** - * Finds an absolute path for {@link ActivationFile#getExists()} or {@link ActivationFile#getMissing()} - */ -@Named -@Singleton -public class ProfileActivationFilePathInterpolator { - - private final PathTranslator pathTranslator; - - private final RootLocator rootLocator; - - private final Interpolator interpolator; - - @Inject - public ProfileActivationFilePathInterpolator( - PathTranslator pathTranslator, RootLocator rootLocator, Interpolator interpolator) { - this.pathTranslator = pathTranslator; - this.rootLocator = rootLocator; - this.interpolator = interpolator; - } - - /** - * Interpolates given {@code path}. - * - * @return absolute path or {@code null} if the input was {@code null} - */ - public String interpolate(String path, ProfileActivationContext context) throws InterpolatorException { - if (path == null) { - return null; - } - - Path basedir = context.getModel().getProjectDirectory(); - - String absolutePath = interpolator.interpolate(path, s -> { - if ("basedir".equals(s) || "project.basedir".equals(s)) { - return basedir != null ? basedir.toFile().getAbsolutePath() : null; - } - if ("project.rootDirectory".equals(s)) { - Path root = rootLocator.findMandatoryRoot(basedir); - return root.toFile().getAbsolutePath(); - } - String r = context.getModel().getProperties().get(s); - if (r == null) { - r = context.getUserProperties().get(s); - } - if (r == null) { - r = context.getSystemProperties().get(s); - } - return r; - }); - - return pathTranslator.alignToBaseDirectory(absolutePath, basedir); - } -} diff --git a/impl/maven-impl/src/main/java/org/apache/maven/internal/impl/model/profile/ConditionFunctions.java b/impl/maven-impl/src/main/java/org/apache/maven/internal/impl/model/profile/ConditionFunctions.java index 82b4c4744c..c154f4b5ed 100644 --- a/impl/maven-impl/src/main/java/org/apache/maven/internal/impl/model/profile/ConditionFunctions.java +++ b/impl/maven-impl/src/main/java/org/apache/maven/internal/impl/model/profile/ConditionFunctions.java @@ -18,21 +18,12 @@ */ package org.apache.maven.internal.impl.model.profile; -import java.io.File; -import java.io.IOException; -import java.nio.file.FileVisitResult; -import java.nio.file.Files; -import java.nio.file.Path; -import java.nio.file.PathMatcher; -import java.nio.file.Paths; -import java.nio.file.SimpleFileVisitor; -import java.nio.file.attribute.BasicFileAttributes; import java.util.List; -import java.util.concurrent.atomic.AtomicBoolean; +import org.apache.maven.api.services.InterpolatorException; +import org.apache.maven.api.services.ModelBuilderException; import org.apache.maven.api.services.VersionParser; import org.apache.maven.api.services.model.ProfileActivationContext; -import org.apache.maven.internal.impl.model.ProfileActivationFilePathInterpolator; import static org.apache.maven.internal.impl.model.profile.ConditionParser.toInt; @@ -45,22 +36,16 @@ import static org.apache.maven.internal.impl.model.profile.ConditionParser.toInt public class ConditionFunctions { private final ProfileActivationContext context; private final VersionParser versionParser; - private final ProfileActivationFilePathInterpolator interpolator; /** * Constructs a new ConditionFunctions instance. * * @param context The profile activation context * @param versionParser The version parser for comparing versions - * @param interpolator The interpolator for resolving file paths */ - public ConditionFunctions( - ProfileActivationContext context, - VersionParser versionParser, - ProfileActivationFilePathInterpolator interpolator) { + public ConditionFunctions(ProfileActivationContext context, VersionParser versionParser) { this.context = context; this.versionParser = versionParser; - this.interpolator = interpolator; } /** @@ -207,75 +192,34 @@ public class ConditionFunctions { * Checks if a file or directory exists at the given path. * * @param args A list containing a single string argument representing the path - * @return true if the file or directory exists, false otherwise + * @return {@code true} if the file or directory exists, {@code false} otherwise * @throws IllegalArgumentException if the number of arguments is not exactly one - * @throws IOException if a problem occurs while walking the file system + * @throws ModelBuilderException if a problem occurs while walking the file system + * @throws InterpolatorException if an error occurs during interpolation */ - public Object exists(List args) throws IOException { + public Object exists(List args) { if (args.size() != 1) { throw new IllegalArgumentException("exists function requires exactly one argument"); } String path = ConditionParser.toString(args.get(0)); - return fileExists(path); + return context.exists(path, true); } /** * Checks if a file or directory is missing at the given path. * * @param args A list containing a single string argument representing the path - * @return true if the file or directory does not exist, false otherwise + * @return {@code true} if the file or directory does not exist, {@code false} otherwise * @throws IllegalArgumentException if the number of arguments is not exactly one - * @throws IOException if a problem occurs while walking the file system + * @throws ModelBuilderException if a problem occurs while walking the file system + * @throws InterpolatorException if an error occurs during interpolation */ - public Object missing(List args) throws IOException { + public Object missing(List args) { if (args.size() != 1) { throw new IllegalArgumentException("missing function requires exactly one argument"); } String path = ConditionParser.toString(args.get(0)); - return !fileExists(path); - } - - private boolean fileExists(String path) throws IOException { - String pattern = interpolator.interpolate(path, context); - int asteriskIndex = pattern.indexOf('*'); - int questionMarkIndex = pattern.indexOf('?'); - int firstWildcardIndex = questionMarkIndex < 0 - ? asteriskIndex - : asteriskIndex < 0 ? questionMarkIndex : Math.min(asteriskIndex, questionMarkIndex); - String fixed, glob; - if (firstWildcardIndex < 0) { - fixed = pattern; - glob = ""; - } else { - int lastSep = pattern.substring(0, firstWildcardIndex).lastIndexOf(File.separatorChar); - if (lastSep < 0) { - fixed = ""; - glob = pattern; - } else { - fixed = pattern.substring(0, lastSep); - glob = pattern.substring(lastSep + 1); - } - } - Path fixedPath = Paths.get(fixed); - if (!Files.exists(fixedPath)) { - return false; - } - if (!glob.isEmpty()) { - PathMatcher matcher = fixedPath.getFileSystem().getPathMatcher("glob:" + glob); - AtomicBoolean found = new AtomicBoolean(false); - Files.walkFileTree(fixedPath, new SimpleFileVisitor<>() { - @Override - public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) { - if (found.get() || matcher.matches(fixedPath.relativize(file))) { - found.set(true); - return FileVisitResult.TERMINATE; - } - return FileVisitResult.CONTINUE; - } - }); - return found.get(); - } - return true; + return !context.exists(path, true); } /** diff --git a/impl/maven-impl/src/main/java/org/apache/maven/internal/impl/model/profile/ConditionProfileActivator.java b/impl/maven-impl/src/main/java/org/apache/maven/internal/impl/model/profile/ConditionProfileActivator.java index 3c0d4b040f..7070905a2e 100644 --- a/impl/maven-impl/src/main/java/org/apache/maven/internal/impl/model/profile/ConditionProfileActivator.java +++ b/impl/maven-impl/src/main/java/org/apache/maven/internal/impl/model/profile/ConditionProfileActivator.java @@ -18,7 +18,6 @@ */ package org.apache.maven.internal.impl.model.profile; -import java.nio.file.Path; import java.util.HashMap; import java.util.Map; import java.util.function.Function; @@ -34,9 +33,7 @@ import org.apache.maven.api.services.ModelProblemCollector; import org.apache.maven.api.services.VersionParser; import org.apache.maven.api.services.model.ProfileActivationContext; import org.apache.maven.api.services.model.ProfileActivator; -import org.apache.maven.api.services.model.RootLocator; import org.apache.maven.internal.impl.model.DefaultInterpolator; -import org.apache.maven.internal.impl.model.ProfileActivationFilePathInterpolator; import static org.apache.maven.internal.impl.model.profile.ConditionParser.toBoolean; @@ -49,22 +46,15 @@ import static org.apache.maven.internal.impl.model.profile.ConditionParser.toBoo public class ConditionProfileActivator implements ProfileActivator { private final VersionParser versionParser; - private final ProfileActivationFilePathInterpolator interpolator; - private final RootLocator rootLocator; /** * Constructs a new ConditionProfileActivator with the necessary dependencies. * * @param versionParser The parser for handling version comparisons - * @param interpolator The interpolator for resolving file paths - * @param rootLocator The locator for finding the project root directory */ @Inject - public ConditionProfileActivator( - VersionParser versionParser, ProfileActivationFilePathInterpolator interpolator, RootLocator rootLocator) { + public ConditionProfileActivator(VersionParser versionParser) { this.versionParser = versionParser; - this.interpolator = interpolator; - this.rootLocator = rootLocator; } /** @@ -82,9 +72,8 @@ public class ConditionProfileActivator implements ProfileActivator { } String condition = profile.getActivation().getCondition(); try { - Map functions = - registerFunctions(context, versionParser, interpolator); - Function propertyResolver = s -> property(context, rootLocator, s); + Map functions = registerFunctions(context, versionParser); + Function propertyResolver = s -> property(context, s); return toBoolean(new ConditionParser(functions, propertyResolver).parse(condition)); } catch (Exception e) { problems.add( @@ -115,16 +104,13 @@ public class ConditionProfileActivator implements ProfileActivator { * * @param context The profile activation context * @param versionParser The parser for handling version comparisons - * @param interpolator The interpolator for resolving file paths * @return A map of function names to their implementations */ public static Map registerFunctions( - ProfileActivationContext context, - VersionParser versionParser, - ProfileActivationFilePathInterpolator interpolator) { + ProfileActivationContext context, VersionParser versionParser) { Map functions = new HashMap<>(); - ConditionFunctions conditionFunctions = new ConditionFunctions(context, versionParser, interpolator); + ConditionFunctions conditionFunctions = new ConditionFunctions(context, versionParser); for (java.lang.reflect.Method method : ConditionFunctions.class.getDeclaredMethods()) { String methodName = method.getName(); @@ -170,43 +156,37 @@ public class ConditionProfileActivator implements ProfileActivator { * @return The value of the property, or null if not found * @throws IllegalArgumentException if the number of arguments is not exactly one */ - static String property(ProfileActivationContext context, RootLocator rootLocator, String name) { - String value = doGetProperty(context, rootLocator, name); - return new DefaultInterpolator().interpolate(value, s -> doGetProperty(context, rootLocator, s)); + static String property(ProfileActivationContext context, String name) { + String value = doGetProperty(context, name); + return new DefaultInterpolator().interpolate(value, s -> doGetProperty(context, s)); } - static String doGetProperty(ProfileActivationContext context, RootLocator rootLocator, String name) { + static String doGetProperty(ProfileActivationContext context, String name) { // Handle special project-related properties if ("project.basedir".equals(name)) { - Path basedir = context.getModel().getProjectDirectory(); - return basedir != null ? basedir.toFile().getAbsolutePath() : null; + return context.getModelBaseDirectory(); } if ("project.rootDirectory".equals(name)) { - Path basedir = context.getModel().getProjectDirectory(); - if (basedir != null) { - Path root = rootLocator.findMandatoryRoot(basedir); - return root.toFile().getAbsolutePath(); - } - return null; + return context.getModelRootDirectory(); } if ("project.artifactId".equals(name)) { - return context.getModel().getArtifactId(); + return context.getModelArtifactId(); } if ("project.packaging".equals(name)) { - return context.getModel().getPackaging(); + return context.getModelPackaging(); } // Check user properties - String v = context.getUserProperties().get(name); + String v = context.getUserProperty(name); if (v == null) { // Check project properties // TODO: this may leads to instability between file model activation and effective model activation // as the effective model properties may be different from the file model - v = context.getModel().getProperties().get(name); + v = context.getModelProperty(name); } if (v == null) { // Check system properties - v = context.getSystemProperties().get(name); + v = context.getSystemProperty(name); } return v; } diff --git a/impl/maven-impl/src/main/java/org/apache/maven/internal/impl/model/profile/FileProfileActivator.java b/impl/maven-impl/src/main/java/org/apache/maven/internal/impl/model/profile/FileProfileActivator.java index b0824a168a..77abd86307 100644 --- a/impl/maven-impl/src/main/java/org/apache/maven/internal/impl/model/profile/FileProfileActivator.java +++ b/impl/maven-impl/src/main/java/org/apache/maven/internal/impl/model/profile/FileProfileActivator.java @@ -18,21 +18,17 @@ */ package org.apache.maven.internal.impl.model.profile; -import java.io.File; - -import org.apache.maven.api.di.Inject; import org.apache.maven.api.di.Named; import org.apache.maven.api.di.Singleton; import org.apache.maven.api.model.Activation; import org.apache.maven.api.model.ActivationFile; import org.apache.maven.api.model.Profile; import org.apache.maven.api.services.BuilderProblem; -import org.apache.maven.api.services.InterpolatorException; +import org.apache.maven.api.services.MavenException; import org.apache.maven.api.services.ModelProblem; import org.apache.maven.api.services.ModelProblemCollector; import org.apache.maven.api.services.model.ProfileActivationContext; import org.apache.maven.api.services.model.ProfileActivator; -import org.apache.maven.internal.impl.model.ProfileActivationFilePathInterpolator; /** * Determines profile activation based on the existence/absence of some file. @@ -46,13 +42,6 @@ import org.apache.maven.internal.impl.model.ProfileActivationFilePathInterpolato @Singleton public class FileProfileActivator implements ProfileActivator { - private final ProfileActivationFilePathInterpolator profileActivationFilePathInterpolator; - - @Inject - public FileProfileActivator(ProfileActivationFilePathInterpolator profileActivationFilePathInterpolator) { - this.profileActivationFilePathInterpolator = profileActivationFilePathInterpolator; - } - @Override public boolean isActive(Profile profile, ProfileActivationContext context, ModelProblemCollector problems) { Activation activation = profile.getActivation(); @@ -92,31 +81,20 @@ public class FileProfileActivator implements ProfileActivator { return false; } + boolean fileExists; try { - path = profileActivationFilePathInterpolator.interpolate(path, context); - } catch (InterpolatorException e) { + fileExists = context.exists(path, false); + } catch (MavenException e) { problems.add( BuilderProblem.Severity.ERROR, ModelProblem.Version.BASE, - "Failed to interpolate file location " + path + " for profile " + profile.getId() + ": " + "Failed to check file existence " + path + " for profile " + profile.getId() + ": " + e.getMessage(), file.getLocation(missing ? "missing" : "exists"), e); return false; } - if (path == null) { - return false; - } - - File f = new File(path); - - if (!f.isAbsolute()) { - return false; - } - - boolean fileExists = f.exists(); - return missing != fileExists; } diff --git a/impl/maven-impl/src/main/java/org/apache/maven/internal/impl/model/profile/JdkVersionProfileActivator.java b/impl/maven-impl/src/main/java/org/apache/maven/internal/impl/model/profile/JdkVersionProfileActivator.java index 2a417a3bc6..b3f95fa750 100644 --- a/impl/maven-impl/src/main/java/org/apache/maven/internal/impl/model/profile/JdkVersionProfileActivator.java +++ b/impl/maven-impl/src/main/java/org/apache/maven/internal/impl/model/profile/JdkVersionProfileActivator.java @@ -60,7 +60,7 @@ public class JdkVersionProfileActivator implements ProfileActivator { return false; } - String version = context.getSystemProperties().get("java.version"); + String version = context.getSystemProperty("java.version"); if (version == null || version.isEmpty()) { problems.add( diff --git a/impl/maven-impl/src/main/java/org/apache/maven/internal/impl/model/profile/OperatingSystemProfileActivator.java b/impl/maven-impl/src/main/java/org/apache/maven/internal/impl/model/profile/OperatingSystemProfileActivator.java index 1d03e26ae2..3d6af38356 100644 --- a/impl/maven-impl/src/main/java/org/apache/maven/internal/impl/model/profile/OperatingSystemProfileActivator.java +++ b/impl/maven-impl/src/main/java/org/apache/maven/internal/impl/model/profile/OperatingSystemProfileActivator.java @@ -56,15 +56,10 @@ public class OperatingSystemProfileActivator implements ProfileActivator { boolean active = ensureAtLeastOneNonNull(os); - String actualOsName = context.getSystemProperties() - .getOrDefault("os.name", Os.OS_NAME) - .toLowerCase(Locale.ENGLISH); - String actualOsArch = context.getSystemProperties() - .getOrDefault("os.arch", Os.OS_ARCH) - .toLowerCase(Locale.ENGLISH); - String actualOsVersion = context.getSystemProperties() - .getOrDefault("os.version", Os.OS_VERSION) - .toLowerCase(Locale.ENGLISH); + String actualOsName = getSystemProperty(context, "os.name", Os.OS_NAME).toLowerCase(Locale.ENGLISH); + String actualOsArch = getSystemProperty(context, "os.arch", Os.OS_ARCH).toLowerCase(Locale.ENGLISH); + String actualOsVersion = + getSystemProperty(context, "os.version", Os.OS_VERSION).toLowerCase(Locale.ENGLISH); if (active && os.getFamily() != null) { active = determineFamilyMatch(os.getFamily(), actualOsName); @@ -82,6 +77,11 @@ public class OperatingSystemProfileActivator implements ProfileActivator { return active; } + private String getSystemProperty(ProfileActivationContext context, String key, String defValue) { + String val = context.getSystemProperty(key); + return val != null ? val : defValue; + } + @Override public boolean presentInConfig(Profile profile, ProfileActivationContext context, ModelProblemCollector problems) { Activation activation = profile.getActivation(); diff --git a/impl/maven-impl/src/main/java/org/apache/maven/internal/impl/model/profile/PackagingProfileActivator.java b/impl/maven-impl/src/main/java/org/apache/maven/internal/impl/model/profile/PackagingProfileActivator.java index 65a508355d..f291f9240e 100644 --- a/impl/maven-impl/src/main/java/org/apache/maven/internal/impl/model/profile/PackagingProfileActivator.java +++ b/impl/maven-impl/src/main/java/org/apache/maven/internal/impl/model/profile/PackagingProfileActivator.java @@ -47,7 +47,7 @@ public class PackagingProfileActivator implements ProfileActivator { } private static boolean isPackaging(ProfileActivationContext context, String p) { - String packaging = context.getModel().getPackaging(); + String packaging = context.getModelPackaging(); return Objects.equals(p, packaging); } diff --git a/impl/maven-impl/src/main/java/org/apache/maven/internal/impl/model/profile/PropertyProfileActivator.java b/impl/maven-impl/src/main/java/org/apache/maven/internal/impl/model/profile/PropertyProfileActivator.java index d36ecda8f9..6ac5e0d3b6 100644 --- a/impl/maven-impl/src/main/java/org/apache/maven/internal/impl/model/profile/PropertyProfileActivator.java +++ b/impl/maven-impl/src/main/java/org/apache/maven/internal/impl/model/profile/PropertyProfileActivator.java @@ -69,12 +69,12 @@ public class PropertyProfileActivator implements ProfileActivator { return false; } - String sysValue = context.getUserProperties().get(name); + String sysValue = context.getUserProperty(name); if (sysValue == null && "packaging".equals(name)) { - sysValue = context.getModel().getPackaging(); + sysValue = context.getModelPackaging(); } if (sysValue == null) { - sysValue = context.getSystemProperties().get(name); + sysValue = context.getSystemProperty(name); } String propValue = property.getValue(); diff --git a/impl/maven-impl/src/test/java/org/apache/maven/internal/impl/model/DefaultModelBuilderTest.java b/impl/maven-impl/src/test/java/org/apache/maven/internal/impl/model/DefaultModelBuilderTest.java new file mode 100644 index 0000000000..cf06e09248 --- /dev/null +++ b/impl/maven-impl/src/test/java/org/apache/maven/internal/impl/model/DefaultModelBuilderTest.java @@ -0,0 +1,66 @@ +/* + * 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.nio.file.Path; +import java.nio.file.Paths; + +import org.apache.maven.api.Session; +import org.apache.maven.api.services.ModelBuilder; +import org.apache.maven.api.services.ModelBuilderRequest; +import org.apache.maven.api.services.ModelBuilderResult; +import org.apache.maven.api.services.ModelSource; +import org.apache.maven.internal.impl.standalone.ApiRunner; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; + +/** + * + */ +class DefaultModelBuilderTest { + + Session session; + ModelBuilder builder; + + @BeforeEach + void setup() { + session = ApiRunner.createSession(); + builder = session.getService(ModelBuilder.class); + assertNotNull(builder); + } + + @Test + public void testPropertiesAndProfiles() { + ModelBuilderRequest request = ModelBuilderRequest.builder() + .session(session) + .requestType(ModelBuilderRequest.RequestType.BUILD_PROJECT) + .source(ModelSource.fromPath(getPom("props-and-profiles"))) + .build(); + ModelBuilderResult result = builder.newSession().build(request); + assertNotNull(result); + assertEquals("21", result.getEffectiveModel().getProperties().get("maven.compiler.release")); + } + + private Path getPom(String name) { + return Paths.get("src/test/resources/poms/factory/" + name + ".xml").toAbsolutePath(); + } +} diff --git a/impl/maven-impl/src/test/java/org/apache/maven/internal/impl/model/profile/AbstractProfileActivatorTest.java b/impl/maven-impl/src/test/java/org/apache/maven/internal/impl/model/profile/AbstractProfileActivatorTest.java index 6e40ef8f10..a55022c183 100644 --- a/impl/maven-impl/src/test/java/org/apache/maven/internal/impl/model/profile/AbstractProfileActivatorTest.java +++ b/impl/maven-impl/src/test/java/org/apache/maven/internal/impl/model/profile/AbstractProfileActivatorTest.java @@ -25,6 +25,8 @@ import org.apache.maven.api.model.Model; import org.apache.maven.api.model.Profile; import org.apache.maven.api.services.model.ProfileActivationContext; import org.apache.maven.api.services.model.ProfileActivator; +import org.apache.maven.internal.impl.model.DefaultInterpolator; +import org.apache.maven.internal.impl.model.DefaultPathTranslator; import org.apache.maven.internal.impl.model.DefaultProfileActivationContext; import org.apache.maven.internal.impl.model.rootlocator.DefaultRootLocator; import org.junit.jupiter.api.AfterEach; @@ -55,7 +57,8 @@ public abstract class AbstractProfileActivatorTest { } protected DefaultProfileActivationContext newContext() { - return new DefaultProfileActivationContext(); + return new DefaultProfileActivationContext( + new DefaultPathTranslator(), new FakeRootLocator(), new DefaultInterpolator()); } protected ProfileActivationContext newContext( diff --git a/impl/maven-impl/src/test/java/org/apache/maven/internal/impl/model/profile/ConditionParserTest.java b/impl/maven-impl/src/test/java/org/apache/maven/internal/impl/model/profile/ConditionParserTest.java index fe6081d28a..c2887d96f7 100644 --- a/impl/maven-impl/src/test/java/org/apache/maven/internal/impl/model/profile/ConditionParserTest.java +++ b/impl/maven-impl/src/test/java/org/apache/maven/internal/impl/model/profile/ConditionParserTest.java @@ -28,7 +28,6 @@ import org.apache.maven.internal.impl.DefaultVersionParser; import org.apache.maven.internal.impl.model.DefaultInterpolator; import org.apache.maven.internal.impl.model.DefaultPathTranslator; import org.apache.maven.internal.impl.model.DefaultProfileActivationContext; -import org.apache.maven.internal.impl.model.ProfileActivationFilePathInterpolator; import org.apache.maven.internal.impl.model.profile.ConditionParser.ExpressionFunction; import org.apache.maven.internal.impl.model.rootlocator.DefaultRootLocator; import org.eclipse.aether.util.version.GenericVersionScheme; @@ -51,19 +50,18 @@ class ConditionParserTest { ProfileActivationContext context = createMockContext(); DefaultVersionParser versionParser = new DefaultVersionParser(new DefaultModelVersionParser(new GenericVersionScheme())); - ProfileActivationFilePathInterpolator interpolator = new ProfileActivationFilePathInterpolator( - new DefaultPathTranslator(), - new AbstractProfileActivatorTest.FakeRootLocator(), - new DefaultInterpolator()); DefaultRootLocator rootLocator = new DefaultRootLocator(); - functions = ConditionProfileActivator.registerFunctions(context, versionParser, interpolator); - propertyResolver = s -> ConditionProfileActivator.property(context, rootLocator, s); + functions = ConditionProfileActivator.registerFunctions(context, versionParser); + propertyResolver = s -> ConditionProfileActivator.property(context, s); parser = new ConditionParser(functions, propertyResolver); } private ProfileActivationContext createMockContext() { - DefaultProfileActivationContext context = new DefaultProfileActivationContext(); + DefaultProfileActivationContext context = new DefaultProfileActivationContext( + new DefaultPathTranslator(), + new AbstractProfileActivatorTest.FakeRootLocator(), + new DefaultInterpolator()); context.setSystemProperties(Map.of( "os.name", "windows", "os.arch", "amd64", diff --git a/impl/maven-impl/src/test/java/org/apache/maven/internal/impl/model/profile/ConditionProfileActivatorTest.java b/impl/maven-impl/src/test/java/org/apache/maven/internal/impl/model/profile/ConditionProfileActivatorTest.java index 71cd0228de..6dd31e293f 100644 --- a/impl/maven-impl/src/test/java/org/apache/maven/internal/impl/model/profile/ConditionProfileActivatorTest.java +++ b/impl/maven-impl/src/test/java/org/apache/maven/internal/impl/model/profile/ConditionProfileActivatorTest.java @@ -33,8 +33,6 @@ import org.apache.maven.internal.impl.DefaultVersionParser; import org.apache.maven.internal.impl.model.DefaultInterpolator; import org.apache.maven.internal.impl.model.DefaultPathTranslator; import org.apache.maven.internal.impl.model.DefaultProfileActivationContext; -import org.apache.maven.internal.impl.model.ProfileActivationFilePathInterpolator; -import org.apache.maven.internal.impl.model.rootlocator.DefaultRootLocator; import org.eclipse.aether.util.version.GenericVersionScheme; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Disabled; @@ -55,10 +53,7 @@ public class ConditionProfileActivatorTest extends AbstractProfileActivatorTest< @Override void setUp() throws Exception { activator = new ConditionProfileActivator( - new DefaultVersionParser(new DefaultModelVersionParser(new GenericVersionScheme())), - new ProfileActivationFilePathInterpolator( - new DefaultPathTranslator(), new FakeRootLocator(), new DefaultInterpolator()), - new DefaultRootLocator()); + new DefaultVersionParser(new DefaultModelVersionParser(new GenericVersionScheme()))); Path file = tempDir.resolve("file.txt"); Files.createFile(file); @@ -484,7 +479,9 @@ public class ConditionProfileActivatorTest extends AbstractProfileActivatorTest< } protected ProfileActivationContext newFileContext(Path path) { - DefaultProfileActivationContext context = new DefaultProfileActivationContext(); + DefaultProfileActivationContext context = new DefaultProfileActivationContext( + new DefaultPathTranslator(), new FakeRootLocator(), new DefaultInterpolator()); + context.setModel(Model.newBuilder().pomFile(path.resolve("pom.xml")).build()); return context; } diff --git a/impl/maven-impl/src/test/java/org/apache/maven/internal/impl/model/profile/FileProfileActivatorTest.java b/impl/maven-impl/src/test/java/org/apache/maven/internal/impl/model/profile/FileProfileActivatorTest.java index bde150d74e..955bb436f4 100644 --- a/impl/maven-impl/src/test/java/org/apache/maven/internal/impl/model/profile/FileProfileActivatorTest.java +++ b/impl/maven-impl/src/test/java/org/apache/maven/internal/impl/model/profile/FileProfileActivatorTest.java @@ -27,10 +27,7 @@ import org.apache.maven.api.model.ActivationFile; import org.apache.maven.api.model.Model; import org.apache.maven.api.model.Profile; import org.apache.maven.api.services.model.RootLocator; -import org.apache.maven.internal.impl.model.DefaultInterpolator; -import org.apache.maven.internal.impl.model.DefaultPathTranslator; import org.apache.maven.internal.impl.model.DefaultProfileActivationContext; -import org.apache.maven.internal.impl.model.ProfileActivationFilePathInterpolator; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.io.TempDir; @@ -47,13 +44,12 @@ class FileProfileActivatorTest extends AbstractProfileActivatorTest { new DefaultDependencyManagementInjector(), new DefaultDependencyManagementImporter(), new DefaultPluginConfigurationExpander(), - new ProfileActivationFilePathInterpolator( - new DefaultPathTranslator(), new DefaultRootLocator(), new DefaultInterpolator()), new DefaultModelVersionParser(getVersionScheme()), List.of(), new DefaultModelCacheFactory(), new DefaultModelResolver(), - new DefaultInterpolator()); + new DefaultInterpolator(), + new DefaultPathTranslator(), + new DefaultRootLocator()); } private RepositorySystem repositorySystem; diff --git a/impl/maven-impl/src/test/resources/poms/factory/props-and-profiles-grand-parent.xml b/impl/maven-impl/src/test/resources/poms/factory/props-and-profiles-grand-parent.xml new file mode 100644 index 0000000000..2623069658 --- /dev/null +++ b/impl/maven-impl/src/test/resources/poms/factory/props-and-profiles-grand-parent.xml @@ -0,0 +1,34 @@ + + + + org.apache.maven.tests + props-and-profiles-grand-parent + 1.0-SNAPSHOT + pom + + + jdk9+ + + true + + + 17 + + + + diff --git a/impl/maven-impl/src/test/resources/poms/factory/props-and-profiles-parent.xml b/impl/maven-impl/src/test/resources/poms/factory/props-and-profiles-parent.xml new file mode 100644 index 0000000000..00ffc6d0c9 --- /dev/null +++ b/impl/maven-impl/src/test/resources/poms/factory/props-and-profiles-parent.xml @@ -0,0 +1,28 @@ + + + + + props-and-profiles-grand-parent.xml + + props-and-profiles + 1.0-SNAPSHOT + pom + + 21 + + diff --git a/impl/maven-impl/src/test/resources/poms/factory/props-and-profiles.xml b/impl/maven-impl/src/test/resources/poms/factory/props-and-profiles.xml new file mode 100644 index 0000000000..02fb63cfd0 --- /dev/null +++ b/impl/maven-impl/src/test/resources/poms/factory/props-and-profiles.xml @@ -0,0 +1,24 @@ + + + + + props-and-profiles-parent.xml + + props-and-profiles + 1.0-SNAPSHOT +