Skip to content

Commit 210dbdc

Browse files
gnodetgnodet
authored
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.
1 parent d4b7e42 commit 210dbdc

File tree

9 files changed

+445
-78
lines changed

9 files changed

+445
-78
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: 77 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,44 @@ 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+
// Check for uninterpolated expressions - these should have been interpolated by now
1605+
Matcher matcher = EXPRESSION_NAME_PATTERN.matcher(repository.getUrl());
1606+
if (matcher.find()) {
1607+
addViolation(
1608+
problems,
1609+
Severity.ERROR,
1610+
Version.V40,
1611+
prefix + prefix2 + "[" + repository.getId() + "].url",
1612+
null,
1613+
"contains an uninterpolated expression.",
1614+
repository);
1615+
}
1616+
}
1617+
}
1618+
15961619
private void validate20EffectiveRepository(
15971620
ModelProblemCollector problems, Repository repository, String prefix, int validationLevel) {
15981621
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
@@ -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,16 +888,23 @@ 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");
900-
assertViolations(result, 0, 0, 0);
898+
// This test runs on raw model without interpolation, so all expressions appear uninterpolated
899+
// In the real flow, supported expressions would be interpolated before validation
900+
assertViolations(result, 0, 3, 0);
901+
}
902+
903+
@Test
904+
void repositoryWithUnsupportedExpression() throws Exception {
905+
SimpleProblemCollector result = validateRaw("raw-model/repository-with-unsupported-expression.xml");
906+
// Unsupported expressions should cause validation errors
907+
assertViolations(result, 0, 1, 0);
901908
}
902909

903910
@Test

0 commit comments

Comments
 (0)