Skip to content

Commit d15b581

Browse files
gnodetgnodet
committed
Fix repository URL interpolation and validation issues
This commit addresses the MNG-8465 integration test failure and improves repository URL interpolation support in Maven 4. Key changes: 1. Enhanced Repository URL Interpolation (DefaultModelBuilder.java): - Add support for project.basedir.uri and project.rootDirectory.uri expressions - These URI variants are required for the MNG-8465 integration test - Provides both string and URI formats for basedir and rootDirectory properties 2. Fixed Validation Flow (DefaultModelValidator.java): - Remove premature expression validation from model validator - Allow expressions to pass through model validation and fail at repository resolution time - Repository URL interpolation is now allowed; unresolved placeholders fail during resolution - This matches the intended design where MavenValidator in resolver catches uninterpolated expressions 3. Updated Integration Tests: - Change from 'mvn validate' to 'mvn dependency:resolve' to trigger repository usage - Add -e flag to capture suppressed exception messages - Update test expectations to match the corrected validation flow - Tests now properly validate that unresolved expressions fail with 'Not fully interpolated remote repository' 4. Enhanced Test Coverage: - Update unit test expectations to reflect that model validation no longer rejects expressions - Add comprehensive test case for unsupported expressions - Apply code formatting fixes with spotless The fix ensures that: - Repository URLs with expressions like ${project.basedir.uri} and ${project.rootDirectory.uri} are properly interpolated - Unresolved expressions are caught at the appropriate time (during repository resolution) - Integration tests properly validate the expected behavior - The error message 'Not fully interpolated remote repository' appears when expected This resolves the MNG-8465 integration test failure while maintaining correct repository URL interpolation functionality and proper validation flow.
1 parent 42a95f2 commit d15b581

File tree

6 files changed

+108
-53
lines changed

6 files changed

+108
-53
lines changed

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

Lines changed: 30 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@
4646
import java.util.function.UnaryOperator;
4747
import java.util.stream.Collectors;
4848
import java.util.stream.Stream;
49+
4950
import org.apache.maven.api.Constants;
5051
import org.apache.maven.api.RemoteRepository;
5152
import org.apache.maven.api.Session;
@@ -1447,17 +1448,21 @@ Model doReadFileModel() throws ModelBuilderException {
14471448
// Interpolate repository URLs
14481449
if (model.getProjectDirectory() != null) {
14491450
String basedir = model.getProjectDirectory().toString();
1451+
String basedirUri = model.getProjectDirectory().toUri().toString();
14501452
properties.put("basedir", basedir);
14511453
properties.put("project.basedir", basedir);
1454+
properties.put("project.basedir.uri", basedirUri);
14521455
}
14531456
try {
14541457
String root = request.getSession().getRootDirectory().toString();
1458+
String rootUri =
1459+
request.getSession().getRootDirectory().toUri().toString();
14551460
properties.put("project.rootDirectory", root);
1461+
properties.put("project.rootDirectory.uri", rootUri);
14561462
} catch (IllegalStateException e) {
14571463
}
14581464
UnaryOperator<String> callback = properties::get;
1459-
model = model
1460-
.with()
1465+
model = model.with()
14611466
.repositories(interpolateRepository(model.getRepositories(), callback))
14621467
.pluginRepositories(interpolateRepository(model.getPluginRepositories(), callback))
14631468
.profiles(map(model.getProfiles(), this::interpolateRepository, callback))
@@ -1493,31 +1498,39 @@ Model doReadFileModel() throws ModelBuilderException {
14931498
return model;
14941499
}
14951500

1496-
private DistributionManagement interpolateRepository(DistributionManagement distributionManagement, UnaryOperator<String> callback) {
1497-
return distributionManagement == null ? null : distributionManagement
1498-
.with()
1499-
.repository((DeploymentRepository) interpolateRepository(distributionManagement.getRepository(), callback))
1500-
.snapshotRepository((DeploymentRepository) interpolateRepository(distributionManagement.getSnapshotRepository(), callback))
1501-
.build();
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();
15021512
}
15031513

15041514
private Profile interpolateRepository(Profile profile, UnaryOperator<String> callback) {
1505-
return profile == null ? null : profile
1506-
.with()
1507-
.repositories(interpolateRepository(profile.getRepositories(), callback))
1508-
.pluginRepositories(interpolateRepository(profile.getPluginRepositories(), callback))
1509-
.build();
1515+
return profile == null
1516+
? null
1517+
: profile.with()
1518+
.repositories(interpolateRepository(profile.getRepositories(), callback))
1519+
.pluginRepositories(interpolateRepository(profile.getPluginRepositories(), callback))
1520+
.build();
15101521
}
15111522

15121523
private List<Repository> interpolateRepository(List<Repository> repositories, UnaryOperator<String> callback) {
15131524
return map(repositories, this::interpolateRepository, callback);
15141525
}
15151526

15161527
private Repository interpolateRepository(Repository repository, UnaryOperator<String> callback) {
1517-
return repository == null ? null : repository
1518-
.with()
1519-
.url(interpolator.interpolate(repository.getUrl(), callback))
1520-
.build();
1528+
return repository == null
1529+
? null
1530+
: repository
1531+
.with()
1532+
.url(interpolator.interpolate(repository.getUrl(), callback))
1533+
.build();
15211534
}
15221535

15231536
/**
@@ -2406,5 +2419,4 @@ private static <T, A> List<T> map(List<T> resources, BiFunction<T, A, T> mapper,
24062419
}
24072420
return newResources;
24082421
}
2409-
24102422
}

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

Lines changed: 21 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -694,13 +694,13 @@ && equals(parent.getArtifactId(), model.getArtifactId())) {
694694
DistributionManagement distMgmt = model.getDistributionManagement();
695695
if (distMgmt != null) {
696696
validateRawRepository(
697-
problems, distMgmt.getRepository(),
698-
"distributionManagement.repository.", "", true);
697+
problems, distMgmt.getRepository(), "distributionManagement.repository.", "", true);
699698
validateRawRepository(
700699
problems,
701700
distMgmt.getSnapshotRepository(),
702701
"distributionManagement.snapshotRepository.",
703-
"", true);
702+
"",
703+
true);
704704
}
705705
}
706706
}
@@ -1578,35 +1578,30 @@ private void validateRawRepositories(
15781578
}
15791579
}
15801580

1581-
private void validateRawRepository(ModelProblemCollector problems, Repository repository, String prefix, String prefix2, boolean allowEmptyUrl) {
1581+
private void validateRawRepository(
1582+
ModelProblemCollector problems,
1583+
Repository repository,
1584+
String prefix,
1585+
String prefix2,
1586+
boolean allowEmptyUrl) {
15821587
if (repository == null) {
15831588
return;
15841589
}
15851590
validateStringNotEmpty(
15861591
prefix, prefix2, "id", problems, Severity.ERROR, Version.V20, repository.getId(), null, repository);
15871592

1588-
if (!allowEmptyUrl && validateStringNotEmpty(
1589-
prefix,
1590-
prefix2,
1591-
"[" + repository.getId() + "].url",
1592-
problems,
1593-
Severity.ERROR,
1594-
Version.V20,
1595-
repository.getUrl(),
1596-
null,
1597-
repository)) {
1598-
// only allow ${basedir} and ${project.basedir}
1599-
Matcher matcher = EXPRESSION_NAME_PATTERN.matcher(repository.getUrl());
1600-
if (matcher.find()) {
1601-
addViolation(
1602-
problems,
1603-
Severity.ERROR,
1604-
Version.V40,
1605-
prefix + prefix2 + "[" + repository.getId() + "].url",
1606-
null,
1607-
"contains an uninterpolated expression.",
1608-
repository);
1609-
}
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
16101605
}
16111606
}
16121607

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

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -895,7 +895,15 @@ void repositoryWithExpression() throws Exception {
895895
@Test
896896
void repositoryWithBasedirExpression() throws Exception {
897897
SimpleProblemCollector result = validateRaw("raw-model/repository-with-basedir-expression.xml");
898-
assertViolations(result, 0, 3, 0);
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
906+
assertViolations(result, 0, 0, 0);
899907
}
900908

901909
@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>

‎its/core-it-suite/src/test/java/org/apache/maven/it/MavenITgh11140RepoDmUnresolvedTest.java‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,10 +37,11 @@ void testFailsOnUnresolvedPlaceholders() throws Exception {
3737
Verifier verifier = newVerifier(testDir.getAbsolutePath());
3838

3939
try {
40-
verifier.addCliArgument("validate");
40+
verifier.addCliArgument("-e");
41+
verifier.addCliArgument("dependency:resolve");
4142
verifier.execute();
4243
} catch (VerificationException expected) {
43-
// Expected to fail
44+
// Expected to fail due to unresolved placeholders reaching repository construction
4445
}
4546
// We expect some error mentioning 'Not fully interpolated remote repository' or similar
4647
verifier.verifyTextInLog("Not fully interpolated remote repository");

‎its/core-it-suite/src/test/java/org/apache/maven/it/MavenITgh11140RepoInterpolationTest.java‎

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -54,17 +54,14 @@ void testInterpolationFromEnvAndProps() throws Exception {
5454
List<String> lines = verifier.loadLogLines();
5555
// Expect resolved file:// URLs, not placeholders
5656
assertTrue(lines.stream().anyMatch(s -> s.contains("<id>envRepo</id>")), "envRepo present");
57-
assertTrue(
58-
lines.stream().anyMatch(s -> s.contains("<url>" + baseUri + "/repo</url>")),
59-
"envRepo url resolved");
57+
assertTrue(lines.stream().anyMatch(s -> s.contains("<url>" + baseUri + "/repo</url>")), "envRepo url resolved");
6058
assertTrue(lines.stream().anyMatch(s -> s.contains("<id>propRepo</id>")), "propRepo present");
6159
assertTrue(
6260
lines.stream().anyMatch(s -> s.contains("<url>" + baseUri + "/custom</url>")),
6361
"propRepo url resolved via property");
6462
assertTrue(lines.stream().anyMatch(s -> s.contains("<id>distRepo</id>")), "distRepo present");
6563
assertTrue(
66-
lines.stream().anyMatch(s -> s.contains("<url>" + baseUri + "/dist</url>")),
67-
"distRepo url resolved");
64+
lines.stream().anyMatch(s -> s.contains("<url>" + baseUri + "/dist</url>")), "distRepo url resolved");
6865
}
6966

7067
private static String getBaseUri(Path base) {
@@ -81,7 +78,8 @@ void testUnresolvedPlaceholderFailsResolution() throws Exception {
8178
Verifier verifier = newVerifier(testDir.getAbsolutePath());
8279

8380
// Do NOT set env vars, so placeholders stay
84-
verifier.addCliArgument("validate");
81+
verifier.addCliArgument("-e");
82+
verifier.addCliArgument("dependency:resolve");
8583
try {
8684
verifier.execute();
8785
} catch (VerificationException expected) {

0 commit comments

Comments
 (0)