Skip to content

Commit 0050d30

Browse files
jevanlingenjevanlingen
authored
Generalize changing of properties in Maven recipes (#5503)
1 parent da2e4dd commit 0050d30

7 files changed

Lines changed: 102 additions & 81 deletions

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

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@
2525
import org.openrewrite.semver.Semver;
2626
import org.openrewrite.semver.VersionComparator;
2727
import org.openrewrite.xml.AddToTagVisitor;
28-
import org.openrewrite.xml.ChangeTagValueVisitor;
2928
import org.openrewrite.xml.RemoveContentVisitor;
3029
import org.openrewrite.xml.tree.Xml;
3130

@@ -239,16 +238,7 @@ private boolean checkIfNewDependencyPresents(@Nullable String groupId, @Nullable
239238
.anyMatch(rd -> (version == null) || version.equals(rd.getVersion()));
240239
}
241240

242-
private Xml.Tag changeChildTagValue(Xml.Tag tag, String childTagName, String newValue, ExecutionContext ctx) {
243-
Optional<Xml.Tag> childTag = tag.getChild(childTagName);
244-
if (childTag.isPresent() && !newValue.equals(childTag.get().getValue().orElse(null))) {
245-
tag = (Xml.Tag) new ChangeTagValueVisitor<>(childTag.get(), newValue).visitNonNull(tag, ctx);
246-
}
247-
return tag;
248-
}
249-
250241
private boolean isDependencyManaged(Scope scope, String groupId, String artifactId) {
251-
252242
MavenResolutionResult result = getResolutionResult();
253243
for (ResolvedManagedDependency managedDependency : result.getPom().getDependencyManagement()) {
254244
if (groupId.equals(managedDependency.getGroupId()) && artifactId.equals(managedDependency.getArtifactId())) {

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

Lines changed: 6 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@
2626
import org.openrewrite.maven.tree.ResolvedPom;
2727
import org.openrewrite.semver.Semver;
2828
import org.openrewrite.semver.VersionComparator;
29-
import org.openrewrite.xml.ChangeTagValueVisitor;
3029
import org.openrewrite.xml.RemoveContentVisitor;
3130
import org.openrewrite.xml.tree.Xml;
3231

@@ -41,11 +40,6 @@ public class ChangeManagedDependencyGroupIdAndArtifactId extends Recipe {
4140
@EqualsAndHashCode.Exclude
4241
MavenMetadataFailures metadataFailures = new MavenMetadataFailures(this);
4342

44-
// there are several implicitly defined version properties that we should never attempt to update
45-
private static final Set<String> implicitlyDefinedVersionProperties = new HashSet<>(Arrays.asList(
46-
"${version}", "${project.version}", "${pom.version}", "${project.parent.version}"
47-
));
48-
4943
@Option(displayName = "Old groupId",
5044
description = "The old groupId to replace. The groupId is the first part of a managed dependency coordinate `com.google.guava:guava:VERSION`.",
5145
example = "org.openrewrite.recipe")
@@ -146,24 +140,17 @@ public Xml.Document visitDocument(Xml.Document document, ExecutionContext ctx) {
146140

147141
@Override
148142
public Xml.Tag visitTag(Xml.Tag tag, ExecutionContext ctx) {
149-
150143
Xml.Tag t = super.visitTag(tag, ctx);
151144
if (isManagedDependencyTag(oldGroupId, oldArtifactId)) {
152-
Optional<Xml.Tag> groupIdTag = t.getChild("groupId");
153-
boolean changed = false;
154-
if (groupIdTag.isPresent() && !newGroupId.equals(groupIdTag.get().getValue().orElse(null))) {
155-
doAfterVisit(new ChangeTagValueVisitor<>(groupIdTag.get(), newGroupId));
156-
changed = true;
145+
if (t.getChild("groupId").isPresent()) {
146+
t = changeChildTagValue(t, "groupId", newGroupId, ctx);
157147
}
158-
Optional<Xml.Tag> artifactIdTag = t.getChild("artifactId");
159-
if (artifactIdTag.isPresent() && !newArtifactId.equals(artifactIdTag.get().getValue().orElse(null))) {
160-
doAfterVisit(new ChangeTagValueVisitor<>(artifactIdTag.get(), newArtifactId));
161-
changed = true;
148+
if (t.getChild("artifactId").isPresent()) {
149+
t = changeChildTagValue(t, "artifactId", newArtifactId, ctx);
162150
}
163151
if (newVersion != null) {
164152
try {
165153
Optional<Xml.Tag> versionTag = t.getChild("version");
166-
167154
if (versionTag.isPresent()) {
168155
String resolvedArtifactId = newArtifactId;
169156
if (resolvedArtifactId.contains("${")) {
@@ -172,14 +159,13 @@ public Xml.Tag visitTag(Xml.Tag tag, ExecutionContext ctx) {
172159
resolvedArtifactId = ResolvedPom.placeholderHelper.replacePlaceholders(newArtifactId, properties::get);
173160
}
174161
String resolvedNewVersion = resolveSemverVersion(ctx, newGroupId, resolvedArtifactId, getResolutionResult().getPom().getValue(versionTag.get().getValue().orElse(null)));
175-
t = (Xml.Tag) new ChangeTagValueVisitor<>(versionTag.get(), resolvedNewVersion).visitNonNull(t, 0, getCursor().getParentOrThrow());
162+
t = changeChildTagValue(t, "version", resolvedNewVersion, ctx);
176163
}
177-
changed = true;
178164
} catch (MavenDownloadingException e) {
179165
return e.warn(t);
180166
}
181167
}
182-
if (changed) {
168+
if (t != tag) {
183169
maybeUpdateModel();
184170
doAfterVisit(new RemoveRedundantDependencyVersions(null, null, null, null).getVisitor());
185171
if (isNewDependencyPresent) {

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

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -23,11 +23,8 @@
2323
import org.openrewrite.Recipe;
2424
import org.openrewrite.TreeVisitor;
2525
import org.openrewrite.maven.table.MavenMetadataFailures;
26-
import org.openrewrite.xml.ChangeTagValueVisitor;
2726
import org.openrewrite.xml.tree.Xml;
2827

29-
import java.util.Optional;
30-
3128
@Value
3229
@EqualsAndHashCode(callSuper = false)
3330
public class ChangePluginGroupIdAndArtifactId extends Recipe {
@@ -105,14 +102,6 @@ public Xml visitTag(Xml.Tag tag, ExecutionContext ctx) {
105102
//noinspection ConstantConditions
106103
return t;
107104
}
108-
109-
private Xml.Tag changeChildTagValue(Xml.Tag tag, String childTagName, String newValue, ExecutionContext ctx) {
110-
Optional<Xml.Tag> childTag = tag.getChild(childTagName);
111-
if (childTag.isPresent() && !newValue.equals(childTag.get().getValue().orElse(null))) {
112-
tag = (Xml.Tag) new ChangeTagValueVisitor<>(childTag.get(), newValue).visitNonNull(tag, ctx);
113-
}
114-
return tag;
115-
}
116105
};
117106
}
118107
}

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

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
import org.openrewrite.TreeVisitor;
2222
import org.openrewrite.maven.internal.MavenPomDownloader;
2323
import org.openrewrite.maven.tree.*;
24+
import org.openrewrite.xml.ChangeTagValueVisitor;
2425
import org.openrewrite.xml.XPathMatcher;
2526
import org.openrewrite.xml.XmlVisitor;
2627
import org.openrewrite.xml.tree.Xml;
@@ -47,6 +48,11 @@ public class MavenVisitor<P> extends XmlVisitor<P> {
4748
static final XPathMatcher PARENT_MATCHER = new XPathMatcher("/project/parent");
4849
static final XPathMatcher PROJECT_MATCHER = new XPathMatcher("/project");
4950

51+
// there are several implicitly defined version properties that we should never attempt to update
52+
private static final Set<String> IMPLICITLY_DEFINED_VERSION_PROPERTIES = new HashSet<>(Arrays.asList(
53+
"${version}", "${project.version}", "${pom.version}", "${project.parent.version}"
54+
));
55+
5056
private transient Xml.@Nullable Document document;
5157

5258
@Nullable
@@ -269,6 +275,32 @@ private boolean isTag(String name) {
269275
return getCursor().getValue() instanceof Xml.Tag && name.equals(getCursor().<Xml.Tag>getValue().getName());
270276
}
271277

278+
protected Xml.Tag changeChildTagValue(Xml.Tag tag, String childTagName, @Nullable String newValue, P p) {
279+
return changeChildTagValue(tag, childTagName, newValue, false, p);
280+
}
281+
282+
protected Xml.Tag changeChildTagValue(Xml.Tag tag, String childTagName, @Nullable String newValue, @Nullable Boolean addPropertyIfMissing, P p) {
283+
Optional<Xml.Tag> childTag = tag.getChild(childTagName);
284+
if (childTag.isPresent()) {
285+
String oldValue = childTag.get().getValue().orElse(null);
286+
if (newValue != null && !newValue.equals(oldValue)) {
287+
if (isProperty(oldValue)) {
288+
String propertyName = oldValue.substring(2, oldValue.length() - 1);
289+
if (getResolutionResult().getPom().getRequested().getProperties().containsKey(propertyName)) {
290+
doAfterVisit((TreeVisitor<?, P>) new ChangePropertyValue(propertyName, newValue, addPropertyIfMissing, false).getVisitor());
291+
}
292+
} else {
293+
tag = (Xml.Tag) new ChangeTagValueVisitor<>(childTag.get(), newValue).visitNonNull(tag, p);
294+
}
295+
}
296+
}
297+
return tag;
298+
}
299+
300+
protected boolean isProperty(@Nullable String value) {
301+
return value != null && value.startsWith("${") && !IMPLICITLY_DEFINED_VERSION_PROPERTIES.contains(value);
302+
}
303+
272304
public @Nullable ResolvedDependency findDependency(Xml.Tag tag) {
273305
Map<Scope, List<ResolvedDependency>> dependencies = getResolutionResult().getDependencies();
274306
Scope scope = Scope.fromName(tag.getChildValue("scope").orElse("compile"));

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

Lines changed: 10 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -52,11 +52,6 @@ public class UpgradeDependencyVersion extends ScanningRecipe<UpgradeDependencyVe
5252
@EqualsAndHashCode.Exclude
5353
transient MavenMetadataFailures metadataFailures = new MavenMetadataFailures(this);
5454

55-
// there are several implicitly defined version properties that we should never attempt to update
56-
private static final Collection<String> implicitlyDefinedVersionProperties = Arrays.asList(
57-
"${version}", "${project.version}", "${pom.version}", "${project.parent.version}"
58-
);
59-
6055
@Option(displayName = "Group",
6156
description = "The first part of a dependency coordinate `com.google.guava:guava:VERSION`. This can be a glob expression.",
6257
example = "com.fasterxml.jackson*")
@@ -156,8 +151,7 @@ public Xml.Tag visitTag(final Xml.Tag tag, final ExecutionContext ctx) {
156151
Optional<Xml.Tag> version = tag.getChild("version");
157152
if (version.isPresent()) {
158153
String requestedVersion = d.getRequested().getVersion();
159-
if (requestedVersion != null && requestedVersion.startsWith("${") &&
160-
!implicitlyDefinedVersionProperties.contains(requestedVersion)) {
154+
if (isProperty(requestedVersion)) {
161155
String propertyName = requestedVersion.substring(2, requestedVersion.length() - 1);
162156
if (!getResolutionResult().getPom().getRequested().getProperties().containsKey(propertyName)) {
163157
storeParentPomProperty(getResolutionResult().getParent(), propertyName, newerVersion);
@@ -180,8 +174,7 @@ public Xml.Tag visitTag(final Xml.Tag tag, final ExecutionContext ctx) {
180174
* @param propertyName the name of the property to update, if found in any the parent pom source file
181175
* @param newerVersion the resolved newer version that any matching parent pom property should be updated to
182176
*/
183-
private void storeParentPomProperty(
184-
@Nullable MavenResolutionResult currentMavenResolutionResult, String propertyName, String newerVersion) {
177+
private void storeParentPomProperty(@Nullable MavenResolutionResult currentMavenResolutionResult, String propertyName, String newerVersion) {
185178
if (currentMavenResolutionResult == null) {
186179
return; // No parent contained the property; might then be in the same source file, or an import BOM
187180
}
@@ -270,10 +263,8 @@ public Xml.Tag visitTag(Xml.Tag tag, ExecutionContext ctx) {
270263
}
271264

272265
private void retainVersions() {
273-
for (Recipe retainVersionRecipe : RetainVersions.plan(this, retainVersions == null ?
274-
emptyList() : retainVersions)) {
275-
doAfterVisit(retainVersionRecipe.getVisitor());
276-
}
266+
RetainVersions.plan(this, retainVersions == null ? emptyList() : retainVersions)
267+
.forEach(it -> doAfterVisit(it.getVisitor()));
277268
}
278269
}
279270

@@ -284,26 +275,13 @@ private Xml.Tag upgradeDependency(ExecutionContext ctx, Xml.Tag t) throws MavenD
284275
// as a source file, attempt to find a new version.
285276
String newerVersion = findNewerVersion(d.getGroupId(), d.getArtifactId(), d.getVersion(), ctx);
286277
if (newerVersion != null) {
287-
Optional<Xml.Tag> version = t.getChild("version");
288-
if (version.isPresent()) {
289-
String requestedVersion = d.getRequested().getVersion();
290-
if (requestedVersion != null && requestedVersion.startsWith("${") && !implicitlyDefinedVersionProperties.contains(requestedVersion)) {
291-
String propertyName = requestedVersion.substring(2, requestedVersion.length() - 1);
292-
if (getResolutionResult().getPom().getRequested().getProperties().containsKey(propertyName)) {
293-
doAfterVisit(new ChangePropertyValue(propertyName, newerVersion, overrideManagedVersion, false).getVisitor());
294-
}
295-
} else {
296-
t = (Xml.Tag) new ChangeTagValueVisitor<>(version.get(), newerVersion).visitNonNull(t, 0, getCursor().getParentOrThrow());
297-
}
278+
if (t.getChild("version").isPresent()) {
279+
t = changeChildTagValue(t, "version", newerVersion, overrideManagedVersion, ctx);
298280
} else if (Boolean.TRUE.equals(overrideManagedVersion)) {
299281
ResolvedManagedDependency dm = findManagedDependency(t);
300282
// if a managed dependency is expressed as a property, change the property value
301283
// do this only when a requested bom is absent, otherwise changing property has no effect
302-
if (dm != null &&
303-
dm.getRequested().getVersion() != null &&
304-
dm.getRequested().getVersion().startsWith("${") &&
305-
!implicitlyDefinedVersionProperties.contains(dm.getRequested().getVersion()) &&
306-
dm.getRequestedBom() == null) {
284+
if (dm != null && isProperty(dm.getRequested().getVersion()) && dm.getRequestedBom() == null) {
307285
doAfterVisit(new ChangePropertyValue(dm.getRequested().getVersion().substring(2,
308286
dm.getRequested().getVersion().length() - 1),
309287
newerVersion, overrideManagedVersion, false).getVisitor());
@@ -359,20 +337,14 @@ private Xml.Tag upgradePluginDependency(ExecutionContext ctx, Xml.Tag t) throws
359337
if (groupId != null && artifactId != null && version != null) {
360338
String newerVersion = findNewerVersion(groupId, artifactId, resolveVersion(version), ctx);
361339
if (newerVersion != null) {
362-
if (version.startsWith("${") && !implicitlyDefinedVersionProperties.contains(version)) {
363-
doAfterVisit(new ChangePropertyValue(version.substring(2, version.length() - 1), newerVersion, overrideManagedVersion, false).getVisitor());
364-
} else {
365-
Optional<Xml.Tag> versionTag = t.getChild("version");
366-
assert versionTag.isPresent();
367-
t = (Xml.Tag) new ChangeTagValueVisitor<>(versionTag.get(), newerVersion).visitNonNull(t, 0, getCursor().getParentOrThrow());
368-
}
340+
t = changeChildTagValue(t, "version", newerVersion, overrideManagedVersion, ctx);
369341
}
370342
}
371343
return t;
372344
}
373345

374346
private String resolveVersion(String version) {
375-
if (version.startsWith("${") && !implicitlyDefinedVersionProperties.contains(version)) {
347+
if (isProperty(version)) {
376348
Map<String, String> properties = getResolutionResult().getPom().getProperties();
377349
String property = version.substring(2, version.length() - 1);
378350
return properties.getOrDefault(property, version);
@@ -384,7 +356,7 @@ private String resolveVersion(String version) {
384356
String newerVersion = findNewerVersion(groupId, artifactId, version2, ctx);
385357
if (newerVersion == null) {
386358
return null;
387-
} else if (requestedVersion != null && requestedVersion.startsWith("${")) {
359+
} else if (isProperty(requestedVersion)) {
388360
//noinspection unchecked
389361
return (TreeVisitor<Xml, ExecutionContext>) new ChangePropertyValue(requestedVersion.substring(2, requestedVersion.length() - 1), newerVersion, overrideManagedVersion, false)
390362
.getVisitor();

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

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1624,4 +1624,56 @@ void changePluginDependencyGroupIdAndArtifactId() {
16241624
)
16251625
);
16261626
}
1627+
1628+
@Test
1629+
void changeDependencyGroupIdAndArtifactIdWithVersionProperty() {
1630+
rewriteRun(
1631+
spec -> spec.recipe(new ChangeDependencyGroupIdAndArtifactId(
1632+
"javax.activation",
1633+
"javax.activation-api",
1634+
"jakarta.activation",
1635+
"jakarta.activation-api",
1636+
"1.2.x",
1637+
null
1638+
)),
1639+
pomXml(
1640+
"""
1641+
<project>
1642+
<modelVersion>4.0.0</modelVersion>
1643+
<groupId>com.mycompany.app</groupId>
1644+
<artifactId>my-app</artifactId>
1645+
<version>1</version>
1646+
<properties>
1647+
<activation.api>1.2.0</activation.api>
1648+
</properties>
1649+
<dependencies>
1650+
<dependency>
1651+
<groupId>javax.activation</groupId>
1652+
<artifactId>javax.activation-api</artifactId>
1653+
<version>${activation.api}</version>
1654+
</dependency>
1655+
</dependencies>
1656+
</project>
1657+
""",
1658+
"""
1659+
<project>
1660+
<modelVersion>4.0.0</modelVersion>
1661+
<groupId>com.mycompany.app</groupId>
1662+
<artifactId>my-app</artifactId>
1663+
<version>1</version>
1664+
<properties>
1665+
<activation.api>1.2.2</activation.api>
1666+
</properties>
1667+
<dependencies>
1668+
<dependency>
1669+
<groupId>jakarta.activation</groupId>
1670+
<artifactId>jakarta.activation-api</artifactId>
1671+
<version>${activation.api}</version>
1672+
</dependency>
1673+
</dependencies>
1674+
</project>
1675+
"""
1676+
)
1677+
);
1678+
}
16271679
}

0 commit comments

Comments
 (0)
⚡