From 322a023fe57262c984b18cc8120e5a070fea63a4 Mon Sep 17 00:00:00 2001 From: Paul Rutledge Date: Mon, 19 Jun 2023 17:50:34 -0500 Subject: [PATCH 1/4] #47: varying key order in maps should produce consistent diffs --- .gitignore | 1 + src/lambdaisland/deep_diff2/diff_impl.cljc | 36 +++++++++------------ test/lambdaisland/deep_diff2/diff_test.cljc | 9 ++++++ 3 files changed, 26 insertions(+), 20 deletions(-) diff --git a/.gitignore b/.gitignore index 91d15fb..9faeae1 100644 --- a/.gitignore +++ b/.gitignore @@ -20,3 +20,4 @@ resources/public/ .store package-lock.json .cache +*.iml \ No newline at end of file diff --git a/src/lambdaisland/deep_diff2/diff_impl.cljc b/src/lambdaisland/deep_diff2/diff_impl.cljc index e864f87..ec0be33 100644 --- a/src/lambdaisland/deep_diff2/diff_impl.cljc +++ b/src/lambdaisland/deep_diff2/diff_impl.cljc @@ -1,5 +1,6 @@ (ns lambdaisland.deep-diff2.diff-impl (:require [clojure.data :as data] + [clojure.set :as set] [lambdaisland.clj-diff.core :as seq-diff])) (declare diff diff-similar) @@ -109,26 +110,21 @@ (remove #(contains? exp %) act))) (defn diff-map [exp act] - (first - (let [exp-ks (keys exp) - act-ks (keys act) - [del ins] (del+ins exp-ks act-ks)] - (reduce - (fn [[m idx] k] - [(cond-> m - (contains? del idx) - (assoc (->Deletion k) (get exp k)) - - (not (contains? del idx)) - (assoc k (diff (get exp k) (get act k))) - - (contains? ins idx) - (into (map (juxt ->Insertion (partial get act))) (get ins idx))) - (inc idx)]) - [(if (contains? ins -1) - (into {} (map (juxt ->Insertion (partial get act))) (get ins -1)) - {}) 0] - exp-ks)))) + (let [exp-ks (set (keys exp)) + act-ks (set (keys act))] + (reduce + (fn [m k] + (cond + (and (contains? exp-ks k) (not (contains? act-ks k))) + (assoc m (->Deletion k) (get exp k)) + + (and (contains? act-ks k) (not (contains? exp-ks k))) + (assoc m (->Insertion k) (get act k)) + + (and (contains? act-ks k) (contains? exp-ks k)) + (assoc m k (diff (get exp k) (get act k))))) + {} + (set/union exp-ks act-ks)))) (defn primitive? [x] (or (number? x) (string? x) (boolean? x) (inst? x) (keyword? x) (symbol? x))) diff --git a/test/lambdaisland/deep_diff2/diff_test.cljc b/test/lambdaisland/deep_diff2/diff_test.cljc index 04c4f58..766546f 100644 --- a/test/lambdaisland/deep_diff2/diff_test.cljc +++ b/test/lambdaisland/deep_diff2/diff_test.cljc @@ -92,6 +92,15 @@ (is (= {:a [1 (diff/->Deletion 2) 3]} (diff/diff {:a [1 2 3]} {:a [1 3]})))) + (testing "map key order doesn't impact diff result" + (is (= {:name (diff/->Mismatch "Alyysa P Hacker" "Alyssa P Hacker"), :age 40} + + (diff/diff (array-map :name "Alyysa P Hacker" :age 40) + (array-map :age 40 :name "Alyssa P Hacker")) + + (diff/diff (array-map :age 40 :name "Alyysa P Hacker") + (array-map :age 40 :name "Alyssa P Hacker"))))) + (testing "records" (is (= {:a (diff/->Mismatch 1 2)} (diff/diff (map->ARecord {:a 1}) (map->ARecord {:a 2})))) From f7fe707cf908ef2c06c18e4adfc2bbc0c3362bf5 Mon Sep 17 00:00:00 2001 From: Paul Rutledge Date: Mon, 19 Jun 2023 18:03:20 -0500 Subject: [PATCH 2/4] #47: varying key order in maps should produce consistent diffs --- src/lambdaisland/deep_diff2/diff_impl.cljc | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/lambdaisland/deep_diff2/diff_impl.cljc b/src/lambdaisland/deep_diff2/diff_impl.cljc index ec0be33..8271eca 100644 --- a/src/lambdaisland/deep_diff2/diff_impl.cljc +++ b/src/lambdaisland/deep_diff2/diff_impl.cljc @@ -114,14 +114,12 @@ act-ks (set (keys act))] (reduce (fn [m k] - (cond - (and (contains? exp-ks k) (not (contains? act-ks k))) + (case [(contains? exp-ks k) (contains? act-ks k)] + [true false] (assoc m (->Deletion k) (get exp k)) - - (and (contains? act-ks k) (not (contains? exp-ks k))) + [false true] (assoc m (->Insertion k) (get act k)) - - (and (contains? act-ks k) (contains? exp-ks k)) + [true true] (assoc m k (diff (get exp k) (get act k))))) {} (set/union exp-ks act-ks)))) From 5e52d24913c931a61eeed9827454f05fc301d3dd Mon Sep 17 00:00:00 2001 From: Paul Rutledge Date: Mon, 19 Jun 2023 18:03:31 -0500 Subject: [PATCH 3/4] add changelog entry --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4304757..8cc5658 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,8 @@ ## Fixed +Varying key order in maps should produce a consistent diff (#47) + ## Changed # 2.9.202 (2023-06-09 / 35494a0) From f9e37b215f0998f8583e76000278238e80dbe117 Mon Sep 17 00:00:00 2001 From: Paul Rutledge Date: Sun, 16 Jul 2023 19:47:27 -0500 Subject: [PATCH 4/4] add a comment about why it's okay to have omitted a case --- src/lambdaisland/deep_diff2/diff_impl.cljc | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/lambdaisland/deep_diff2/diff_impl.cljc b/src/lambdaisland/deep_diff2/diff_impl.cljc index 8271eca..ce90472 100644 --- a/src/lambdaisland/deep_diff2/diff_impl.cljc +++ b/src/lambdaisland/deep_diff2/diff_impl.cljc @@ -120,7 +120,10 @@ [false true] (assoc m (->Insertion k) (get act k)) [true true] - (assoc m k (diff (get exp k) (get act k))))) + (assoc m k (diff (get exp k) (get act k))) + ; `[false false]` will never occur because `k` necessarily + ; originated from at least one of the two sets + )) {} (set/union exp-ks act-ks))))