Skip to content

Commit 6bc827f

Browse files
timtebeektimtebeek
committed
Fix glob pattern leaking as newArtifactId in managed dependency delegation
ChangeDependencyGroupIdAndArtifactId was passing glob patterns (e.g. "swagger-*") as concrete newArtifactId/newGroupId values when delegating to ChangeManagedDependencyGroupIdAndArtifactId. Fix the root cause by making newGroupId/newArtifactId nullable (null = preserve existing value) and passing them directly instead of falling back to old glob values. Also move the duplicate dependency check from visitDocument into visitTag so it uses the actual tag coordinates rather than potentially-null or glob new values.
1 parent 4635b38 commit 6bc827f

4 files changed

Lines changed: 104 additions & 24 deletions

File tree

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -266,8 +266,8 @@ public Xml visitDocument(Xml.Document document, ExecutionContext ctx) {
266266
if (configuredToChangeManagedDependency) {
267267
doAfterVisit(new ChangeManagedDependencyGroupIdAndArtifactId(
268268
oldGroupId, oldArtifactId,
269-
Optional.ofNullable(newGroupId).orElse(oldGroupId),
270-
Optional.ofNullable(newArtifactId).orElse(oldArtifactId),
269+
newGroupId,
270+
newArtifactId,
271271
newVersion, versionPattern).getVisitor());
272272
}
273273
// Update any exclusions that reference the old coordinates

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

Lines changed: 21 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -54,13 +54,17 @@ public class ChangeManagedDependencyGroupIdAndArtifactId extends Recipe {
5454
String oldArtifactId;
5555

5656
@Option(displayName = "New groupId",
57-
description = "The new groupId to use.",
58-
example = "corp.internal.openrewrite.recipe")
57+
description = "The new groupId to use. Defaults to the existing group id.",
58+
example = "corp.internal.openrewrite.recipe",
59+
required = false)
60+
@Nullable
5961
String newGroupId;
6062

6163
@Option(displayName = "New artifactId",
62-
description = "The new artifactId to use.",
63-
example = "rewrite-testing-frameworks")
64+
description = "The new artifactId to use. Defaults to the existing artifact id.",
65+
example = "rewrite-testing-frameworks",
66+
required = false)
67+
@Nullable
6468
String newArtifactId;
6569

6670
@Option(displayName = "New version",
@@ -78,7 +82,7 @@ public class ChangeManagedDependencyGroupIdAndArtifactId extends Recipe {
7882
@Nullable
7983
String versionPattern;
8084

81-
public ChangeManagedDependencyGroupIdAndArtifactId(String oldGroupId, String oldArtifactId, String newGroupId, String newArtifactId, @Nullable String newVersion) {
85+
public ChangeManagedDependencyGroupIdAndArtifactId(String oldGroupId, String oldArtifactId, @Nullable String newGroupId, @Nullable String newArtifactId, @Nullable String newVersion) {
8286
this.oldGroupId = oldGroupId;
8387
this.oldArtifactId = oldArtifactId;
8488
this.newGroupId = newGroupId;
@@ -88,7 +92,7 @@ public ChangeManagedDependencyGroupIdAndArtifactId(String oldGroupId, String old
8892
}
8993

9094
@JsonCreator
91-
public ChangeManagedDependencyGroupIdAndArtifactId(String oldGroupId, String oldArtifactId, String newGroupId, String newArtifactId, @Nullable String newVersion, @Nullable String versionPattern) {
95+
public ChangeManagedDependencyGroupIdAndArtifactId(String oldGroupId, String oldArtifactId, @Nullable String newGroupId, @Nullable String newArtifactId, @Nullable String newVersion, @Nullable String versionPattern) {
9296
this.oldGroupId = oldGroupId;
9397
this.oldArtifactId = oldArtifactId;
9498
this.newGroupId = newGroupId;
@@ -126,12 +130,10 @@ public TreeVisitor<?, ExecutionContext> getVisitor() {
126130
final VersionComparator versionComparator = newVersion != null ? Semver.validate(newVersion, versionPattern).getValue() : null;
127131
@Nullable
128132
private Collection<String> availableVersions;
129-
private boolean isNewDependencyPresent;
130133
private Set<String> safeVersionPlaceholdersToChange = new HashSet<>();
131134

132135
@Override
133136
public Xml.Document visitDocument(Xml.Document document, ExecutionContext ctx) {
134-
isNewDependencyPresent = checkIfNewDependencyPresent(newGroupId, newArtifactId, newVersion);
135137
safeVersionPlaceholdersToChange = getSafeVersionPlaceholdersToChange(oldGroupId, oldArtifactId, ctx);
136138
return super.visitDocument(document, ctx);
137139
}
@@ -147,29 +149,26 @@ public Xml.Tag visitTag(Xml.Tag tag, ExecutionContext ctx) {
147149
}
148150
}
149151

150-
if (!newGroupId.equals(oldGroupId)) {
152+
if (newGroupId != null) {
151153
t = (Xml.Tag) new ChangeTagValueVisitor<>(t.getChild("groupId").orElse(null), newGroupId).visitNonNull(t, ctx);
152154
}
153-
if (!newArtifactId.equals(oldArtifactId)) {
155+
if (newArtifactId != null) {
154156
t = (Xml.Tag) new ChangeTagValueVisitor<>(t.getChild("artifactId").orElse(null), newArtifactId).visitNonNull(t, ctx);
155157
}
156158
if (newVersion != null) {
157159
try {
158160
Optional<Xml.Tag> versionTag = t.getChild("version");
159161
if (versionTag.isPresent()) {
160-
String resolvedGroupId = t.getChildValue("groupId").orElse(newGroupId);
161-
String resolvedArtifactId = t.getChildValue("artifactId").orElse(newArtifactId);
162+
ResolvedPom pom = getResolutionResult().getPom();
163+
String resolvedGroupId = t.getChildValue("groupId").orElse(newGroupId != null ? newGroupId : oldGroupId);
164+
String resolvedArtifactId = t.getChildValue("artifactId").orElse(newArtifactId != null ? newArtifactId : oldArtifactId);
162165
if (resolvedArtifactId.contains("${")) {
163-
ResolvedPom pom = getResolutionResult().getPom();
164-
Map<String, String> properties = pom.getProperties();
165-
resolvedArtifactId = ResolvedPom.placeholderHelper.replacePlaceholders(resolvedArtifactId, properties::get);
166+
resolvedArtifactId = ResolvedPom.placeholderHelper.replacePlaceholders(resolvedArtifactId, pom.getProperties()::get);
166167
}
167168
if (resolvedGroupId.contains("${")) {
168-
ResolvedPom pom = getResolutionResult().getPom();
169-
Map<String, String> properties = pom.getProperties();
170-
resolvedGroupId = ResolvedPom.placeholderHelper.replacePlaceholders(resolvedGroupId, properties::get);
169+
resolvedGroupId = ResolvedPom.placeholderHelper.replacePlaceholders(resolvedGroupId, pom.getProperties()::get);
171170
}
172-
String resolvedNewVersion = resolveSemverVersion(ctx, resolvedGroupId, resolvedArtifactId, getResolutionResult().getPom().getValue(versionTag.get().getValue().orElse(null)));
171+
String resolvedNewVersion = resolveSemverVersion(ctx, resolvedGroupId, resolvedArtifactId, pom.getValue(versionTag.get().getValue().orElse(null)));
173172
String versionTagValue = t.getChildValue("version").orElse(null);
174173
if (versionTagValue == null || !safeVersionPlaceholdersToChange.contains(versionTagValue)) {
175174
t = (Xml.Tag) new ChangeTagValueVisitor<>(versionTag.get(), resolvedNewVersion).visitNonNull(t, ctx);
@@ -184,7 +183,9 @@ public Xml.Tag visitTag(Xml.Tag tag, ExecutionContext ctx) {
184183
if (t != tag) {
185184
maybeUpdateModel();
186185
doAfterVisit(new RemoveRedundantDependencyVersions(null, null, null, null).getVisitor());
187-
if (isNewDependencyPresent) {
186+
String effectiveGroupId = newGroupId != null ? newGroupId : tag.getChildValue("groupId").orElse(null);
187+
String effectiveArtifactId = newArtifactId != null ? newArtifactId : tag.getChildValue("artifactId").orElse(null);
188+
if (checkIfNewDependencyPresent(effectiveGroupId, effectiveArtifactId, newVersion)) {
188189
doAfterVisit(new RemoveContentVisitor<>(t, true, true));
189190
maybeUpdateModel();
190191
}

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

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3149,6 +3149,85 @@ void handlesGlobCorrectlyWhenAlsoConflictingManagedLeavingPropertyIfNecessary()
31493149
);
31503150
}
31513151

3152+
@Test
3153+
void handlesGlobCorrectlyWithManagedDependencies() {
3154+
rewriteRun(
3155+
spec -> spec.recipe(new ChangeDependencyGroupIdAndArtifactId(
3156+
"io.swagger",
3157+
"swagger-*",
3158+
"io.swagger.core.v3",
3159+
null,
3160+
"2.2.0",
3161+
null
3162+
)),
3163+
//language=xml
3164+
pomXml(
3165+
"""
3166+
<project>
3167+
<groupId>com.example</groupId>
3168+
<artifactId>demo-child</artifactId>
3169+
<version>0.0.1-SNAPSHOT</version>
3170+
<dependencyManagement>
3171+
<dependencies>
3172+
<dependency>
3173+
<groupId>io.swagger</groupId>
3174+
<artifactId>swagger-annotations</artifactId>
3175+
<version>1.5.16</version>
3176+
</dependency>
3177+
<dependency>
3178+
<groupId>io.swagger</groupId>
3179+
<artifactId>swagger-models</artifactId>
3180+
<version>1.5.16</version>
3181+
</dependency>
3182+
</dependencies>
3183+
</dependencyManagement>
3184+
<dependencies>
3185+
<dependency>
3186+
<groupId>io.swagger</groupId>
3187+
<artifactId>swagger-annotations</artifactId>
3188+
</dependency>
3189+
<dependency>
3190+
<groupId>io.swagger</groupId>
3191+
<artifactId>swagger-models</artifactId>
3192+
</dependency>
3193+
</dependencies>
3194+
</project>
3195+
""",
3196+
"""
3197+
<project>
3198+
<groupId>com.example</groupId>
3199+
<artifactId>demo-child</artifactId>
3200+
<version>0.0.1-SNAPSHOT</version>
3201+
<dependencyManagement>
3202+
<dependencies>
3203+
<dependency>
3204+
<groupId>io.swagger.core.v3</groupId>
3205+
<artifactId>swagger-annotations</artifactId>
3206+
<version>2.2.0</version>
3207+
</dependency>
3208+
<dependency>
3209+
<groupId>io.swagger.core.v3</groupId>
3210+
<artifactId>swagger-models</artifactId>
3211+
<version>2.2.0</version>
3212+
</dependency>
3213+
</dependencies>
3214+
</dependencyManagement>
3215+
<dependencies>
3216+
<dependency>
3217+
<groupId>io.swagger.core.v3</groupId>
3218+
<artifactId>swagger-annotations</artifactId>
3219+
</dependency>
3220+
<dependency>
3221+
<groupId>io.swagger.core.v3</groupId>
3222+
<artifactId>swagger-models</artifactId>
3223+
</dependency>
3224+
</dependencies>
3225+
</project>
3226+
"""
3227+
)
3228+
);
3229+
}
3230+
31523231
@Test
31533232
void handlesCollisionInDeepRemoteParent() {
31543233
rewriteRun(

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -632,13 +632,13 @@ void doesNotMakeChangeWhenChangingBomImportManagedDependency() {
632632
}
633633

634634
@Test
635-
void globArtifactIdRetainedWhenNewMatchesOld() {
635+
void globArtifactIdRetainedWhenNewArtifactIdIsNull() {
636636
rewriteRun(
637637
spec -> spec.recipe(new ChangeManagedDependencyGroupIdAndArtifactId(
638638
"javax.activation",
639639
"javax.activation-*",
640640
"jakarta.activation",
641-
"javax.activation-*",
641+
null,
642642
null
643643
)),
644644
pomXml(

0 commit comments

Comments
 (0)
⚡