Skip to content

Commit 86034d0

Browse files
MBoegerstimtebeek
andauthored
ChangeDependencyGroupIdAndArtifactId updates version properties in parent POMs (#6630)
* `ChangeDependencyGroupIdAndArtifactId` updates version properties in parent POMs Convert `ChangeDependencyGroupIdAndArtifactId` from `Recipe` to `ScanningRecipe` so that version properties defined in parent POMs are updated when the dependency referencing that property is in a child POM. The scanner phase identifies version properties inherited from parent POMs and records which source file defines each property. The visitor phase then applies property updates when visiting those parent POM files. All existing visitor logic is preserved unchanged, avoiding the recipe-chaining issues that caused the previous attempt (#5815) to be reverted. Fixes moderneinc/customer-requests#1374 * Add test for shared property overlap scenario Verifies that when a property in a parent POM is shared by multiple dependencies in a child POM, and only one dependency matches the change criteria, the property is left unchanged and the version is inlined instead. * Check sibling modules for overlapping property usage When a version property in a parent POM is shared by dependencies across different child modules, changing it would break non-matching siblings. The existing overlap filter only checked the current module's children and parent dependencies, but not sibling modules. Walk up the parent chain and check each parent's children to detect property usage conflicts in sibling modules. When overlap is found, the version is inlined instead of updating the shared property. * Add test documenting conservative behavior for redefined child properties When a child module redefines a parent property used by a non-matching dependency, the overlap filter conservatively prevents updating the parent property. This is safe because ChangePropertyValue via doAfterVisit does not scope to a single document and would also change the child's intentional override. * Avoid traversal when newVersion or compartor is null * Add an early return after handling property tag --------- Co-authored-by: Tim te Beek <tim@moderne.io>
1 parent 715a521 commit 86034d0

2 files changed

Lines changed: 546 additions & 5 deletions

File tree

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

Lines changed: 146 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,9 +29,11 @@
2929
import org.openrewrite.xml.RemoveContentVisitor;
3030
import org.openrewrite.xml.tree.Xml;
3131

32+
import java.nio.file.Path;
3233
import java.util.*;
3334

3435
import static java.util.Collections.max;
36+
import static java.util.Objects.requireNonNull;
3537
import static java.util.stream.Collectors.toSet;
3638
import static org.openrewrite.Validated.required;
3739
import static org.openrewrite.Validated.test;
@@ -41,7 +43,8 @@
4143

4244
@Value
4345
@EqualsAndHashCode(callSuper = false)
44-
public class ChangeDependencyGroupIdAndArtifactId extends Recipe {
46+
public class ChangeDependencyGroupIdAndArtifactId extends ScanningRecipe<ChangeDependencyGroupIdAndArtifactId.Accumulator> {
47+
@EqualsAndHashCode.Exclude
4548
transient MavenMetadataFailures metadataFailures = new MavenMetadataFailures(this);
4649

4750
@Option(displayName = "Old groupId",
@@ -142,7 +145,110 @@ public Validated<Object> validate() {
142145
}
143146

144147
@Override
145-
public TreeVisitor<?, ExecutionContext> getVisitor() {
148+
public Accumulator getInitialValue(ExecutionContext ctx) {
149+
return new Accumulator();
150+
}
151+
152+
@Override
153+
public TreeVisitor<?, ExecutionContext> getScanner(Accumulator acc) {
154+
if (newVersion == null) {
155+
return TreeVisitor.noop();
156+
}
157+
final VersionComparator versionComparator = Semver.validate(newVersion, versionPattern).getValue();
158+
if (versionComparator == null) {
159+
return TreeVisitor.noop();
160+
}
161+
return new MavenIsoVisitor<ExecutionContext>() {
162+
final boolean configuredToChangeManagedDependency = changeManagedDependency == null || changeManagedDependency;
163+
164+
@Override
165+
public Xml.Tag visitTag(Xml.Tag tag, ExecutionContext ctx) {
166+
if (!isDependencyTag(oldGroupId, oldArtifactId) &&
167+
!isPluginDependencyTag(oldGroupId, oldArtifactId) &&
168+
!isAnnotationProcessorPathTag(oldGroupId, oldArtifactId)) {
169+
return super.visitTag(tag, ctx);
170+
}
171+
String currentVersion = tag.getChildValue("version").orElse(null);
172+
if (!isProperty(currentVersion)) {
173+
return super.visitTag(tag, ctx);
174+
}
175+
String propertyName = currentVersion.substring(2, currentVersion.length() - 1);
176+
if (getResolutionResult().getPom().getRequested().getProperties().containsKey(propertyName)) {
177+
return super.visitTag(tag, ctx);
178+
}
179+
// Property is inherited from a parent POM; check if it is safe to change
180+
Set<String> safeProperties = getSafeVersionPlaceholdersToChange(oldGroupId, oldArtifactId, ctx);
181+
if (!safeProperties.contains(currentVersion)) {
182+
return super.visitTag(tag, ctx);
183+
}
184+
try {
185+
String groupId = Optional.ofNullable(newGroupId).orElse(tag.getChildValue("groupId").orElse(oldGroupId));
186+
String artifactId = Optional.ofNullable(newArtifactId).orElse(tag.getChildValue("artifactId").orElse(oldArtifactId));
187+
String resolvedVersion = resolveSemverVersion(ctx, groupId, artifactId, currentVersion);
188+
storeParentPomProperty(getResolutionResult().getParent(), propertyName, resolvedVersion, acc);
189+
} catch (MavenDownloadingException e) {
190+
return e.warn(tag);
191+
}
192+
return super.visitTag(tag, ctx);
193+
}
194+
195+
private Set<String> getSafeVersionPlaceholdersToChange(String groupId, String artifactId, ExecutionContext ctx) {
196+
MavenResolutionResult result = getResolutionResult();
197+
ResolvedPom resolvedPom = result.getPom();
198+
Pom requestedPom = resolvedPom.getRequested();
199+
Set<String> relevantProperties = requestedPom.getDependencies().stream()
200+
.filter(d -> isProperty(d.getVersion()) &&
201+
matchesGlob(resolvedPom.getValue(d.getGroupId()), groupId) &&
202+
matchesGlob(resolvedPom.getValue(d.getArtifactId()), artifactId))
203+
.map(Dependency::getVersion)
204+
.collect(toSet());
205+
relevantProperties = filterPropertiesWithOverlapInDependencies(relevantProperties, groupId, artifactId, requestedPom, resolvedPom, configuredToChangeManagedDependency);
206+
relevantProperties = filterPropertiesWithOverlapInChildren(relevantProperties, groupId, artifactId, result, configuredToChangeManagedDependency);
207+
relevantProperties = filterPropertiesWithOverlapInParents(relevantProperties, groupId, artifactId, result, configuredToChangeManagedDependency, ctx);
208+
// Also check sibling modules for overlapping property usage
209+
MavenResolutionResult current = result;
210+
while (current.parentPomIsProjectPom()) {
211+
current = requireNonNull(current.getParent());
212+
relevantProperties = filterPropertiesWithOverlapInChildren(relevantProperties, groupId, artifactId, current, configuredToChangeManagedDependency);
213+
}
214+
return relevantProperties;
215+
}
216+
217+
@SuppressWarnings("ConstantConditions")
218+
private String resolveSemverVersion(ExecutionContext ctx, String groupId, String artifactId, @Nullable String currentVersion) throws MavenDownloadingException {
219+
if (versionComparator == null) {
220+
return newVersion;
221+
}
222+
String finalCurrentVersion = currentVersion != null ? currentVersion : newVersion;
223+
List<String> availableVersions = new ArrayList<>();
224+
MavenMetadata mavenMetadata = metadataFailures.insertRows(ctx, () -> downloadMetadata(groupId, artifactId, ctx));
225+
for (String v : mavenMetadata.getVersioning().getVersions()) {
226+
if (versionComparator.isValid(finalCurrentVersion, v)) {
227+
availableVersions.add(v);
228+
}
229+
}
230+
return availableVersions.isEmpty() ? newVersion : max(availableVersions, versionComparator);
231+
}
232+
233+
private void storeParentPomProperty(@Nullable MavenResolutionResult parent, String propertyName, String newValue, Accumulator acc) {
234+
if (parent == null) {
235+
return;
236+
}
237+
Pom pom = parent.getPom().getRequested();
238+
if (pom.getSourcePath() == null) {
239+
return;
240+
}
241+
if (pom.getProperties().containsKey(propertyName)) {
242+
acc.getPomProperties().add(new PomProperty(pom.getSourcePath(), propertyName, newValue));
243+
return;
244+
}
245+
storeParentPomProperty(parent.getParent(), propertyName, newValue, acc);
246+
}
247+
};
248+
}
249+
250+
@Override
251+
public TreeVisitor<?, ExecutionContext> getVisitor(Accumulator acc) {
146252
return new MavenVisitor<ExecutionContext>() {
147253
@Nullable
148254
final VersionComparator versionComparator = newVersion != null ? Semver.validate(newVersion, versionPattern).getValue() : null;
@@ -176,6 +282,24 @@ public Xml visitDocument(Xml.Document document, ExecutionContext ctx) {
176282
@Override
177283
public Xml visitTag(Xml.Tag tag, ExecutionContext ctx) {
178284
Xml.Tag t = (Xml.Tag) super.visitTag(tag, ctx);
285+
286+
// Update version properties in parent POMs based on scanner results
287+
if (isPropertyTag()) {
288+
Path pomSourcePath = getResolutionResult().getPom().getRequested().getSourcePath();
289+
for (PomProperty prop : acc.getPomProperties()) {
290+
if (!prop.getPomFilePath().equals(pomSourcePath) || !prop.getPropertyName().equals(tag.getName())) {
291+
continue;
292+
}
293+
Optional<String> value = tag.getValue();
294+
if (!value.isPresent() || !value.get().equals(prop.getNewValue())) {
295+
doAfterVisit(new ChangeTagValueVisitor<>(tag, prop.getNewValue()));
296+
maybeUpdateModel();
297+
}
298+
break;
299+
}
300+
return t;
301+
}
302+
179303
boolean isOldDependencyTag = isDependencyTag(oldGroupId, oldArtifactId);
180304
if (isOldDependencyTag && isNewDependencyPresent) {
181305
doAfterVisit(new RemoveContentVisitor<>(tag, true, true));
@@ -303,7 +427,14 @@ private Set<String> getSafeVersionPlaceholdersToChange(String groupId, String ar
303427
.collect(toSet());
304428
relevantProperties = filterPropertiesWithOverlapInDependencies(relevantProperties, groupId, artifactId, requestedPom, resolvedPom, configuredToChangeManagedDependency);
305429
relevantProperties = filterPropertiesWithOverlapInChildren(relevantProperties, groupId, artifactId, result, configuredToChangeManagedDependency);
306-
return filterPropertiesWithOverlapInParents(relevantProperties, groupId, artifactId, result, configuredToChangeManagedDependency, ctx);
430+
relevantProperties = filterPropertiesWithOverlapInParents(relevantProperties, groupId, artifactId, result, configuredToChangeManagedDependency, ctx);
431+
// Also check sibling modules for overlapping property usage
432+
MavenResolutionResult current = result;
433+
while (current.parentPomIsProjectPom()) {
434+
current = requireNonNull(current.getParent());
435+
relevantProperties = filterPropertiesWithOverlapInChildren(relevantProperties, groupId, artifactId, current, configuredToChangeManagedDependency);
436+
}
437+
return relevantProperties;
307438
}
308439

309440

@@ -327,4 +458,16 @@ private String resolveSemverVersion(ExecutionContext ctx, String groupId, String
327458
}
328459
};
329460
}
461+
462+
@Value
463+
public static class Accumulator {
464+
Set<PomProperty> pomProperties = new HashSet<>();
465+
}
466+
467+
@Value
468+
public static class PomProperty {
469+
Path pomFilePath;
470+
String propertyName;
471+
String newValue;
472+
}
330473
}

0 commit comments

Comments
 (0)