Skip to content

Commit

Permalink
Merge pull request #48 from RutledgePaulV/fix-map-diffing-under-varia…
Browse files Browse the repository at this point in the history
…ble-key-order

varying key order in maps should produce consistent diffs
  • Loading branch information
alysbrooks authored Jul 24, 2023
2 parents 5a3c15a + f9e37b2 commit 4d92566
Show file tree
Hide file tree
Showing 4 changed files with 29 additions and 20 deletions.
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -20,3 +20,4 @@ resources/public/
.store
package-lock.json
.cache
*.iml
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@

## Fixed

Varying key order in maps should produce a consistent diff (#47)

## Changed

# 2.9.202 (2023-06-09 / 35494a0)
Expand Down
37 changes: 17 additions & 20 deletions src/lambdaisland/deep_diff2/diff_impl.cljc
Original file line number Diff line number Diff line change
@@ -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)
Expand Down Expand Up @@ -109,26 +110,22 @@
(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]
(case [(contains? exp-ks k) (contains? act-ks k)]
[true false]
(assoc m (->Deletion k) (get exp k))
[false true]
(assoc m (->Insertion k) (get act k))
[true true]
(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))))

(defn primitive? [x]
(or (number? x) (string? x) (boolean? x) (inst? x) (keyword? x) (symbol? x)))
Expand Down
9 changes: 9 additions & 0 deletions test/lambdaisland/deep_diff2/diff_test.cljc
Original file line number Diff line number Diff line change
Expand Up @@ -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}))))
Expand Down

0 comments on commit 4d92566

Please sign in to comment.