Skip to content

Commit 74b04b7

Browse files
gnodetclaude
andcommitted
mvnup: comment out repositories with undefined property expressions (PR #12204)
Cherry-pick from fix/filter-unresolvable-project-repos branch. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 52f382b commit 74b04b7

2 files changed

Lines changed: 259 additions & 0 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 {

0 commit comments

Comments
 (0)