Skip to content

Commit 29d6b70

Browse files
Jenson3210Jenson3210
committed
Add sibling exclusion for new coordinates instead of replacing old ones
ChangeDependencyGroupIdAndArtifactId previously called ChangeExclusion to rename exclusions matching the old coordinates to the new ones. This was incorrect because the old exclusion exists to block the OLD vulnerable transitive dependency that other libraries may still pull in. Now, when changing dependency coordinates, existing exclusions for the old coordinates are preserved and a sibling exclusion for the new coordinates is added alongside them. This ensures both old and new transitive dependencies are excluded.
1 parent aab13f2 commit 29d6b70

2 files changed

Lines changed: 56 additions & 36 deletions

File tree

‎rewrite-maven/src/main/java/org/openrewrite/maven/ChangeDependencyGroupIdAndArtifactId.java‎

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,8 @@ public String getInstanceNameSuffix() {
122122
}
123123

124124
String description = "Change a Maven dependency coordinates. The `newGroupId` or `newArtifactId` **MUST** be different from before. " +
125-
"Matching `<dependencyManagement>` coordinates are also updated if a `newVersion` or `versionPattern` is provided.";
125+
"Matching `<dependencyManagement>` coordinates are also updated if a `newVersion` or `versionPattern` is provided. " +
126+
"Exclusions that reference the old dependency coordinates are preserved, and a sibling exclusion for the new coordinates is added alongside them.";
126127

127128
@Override
128129
public Validated<Object> validate() {
@@ -269,6 +270,36 @@ public Xml visitDocument(Xml.Document document, ExecutionContext ctx) {
269270
newArtifactId,
270271
newVersion, versionPattern).getVisitor());
271272
}
273+
// Add sibling exclusions for the new coordinates alongside existing old exclusions
274+
if (newGroupId != null || newArtifactId != null) {
275+
String effectiveNewGroupId = newGroupId != null ? newGroupId : oldGroupId;
276+
String effectiveNewArtifactId = newArtifactId != null ? newArtifactId : oldArtifactId;
277+
doAfterVisit(new MavenVisitor<ExecutionContext>() {
278+
@Override
279+
public Xml visitTag(Xml.Tag tag, ExecutionContext ctx) {
280+
Xml.Tag t = (Xml.Tag) super.visitTag(tag, ctx);
281+
if (!"exclusions".equals(t.getName())) {
282+
return t;
283+
}
284+
boolean hasOldExclusion = t.getChildren("exclusion").stream()
285+
.anyMatch(e -> matchesGlob(e.getChildValue("groupId").orElse(null), oldGroupId) &&
286+
matchesGlob(e.getChildValue("artifactId").orElse(null), oldArtifactId));
287+
boolean hasNewExclusion = t.getChildren("exclusion").stream()
288+
.anyMatch(e -> effectiveNewGroupId.equals(e.getChildValue("groupId").orElse(null)) &&
289+
effectiveNewArtifactId.equals(e.getChildValue("artifactId").orElse(null)));
290+
if (hasOldExclusion && !hasNewExclusion) {
291+
t = (Xml.Tag) new AddToTagVisitor<>(t, Xml.Tag.build(
292+
"<exclusion>\n" +
293+
"<groupId>" + effectiveNewGroupId + "</groupId>\n" +
294+
"<artifactId>" + effectiveNewArtifactId + "</artifactId>\n" +
295+
"</exclusion>"))
296+
.visitNonNull(t, ctx, getCursor().getParentOrThrow());
297+
maybeUpdateModel();
298+
}
299+
return t;
300+
}
301+
});
302+
}
272303
return super.visitDocument(document, ctx);
273304
}
274305

‎rewrite-maven/src/test/java/org/openrewrite/maven/ChangeDependencyGroupIdAndArtifactIdTest.java‎

Lines changed: 24 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -3489,11 +3489,10 @@ void versionNotDroppedWhenSwaggerMigratedFirst() {
34893489

34903490
@Issue("https://github.com/moderneinc/customer-requests/issues/1330")
34913491
@Test
3492-
void exclusionNotUpdatedWhenDependencyGroupIdChanges() {
3492+
void exclusionPreservedAndSiblingAddedWhenDependencyGroupIdChanges() {
34933493
// When changing a dependency's groupId/artifactId, exclusions that match
3494-
// the old coordinates should NOT be updated. The exclusion exists to block
3495-
// the old transitive dependency, and other libraries may still depend on
3496-
// the old artifact transitively.
3494+
// the old coordinates should be preserved, and a sibling exclusion for the
3495+
// new coordinates should be added alongside them.
34973496
rewriteRun(
34983497
spec -> spec.recipe(new ChangeDependencyGroupIdAndArtifactId(
34993498
"com.fasterxml.jackson.jaxrs",
@@ -3532,19 +3531,26 @@ void exclusionNotUpdatedWhenDependencyGroupIdChanges() {
35323531
</project>
35333532
""",
35343533
spec -> spec.after(actual -> assertThat(actual)
3535-
.containsPattern("<groupId>com\\.fasterxml\\.jackson\\.jakarta\\.rs</groupId>\\s*<artifactId>jackson-jakarta-rs-json-provider</artifactId>\\s*<version>2\\.\\d+\\.\\d+</version>")
3534+
// Old exclusion preserved
3535+
.contains("<groupId>com.fasterxml.jackson.jaxrs</groupId>")
3536+
.contains("<artifactId>jackson-jaxrs-json-provider</artifactId>")
3537+
// Sibling exclusion added for new coordinates
3538+
.contains("<groupId>com.fasterxml.jackson.jakarta.rs</groupId>")
3539+
.contains("<artifactId>jackson-jakarta-rs-json-provider</artifactId>")
3540+
// Version updated
3541+
.containsPattern("<version>2\\.\\d+\\.\\d+</version>")
35363542
.doesNotContain("<version>2.9.7</version>")
35373543
.actual())
35383544
)
35393545
);
35403546
}
35413547

35423548
@Test
3543-
void exclusionsForOldDependencyShouldNotBeChanged() {
3549+
void exclusionPreservedAndSiblingAddedForNewCoordinates() {
35443550
// When changing commons-lang:commons-lang to org.apache.commons:commons-lang3,
3545-
// exclusions for commons-lang:commons-lang in OTHER dependencies should NOT be updated.
3546-
// The exclusion exists to block the old vulnerable transitive dependency, and
3547-
// other libraries may still transitively depend on commons-lang:commons-lang.
3551+
// the old exclusion for commons-lang:commons-lang should be preserved (to block
3552+
// the vulnerable transitive dependency), and a sibling exclusion for the new
3553+
// coordinates should be added alongside it.
35483554
rewriteRun(
35493555
spec -> spec.recipe(new ChangeDependencyGroupIdAndArtifactId(
35503556
"commons-lang",
@@ -3581,32 +3587,15 @@ void exclusionsForOldDependencyShouldNotBeChanged() {
35813587
</dependencies>
35823588
</project>
35833589
""",
3584-
"""
3585-
<project>
3586-
<modelVersion>4.0.0</modelVersion>
3587-
<groupId>com.mycompany.app</groupId>
3588-
<artifactId>my-app</artifactId>
3589-
<version>1</version>
3590-
<dependencies>
3591-
<dependency>
3592-
<groupId>org.apache.commons</groupId>
3593-
<artifactId>commons-lang3</artifactId>
3594-
<version>3.20.0</version>
3595-
</dependency>
3596-
<dependency>
3597-
<groupId>org.apache.struts</groupId>
3598-
<artifactId>struts2-core</artifactId>
3599-
<version>2.5.30</version>
3600-
<exclusions>
3601-
<exclusion>
3602-
<groupId>commons-lang</groupId>
3603-
<artifactId>commons-lang</artifactId>
3604-
</exclusion>
3605-
</exclusions>
3606-
</dependency>
3607-
</dependencies>
3608-
</project>
3609-
"""
3590+
spec -> spec.after(actual -> assertThat(actual)
3591+
// Direct dependency changed to commons-lang3
3592+
.containsPattern("<groupId>org\\.apache\\.commons</groupId>\\s*<artifactId>commons-lang3</artifactId>\\s*<version>3\\.\\d+\\.\\d+</version>")
3593+
.doesNotContain("<version>2.6</version>")
3594+
// Old exclusion preserved
3595+
.containsPattern("<exclusion>\\s*<groupId>commons-lang</groupId>\\s*<artifactId>commons-lang</artifactId>\\s*</exclusion>")
3596+
// Sibling exclusion added for new coordinates
3597+
.containsPattern("<exclusion>\\s*<groupId>org\\.apache\\.commons</groupId>\\s*<artifactId>commons-lang3</artifactId>\\s*</exclusion>")
3598+
.actual())
36103599
)
36113600
);
36123601
}

0 commit comments

Comments
 (0)
⚡