From 576f64ad61dc20dbc305785bdff554ca803dda69 Mon Sep 17 00:00:00 2001 From: Jeremy Heiler Date: Fri, 21 Dec 2018 10:27:19 -0500 Subject: [PATCH] Improve `combine` to not return lazy sequences The `combine` function attempts to add new values to a collection by merging them. However, for lists and a vectors, a lazy sequence is returned rather than a list or a vector. For lists, `(apply list (concat ...` is used instead of `(into ...` in order to preserve element order for backwards compatibility, as `into` would put the new elements at the head of the list. For vectors, `into` is used, as new elements are added to the tail. The implementation for sets is simplified. --- CHANGES.markdown | 1 + src/liberator/util.clj | 6 +++--- test/test_util.clj | 4 ++++ 3 files changed, 8 insertions(+), 3 deletions(-) diff --git a/CHANGES.markdown b/CHANGES.markdown index 5b9d069..99e0059 100644 --- a/CHANGES.markdown +++ b/CHANGES.markdown @@ -6,6 +6,7 @@ version which blocked updating some dependencies * Update clojure versions in the build matrix. * Allow `defresource` to have a docstring (#305) +* Improve `liberator.util/combine` to not return lazy sequences (#304) ## Bugs fixed diff --git a/src/liberator/util.clj b/src/liberator/util.clj index 14b9c6c..f334ef3 100644 --- a/src/liberator/util.clj +++ b/src/liberator/util.clj @@ -76,9 +76,9 @@ (cond (-> newval meta :replace) newval (and (map? curr) (map? newval)) (merge-with combine curr newval) - (and (list? curr) (coll? newval)) (concat curr newval) - (and (vector? curr) (coll? newval)) (concat curr newval) - (and (set? curr) (coll? newval)) (set (concat curr newval)) + (and (list? curr) (coll? newval)) (apply list (concat curr newval)) + (and (vector? curr) (coll? newval)) (into curr newval) + (and (set? curr) (coll? newval)) (into curr newval) :otherwise newval)) (defn is-protocol-exception? diff --git a/test/test_util.clj b/test/test_util.clj index 1f4bbb8..4db8629 100644 --- a/test/test_util.clj +++ b/test/test_util.clj @@ -5,9 +5,13 @@ (facts "combine function" (facts "simple combinations" (fact "merges map" (combine {:a 1} {:b 2}) => {:a 1 :b 2}) + (fact "returns a map" (combine {:a 1} {:b 2}) => map?) (fact "concats list" (combine '(1 2) [3 4]) => '(1 2 3 4)) + (fact "returns a list" (combine '(1 2) [3 4]) => list?) (fact "concats vector" (combine [1 2] '(3 4)) => [1 2 3 4]) + (fact "returns a vector" (combine [1 2] '(3 4)) => vector?) (fact "concats set" (combine #{1 2} [3 4]) => #{1 2 3 4}) + (fact "returns a set" (combine #{1 2} [3 4]) => set?) (facts "replaces other types" (fact (combine 123 456) => 456) (fact (combine "abc" 123) => 123)