Skip to content

Commit 616d6ea

Browse files
gnodetgnodet
committed
Allow repository URL interpolation with improved validation (#11140)
This commit enables repository URL interpolation in Maven 4 while maintaining backward compatibility and providing early validation of unresolved expressions. Repository URLs can now use expressions like ${env.REPO_URL} and ${project.basedir.uri} which are interpolated during model building. Key changes: 1. DefaultModelBuilder: Add repository URL interpolation during model building - Support for repositories, pluginRepositories, profiles, and distributionManagement - Provide basedir, project.basedir, project.basedir.uri, project.rootDirectory, and project.rootDirectory.uri properties for interpolation - Enable environment variable and project property interpolation in repository URLs 2. DefaultModelValidator: Validate interpolated repository URLs for unresolved expressions - Repository URL expressions are interpolated during model building - After interpolation, any remaining ${...} expressions cause validation errors - Early failure during model validation provides clear error messages 3. CompatibilityFixStrategy: Remove repository disabling logic, replace with informational logging for interpolated URLs 4. Add integration tests for repository URL interpolation: - Test successful interpolation from environment variables and project properties - Test early failure when expressions cannot be resolved during model building The new approach enables legitimate use cases while providing early, clear error messages for unresolved expressions during the validate phase rather than later during repository resolution. (cherry picked from commit 210dbdc) # Conflicts: # impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelBuilder.java # impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelValidator.java
1 parent b2e4fd4 commit 616d6ea

File tree

9 files changed

+444
-68
lines changed

9 files changed

+444
-68
lines changed

‎impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvnup/goals/CompatibilityFixStrategy.java‎

Lines changed: 4 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,6 @@
3333
import org.apache.maven.api.di.Singleton;
3434
import org.apache.maven.cling.invoker.mvnup.UpgradeContext;
3535
import org.jdom2.Attribute;
36-
import org.jdom2.Comment;
3736
import org.jdom2.Content;
3837
import org.jdom2.Document;
3938
import org.jdom2.Element;
@@ -498,25 +497,12 @@ private boolean fixRepositoryExpressions(Element repositoriesElement, Namespace
498497
Element urlElement = repository.getChild("url", namespace);
499498
if (urlElement != null) {
500499
String url = urlElement.getTextTrim();
501-
if (url.contains("${")
502-
&& !url.contains("${project.basedir}")
503-
&& !url.contains("${project.rootDirectory}")) {
500+
if (url.contains("${")) {
501+
// Allow repository URL interpolation; do not disable.
502+
// Keep a gentle warning to help users notice unresolved placeholders at build time.
504503
String repositoryId = getChildText(repository, "id", namespace);
505-
context.warning("Found unsupported expression in " + elementType + " URL (id: " + repositoryId
504+
context.info("Detected interpolated expression in " + elementType + " URL (id: " + repositoryId
506505
+ "): " + url);
507-
context.warning(
508-
"Maven 4 only supports ${project.basedir} and ${project.rootDirectory} expressions in repository URLs");
509-
510-
// Comment out the problematic repository
511-
Comment comment =
512-
new Comment(" Repository disabled due to unsupported expression in URL: " + url + " ");
513-
Element parent = repository.getParentElement();
514-
parent.addContent(parent.indexOf(repository), comment);
515-
removeElementWithFormatting(repository);
516-
517-
context.detail("Fixed: " + "Commented out " + elementType + " with unsupported URL expression (id: "
518-
+ repositoryId + ")");
519-
fixed = true;
520506
}
521507
}
522508
}

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

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@
4242
import java.util.concurrent.Executor;
4343
import java.util.concurrent.Executors;
4444
import java.util.concurrent.atomic.AtomicReference;
45+
import java.util.function.BiFunction;
4546
import java.util.function.Supplier;
4647
import java.util.function.UnaryOperator;
4748
import java.util.stream.Collectors;
@@ -63,12 +64,15 @@
6364
import org.apache.maven.api.model.Activation;
6465
import org.apache.maven.api.model.Dependency;
6566
import org.apache.maven.api.model.DependencyManagement;
67+
import org.apache.maven.api.model.DeploymentRepository;
68+
import org.apache.maven.api.model.DistributionManagement;
6669
import org.apache.maven.api.model.Exclusion;
6770
import org.apache.maven.api.model.InputLocation;
6871
import org.apache.maven.api.model.InputSource;
6972
import org.apache.maven.api.model.Model;
7073
import org.apache.maven.api.model.Parent;
7174
import org.apache.maven.api.model.Profile;
75+
import org.apache.maven.api.model.Repository;
7276
import org.apache.maven.api.services.BuilderProblem;
7377
import org.apache.maven.api.services.BuilderProblem.Severity;
7478
import org.apache.maven.api.services.Interpolator;
@@ -1415,6 +1419,29 @@ Model doReadFileModel() throws ModelBuilderException {
14151419
model.getParent().getVersion()))
14161420
: null)
14171421
.build();
1422+
// Interpolate repository URLs
1423+
if (model.getProjectDirectory() != null) {
1424+
String basedir = model.getProjectDirectory().toString();
1425+
String basedirUri = model.getProjectDirectory().toUri().toString();
1426+
properties.put("basedir", basedir);
1427+
properties.put("project.basedir", basedir);
1428+
properties.put("project.basedir.uri", basedirUri);
1429+
}
1430+
try {
1431+
String root = request.getSession().getRootDirectory().toString();
1432+
String rootUri =
1433+
request.getSession().getRootDirectory().toUri().toString();
1434+
properties.put("project.rootDirectory", root);
1435+
properties.put("project.rootDirectory.uri", rootUri);
1436+
} catch (IllegalStateException e) {
1437+
}
1438+
UnaryOperator<String> callback = properties::get;
1439+
model = model.with()
1440+
.repositories(interpolateRepository(model.getRepositories(), callback))
1441+
.pluginRepositories(interpolateRepository(model.getPluginRepositories(), callback))
1442+
.profiles(map(model.getProfiles(), this::interpolateRepository, callback))
1443+
.distributionManagement(interpolateRepository(model.getDistributionManagement(), callback))
1444+
.build();
14181445
// Override model properties with user properties
14191446
Map<String, String> newProps = merge(model.getProperties(), session.getUserProperties());
14201447
if (newProps != null) {
@@ -1445,6 +1472,41 @@ Model doReadFileModel() throws ModelBuilderException {
14451472
return model;
14461473
}
14471474

1475+
private DistributionManagement interpolateRepository(
1476+
DistributionManagement distributionManagement, UnaryOperator<String> callback) {
1477+
return distributionManagement == null
1478+
? null
1479+
: distributionManagement
1480+
.with()
1481+
.repository((DeploymentRepository)
1482+
interpolateRepository(distributionManagement.getRepository(), callback))
1483+
.snapshotRepository((DeploymentRepository)
1484+
interpolateRepository(distributionManagement.getSnapshotRepository(), callback))
1485+
.build();
1486+
}
1487+
1488+
private Profile interpolateRepository(Profile profile, UnaryOperator<String> callback) {
1489+
return profile == null
1490+
? null
1491+
: profile.with()
1492+
.repositories(interpolateRepository(profile.getRepositories(), callback))
1493+
.pluginRepositories(interpolateRepository(profile.getPluginRepositories(), callback))
1494+
.build();
1495+
}
1496+
1497+
private List<Repository> interpolateRepository(List<Repository> repositories, UnaryOperator<String> callback) {
1498+
return map(repositories, this::interpolateRepository, callback);
1499+
}
1500+
1501+
private Repository interpolateRepository(Repository repository, UnaryOperator<String> callback) {
1502+
return repository == null
1503+
? null
1504+
: repository
1505+
.with()
1506+
.url(interpolator.interpolate(repository.getUrl(), callback))
1507+
.build();
1508+
}
1509+
14481510
/**
14491511
* Merges a list of model profiles with user-defined properties.
14501512
* For each property defined in both the model and user properties, the user property value
@@ -2250,4 +2312,21 @@ Set<String> getContexts() {
22502312
return contexts;
22512313
}
22522314
}
2315+
2316+
private static <T, A> List<T> map(List<T> resources, BiFunction<T, A, T> mapper, A argument) {
2317+
List<T> newResources = null;
2318+
if (resources != null) {
2319+
for (int i = 0; i < resources.size(); i++) {
2320+
T resource = resources.get(i);
2321+
T newResource = mapper.apply(resource, argument);
2322+
if (newResource != resource) {
2323+
if (newResources == null) {
2324+
newResources = new ArrayList<>(resources);
2325+
}
2326+
newResources.set(i, newResource);
2327+
}
2328+
}
2329+
}
2330+
return newResources;
2331+
}
22532332
}

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

Lines changed: 76 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -556,16 +556,6 @@ public void validateFileModel(Session s, Model m, int validationLevel, ModelProb
556556
validationLevel);
557557
}
558558

559-
validateRawRepositories(
560-
problems, profile.getRepositories(), prefix, "repositories.repository.", validationLevel);
561-
562-
validateRawRepositories(
563-
problems,
564-
profile.getPluginRepositories(),
565-
prefix,
566-
"pluginRepositories.pluginRepository.",
567-
validationLevel);
568-
569559
BuildBase buildBase = profile.getBuild();
570560
if (buildBase != null) {
571561
validate20RawPlugins(problems, buildBase.getPlugins(), prefix, "plugins.plugin.", validationLevel);
@@ -635,6 +625,43 @@ public void validateRawModel(Session s, Model m, int validationLevel, ModelProbl
635625
parent);
636626
}
637627
}
628+
629+
if (validationLevel > VALIDATION_LEVEL_MINIMAL) {
630+
validateRawRepositories(problems, m.getRepositories(), "repositories.repository.", EMPTY, validationLevel);
631+
632+
validateRawRepositories(
633+
problems,
634+
m.getPluginRepositories(),
635+
"pluginRepositories.pluginRepository.",
636+
EMPTY,
637+
validationLevel);
638+
639+
for (Profile profile : m.getProfiles()) {
640+
String prefix = "profiles.profile[" + profile.getId() + "].";
641+
642+
validateRawRepositories(
643+
problems, profile.getRepositories(), prefix, "repositories.repository.", validationLevel);
644+
645+
validateRawRepositories(
646+
problems,
647+
profile.getPluginRepositories(),
648+
prefix,
649+
"pluginRepositories.pluginRepository.",
650+
validationLevel);
651+
}
652+
653+
DistributionManagement distMgmt = m.getDistributionManagement();
654+
if (distMgmt != null) {
655+
validateRawRepository(
656+
problems, distMgmt.getRepository(), "distributionManagement.repository.", "", true);
657+
validateRawRepository(
658+
problems,
659+
distMgmt.getSnapshotRepository(),
660+
"distributionManagement.snapshotRepository.",
661+
"",
662+
true);
663+
}
664+
}
638665
}
639666

640667
private void validate30RawProfileActivation(ModelProblemCollector problems, Activation activation, String prefix) {
@@ -1444,40 +1471,7 @@ private void validateRawRepositories(
14441471
Map<String, Repository> index = new HashMap<>();
14451472

14461473
for (Repository repository : repositories) {
1447-
validateStringNotEmpty(
1448-
prefix, prefix2, "id", problems, Severity.ERROR, Version.V20, repository.getId(), null, repository);
1449-
1450-
if (validateStringNotEmpty(
1451-
prefix,
1452-
prefix2,
1453-
"[" + repository.getId() + "].url",
1454-
problems,
1455-
Severity.ERROR,
1456-
Version.V20,
1457-
repository.getUrl(),
1458-
null,
1459-
repository)) {
1460-
// only allow ${basedir} and ${project.basedir}
1461-
Matcher m = EXPRESSION_NAME_PATTERN.matcher(repository.getUrl());
1462-
while (m.find()) {
1463-
String expr = m.group(1);
1464-
if (!("basedir".equals(expr)
1465-
|| "project.basedir".equals(expr)
1466-
|| expr.startsWith("project.basedir.")
1467-
|| "project.rootDirectory".equals(expr)
1468-
|| expr.startsWith("project.rootDirectory."))) {
1469-
addViolation(
1470-
problems,
1471-
Severity.ERROR,
1472-
Version.V40,
1473-
prefix + prefix2 + "[" + repository.getId() + "].url",
1474-
null,
1475-
"contains an unsupported expression (only expressions starting with 'project.basedir' or 'project.rootDirectory' are supported).",
1476-
repository);
1477-
break;
1478-
}
1479-
}
1480-
}
1474+
validateRawRepository(problems, repository, prefix, prefix2, false);
14811475

14821476
String key = repository.getId();
14831477

@@ -1501,6 +1495,44 @@ private void validateRawRepositories(
15011495
}
15021496
}
15031497

1498+
private void validateRawRepository(
1499+
ModelProblemCollector problems,
1500+
Repository repository,
1501+
String prefix,
1502+
String prefix2,
1503+
boolean allowEmptyUrl) {
1504+
if (repository == null) {
1505+
return;
1506+
}
1507+
validateStringNotEmpty(
1508+
prefix, prefix2, "id", problems, Severity.ERROR, Version.V20, repository.getId(), null, repository);
1509+
1510+
if (!allowEmptyUrl
1511+
&& validateStringNotEmpty(
1512+
prefix,
1513+
prefix2,
1514+
"[" + repository.getId() + "].url",
1515+
problems,
1516+
Severity.ERROR,
1517+
Version.V20,
1518+
repository.getUrl(),
1519+
null,
1520+
repository)) {
1521+
// Check for uninterpolated expressions - these should have been interpolated by now
1522+
Matcher matcher = EXPRESSION_NAME_PATTERN.matcher(repository.getUrl());
1523+
if (matcher.find()) {
1524+
addViolation(
1525+
problems,
1526+
Severity.ERROR,
1527+
Version.V40,
1528+
prefix + prefix2 + "[" + repository.getId() + "].url",
1529+
null,
1530+
"contains an uninterpolated expression.",
1531+
repository);
1532+
}
1533+
}
1534+
}
1535+
15041536
private void validate20EffectiveRepository(
15051537
ModelProblemCollector problems, Repository repository, String prefix, int validationLevel) {
15061538
if (repository != null) {

‎impl/maven-impl/src/test/java/org/apache/maven/impl/model/DefaultModelValidatorTest.java‎

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -336,7 +336,7 @@ void testEmptyPluginVersion() throws Exception {
336336
@Test
337337
void testMissingRepositoryId() throws Exception {
338338
SimpleProblemCollector result =
339-
validateFile("missing-repository-id-pom.xml", ModelValidator.VALIDATION_LEVEL_STRICT);
339+
validateRaw("missing-repository-id-pom.xml", ModelValidator.VALIDATION_LEVEL_STRICT);
340340

341341
assertViolations(result, 0, 4, 0);
342342

@@ -855,16 +855,23 @@ void testParentVersionRELEASE() throws Exception {
855855
@Test
856856
void repositoryWithExpression() throws Exception {
857857
SimpleProblemCollector result = validateFile("raw-model/repository-with-expression.xml");
858-
assertViolations(result, 0, 1, 0);
859-
assertEquals(
860-
"'repositories.repository.[repo].url' contains an unsupported expression (only expressions starting with 'project.basedir' or 'project.rootDirectory' are supported).",
861-
result.getErrors().get(0));
858+
// Interpolation in repository URLs is allowed; unresolved placeholders will fail later during resolution
859+
assertViolations(result, 0, 0, 0);
862860
}
863861

864862
@Test
865863
void repositoryWithBasedirExpression() throws Exception {
866864
SimpleProblemCollector result = validateRaw("raw-model/repository-with-basedir-expression.xml");
867-
assertViolations(result, 0, 0, 0);
865+
// This test runs on raw model without interpolation, so all expressions appear uninterpolated
866+
// In the real flow, supported expressions would be interpolated before validation
867+
assertViolations(result, 0, 3, 0);
868+
}
869+
870+
@Test
871+
void repositoryWithUnsupportedExpression() throws Exception {
872+
SimpleProblemCollector result = validateRaw("raw-model/repository-with-unsupported-expression.xml");
873+
// Unsupported expressions should cause validation errors
874+
assertViolations(result, 0, 1, 0);
868875
}
869876

870877
@Test

‎impl/maven-impl/src/test/resources/poms/validation/raw-model/repository-with-unsupported-expression.xml‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
<?xml version="1.0" encoding="UTF-8"?>
2+
3+
<!--
4+
Licensed to the Apache Software Foundation (ASF) under one
5+
or more contributor license agreements. See the NOTICE file
6+
distributed with this work for additional information
7+
regarding copyright ownership. The ASF licenses this file
8+
to you under the Apache License, Version 2.0 (the
9+
"License"); you may not use this file except in compliance
10+
with the License. You may obtain a copy of the License at
11+
12+
http://www.apache.org/licenses/LICENSE-2.0
13+
14+
Unless required by applicable law or agreed to in writing,
15+
software distributed under the License is distributed on an
16+
"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
17+
KIND, either express or implied. See the License for the
18+
specific language governing permissions and limitations
19+
under the License.
20+
-->
21+
22+
<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
23+
xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 https://maven.apache.org/xsd/maven-4.0.0.xsd">
24+
<modelVersion>4.1.0</modelVersion>
25+
26+
<groupId>org.apache.maven.its.mng0000</groupId>
27+
<artifactId>test</artifactId>
28+
<version>1.0-SNAPSHOT</version>
29+
<packaging>pom</packaging>
30+
31+
<name>Maven Integration Test :: Test</name>
32+
<description>Test unsupported repository URL expressions that should cause validation errors.</description>
33+
34+
<repositories>
35+
<repository>
36+
<id>repo-unsupported</id>
37+
<url>${project.baseUri}/sdk/maven/repo</url>
38+
</repository>
39+
</repositories>
40+
41+
</project>

0 commit comments

Comments
 (0)