From 46b22c4bc40e85c21b2af32f6fdaa307ae5b07c0 Mon Sep 17 00:00:00 2001 From: Tommi Reiman Date: Sun, 3 Sep 2023 14:03:52 +0300 Subject: [PATCH 1/7] fix #910 --- src/malli/core.cljc | 24 +++++++++++--------- src/malli/impl/util.cljc | 2 +- test/malli/core_test.cljc | 47 +++++++++++++++++++++++++++++++-------- 3 files changed, 53 insertions(+), 20 deletions(-) diff --git a/src/malli/core.cljc b/src/malli/core.cljc index 07c087d62..7520a91de 100644 --- a/src/malli/core.cljc +++ b/src/malli/core.cljc @@ -764,11 +764,13 @@ (-intercepting this-transformer (if (= :decode method) (fn [x] - (reduce-kv - (fn [x i transformer] - (let [x* (transformer x)] - (if ((nth validators i) x*) (reduced x*) x))) - x transformers)) + (key (reduce-kv + (fn [acc i transformer] + (let [x* (transformer (val acc))] + (if ((nth validators i) x*) + (reduced (miu/-tagged x*)) + (miu/-tagged (or (key acc) x*) x)))) + (miu/-tagged nil x) transformers))) (fn [x] (reduce-kv (fn [x i validator] (if (validator x) (reduced ((nth transformers i) x)) x)) @@ -839,11 +841,13 @@ (-intercepting this-transformer (if (= :decode method) (fn [x] - (reduce-kv - (fn [x i transformer] - (let [x* (transformer x)] - (if ((nth validators i) x*) (reduced x*) x))) - x transformers)) + (key (reduce-kv + (fn [acc i transformer] + (let [x* (transformer (val acc))] + (if ((nth validators i) x*) + (reduced (miu/-tagged x*)) + (miu/-tagged (or (key acc) x*) x)))) + (miu/-tagged nil x) transformers))) (fn [x] (reduce-kv (fn [x i validator] (if (validator x) (reduced ((nth transformers i) x)) x)) diff --git a/src/malli/impl/util.cljc b/src/malli/impl/util.cljc index 440336a81..264e5be99 100644 --- a/src/malli/impl/util.cljc +++ b/src/malli/impl/util.cljc @@ -5,7 +5,7 @@ (def ^:const +max-size+ #?(:clj Long/MAX_VALUE, :cljs (.-MAX_VALUE js/Number))) -(defn -tagged [k v] #?(:clj (MapEntry. k v), :cljs (MapEntry. k v nil))) +(defn -tagged ([v] (-tagged v v)) ([k v] #?(:clj (MapEntry. k v), :cljs (MapEntry. k v nil)))) (defn -tagged? [v] (instance? MapEntry v)) (defn -invalid? [x] #?(:clj (identical? x :malli.core/invalid), :cljs (keyword-identical? x :malli.core/invalid))) diff --git a/test/malli/core_test.cljc b/test/malli/core_test.cljc index 8f35acc61..cb3fca595 100644 --- a/test/malli/core_test.cljc +++ b/test/malli/core_test.cljc @@ -330,15 +330,44 @@ [:y keyword?] [:enter boolean?]]]]]] (are [input result] - (= (m/decode (mu/closed-schema schema) input mt/string-transformer) result) - - {:x "true"} {:x :true, :enter true, :leave true} - {:x "true", :enter "invalid"} {:x "true", :enter "invalid", :leave true} - - {:y "true"} {:y :true, :enter true, :leave true} - {:y "true", :leave "invalid"} {:y "true", :enter true, :leave "invalid"} - - {:x "true", :y "true"} {:x "true", :y "true", :enter true, :leave true})))) + (= (m/decode (mu/closed-schema schema) input (mt/string-transformer)) result) + + {:x "true"} {:x :true, :enter true, :leave true} ;; first + {:x "true", :enter "invalid"} {:x :true, :enter "invalid", :leave true} ;; first (fallback) + {:y "true"} {:y :true, :enter true, :leave true} ;; second + {:y "true", :leave "invalid"} {:y "true", :enter true, :leave "invalid"} ;; no match + {:x "true", :y "true"} {:x :true, :y "true", :enter true, :leave true}))) ;; first (fallback)) + + (testing "branch short-circuit" + + (let [math (mt/transformer {:name :math}) + math-string [:string {:decode/math (partial str "math_")}] + math-kw-string [:and math-string [:any {:decode/math keyword}]] + bono-string [:string {:decode/math (partial str "such_")}]] + + (testing "first successful branch is selected" + (is (= "math_1" + (m/decode math-string 1 math) + (m/decode [:and math-string] 1 math) + (m/decode [:or math-string] 1 math) + (m/decode [:or + math-kw-string + math-string + bono-string] 1 math) + (m/decode [:orn ["string" math-string]] 1 math) + (m/decode [:orn + ["kw-math" math-kw-string] + ["math" math-string] + ["bono" bono-string]] 1 math)))) + + (testing "first branch is selected, even if invalid" + (is (= :math_1 + (m/decode [:or + math-kw-string + :string] 1 math) + (m/decode [:orn + ["kw-math" math-kw-string] + ["string" :string]] 1 math))))))) (testing "explain with branches" (let [schema [:and pos-int? neg-int?]] From 1053cd726e725230c5f6dd78a3ad5551df374150 Mon Sep 17 00:00:00 2001 From: Tommi Reiman Date: Sun, 3 Sep 2023 14:51:57 +0300 Subject: [PATCH 2/7] deduplicate code --- src/malli/core.cljc | 63 +++++++++++++++++---------------------------- 1 file changed, 24 insertions(+), 39 deletions(-) diff --git a/src/malli/core.cljc b/src/malli/core.cljc index 7520a91de..d2cb2ba6c 100644 --- a/src/malli/core.cljc +++ b/src/malli/core.cljc @@ -727,6 +727,28 @@ (-get [_ key default] (get children key default)) (-set [this key value] (-set-assoc-children this key value))))))) +(defn -or-transformer [this transformer child-schemas method options] + (let [this-transformer (-value-transformer transformer this method options)] + (if (seq child-schemas) + (let [transformers (-vmap #(or (-transformer % transformer method options) identity) child-schemas) + validators (-vmap -validator child-schemas)] + (-intercepting this-transformer + (if (= :decode method) + (fn [x] + (key (reduce-kv + (fn [acc i transformer] + (let [x* (transformer (val acc))] + (if ((nth validators i) x*) + (reduced (miu/-tagged x*)) + (miu/-tagged (or (key acc) x*) x)))) + (miu/-tagged nil x) transformers))) + (fn [x] + (reduce-kv + (fn [x i validator] (if (validator x) (reduced ((nth transformers i) x)) x)) + x validators))))) + (-intercepting this-transformer)))) + + (defn -or-schema [] ^{:type ::into-schema} (reify IntoSchema @@ -757,25 +779,7 @@ (-parser [_] (->parser -parser)) (-unparser [_] (->parser -unparser)) (-transformer [this transformer method options] - (let [this-transformer (-value-transformer transformer this method options)] - (if (seq children) - (let [transformers (-vmap #(or (-transformer % transformer method options) identity) children) - validators (-vmap -validator children)] - (-intercepting this-transformer - (if (= :decode method) - (fn [x] - (key (reduce-kv - (fn [acc i transformer] - (let [x* (transformer (val acc))] - (if ((nth validators i) x*) - (reduced (miu/-tagged x*)) - (miu/-tagged (or (key acc) x*) x)))) - (miu/-tagged nil x) transformers))) - (fn [x] - (reduce-kv - (fn [x i validator] (if (validator x) (reduced ((nth transformers i) x)) x)) - x validators))))) - (-intercepting this-transformer)))) + (-or-transformer this transformer children method options)) (-walk [this walker path options] (-walk-indexed this walker path options)) (-properties [_] properties) (-options [_] options) @@ -833,26 +837,7 @@ ::invalid) ::invalid)))) (-transformer [this transformer method options] - (let [this-transformer (-value-transformer transformer this method options)] - (if (seq (-children this)) - (let [transformers (-vmap (fn [[_ _ c]] (or (-transformer c transformer method options) identity)) - (-children this)) - validators (-vmap (fn [[_ _ c]] (-validator c)) (-children this))] - (-intercepting this-transformer - (if (= :decode method) - (fn [x] - (key (reduce-kv - (fn [acc i transformer] - (let [x* (transformer (val acc))] - (if ((nth validators i) x*) - (reduced (miu/-tagged x*)) - (miu/-tagged (or (key acc) x*) x)))) - (miu/-tagged nil x) transformers))) - (fn [x] - (reduce-kv - (fn [x i validator] (if (validator x) (reduced ((nth transformers i) x)) x)) - x validators))))) - (-intercepting this-transformer)))) + (-or-transformer this transformer (-vmap #(nth % 2) (-children this)) method options)) (-walk [this walker path options] (-walk-entries this walker path options)) (-properties [_] properties) (-options [_] options) From d16f1108f13196ec78c2e3706ce12d773a879b63 Mon Sep 17 00:00:00 2001 From: Tommi Reiman Date: Sun, 3 Sep 2023 14:58:04 +0300 Subject: [PATCH 3/7] move code into correct place in file --- src/malli/core.cljc | 43 +++++++++++++++++++++---------------------- 1 file changed, 21 insertions(+), 22 deletions(-) diff --git a/src/malli/core.cljc b/src/malli/core.cljc index d2cb2ba6c..471979ed8 100644 --- a/src/malli/core.cljc +++ b/src/malli/core.cljc @@ -547,6 +547,27 @@ x)))) :cljs (fn [x] (into (when x empty) (map t) x)))) +(defn -or-transformer [this transformer child-schemas method options] + (let [this-transformer (-value-transformer transformer this method options)] + (if (seq child-schemas) + (let [transformers (-vmap #(or (-transformer % transformer method options) identity) child-schemas) + validators (-vmap -validator child-schemas)] + (-intercepting this-transformer + (if (= :decode method) + (fn [x] + (key (reduce-kv + (fn [acc i transformer] + (let [x* (transformer (val acc))] + (if ((nth validators i) x*) + (reduced (miu/-tagged x*)) + (miu/-tagged (or (key acc) x*) x)))) + (miu/-tagged nil x) transformers))) + (fn [x] + (reduce-kv + (fn [x i validator] (if (validator x) (reduced ((nth transformers i) x)) x)) + x validators))))) + (-intercepting this-transformer)))) + ;; ;; ast ;; @@ -727,28 +748,6 @@ (-get [_ key default] (get children key default)) (-set [this key value] (-set-assoc-children this key value))))))) -(defn -or-transformer [this transformer child-schemas method options] - (let [this-transformer (-value-transformer transformer this method options)] - (if (seq child-schemas) - (let [transformers (-vmap #(or (-transformer % transformer method options) identity) child-schemas) - validators (-vmap -validator child-schemas)] - (-intercepting this-transformer - (if (= :decode method) - (fn [x] - (key (reduce-kv - (fn [acc i transformer] - (let [x* (transformer (val acc))] - (if ((nth validators i) x*) - (reduced (miu/-tagged x*)) - (miu/-tagged (or (key acc) x*) x)))) - (miu/-tagged nil x) transformers))) - (fn [x] - (reduce-kv - (fn [x i validator] (if (validator x) (reduced ((nth transformers i) x)) x)) - x validators))))) - (-intercepting this-transformer)))) - - (defn -or-schema [] ^{:type ::into-schema} (reify IntoSchema From b751c1def8bd2871517f86d6732808fa6f8d3234 Mon Sep 17 00:00:00 2001 From: Tommi Reiman Date: Sun, 3 Sep 2023 15:00:02 +0300 Subject: [PATCH 4/7] tagging requires always k & v --- src/malli/core.cljc | 2 +- src/malli/impl/util.cljc | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/malli/core.cljc b/src/malli/core.cljc index 471979ed8..3849fec04 100644 --- a/src/malli/core.cljc +++ b/src/malli/core.cljc @@ -559,7 +559,7 @@ (fn [acc i transformer] (let [x* (transformer (val acc))] (if ((nth validators i) x*) - (reduced (miu/-tagged x*)) + (reduced (miu/-tagged x* nil)) (miu/-tagged (or (key acc) x*) x)))) (miu/-tagged nil x) transformers))) (fn [x] diff --git a/src/malli/impl/util.cljc b/src/malli/impl/util.cljc index 264e5be99..440336a81 100644 --- a/src/malli/impl/util.cljc +++ b/src/malli/impl/util.cljc @@ -5,7 +5,7 @@ (def ^:const +max-size+ #?(:clj Long/MAX_VALUE, :cljs (.-MAX_VALUE js/Number))) -(defn -tagged ([v] (-tagged v v)) ([k v] #?(:clj (MapEntry. k v), :cljs (MapEntry. k v nil)))) +(defn -tagged [k v] #?(:clj (MapEntry. k v), :cljs (MapEntry. k v nil))) (defn -tagged? [v] (instance? MapEntry v)) (defn -invalid? [x] #?(:clj (identical? x :malli.core/invalid), :cljs (keyword-identical? x :malli.core/invalid))) From 1d0f512a723161554755c666b591ed0a6742fa7a Mon Sep 17 00:00:00 2001 From: Tommi Reiman Date: Sun, 3 Sep 2023 15:28:50 +0300 Subject: [PATCH 5/7] remove duplicate tests --- test/malli/core_test.cljc | 80 ++++++++++++++------------------------- 1 file changed, 29 insertions(+), 51 deletions(-) diff --git a/test/malli/core_test.cljc b/test/malli/core_test.cljc index cb3fca595..367a1867f 100644 --- a/test/malli/core_test.cljc +++ b/test/malli/core_test.cljc @@ -292,25 +292,34 @@ (is (= [:and 'int? [:orn [:pos 'pos-int?] [:neg 'neg-int?]]] (m/form schema*)))) (testing "transforming :or" - (testing "first valid transformed branch is used" - (doseq [schema [[:or - [:map [:x keyword?]] - int? - [:map [:y keyword?]] - keyword?] - [:orn - [:äxy [:map [:x keyword?]]] - [:n int?] - [:yxy [:map [:y keyword?]]] - [:kw keyword?]]]] - (are [input result] - (= (m/decode schema input mt/string-transformer) result) - - {:x "true", :y "true"} {:x :true, :y "true"} - {:x false, :y "true"} {:x false, :y :true} - {:x false, :y false} {:x false, :y false} - 1 1 - "kikka" :kikka))) + (let [math (mt/transformer {:name :math}) + math-string [:string {:decode/math (partial str "math_")}] + math-kw-string [:and math-string [:any {:decode/math keyword}]] + bono-string [:string {:decode/math (partial str "such_")}]] + + (testing "first successful branch is selected" + (is (= "math_1" + (m/decode math-string 1 math) + (m/decode [:and math-string] 1 math) + (m/decode [:or math-string] 1 math) + (m/decode [:or + math-kw-string + math-string + bono-string] 1 math) + (m/decode [:orn ["string" math-string]] 1 math) + (m/decode [:orn + ["kw-math" math-kw-string] + ["math" math-string] + ["bono" bono-string]] 1 math)))) + + (testing "first branch value is selected as fallback, even if invalid" + (is (= :math_1 + (m/decode [:or + math-kw-string + :string] 1 math) + (m/decode [:orn + ["kw-math" math-kw-string] + ["string" :string]] 1 math))))) (testing "top-level transformations are retained" (doseq [schema [[:or {:decode/string {:enter (fn [m] (update m :enter #(or % true))) @@ -336,38 +345,7 @@ {:x "true", :enter "invalid"} {:x :true, :enter "invalid", :leave true} ;; first (fallback) {:y "true"} {:y :true, :enter true, :leave true} ;; second {:y "true", :leave "invalid"} {:y "true", :enter true, :leave "invalid"} ;; no match - {:x "true", :y "true"} {:x :true, :y "true", :enter true, :leave true}))) ;; first (fallback)) - - (testing "branch short-circuit" - - (let [math (mt/transformer {:name :math}) - math-string [:string {:decode/math (partial str "math_")}] - math-kw-string [:and math-string [:any {:decode/math keyword}]] - bono-string [:string {:decode/math (partial str "such_")}]] - - (testing "first successful branch is selected" - (is (= "math_1" - (m/decode math-string 1 math) - (m/decode [:and math-string] 1 math) - (m/decode [:or math-string] 1 math) - (m/decode [:or - math-kw-string - math-string - bono-string] 1 math) - (m/decode [:orn ["string" math-string]] 1 math) - (m/decode [:orn - ["kw-math" math-kw-string] - ["math" math-string] - ["bono" bono-string]] 1 math)))) - - (testing "first branch is selected, even if invalid" - (is (= :math_1 - (m/decode [:or - math-kw-string - :string] 1 math) - (m/decode [:orn - ["kw-math" math-kw-string] - ["string" :string]] 1 math))))))) + {:x "true", :y "true"} {:x :true, :y "true", :enter true, :leave true})))) ;; first (fallback)) (testing "explain with branches" (let [schema [:and pos-int? neg-int?]] From 85de30f2c118dad854e4244fae88413e65fe7392 Mon Sep 17 00:00:00 2001 From: Tommi Reiman Date: Mon, 4 Sep 2023 13:52:45 +0300 Subject: [PATCH 6/7] simpler loop --- src/malli/core.cljc | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/malli/core.cljc b/src/malli/core.cljc index 3849fec04..6cdd31ae4 100644 --- a/src/malli/core.cljc +++ b/src/malli/core.cljc @@ -555,13 +555,13 @@ (-intercepting this-transformer (if (= :decode method) (fn [x] - (key (reduce-kv - (fn [acc i transformer] - (let [x* (transformer (val acc))] - (if ((nth validators i) x*) - (reduced (miu/-tagged x* nil)) - (miu/-tagged (or (key acc) x*) x)))) - (miu/-tagged nil x) transformers))) + (reduce-kv + (fn [acc i transformer] + (let [x* (transformer x)] + (if ((nth validators i) x*) + (reduced x*) + (or acc x*)))) + nil transformers)) (fn [x] (reduce-kv (fn [x i validator] (if (validator x) (reduced ((nth transformers i) x)) x)) From 0ab9170aa7e321524ad97cdb5ee0bc0d09c5d4b2 Mon Sep 17 00:00:00 2001 From: Tommi Reiman Date: Tue, 5 Sep 2023 18:37:39 +0300 Subject: [PATCH 7/7] allow nil as valid fallback value --- src/malli/core.cljc | 4 ++-- test/malli/core_test.cljc | 10 +++++++++- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/src/malli/core.cljc b/src/malli/core.cljc index 6cdd31ae4..8808e22f2 100644 --- a/src/malli/core.cljc +++ b/src/malli/core.cljc @@ -560,8 +560,8 @@ (let [x* (transformer x)] (if ((nth validators i) x*) (reduced x*) - (or acc x*)))) - nil transformers)) + (if (-equals acc ::nil) x* acc)))) + ::nil transformers)) (fn [x] (reduce-kv (fn [x i validator] (if (validator x) (reduced ((nth transformers i) x)) x)) diff --git a/test/malli/core_test.cljc b/test/malli/core_test.cljc index 367a1867f..2ef6fba5b 100644 --- a/test/malli/core_test.cljc +++ b/test/malli/core_test.cljc @@ -319,7 +319,15 @@ :string] 1 math) (m/decode [:orn ["kw-math" math-kw-string] - ["string" :string]] 1 math))))) + ["string" :string]] 1 math)))) + + (testing "first branch nil can be selected as a fallback" + (is (= nil (m/decode + [:or + [:keyword {:decode/math (constantly nil)}] + :keyword] + "kikka" + (mt/transformer {:name :math})))))) (testing "top-level transformations are retained" (doseq [schema [[:or {:decode/string {:enter (fn [m] (update m :enter #(or % true)))