Merge pull request #566 from bhagany/nested-diff

Correctly diff nested data structures
boot-clj · Sep 26, 2017 · 3c3ed6b · 3c3ed6b
2 parents ef42889 + bf71f03
commit 3c3ed6b
Show file tree

Hide file tree

Showing 3 changed files with 86 additions and 5 deletions.
diff --git a/CHANGES.md b/CHANGES.md
@@ -5,6 +5,7 @@
 #### Fixed
 
 - When directories or files cannot be opened by boot, don't fail but log something in debug level [#598][598] & [#629][629]
+- `fileset-diff` correctly handles nested data structures [#566][566]
 
 [598]: https://github.com/boot-clj/boot/pull/598
 [629]: https://github.com/boot-clj/boot/pull/629

diff --git a/boot/pod/src/boot/tmpdir.clj b/boot/pod/src/boot/tmpdir.clj
@@ -210,16 +210,17 @@
 (defn- diff*
   [{t1 :tree :as before} {t2 :tree :as after} props]
   (if-not before
-    {:added   after
+    {:added   (or after {:tree {}})
      :removed (assoc after :tree {})
      :changed (assoc after :tree {})}
     (let [props   (or (seq props) [:id])
           d1      (diff-tree t1 props)
           d2      (diff-tree t2 props)
-          [x y _] (map (comp set keys) (data/diff d1 d2))]
-      {:added   (->> (set/difference   y x) (select-keys t2) (assoc after :tree))
-       :removed (->> (set/difference   x y) (select-keys t1) (assoc after :tree))
-       :changed (->> (set/intersection x y) (select-keys t2) (assoc after :tree))})))
+          [x y z] (map (comp set keys) (data/diff d1 d2))
+          changed-keys (set/union (set/intersection x y) (set/intersection (set/union x y) z))]
+      {:added   (->> (set/difference y x changed-keys) (select-keys t2) (assoc after :tree))
+       :removed (->> (set/difference x y changed-keys) (select-keys t1) (assoc after :tree))
+       :changed (->> changed-keys                      (select-keys t2) (assoc after :tree))})))
 
 (defn- fatal-conflict?
   [^File dest]

diff --git a/boot/pod/test/boot/tmpdir_test.clj b/boot/pod/test/boot/tmpdir_test.clj
@@ -0,0 +1,79 @@
+(ns boot.tmpdir-test
+  (:refer-clojure :exclude [hash time])
+  (:require
+    [clojure.test :refer :all]
+    [boot.tmpdir :as tmpd]))
+
+(defn has-path? [fs path]
+  (contains? (:tree fs) path))
+
+(deftest diff*-scalar-test
+  (let [before (tmpd/map->TmpFileSet {:tree {}})
+        after (tmpd/map->TmpFileSet {:tree {"path" {:adding true}}})
+        diff (#'tmpd/diff* before after [:adding])]
+    (testing "Added"
+      (is (has-path? (:added diff) "path"))
+      (is (not (has-path? (:removed diff) "path")))
+      (is (not (has-path? (:changed diff) "path")))))
+  (let [before (tmpd/map->TmpFileSet {:tree {"path" {:removing true}}})
+        after (tmpd/map->TmpFileSet {:tree {}})
+        diff (#'tmpd/diff* before after [:removing])]
+    (testing "Removed"
+      (is (has-path? (:removed diff) "path"))
+      (is (not (has-path? (:added diff) "path")))
+      (is (not (has-path? (:changed diff) "path")))))
+  (let [before (tmpd/map->TmpFileSet {:tree {"path" {:changing true}}})
+        after (tmpd/map->TmpFileSet {:tree {"path" {:changing false}}})
+        diff (#'tmpd/diff* before after [:changing])]
+    (testing "Changed"
+      (is (has-path? (:changed diff) "path"))
+      (is (not (has-path? (:added diff) "path")))
+      (is (not (has-path? (:removed diff) "path"))))))
+
+(deftest diff*-nested-test
+  (let [before (tmpd/map->TmpFileSet {:tree {}})
+        after (tmpd/map->TmpFileSet {:tree {"path" {:nested {:adding true}}}})
+        diff (#'tmpd/diff* before after [:nested])]
+    (testing "Added"
+      (is (has-path? (:added diff) "path"))
+      (is (not (has-path? (:removed diff) "path")))
+      (is (not (has-path? (:changed diff) "path")))))
+  (let [before (tmpd/map->TmpFileSet {:tree {"path" {:nested {:removing true}}}})
+        after (tmpd/map->TmpFileSet {:tree {}})
+        diff (#'tmpd/diff* before after [:nested])]
+    (testing "Removed"
+      (is (has-path? (:removed diff) "path"))
+      (is (not (has-path? (:added diff) "path")))
+      (is (not (has-path? (:changed diff) "path")))))
+  (let [before (tmpd/map->TmpFileSet {:tree {"path" {:nested {:changing true}}}})
+        after (tmpd/map->TmpFileSet {:tree {"path" {:nested {:changing false}}}})
+        diff (#'tmpd/diff* before after [:nested])]
+    (testing "Changed (simple)"
+      (is (has-path? (:changed diff) "path"))
+      (is (not (has-path? (:added diff) "path")))
+      (is (not (has-path? (:removed diff) "path")))))
+  (let [before (tmpd/map->TmpFileSet {:tree {"path" {:nested {:staying true :changing true}}}})
+        after (tmpd/map->TmpFileSet {:tree {"path" {:nested {:staying true :changing false}}}})
+        diff (#'tmpd/diff* before after [:nested])]
+    (testing "Changed (complex)"
+      (is (has-path? (:changed diff) "path"))
+      (is (not (has-path? (:added diff) "path")))
+      (is (not (has-path? (:removed diff) "path"))))))
+
+(deftest diff*-edge-test
+  (let [before {}
+        after {}
+        diff (#'tmpd/diff* before after [:hash :time])
+        empty-fs {:tree {}}]
+    (testing "Empty dicts"
+      (is (= empty-fs (:added diff)))
+      (is (= empty-fs (:removed diff)))
+      (is (= empty-fs (:changed diff)))))
+  (let [before nil
+        after nil
+        diff (#'tmpd/diff* before after [:hash :time])
+        empty-fs {:tree {}}]
+    (testing "Nil"
+      (is (= empty-fs (:added diff)))
+      (is (= empty-fs (:removed diff)))
+      (is (= empty-fs (:changed diff))))))