Skip to content

Commit 48d403b

Browse files
authored
Fix unchanged Quark files appearing in changeset results (#7130)
* Fix unchanged Quark and Binary files appearing in changeset results Quark.printAll() and Binary.printAll() throw UnsupportedOperationException, causing Result.isLocalAndHasNoChanges() to incorrectly report them as changed. Add explicit handling for these types before calling printAll(). Fixes #3948 * Add test for Quark replaced by Remote (wrapper update scenario) * Remove binary file comparison from Result Removed binary file comparison logic from Result class.
1 parent 89ee6c9 commit 48d403b

File tree

2 files changed

+47
-5
lines changed

2 files changed

+47
-5
lines changed

rewrite-core/src/main/java/org/openrewrite/Result.java

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@
2323
import org.openrewrite.marker.DeserializationError;
2424
import org.openrewrite.marker.RecipesThatMadeChanges;
2525
import org.openrewrite.marker.SearchResult;
26+
import org.openrewrite.binary.Binary;
27+
import org.openrewrite.quark.Quark;
2628
import org.openrewrite.remote.Remote;
2729

2830
import java.nio.file.Path;
@@ -268,11 +270,21 @@ public String toString() {
268270

269271
public static boolean isLocalAndHasNoChanges(@Nullable SourceFile before, @Nullable SourceFile after) {
270272
try {
271-
return (before == after) ||
272-
(before != null && after != null &&
273-
// Remote source files are fetched on `printAll`, let's avoid that cost.
274-
!(before instanceof Remote) && !(after instanceof Remote) &&
275-
before.printAll().equals(after.printAll()));
273+
if (before == after) {
274+
return true;
275+
}
276+
if (before == null || after == null) {
277+
return false;
278+
}
279+
// Remote source files are fetched on `printAll`, let's avoid that cost.
280+
if (before instanceof Remote || after instanceof Remote) {
281+
return false;
282+
}
283+
// Quarks don't support printAll(); compare by source path instead
284+
if (before instanceof Quark && after instanceof Quark) {
285+
return before.getSourcePath().equals(after.getSourcePath());
286+
}
287+
return before.printAll().equals(after.printAll());
276288
} catch (Exception e) {
277289
return false;
278290
}

rewrite-core/src/test/java/org/openrewrite/quark/QuarkTest.java

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,15 +18,45 @@
1818
import org.jspecify.annotations.Nullable;
1919
import org.junit.jupiter.api.Test;
2020
import org.openrewrite.*;
21+
import org.openrewrite.marker.Markers;
2122
import org.openrewrite.marker.SearchResult;
23+
import org.openrewrite.remote.Remote;
2224
import org.openrewrite.test.RewriteTest;
2325

26+
import java.net.URI;
27+
import java.nio.file.Path;
28+
import java.nio.file.Paths;
29+
import java.util.UUID;
30+
2431
import static java.util.Objects.requireNonNull;
32+
import static org.assertj.core.api.Assertions.assertThat;
2533
import static org.openrewrite.test.RewriteTest.toRecipe;
2634
import static org.openrewrite.test.SourceSpecs.other;
2735

2836
class QuarkTest implements RewriteTest {
2937

38+
@Test
39+
void unchangedQuarkHasNoChanges() {
40+
Quark before = new Quark(UUID.randomUUID(), Paths.get("file.jks"), Markers.EMPTY, null, null);
41+
Quark after = new Quark(UUID.randomUUID(), Paths.get("file.jks"), Markers.EMPTY, null, null);
42+
assertThat(Result.isLocalAndHasNoChanges(before, after)).isTrue();
43+
}
44+
45+
@Test
46+
void quarkWithDifferentPathHasChanges() {
47+
Quark before = new Quark(UUID.randomUUID(), Paths.get("old.jks"), Markers.EMPTY, null, null);
48+
Quark after = new Quark(UUID.randomUUID(), Paths.get("new.jks"), Markers.EMPTY, null, null);
49+
assertThat(Result.isLocalAndHasNoChanges(before, after)).isFalse();
50+
}
51+
52+
@Test
53+
void quarkReplacedByRemoteHasChanges() {
54+
Path path = Paths.get(".mvn/wrapper/maven-wrapper.jar");
55+
Quark before = new Quark(UUID.randomUUID(), path, Markers.EMPTY, null, null);
56+
Remote after = Remote.builder(before).build(URI.create("https://repo.maven.apache.org/maven2/org/apache/maven/wrapper/maven-wrapper/3.3.2/maven-wrapper-3.3.2.jar"));
57+
assertThat(Result.isLocalAndHasNoChanges(before, after)).isFalse();
58+
}
59+
3060
@DocumentExample
3161
@Test
3262
void renderMarkersOnQuarks() {

0 commit comments

Comments
 (0)