Skip to content

Commit 829b79f

Browse files
gnodetgnodet
committed
Allow repository URL interpolation with improved validation
This commit enables repository URL interpolation in Maven 4 while maintaining backward compatibility. The changes allow repositories to 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: Allow expressions in repository URLs during model validation - Repository URL expressions are permitted and will be interpolated during model building - Unresolved placeholders will fail later during repository resolution with appropriate 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 failure handling when placeholders remain unresolved during repository usage The new approach enables legitimate use cases while maintaining Maven's robustness by catching unresolved placeholders when repositories are actually used.
1 parent 93567fe commit 829b79f

File tree

9 files changed

+434
-77
lines changed

9 files changed

+434
-77
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
@@ -41,6 +41,7 @@
4141
import java.util.concurrent.Executor;
4242
import java.util.concurrent.Executors;
4343
import java.util.concurrent.atomic.AtomicReference;
44+
import java.util.function.BiFunction;
4445
import java.util.function.Supplier;
4546
import java.util.function.UnaryOperator;
4647
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.Mixin;
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;
@@ -1441,6 +1445,29 @@ Model doReadFileModel() throws ModelBuilderException {
14411445
model.getParent().getVersion()))
14421446
: null)
14431447
.build();
1448+
// Interpolate repository URLs
1449+
if (model.getProjectDirectory() != null) {
1450+
String basedir = model.getProjectDirectory().toString();
1451+
String basedirUri = model.getProjectDirectory().toUri().toString();
1452+
properties.put("basedir", basedir);
1453+
properties.put("project.basedir", basedir);
1454+
properties.put("project.basedir.uri", basedirUri);
1455+
}
1456+
try {
1457+
String root = request.getSession().getRootDirectory().toString();
1458+
String rootUri =
1459+
request.getSession().getRootDirectory().toUri().toString();
1460+
properties.put("project.rootDirectory", root);
1461+
properties.put("project.rootDirectory.uri", rootUri);
1462+
} catch (IllegalStateException e) {
1463+
}
1464+
UnaryOperator<String> callback = properties::get;
1465+
model = model.with()
1466+
.repositories(interpolateRepository(model.getRepositories(), callback))
1467+
.pluginRepositories(interpolateRepository(model.getPluginRepositories(), callback))
1468+
.profiles(map(model.getProfiles(), this::interpolateRepository, callback))
1469+
.distributionManagement(interpolateRepository(model.getDistributionManagement(), callback))
1470+
.build();
14441471
// Override model properties with user properties
14451472
Map<String, String> newProps = merge(model.getProperties(), session.getUserProperties());
14461473
if (newProps != null) {
@@ -1471,6 +1498,41 @@ Model doReadFileModel() throws ModelBuilderException {
14711498
return model;
14721499
}
14731500

1501+
private DistributionManagement interpolateRepository(
1502+
DistributionManagement distributionManagement, UnaryOperator<String> callback) {
1503+
return distributionManagement == null
1504+
? null
1505+
: distributionManagement
1506+
.with()
1507+
.repository((DeploymentRepository)
1508+
interpolateRepository(distributionManagement.getRepository(), callback))
1509+
.snapshotRepository((DeploymentRepository)
1510+
interpolateRepository(distributionManagement.getSnapshotRepository(), callback))
1511+
.build();
1512+
}
1513+
1514+
private Profile interpolateRepository(Profile profile, UnaryOperator<String> callback) {
1515+
return profile == null
1516+
? null
1517+
: profile.with()
1518+
.repositories(interpolateRepository(profile.getRepositories(), callback))
1519+
.pluginRepositories(interpolateRepository(profile.getPluginRepositories(), callback))
1520+
.build();
1521+
}
1522+
1523+
private List<Repository> interpolateRepository(List<Repository> repositories, UnaryOperator<String> callback) {
1524+
return map(repositories, this::interpolateRepository, callback);
1525+
}
1526+
1527+
private Repository interpolateRepository(Repository repository, UnaryOperator<String> callback) {
1528+
return repository == null
1529+
? null
1530+
: repository
1531+
.with()
1532+
.url(interpolator.interpolate(repository.getUrl(), callback))
1533+
.build();
1534+
}
1535+
14741536
/**
14751537
* Merges a list of model profiles with user-defined properties.
14761538
* For each property defined in both the model and user properties, the user property value
@@ -2340,4 +2402,21 @@ private Object getOuterRequest(Request<?> req) {
23402402
}
23412403
return req;
23422404
}
2405+
2406+
private static <T, A> List<T> map(List<T> resources, BiFunction<T, A, T> mapper, A argument) {
2407+
List<T> newResources = null;
2408+
if (resources != null) {
2409+
for (int i = 0; i < resources.size(); i++) {
2410+
T resource = resources.get(i);
2411+
T newResource = mapper.apply(resource, argument);
2412+
if (newResource != resource) {
2413+
if (newResources == null) {
2414+
newResources = new ArrayList<>(resources);
2415+
}
2416+
newResources.set(i, newResource);
2417+
}
2418+
}
2419+
}
2420+
return newResources;
2421+
}
23432422
}

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

Lines changed: 66 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -542,16 +542,6 @@ && equals(parent.getArtifactId(), model.getArtifactId())) {
542542
validationLevel);
543543
}
544544

545-
validateRawRepositories(
546-
problems, model.getRepositories(), "repositories.repository.", EMPTY, validationLevel);
547-
548-
validateRawRepositories(
549-
problems,
550-
model.getPluginRepositories(),
551-
"pluginRepositories.pluginRepository.",
552-
EMPTY,
553-
validationLevel);
554-
555545
Build build = model.getBuild();
556546
if (build != null) {
557547
validate20RawPlugins(problems, build.getPlugins(), "build.plugins.plugin.", EMPTY, validationLevel);
@@ -605,16 +595,6 @@ && equals(parent.getArtifactId(), model.getArtifactId())) {
605595
validationLevel);
606596
}
607597

608-
validateRawRepositories(
609-
problems, profile.getRepositories(), prefix, "repositories.repository.", validationLevel);
610-
611-
validateRawRepositories(
612-
problems,
613-
profile.getPluginRepositories(),
614-
prefix,
615-
"pluginRepositories.pluginRepository.",
616-
validationLevel);
617-
618598
BuildBase buildBase = profile.getBuild();
619599
if (buildBase != null) {
620600
validate20RawPlugins(problems, buildBase.getPlugins(), prefix, "plugins.plugin.", validationLevel);
@@ -685,6 +665,44 @@ && equals(parent.getArtifactId(), model.getArtifactId())) {
685665
parent);
686666
}
687667
}
668+
669+
if (validationLevel > VALIDATION_LEVEL_MINIMAL) {
670+
validateRawRepositories(
671+
problems, model.getRepositories(), "repositories.repository.", EMPTY, validationLevel);
672+
673+
validateRawRepositories(
674+
problems,
675+
model.getPluginRepositories(),
676+
"pluginRepositories.pluginRepository.",
677+
EMPTY,
678+
validationLevel);
679+
680+
for (Profile profile : model.getProfiles()) {
681+
String prefix = "profiles.profile[" + profile.getId() + "].";
682+
683+
validateRawRepositories(
684+
problems, profile.getRepositories(), prefix, "repositories.repository.", validationLevel);
685+
686+
validateRawRepositories(
687+
problems,
688+
profile.getPluginRepositories(),
689+
prefix,
690+
"pluginRepositories.pluginRepository.",
691+
validationLevel);
692+
}
693+
694+
DistributionManagement distMgmt = model.getDistributionManagement();
695+
if (distMgmt != null) {
696+
validateRawRepository(
697+
problems, distMgmt.getRepository(), "distributionManagement.repository.", "", true);
698+
validateRawRepository(
699+
problems,
700+
distMgmt.getSnapshotRepository(),
701+
"distributionManagement.snapshotRepository.",
702+
"",
703+
true);
704+
}
705+
}
688706
}
689707

690708
private void validate30RawProfileActivation(ModelProblemCollector problems, Activation activation, String prefix) {
@@ -1536,40 +1554,7 @@ private void validateRawRepositories(
15361554
Map<String, Repository> index = new HashMap<>();
15371555

15381556
for (Repository repository : repositories) {
1539-
validateStringNotEmpty(
1540-
prefix, prefix2, "id", problems, Severity.ERROR, Version.V20, repository.getId(), null, repository);
1541-
1542-
if (validateStringNotEmpty(
1543-
prefix,
1544-
prefix2,
1545-
"[" + repository.getId() + "].url",
1546-
problems,
1547-
Severity.ERROR,
1548-
Version.V20,
1549-
repository.getUrl(),
1550-
null,
1551-
repository)) {
1552-
// only allow ${basedir} and ${project.basedir}
1553-
Matcher matcher = EXPRESSION_NAME_PATTERN.matcher(repository.getUrl());
1554-
while (matcher.find()) {
1555-
String expr = matcher.group(1);
1556-
if (!("basedir".equals(expr)
1557-
|| "project.basedir".equals(expr)
1558-
|| expr.startsWith("project.basedir.")
1559-
|| "project.rootDirectory".equals(expr)
1560-
|| expr.startsWith("project.rootDirectory."))) {
1561-
addViolation(
1562-
problems,
1563-
Severity.ERROR,
1564-
Version.V40,
1565-
prefix + prefix2 + "[" + repository.getId() + "].url",
1566-
null,
1567-
"contains an unsupported expression (only expressions starting with 'project.basedir' or 'project.rootDirectory' are supported).",
1568-
repository);
1569-
break;
1570-
}
1571-
}
1572-
}
1557+
validateRawRepository(problems, repository, prefix, prefix2, false);
15731558

15741559
String key = repository.getId();
15751560

@@ -1593,6 +1578,33 @@ private void validateRawRepositories(
15931578
}
15941579
}
15951580

1581+
private void validateRawRepository(
1582+
ModelProblemCollector problems,
1583+
Repository repository,
1584+
String prefix,
1585+
String prefix2,
1586+
boolean allowEmptyUrl) {
1587+
if (repository == null) {
1588+
return;
1589+
}
1590+
validateStringNotEmpty(
1591+
prefix, prefix2, "id", problems, Severity.ERROR, Version.V20, repository.getId(), null, repository);
1592+
1593+
if (!allowEmptyUrl) {
1594+
validateStringNotEmpty(
1595+
prefix,
1596+
prefix2,
1597+
"[" + repository.getId() + "].url",
1598+
problems,
1599+
Severity.ERROR,
1600+
Version.V20,
1601+
repository.getUrl(),
1602+
null,
1603+
repository);
1604+
// Repository URL interpolation is allowed; unresolved placeholders will fail later during resolution
1605+
}
1606+
}
1607+
15961608
private void validate20EffectiveRepository(
15971609
ModelProblemCollector problems, Repository repository, String prefix, int validationLevel) {
15981610
if (repository != null) {

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

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -365,7 +365,7 @@ void testEmptyPluginVersion() throws Exception {
365365
@Test
366366
void testMissingRepositoryId() throws Exception {
367367
SimpleProblemCollector result =
368-
validateFile("missing-repository-id-pom.xml", ModelValidator.VALIDATION_LEVEL_STRICT);
368+
validateRaw("missing-repository-id-pom.xml", ModelValidator.VALIDATION_LEVEL_STRICT);
369369

370370
assertViolations(result, 0, 4, 0);
371371

@@ -888,15 +888,21 @@ void testParentVersionRELEASE() throws Exception {
888888
@Test
889889
void repositoryWithExpression() throws Exception {
890890
SimpleProblemCollector result = validateFile("raw-model/repository-with-expression.xml");
891-
assertViolations(result, 0, 1, 0);
892-
assertEquals(
893-
"'repositories.repository.[repo].url' contains an unsupported expression (only expressions starting with 'project.basedir' or 'project.rootDirectory' are supported).",
894-
result.getErrors().get(0));
891+
// Interpolation in repository URLs is allowed; unresolved placeholders will fail later during resolution
892+
assertViolations(result, 0, 0, 0);
895893
}
896894

897895
@Test
898896
void repositoryWithBasedirExpression() throws Exception {
899897
SimpleProblemCollector result = validateRaw("raw-model/repository-with-basedir-expression.xml");
898+
// Repository URL interpolation is allowed; unresolved placeholders will fail later during resolution
899+
assertViolations(result, 0, 0, 0);
900+
}
901+
902+
@Test
903+
void repositoryWithUnsupportedExpression() throws Exception {
904+
SimpleProblemCollector result = validateRaw("raw-model/repository-with-unsupported-expression.xml");
905+
// Repository URL interpolation is allowed; unresolved placeholders will fail later during resolution
900906
assertViolations(result, 0, 0, 0);
901907
}
902908

‎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)