Validate that Change(Managed)DependencyGroupIdAndArtifactId updates G…

…AV (#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 openrewrite/rewrite-java-dependencies#55
openrewrite · Jan 13, 2024 · b5e239c · b5e239c
1 parent 02d0ee9
commit b5e239c
Show file tree

Hide file tree

Showing 4 changed files with 83 additions and 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
@@ -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<Object> 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/...aven/src/main/java/org/openrewrite/maven/ChangeManagedDependencyGroupIdAndArtifactId.java b/...aven/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<Object> 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/...e-maven/src/test/java/org/openrewrite/maven/ChangeDependencyGroupIdAndArtifactIdTest.java b/...e-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/.../src/test/java/org/openrewrite/maven/ChangeManagedDependencyGroupIdAndArtifactIdTest.java b/.../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(