Skip to content

Commit 3c62e2e

Browse files
gnodetclaude
gnodet
and
claude
authored
Filter project repos with uninterpolated property expressions (#12204) (#12229)
* mvnup: comment out repositories with undefined property expressions Projects like uima-uimaj, opennlp-sandbox, and seatunnel-shade define repositories with ${eclipseP2RepoId} in the repo ID. This property is never defined — it was used as a literal string in Maven 3. Maven 4 rejects it with IllegalArgumentException at runtime. Extend the mvnup compatibility fix strategy to detect and comment out repositories and plugin repositories whose id or url contain undefined property expressions, following the same pattern used for dependencies (#12080). Handles both root-level and profile-level repositories. * Filter project repositories with uninterpolated property expressions When a project POM defines repositories with ${...} expressions that cannot be resolved (e.g. ${eclipseP2RepoId}), Maven 4 previously failed with InternalErrorException. This change downgrades the model validation from ERROR to WARNING and filters such repositories before they reach the resolver, logging a warning instead of failing the build. --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent b9eadbd commit 3c62e2e

7 files changed

Lines changed: 297 additions & 31 deletions

File tree

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

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -149,6 +149,7 @@ public UpgradeResult doApply(UpgradeContext context, Map<Path, Document> pomMap)
149149
hasIssues |= fixUnsupportedRepositoryExpressions(pomDocument, context);
150150
hasIssues |= fixIncorrectParentRelativePaths(pomDocument, pomPath, pomMap, context);
151151
hasIssues |= fixUndefinedPropertyExpressions(pomDocument, allDefinedProperties, context);
152+
hasIssues |= fixUndefinedPropertyExpressionsInRepositories(pomDocument, allDefinedProperties, context);
152153

153154
if (hasIssues) {
154155
context.success("Maven 4 compatibility issues fixed");
@@ -429,6 +430,89 @@ private boolean fixUndefinedPropertyExpressions(
429430
.reduce(false, Boolean::logicalOr);
430431
}
431432

433+
/**
434+
* Fixes repositories with undefined property expressions by commenting them out.
435+
*/
436+
private boolean fixUndefinedPropertyExpressionsInRepositories(
437+
Document pomDocument, Set<String> allDefinedProperties, UpgradeContext context) {
438+
Element root = pomDocument.root();
439+
440+
Stream<RepositoryContainer> repositoryContainers = Stream.concat(
441+
Stream.of(
442+
new RepositoryContainer(
443+
root.childElement(REPOSITORIES).orElse(null), REPOSITORY, REPOSITORIES),
444+
new RepositoryContainer(
445+
root.childElement(PLUGIN_REPOSITORIES).orElse(null),
446+
PLUGIN_REPOSITORY,
447+
PLUGIN_REPOSITORIES))
448+
.filter(c -> c.element != null),
449+
root.childElement(PROFILES).stream()
450+
.flatMap(profiles -> profiles.childElements(PROFILE))
451+
.flatMap(profile -> Stream.of(
452+
new RepositoryContainer(
453+
profile.childElement(REPOSITORIES)
454+
.orElse(null),
455+
REPOSITORY,
456+
"profile repositories"),
457+
new RepositoryContainer(
458+
profile.childElement(PLUGIN_REPOSITORIES)
459+
.orElse(null),
460+
PLUGIN_REPOSITORY,
461+
"profile pluginRepositories"))
462+
.filter(c -> c.element != null)));
463+
464+
return repositoryContainers
465+
.map(c -> fixUndefinedPropertyExpressionsInRepositorySection(
466+
c.element, c.elementType, allDefinedProperties, pomDocument, context, c.sectionName))
467+
.reduce(false, Boolean::logicalOr);
468+
}
469+
470+
private record RepositoryContainer(Element element, String elementType, String sectionName) {}
471+
472+
private boolean fixUndefinedPropertyExpressionsInRepositorySection(
473+
Element repositoriesElement,
474+
String elementType,
475+
Set<String> allDefinedProperties,
476+
Document pomDocument,
477+
UpgradeContext context,
478+
String sectionName) {
479+
boolean fixed = false;
480+
List<Element> repositories =
481+
repositoriesElement.childElements(elementType).toList();
482+
Editor editor = new Editor(pomDocument);
483+
484+
for (Element repository : repositories) {
485+
Set<String> undefinedProps = findUndefinedPropertiesInRepository(repository, allDefinedProperties);
486+
if (!undefinedProps.isEmpty()) {
487+
String propLabel = undefinedProps.size() > 1 ? "properties" : "property";
488+
String propsStr = "'" + String.join("', '", undefinedProps) + "'";
489+
490+
Comment comment = editor.commentOutElement(repository);
491+
String elementXml = comment.content().trim();
492+
comment.content(
493+
" mvnup: commented out - undefined " + propLabel + " " + propsStr + "\n" + elementXml + " ");
494+
495+
context.detail("Fixed: Commented out " + elementType + " with undefined " + propLabel + " " + propsStr
496+
+ " in " + sectionName);
497+
fixed = true;
498+
}
499+
}
500+
501+
return fixed;
502+
}
503+
504+
private Set<String> findUndefinedPropertiesInRepository(Element repository, Set<String> allDefinedProperties) {
505+
Set<String> undefinedProperties = new HashSet<>();
506+
507+
String id = repository.childText("id");
508+
String url = repository.childText("url");
509+
510+
collectUndefinedExpressions(id, allDefinedProperties, undefinedProperties);
511+
collectUndefinedExpressions(url, allDefinedProperties, undefinedProperties);
512+
513+
return undefinedProperties;
514+
}
515+
432516
/**
433517
* Fixes undefined property expressions in a specific dependencies section.
434518
*/

‎impl/maven-cli/src/test/java/org/apache/maven/cling/invoker/mvnup/goals/CompatibilityFixStrategyTest.java‎

Lines changed: 175 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1457,6 +1457,181 @@ void shouldCommentOutWhenPropertyTrulyUndefinedInEffectiveModel() throws Excepti
14571457
}
14581458
}
14591459

1460+
@Nested
1461+
@DisplayName("Undefined Property Expression in Repositories Fixes")
1462+
class UndefinedPropertyExpressionInRepositoriesTests {
1463+
1464+
@Test
1465+
@DisplayName("should comment out repository with undefined property in id")
1466+
void shouldCommentOutRepositoryWithUndefinedPropertyInId() throws Exception {
1467+
String pomXml = """
1468+
<?xml version="1.0" encoding="UTF-8"?>
1469+
<project xmlns="http://maven.apache.org/POM/4.0.0">
1470+
<modelVersion>4.0.0</modelVersion>
1471+
<groupId>test</groupId>
1472+
<artifactId>test</artifactId>
1473+
<version>1.0.0</version>
1474+
<repositories>
1475+
<repository>
1476+
<id>${eclipseP2RepoId}</id>
1477+
<url>https://repo.example.com</url>
1478+
</repository>
1479+
</repositories>
1480+
</project>
1481+
""";
1482+
1483+
Document document = Document.of(pomXml);
1484+
Map<Path, Document> pomMap = Map.of(Paths.get("pom.xml"), document);
1485+
1486+
UpgradeContext context = createMockContext();
1487+
UpgradeResult result = strategy.doApply(context, pomMap);
1488+
1489+
assertTrue(result.success(), "Compatibility fix should succeed");
1490+
assertTrue(result.modifiedCount() > 0, "Should have commented out repository");
1491+
1492+
String xml = DomUtils.toXml(document);
1493+
assertTrue(xml.contains("mvnup: commented out"), "Should contain comment-out marker");
1494+
assertTrue(xml.contains("eclipseP2RepoId"), "Should mention the undefined property");
1495+
1496+
Element root = document.root();
1497+
Element repos = DomUtils.findChildElement(root, "repositories");
1498+
assertEquals(0, repos.childElements("repository").count(), "Should have no repository elements");
1499+
}
1500+
1501+
@Test
1502+
@DisplayName("should comment out repository with undefined property in url")
1503+
void shouldCommentOutRepositoryWithUndefinedPropertyInUrl() throws Exception {
1504+
String pomXml = """
1505+
<?xml version="1.0" encoding="UTF-8"?>
1506+
<project xmlns="http://maven.apache.org/POM/4.0.0">
1507+
<modelVersion>4.0.0</modelVersion>
1508+
<groupId>test</groupId>
1509+
<artifactId>test</artifactId>
1510+
<version>1.0.0</version>
1511+
<repositories>
1512+
<repository>
1513+
<id>my-repo</id>
1514+
<url>${undefinedBaseUrl}/releases</url>
1515+
</repository>
1516+
</repositories>
1517+
</project>
1518+
""";
1519+
1520+
Document document = Document.of(pomXml);
1521+
Map<Path, Document> pomMap = Map.of(Paths.get("pom.xml"), document);
1522+
1523+
UpgradeContext context = createMockContext();
1524+
UpgradeResult result = strategy.doApply(context, pomMap);
1525+
1526+
assertTrue(result.success(), "Compatibility fix should succeed");
1527+
assertTrue(result.modifiedCount() > 0, "Should have commented out repository");
1528+
1529+
String xml = DomUtils.toXml(document);
1530+
assertTrue(xml.contains("mvnup: commented out"), "Should contain comment-out marker");
1531+
assertTrue(xml.contains("undefinedBaseUrl"), "Should mention the undefined property");
1532+
}
1533+
1534+
@Test
1535+
@DisplayName("should not comment out repository with defined property")
1536+
void shouldNotCommentOutRepositoryWithDefinedProperty() throws Exception {
1537+
String pomXml = """
1538+
<?xml version="1.0" encoding="UTF-8"?>
1539+
<project xmlns="http://maven.apache.org/POM/4.0.0">
1540+
<modelVersion>4.0.0</modelVersion>
1541+
<groupId>test</groupId>
1542+
<artifactId>test</artifactId>
1543+
<version>1.0.0</version>
1544+
<properties>
1545+
<repoId>my-custom-repo</repoId>
1546+
</properties>
1547+
<repositories>
1548+
<repository>
1549+
<id>${repoId}</id>
1550+
<url>https://repo.example.com</url>
1551+
</repository>
1552+
</repositories>
1553+
</project>
1554+
""";
1555+
1556+
Document document = Document.of(pomXml);
1557+
Map<Path, Document> pomMap = Map.of(Paths.get("pom.xml"), document);
1558+
1559+
UpgradeContext context = createMockContext();
1560+
strategy.doApply(context, pomMap);
1561+
1562+
Element root = document.root();
1563+
Element repos = DomUtils.findChildElement(root, "repositories");
1564+
assertEquals(1, repos.childElements("repository").count(), "Repository should still be present");
1565+
}
1566+
1567+
@Test
1568+
@DisplayName("should comment out plugin repository with undefined property")
1569+
void shouldCommentOutPluginRepositoryWithUndefinedProperty() throws Exception {
1570+
String pomXml = """
1571+
<?xml version="1.0" encoding="UTF-8"?>
1572+
<project xmlns="http://maven.apache.org/POM/4.0.0">
1573+
<modelVersion>4.0.0</modelVersion>
1574+
<groupId>test</groupId>
1575+
<artifactId>test</artifactId>
1576+
<version>1.0.0</version>
1577+
<pluginRepositories>
1578+
<pluginRepository>
1579+
<id>${eclipseP2RepoId}</id>
1580+
<url>https://plugins.example.com</url>
1581+
</pluginRepository>
1582+
</pluginRepositories>
1583+
</project>
1584+
""";
1585+
1586+
Document document = Document.of(pomXml);
1587+
Map<Path, Document> pomMap = Map.of(Paths.get("pom.xml"), document);
1588+
1589+
UpgradeContext context = createMockContext();
1590+
UpgradeResult result = strategy.doApply(context, pomMap);
1591+
1592+
assertTrue(result.success(), "Compatibility fix should succeed");
1593+
assertTrue(result.modifiedCount() > 0, "Should have commented out plugin repository");
1594+
1595+
String xml = DomUtils.toXml(document);
1596+
assertTrue(xml.contains("mvnup: commented out"), "Should contain comment-out marker");
1597+
1598+
Element root = document.root();
1599+
Element repos = DomUtils.findChildElement(root, "pluginRepositories");
1600+
assertEquals(
1601+
0, repos.childElements("pluginRepository").count(), "Should have no pluginRepository elements");
1602+
}
1603+
1604+
@Test
1605+
@DisplayName("should not comment out repository with well-known property")
1606+
void shouldNotCommentOutRepositoryWithWellKnownProperty() throws Exception {
1607+
String pomXml = """
1608+
<?xml version="1.0" encoding="UTF-8"?>
1609+
<project xmlns="http://maven.apache.org/POM/4.0.0">
1610+
<modelVersion>4.0.0</modelVersion>
1611+
<groupId>test</groupId>
1612+
<artifactId>test</artifactId>
1613+
<version>1.0.0</version>
1614+
<repositories>
1615+
<repository>
1616+
<id>local-repo</id>
1617+
<url>file://${project.basedir}/repo</url>
1618+
</repository>
1619+
</repositories>
1620+
</project>
1621+
""";
1622+
1623+
Document document = Document.of(pomXml);
1624+
Map<Path, Document> pomMap = Map.of(Paths.get("pom.xml"), document);
1625+
1626+
UpgradeContext context = createMockContext();
1627+
strategy.doApply(context, pomMap);
1628+
1629+
Element root = document.root();
1630+
Element repos = DomUtils.findChildElement(root, "repositories");
1631+
assertEquals(1, repos.childElements("repository").count(), "Repository should still be present");
1632+
}
1633+
}
1634+
14601635
@Nested
14611636
@DisplayName("Strategy Description")
14621637
class StrategyDescriptionTests {

‎impl/maven-core/src/main/java/org/apache/maven/project/DefaultProjectBuildingHelper.java‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,13 @@ public List<ArtifactRepository> createArtifactRepositories(
9292
List<ArtifactRepository> internalRepositories = new ArrayList<>();
9393

9494
for (Repository repository : pomRepositories) {
95+
if (containsExpression(repository.getId()) || containsExpression(repository.getUrl())) {
96+
logger.warn(
97+
"Skipping repository '{}' (url: '{}') containing an uninterpolated property expression",
98+
repository.getId(),
99+
repository.getUrl());
100+
continue;
101+
}
95102
internalRepositories.add(MavenRepositorySystem.buildArtifactRepository(repository));
96103
}
97104

@@ -267,6 +274,10 @@ public void selectProjectRealm(MavenProject project) {
267274
Thread.currentThread().setContextClassLoader(projectRealm);
268275
}
269276

277+
private static boolean containsExpression(String value) {
278+
return value != null && value.contains("${");
279+
}
280+
270281
private List<org.eclipse.aether.artifact.Artifact> toAetherArtifacts(final List<Artifact> pluginArtifacts) {
271282
return new ArrayList<>(RepositoryUtils.toArtifacts(pluginArtifacts));
272283
}

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1535,11 +1535,11 @@ private void validateRawRepository(
15351535
if (matcher.find()) {
15361536
addViolation(
15371537
problems,
1538-
Severity.ERROR,
1538+
Severity.WARNING,
15391539
Version.V40,
15401540
prefix + prefix2 + "[" + repository.getId() + "].id",
15411541
null,
1542-
"contains an uninterpolated expression.",
1542+
"contains an uninterpolated expression; the repository will be skipped.",
15431543
repository);
15441544
}
15451545
}
@@ -1561,11 +1561,11 @@ && validateStringNotEmpty(
15611561
if (matcher.find()) {
15621562
addViolation(
15631563
problems,
1564-
Severity.ERROR,
1564+
Severity.WARNING,
15651565
Version.V40,
15661566
prefix + prefix2 + "[" + repository.getId() + "].url",
15671567
null,
1568-
"contains an uninterpolated expression.",
1568+
"contains an uninterpolated expression; the repository will be skipped.",
15691569
repository);
15701570
}
15711571
}

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

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -907,31 +907,33 @@ void repositoryWithBasedirExpression() throws Exception {
907907
SimpleProblemCollector result = validateRaw("raw-model/repository-with-basedir-expression.xml");
908908
// This test runs on raw model without interpolation, so all expressions appear uninterpolated
909909
// In the real flow, supported expressions would be interpolated before validation
910-
assertViolations(result, 0, 3, 0);
910+
assertViolations(result, 0, 0, 3);
911911
}
912912

913913
@Test
914914
void repositoryWithUnsupportedExpression() throws Exception {
915915
SimpleProblemCollector result = validateRaw("raw-model/repository-with-unsupported-expression.xml");
916-
// Unsupported expressions should cause validation errors
917-
assertViolations(result, 0, 1, 0);
916+
// Unsupported expressions should cause validation warnings (repos will be skipped at build time)
917+
assertViolations(result, 0, 0, 1);
918918
}
919919

920920
@Test
921921
void repositoryWithUninterpolatedId() throws Exception {
922922
SimpleProblemCollector result = validateRaw("raw-model/repository-with-uninterpolated-id.xml");
923-
// Uninterpolated expressions in repository IDs should cause validation errors
923+
// Uninterpolated expressions in repository IDs should cause validation warnings
924+
// (repos will be skipped at build time)
924925
// distributionManagement repositories skip expression check since parent properties
925926
// may not be available at file model validation stage
926-
assertViolations(result, 0, 2, 0);
927+
assertViolations(result, 0, 0, 2);
927928

928-
// Check that repository ID validation errors are present for repositories and pluginRepositories
929-
assertTrue(result.getErrors().stream()
930-
.anyMatch(error -> error.contains("repositories.repository.[${repository.id}].id")
931-
&& error.contains("contains an uninterpolated expression")));
932-
assertTrue(result.getErrors().stream()
933-
.anyMatch(error -> error.contains("pluginRepositories.pluginRepository.[${plugin.repository.id}].id")
934-
&& error.contains("contains an uninterpolated expression")));
929+
// Check that repository ID validation warnings are present for repositories and pluginRepositories
930+
assertTrue(result.getWarnings().stream()
931+
.anyMatch(warning -> warning.contains("repositories.repository.[${repository.id}].id")
932+
&& warning.contains("contains an uninterpolated expression")));
933+
assertTrue(result.getWarnings().stream()
934+
.anyMatch(
935+
warning -> warning.contains("pluginRepositories.pluginRepository.[${plugin.repository.id}].id")
936+
&& warning.contains("contains an uninterpolated expression")));
935937
}
936938

937939
@Test

0 commit comments

Comments
 (0)