slawekjaranowski commented on code in PR #13891:
URL: https://github.com/apache/maven/pull/13891#discussion_r4507246671
##########
maven-model-builder/src/main/java/org/apache/maven/model/interpolation/AbstractStringBasedModelInterpolator.java:
##########
@@ -42,6 +42,7 @@
import org.codehaus.plexus.interpolation.PrefixedObjectValueSource;
import org.codehaus.plexus.interpolation.PrefixedValueSourceWrapper;
import org.codehaus.plexus.interpolation.RecursionInterceptor;
+import org.codehaus.plexus.interpolation.SimpleRecursionInterceptor;
import org.codehaus.plexus.interpolation.SingleResponseValueSource;
import org.codehaus.plexus.interpolation.ValueSource;
Review Comment:
```suggestion
import org.codehaus.plexus.interpolation.SingleResponseValueSource;
import org.codehaus.plexus.interpolation.ValueSource;
import org.codehaus.plexus.interpolation.util.ValueSourceUtils;
```
##########
maven-model-builder/src/main/java/org/apache/maven/model/interpolation/AbstractStringBasedModelInterpolator.java:
##########
@@ -292,6 +293,47 @@ protected List<? extends InterpolationPostProcessor>
createPostProcessors(
}
protected RecursionInterceptor createRecursionInterceptor() {
- return new PrefixAwareRecursionInterceptor(PROJECT_PREFIXES);
+ return new PrefixAwareRecursionInterceptor(PROJECT_PREFIXES) {
+ // The custom property is distinct from project.rootDirectory and
pom.rootDirectory.
+ private final RecursionInterceptor rootDirectory = new
SimpleRecursionInterceptor();
+
+ @Override
+ public boolean hasRecursiveExpression(String expression) {
+ return "rootDirectory".equals(expression)
+ ? rootDirectory.hasRecursiveExpression(expression)
+ : super.hasRecursiveExpression(expression);
+ }
+
+ @Override
+ public void expressionResolutionStarted(String expression) {
+ if ("rootDirectory".equals(expression)) {
+ rootDirectory.expressionResolutionStarted(expression);
+ } else {
+ super.expressionResolutionStarted(expression);
+ }
+ }
+
+ @Override
+ public void expressionResolutionFinished(String expression) {
+ if ("rootDirectory".equals(expression)) {
+ rootDirectory.expressionResolutionFinished(expression);
+ } else {
+ super.expressionResolutionFinished(expression);
+ }
+ }
+
+ @Override
+ public List<?> getExpressionCycle(String expression) {
+ return "rootDirectory".equals(expression)
+ ? rootDirectory.getExpressionCycle(expression)
+ : super.getExpressionCycle(expression);
+ }
+
+ @Override
+ public void clear() {
+ rootDirectory.clear();
+ super.clear();
+ }
+ };
Review Comment:
Single stack with a key function: fixes `baseUri` too and keeps full cycle
messages.
```suggestion
return new RecursionInterceptor() {
// Expressions resolved only with a project prefix: unprefixed,
they are unrelated custom properties.
private final Collection<String> prefixOnlyExpressions =
Arrays.asList("rootDirectory", "baseUri");
private final List<String> expressions = new ArrayList<>();
private String key(String expression) {
String naked = ValueSourceUtils.trimPrefix(expression,
PROJECT_PREFIXES, true);
if (prefixOnlyExpressions.contains(naked) &&
!naked.equals(expression)) {
return "project." + naked;
}
return naked;
}
@Override
public void expressionResolutionStarted(String expression) {
expressions.add(key(expression));
}
@Override
public void expressionResolutionFinished(String expression) {
expressions.remove(expressions.size() - 1);
}
@Override
public boolean hasRecursiveExpression(String expression) {
return expressions.contains(key(expression));
}
@Override
public List<?> getExpressionCycle(String expression) {
int idx = expressions.indexOf(key(expression));
return idx < 0 ? new ArrayList<>() : new
ArrayList<>(expressions.subList(idx, expressions.size()));
}
@Override
public void clear() {
expressions.clear();
}
};
```
##########
maven-model-builder/src/test/java/org/apache/maven/model/interpolation/AbstractModelInterpolatorTest.java:
##########
@@ -425,6 +432,84 @@ public void testUnprefixedRootDirectoryIsNotInterpolated()
throws Exception {
assertEquals("${rootDirectory}/temp-repo",
out.getRepositories().get(0).getUrl());
}
+ @Test
+ public void testRootDirectoryAliases(@TempDir Path directory) throws
Exception {
+ for (String prefix : new String[] {"project.", "pom."}) {
+ for (String source : new String[] {"model", "user", "system"}) {
+ Path root = Files.createDirectory(directory.resolve(prefix +
source));
+ Model model = new Model();
+ ModelBuildingRequest request = new
DefaultModelBuildingRequest();
+ Properties properties = "model".equals(source)
+ ? model.getProperties()
+ : "user".equals(source) ? request.getUserProperties()
: request.getSystemProperties();
+ properties.setProperty("rootDirectory", "${" + prefix +
"rootDirectory}");
+
+ assertRootDirectoryInterpolation(root, model, request,
root.toString());
+ }
+ }
+ }
+
+ @Test
+ public void testRootDirectoryCycleIsDetected() throws Exception {
+ Model model = new Model();
+ model.setName("${rootDirectory}");
+ model.getProperties().setProperty("rootDirectory", "${other}");
+ model.getProperties().setProperty("other", "${rootDirectory}");
+ SimpleProblemCollector collector = new SimpleProblemCollector();
+
+ createInterpolator().interpolateModel(model, null, new
DefaultModelBuildingRequest(), collector);
+
+ assertFalse(collector.getErrors().isEmpty());
+ }
+
+ @Test
+ public void testRootDirectoryRecursionNamespaces() throws Exception {
+ RecursionInterceptor interceptor =
+ ((AbstractStringBasedModelInterpolator)
createInterpolator()).createRecursionInterceptor();
+ interceptor.expressionResolutionStarted("rootDirectory");
+
assertFalse(interceptor.hasRecursiveExpression("project.rootDirectory"));
+ interceptor.expressionResolutionStarted("project.rootDirectory");
+ assertTrue(interceptor.hasRecursiveExpression("pom.rootDirectory"));
+ interceptor.expressionResolutionFinished("project.rootDirectory");
+ assertTrue(interceptor.hasRecursiveExpression("rootDirectory"));
+ interceptor.expressionResolutionFinished("rootDirectory");
+ assertFalse(interceptor.hasRecursiveExpression("rootDirectory"));
+
+ interceptor.expressionResolutionStarted("rootDirectory");
+ interceptor.expressionResolutionStarted("project.rootDirectory");
+ interceptor.clear();
+ assertFalse(interceptor.hasRecursiveExpression("rootDirectory"));
+
assertFalse(interceptor.hasRecursiveExpression("project.rootDirectory"));
+ }
Review Comment:
Regression tests for `baseUri` and for the cycle message.
```suggestion
}
@Test
public void testBaseUriAlias(@TempDir Path directory) throws Exception {
Model model = new Model();
model.getProperties().setProperty("baseUri", "${project.baseUri}");
model.getProperties().setProperty("customBase", "${baseUri}");
model.setName("${baseUri}");
Site site = new Site();
site.setName("${baseUri}");
DistributionManagement distributionManagement = new
DistributionManagement();
distributionManagement.setSite(site);
model.setDistributionManagement(distributionManagement);
SimpleProblemCollector collector = new SimpleProblemCollector();
Model out = createInterpolator()
.interpolateModel(model, directory.toFile(), new
DefaultModelBuildingRequest(), collector);
assertProblemFree(collector);
String expected = directory.toUri().toASCIIString();
assertEquals(expected, out.getName());
assertEquals(expected,
out.getProperties().getProperty("customBase"));
assertEquals(expected,
out.getDistributionManagement().getSite().getName());
}
@Test
public void testRootDirectoryCycleMessage() throws Exception {
Model model = new Model();
model.setName("${rootDirectory}");
model.getProperties().setProperty("rootDirectory", "${other}");
model.getProperties().setProperty("other", "${rootDirectory}");
SimpleProblemCollector collector = new SimpleProblemCollector();
createInterpolator().interpolateModel(model, null, new
DefaultModelBuildingRequest(), collector);
assertTrue(collector.getErrors().stream().anyMatch(e ->
e.contains("[rootDirectory, other]")));
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
