From b5e239c85b869182b1bf9c0f522fd825f19cccc8 Mon Sep 17 00:00:00 2001 From: Tobias Lidskog Date: Sat, 13 Jan 2024 02:12:47 +0100 Subject: [PATCH] Validate that Change(Managed)DependencyGroupIdAndArtifactId updates GAV (#3889) ChangeDependencyGroupIdAndArtifactId and ChangeManagedDependencyGroupIdAndArtifactId are intended to be set up to *change* maven coordinates. If the pre- and post-coordinates are the same, it's likely that the UpgradeDependencyVersion recipe was intended. This change adds validation of the newGroupId/newArtifactId options. See https://github.com/openrewrite/rewrite-java-dependencies/issues/55 --- .../ChangeDependencyGroupIdAndArtifactId.java | 18 +++++++++- ...ManagedDependencyGroupIdAndArtifactId.java | 14 ++++++++ ...ngeDependencyGroupIdAndArtifactIdTest.java | 35 +++++++++++++++++++ ...gedDependencyGroupIdAndArtifactIdTest.java | 17 +++++++++ 4 files changed, 83 insertions(+), 1 deletion(-) diff --git a/rewrite-maven/src/main/java/org/openrewrite/maven/ChangeDependencyGroupIdAndArtifactId.java b/rewrite-maven/src/main/java/org/openrewrite/maven/ChangeDependencyGroupIdAndArtifactId.java index 656cac359ab..759033a166e 100755 --- a/rewrite-maven/src/main/java/org/openrewrite/maven/ChangeDependencyGroupIdAndArtifactId.java +++ b/rewrite-maven/src/main/java/org/openrewrite/maven/ChangeDependencyGroupIdAndArtifactId.java @@ -34,6 +34,10 @@ import java.util.*; +import static org.openrewrite.Validated.required; +import static org.openrewrite.Validated.test; +import static org.openrewrite.internal.StringUtils.isBlank; + @Value @EqualsAndHashCode(callSuper = false) public class ChangeDependencyGroupIdAndArtifactId extends Recipe { @@ -90,8 +94,8 @@ public ChangeDependencyGroupIdAndArtifactId(String oldGroupId, String oldArtifac this.newGroupId = newGroupId; this.newArtifactId = newArtifactId; this.newVersion = newVersion; - this.overrideManagedVersion = false; this.versionPattern = versionPattern; + this.overrideManagedVersion = false; } @JsonCreator @@ -111,6 +115,18 @@ public Validated validate() { if (newVersion != null) { validated = validated.and(Semver.validate(newVersion, versionPattern)); } + validated = validated.and(required("newGroupId", newGroupId).or(required("newArtifactId", newArtifactId))); + validated = + validated.and(test( + "coordinates", + "newGroupId OR newArtifactId must be different from before", + this, + r -> { + boolean sameGroupId = isBlank(r.newGroupId) || Objects.equals(r.oldGroupId, r.newGroupId); + boolean sameArtifactId = isBlank(r.newArtifactId) || Objects.equals(r.oldArtifactId, r.newArtifactId); + return !(sameGroupId && sameArtifactId); + } + )); return validated; } diff --git a/rewrite-maven/src/main/java/org/openrewrite/maven/ChangeManagedDependencyGroupIdAndArtifactId.java b/rewrite-maven/src/main/java/org/openrewrite/maven/ChangeManagedDependencyGroupIdAndArtifactId.java index 5b52ba8f3b9..dec4f2d5b6c 100644 --- a/rewrite-maven/src/main/java/org/openrewrite/maven/ChangeManagedDependencyGroupIdAndArtifactId.java +++ b/rewrite-maven/src/main/java/org/openrewrite/maven/ChangeManagedDependencyGroupIdAndArtifactId.java @@ -29,6 +29,9 @@ import java.util.*; +import static org.openrewrite.Validated.test; +import static org.openrewrite.internal.StringUtils.isBlank; + @Value @EqualsAndHashCode(callSuper = true) public class ChangeManagedDependencyGroupIdAndArtifactId extends Recipe { @@ -100,6 +103,17 @@ public Validated validate() { if (newVersion != null) { validated = validated.and(Semver.validate(newVersion, versionPattern)); } + validated = + validated.and(test( + "coordinates", + "newGroupId OR newArtifactId must be different from before", + this, + r -> { + boolean sameGroupId = isBlank(r.newGroupId) || Objects.equals(r.oldGroupId, r.newGroupId); + boolean sameArtifactId = isBlank(r.newArtifactId) || Objects.equals(r.oldArtifactId, r.newArtifactId); + return !(sameGroupId && sameArtifactId); + } + )); return validated; } diff --git a/rewrite-maven/src/test/java/org/openrewrite/maven/ChangeDependencyGroupIdAndArtifactIdTest.java b/rewrite-maven/src/test/java/org/openrewrite/maven/ChangeDependencyGroupIdAndArtifactIdTest.java index 78b75092938..d6c67b138f2 100644 --- a/rewrite-maven/src/test/java/org/openrewrite/maven/ChangeDependencyGroupIdAndArtifactIdTest.java +++ b/rewrite-maven/src/test/java/org/openrewrite/maven/ChangeDependencyGroupIdAndArtifactIdTest.java @@ -20,6 +20,7 @@ import org.openrewrite.Issue; import org.openrewrite.test.RewriteTest; +import static org.assertj.core.api.Assertions.assertThatExceptionOfType; import static org.openrewrite.java.Assertions.mavenProject; import static org.openrewrite.maven.Assertions.pomXml; @@ -98,6 +99,40 @@ void changeDependencyGroupIdAndArtifactId() { ); } + @Test + @Issue("https://github.com/openrewrite/rewrite-java-dependencies/issues/55") + void requireNewGroupIdOrNewArtifactId() { + assertThatExceptionOfType(AssertionError.class) + .isThrownBy(() -> rewriteRun( + spec -> spec.recipe(new ChangeDependencyGroupIdAndArtifactId( + "javax.activation", + "javax.activation-api", + null, + null, + null, + null, + null + )) + )).withMessageContaining("newGroupId OR newArtifactId must be different from before"); + } + + @Test + @Issue("https://github.com/openrewrite/rewrite-java-dependencies/issues/55") + void requireNewGroupIdOrNewArtifactIdToBeDifferentFromBefore() { + assertThatExceptionOfType(AssertionError.class) + .isThrownBy(() -> rewriteRun( + spec -> spec.recipe(new ChangeDependencyGroupIdAndArtifactId( + "javax.activation", + "javax.activation-api", + "javax.activation", + null, + null, + null, + null + )) + )).withMessageContaining("newGroupId OR newArtifactId must be different from before"); + } + @Test void overrideManagedDependency() { rewriteRun( diff --git a/rewrite-maven/src/test/java/org/openrewrite/maven/ChangeManagedDependencyGroupIdAndArtifactIdTest.java b/rewrite-maven/src/test/java/org/openrewrite/maven/ChangeManagedDependencyGroupIdAndArtifactIdTest.java index 8fe435bccc9..79d18e92325 100644 --- a/rewrite-maven/src/test/java/org/openrewrite/maven/ChangeManagedDependencyGroupIdAndArtifactIdTest.java +++ b/rewrite-maven/src/test/java/org/openrewrite/maven/ChangeManagedDependencyGroupIdAndArtifactIdTest.java @@ -17,9 +17,11 @@ import org.junit.jupiter.api.Test; import org.openrewrite.DocumentExample; +import org.openrewrite.Issue; import org.openrewrite.test.RewriteTest; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatExceptionOfType; import static org.openrewrite.maven.Assertions.pomXml; class ChangeManagedDependencyGroupIdAndArtifactIdTest implements RewriteTest { @@ -74,6 +76,21 @@ void changeManagedDependencyGroupIdAndArtifactId() { ); } + @Test + @Issue("https://github.com/openrewrite/rewrite-java-dependencies/issues/55") + void requireNewGroupIdOrNewArtifactIdToBeDifferentFromBefore() { + assertThatExceptionOfType(AssertionError.class) + .isThrownBy(() -> rewriteRun( + spec -> spec.recipe(new ChangeManagedDependencyGroupIdAndArtifactId( + "javax.activation", + "javax.activation-api", + "javax.activation", + "javax.activation-api", + null + )) + )).withMessageContaining("newGroupId OR newArtifactId must be different from before"); + } + @Test void changeManagedDependencyWithDynamicVersion() { rewriteRun(