From c41d4cb2d0dc6cf7a98f834feea5a24e39fbaf43 Mon Sep 17 00:00:00 2001 From: Roman Shevchenko Date: Tue, 14 Mar 2017 11:28:48 +0100 Subject: [PATCH] [updater] avoids using optional candidates in moved files lookup --- .../com/intellij/updater/DiffCalculator.java | 12 ++++--- updater/src/com/intellij/updater/Patch.java | 2 +- .../src/com/intellij/updater/PatchAction.java | 2 -- .../intellij/updater/PatchCreationTest.java | 36 +++++++++++++++++++ .../com/intellij/updater/PatchTestCase.java | 8 +++-- .../com/intellij/updater/UpdaterTestCase.java | 4 +++ 6 files changed, 54 insertions(+), 10 deletions(-) diff --git a/updater/src/com/intellij/updater/DiffCalculator.java b/updater/src/com/intellij/updater/DiffCalculator.java index c263a4e42fe0..bc0a5878d7d8 100644 --- a/updater/src/com/intellij/updater/DiffCalculator.java +++ b/updater/src/com/intellij/updater/DiffCalculator.java @@ -20,12 +20,13 @@ import java.util.*; public class DiffCalculator { public static Result calculate(Map oldChecksums, Map newChecksums) { - return calculate(oldChecksums, newChecksums, Collections.emptyList(), false); + return calculate(oldChecksums, newChecksums, Collections.emptyList(), Collections.emptyList(), false); } public static Result calculate(Map oldChecksums, Map newChecksums, List critical, + List optional, boolean lookForMoved) { Result result = new Result(); result.commonFiles = collect(oldChecksums, newChecksums, critical, true); @@ -54,12 +55,12 @@ public class DiffCalculator { for (Map.Entry create : toCreate.entrySet()) { if (Digester.isFile(create.getValue())) { List sameContent = byContent.get(create.getValue()); - String source = findBestCandidateForMove(sameContent, create.getKey()); + String source = findBestCandidateForMove(sameContent, create.getKey(), optional); boolean move = true; if (source == null) { List sameName = byName.get(new File(create.getKey()).getName()); - source = findBestCandidateForMove(sameName, create.getKey()); + source = findBestCandidateForMove(sameName, create.getKey(), optional); move = false; } @@ -89,15 +90,16 @@ public class DiffCalculator { return matches; } - private static String findBestCandidateForMove(List paths, String path) { + private static String findBestCandidateForMove(List paths, String path, List optional) { if (paths == null) return null; - if (paths.size() == 1) return paths.get(0); + boolean mandatory = !optional.contains(path); String best = ""; String[] dirs = path.split("/"); int common = 0; for (String other : paths) { + if (mandatory && optional.contains(other)) continue; // mandatory targets must not use optional sources String[] others = other.split("/"); for (int i = 0; i < dirs.length && i < others.length; i++) { if (dirs[dirs.length - i - 1].equals(others[others.length - i - 1])) { diff --git a/updater/src/com/intellij/updater/Patch.java b/updater/src/com/intellij/updater/Patch.java index f5b844d9331a..a374f7178413 100644 --- a/updater/src/com/intellij/updater/Patch.java +++ b/updater/src/com/intellij/updater/Patch.java @@ -70,7 +70,7 @@ public class Patch { File newerDir = new File(spec.getNewFolder()); Map oldChecksums = digestFiles(olderDir, spec.getIgnoredFiles(), isNormalized(), ui); Map newChecksums = digestFiles(newerDir, spec.getIgnoredFiles(), false, ui); - DiffCalculator.Result diff = DiffCalculator.calculate(oldChecksums, newChecksums, spec.getCriticalFiles(), true); + DiffCalculator.Result diff = DiffCalculator.calculate(oldChecksums, newChecksums, spec.getCriticalFiles(), spec.getOptionalFiles(), true); List tempActions = new ArrayList<>(); diff --git a/updater/src/com/intellij/updater/PatchAction.java b/updater/src/com/intellij/updater/PatchAction.java index 12ef8889861f..1e44950cc4a3 100644 --- a/updater/src/com/intellij/updater/PatchAction.java +++ b/updater/src/com/intellij/updater/PatchAction.java @@ -226,7 +226,6 @@ public abstract class PatchAction { PatchAction that = (PatchAction)o; - if (myFlags != that.myFlags) return false; if (myChecksum != that.myChecksum) return false; if (!Objects.equals(myPath, that.myPath)) return false; @@ -237,7 +236,6 @@ public abstract class PatchAction { public int hashCode() { int result = Objects.hashCode(myPath); result = 31 * result + (int)(myChecksum ^ (myChecksum >>> 32)); - result = 31 * result + myFlags; return result; } } \ No newline at end of file diff --git a/updater/testSrc/com/intellij/updater/PatchCreationTest.java b/updater/testSrc/com/intellij/updater/PatchCreationTest.java index f8dd8cc5e85d..21600388fc32 100644 --- a/updater/testSrc/com/intellij/updater/PatchCreationTest.java +++ b/updater/testSrc/com/intellij/updater/PatchCreationTest.java @@ -240,6 +240,42 @@ public class PatchCreationTest extends PatchTestCase { new UpdateAction(patch, "lib64/redist/annotations.jar", "lib64/annotations.jar", CHECKSUMS.ANNOTATIONS_JAR, true)); } + @Test + public void testNoOptionalFileMove1() throws Exception { + resetNewerDir(); + FileUtil.copy(new File(dataDir, "lib/annotations.jar"), new File(myOlderDir, "lib/annotations.bin")); + FileUtil.copy(new File(dataDir, "lib/annotations_changed.jar"), new File(myOlderDir, "lib64/annotations.bin")); + FileUtil.copy(new File(dataDir, "lib/annotations.jar"), new File(myNewerDir, "lib/redist/annotations.bin")); + FileUtil.copy(new File(dataDir, "lib/annotations.jar"), new File(myNewerDir, "lib64/redist/annotations.bin")); + + Patch patch = createPatch(spec -> spec.setOptionalFiles(Arrays.asList("lib/annotations.bin", "lib/redist/annotations.bin"))); + assertThat(sortActions(patch.getActions())).containsExactly( + new DeleteAction(patch, "lib/annotations.bin", CHECKSUMS.ANNOTATIONS_JAR_BIN), + new DeleteAction(patch, "lib64/annotations.bin", CHECKSUMS.ANNOTATIONS_CHANGED_JAR_BIN), + new CreateAction(patch, "lib/redist/"), + new CreateAction(patch, "lib64/redist/"), + new UpdateAction(patch, "lib/redist/annotations.bin", "lib/annotations.bin", CHECKSUMS.ANNOTATIONS_JAR_BIN, true), + new UpdateAction(patch, "lib64/redist/annotations.bin", "lib64/annotations.bin", CHECKSUMS.ANNOTATIONS_CHANGED_JAR_BIN, false)); + } + + @Test + public void testNoOptionalFileMove2() throws Exception { + resetNewerDir(); + FileUtil.copy(new File(dataDir, "lib/annotations_changed.jar"), new File(myOlderDir, "lib/annotations.bin")); + FileUtil.copy(new File(dataDir, "lib/annotations.jar"), new File(myOlderDir, "lib64/annotations.bin")); + FileUtil.copy(new File(dataDir, "lib/annotations.jar"), new File(myNewerDir, "lib/redist/annotations.bin")); + FileUtil.copy(new File(dataDir, "lib/annotations.jar"), new File(myNewerDir, "lib64/redist/annotations.bin")); + + Patch patch = createPatch(spec -> spec.setOptionalFiles(Arrays.asList("lib/annotations.bin", "lib/redist/annotations.bin"))); + assertThat(sortActions(patch.getActions())).containsExactly( + new DeleteAction(patch, "lib/annotations.bin", CHECKSUMS.ANNOTATIONS_CHANGED_JAR_BIN), + new DeleteAction(patch, "lib64/annotations.bin", CHECKSUMS.ANNOTATIONS_JAR_BIN), + new CreateAction(patch, "lib/redist/"), + new CreateAction(patch, "lib64/redist/"), + new UpdateAction(patch, "lib/redist/annotations.bin", "lib64/annotations.bin", CHECKSUMS.ANNOTATIONS_JAR_BIN, true), + new UpdateAction(patch, "lib64/redist/annotations.bin", "lib64/annotations.bin", CHECKSUMS.ANNOTATIONS_JAR_BIN, true)); + } + @Test public void testSaveLoad() throws Exception { Patch original = createPatch(); diff --git a/updater/testSrc/com/intellij/updater/PatchTestCase.java b/updater/testSrc/com/intellij/updater/PatchTestCase.java index 4d69718c8ce7..0acddb83bba5 100644 --- a/updater/testSrc/com/intellij/updater/PatchTestCase.java +++ b/updater/testSrc/com/intellij/updater/PatchTestCase.java @@ -60,9 +60,13 @@ public abstract class PatchTestCase extends UpdaterTestCase { } protected Patch createPatch() throws IOException, OperationCancelledException { - PatchSpec spec = new PatchSpec() + return createPatch(Function.identity()); + } + + protected Patch createPatch(Function tuner) throws IOException, OperationCancelledException { + PatchSpec spec = tuner.apply(new PatchSpec() .setOldFolder(myOlderDir.getAbsolutePath()) - .setNewFolder(myNewerDir.getAbsolutePath()); + .setNewFolder(myNewerDir.getAbsolutePath())); return new Patch(spec, TEST_UI); } diff --git a/updater/testSrc/com/intellij/updater/UpdaterTestCase.java b/updater/testSrc/com/intellij/updater/UpdaterTestCase.java index f668806ea738..4bfc8dfb17a2 100644 --- a/updater/testSrc/com/intellij/updater/UpdaterTestCase.java +++ b/updater/testSrc/com/intellij/updater/UpdaterTestCase.java @@ -60,10 +60,12 @@ public abstract class UpdaterTestCase { public final long README_TXT; public final long IDEA_BAT; public final long ANNOTATIONS_JAR; + public final long ANNOTATIONS_JAR_BIN; public final long BOOTSTRAP_JAR; public final long BOOTSTRAP_JAR_BINARY; public final long FOCUS_KILLER_DLL; public final long ANNOTATIONS_JAR_NORM; + public final long ANNOTATIONS_CHANGED_JAR_BIN; public final long ANNOTATIONS_CHANGED_JAR_NORM; public final long BOOT_JAR_NORM; public final long BOOT2_JAR_NORM; @@ -78,10 +80,12 @@ public abstract class UpdaterTestCase { README_TXT = windowsLineEnds ? 1272723667L : 7256327L; IDEA_BAT = windowsLineEnds ? 3088608749L : 1493936069L; ANNOTATIONS_JAR = 2119442657L; + ANNOTATIONS_JAR_BIN = 2525796836L; BOOTSTRAP_JAR = 2082851308L; BOOTSTRAP_JAR_BINARY = 2745721972L; FOCUS_KILLER_DLL = 1991212227L; ANNOTATIONS_JAR_NORM = 2119442657L; + ANNOTATIONS_CHANGED_JAR_BIN = 2587736223L; ANNOTATIONS_CHANGED_JAR_NORM = 4088078858L; BOOT_JAR_NORM = 3018038682L; BOOT2_JAR_NORM = 2406818996L;