metosin / malli

High-performance data-driven data specification library for Clojure/Script.
Eclipse Public License 2.0
1.44k stars 204 forks source link

Problematic execution order of decode/encode #938

Open solita-nikoa opened 10 months ago

solita-nikoa commented 10 months ago

Consider following example. I have a map with some structure, some properties are simply typed and some are more complex and hence custom schema. In real world :complex would be somehow dependent from :simple.

Also I think explicit [:and [:map ...]] is very practical due schema support some built-in features like closed-schema etc.

So now I want to make some meaningful business logic and of course with decoded values. And that's works fine on decode but on encode I need to operate on decoded values. If I flip :and definition order, I have opposite problem.

There also seems to be :enter and :leave phases. I don't really understand them, they sound good fit but do nothing at least on this case (bug?).

To me this seems pretty much the same case how middlewares as general concept are handled in many http backend libraries. There execution order of http response is inversed compared to response execution order.

(def bug-hunt-schema
  [:and
   [:map
    [:simple :keyword]
    [:complex :any]]
   (m/-simple-schema
    {:type :bug-hunt-schema
     :pred (fn [_value] true)
     :type-properties
     {:decode/string {:enter (fn [value] (prn :decode-enter value) value)
                      :leave (fn [value] (prn :decode-leave value) value)}
      :encode/string {:enter (fn [value] (prn :encode-enter value) value)
                      :leave (fn [value] (prn :encode-leave value) value)}}})])

(m/decode
 (mu/closed-schema
  bug-hunt-schema)
 {:simple "hello" :complex 42}
 mt/string-transformer)

;; :decode-enter {:simple :hello, :complex 42}
;; :decode-leave {:simple :hello, :complex 42}
;; => {:simple :hello, :complex 42}

(m/encode
 (mu/closed-schema
  bug-hunt-schema)
 {:simple :hello :complex 42}
 mt/string-transformer)

;; :encode-enter {:simple "hello", :complex 42}
;; :encode-leave {:simple "hello", :complex 42}
;; {:simple "hello", :complex 42}

[Minor side note]

So maybe I just bite the bullet and re-decode inside encoder. Btw using what transformer? This depends from context but I claim that it would be practical that encode/decode/transfomers would have access to current transformer so that certain recursive taks would be made without being over specific which transformer is used.

ikitommi commented 10 months ago

Interesting. All schemas run both :decode and :encode in children in same order. I also think this is wrong. I need to revisit the ordering, thanks for reporting.

Tested with a dirty (and invalid) fix:

diff --git a/src/malli/core.cljc b/src/malli/core.cljc
index d66b3f9..a2b3b1e 100644
--- a/src/malli/core.cljc
+++ b/src/malli/core.cljc
@@ -519,7 +519,8 @@
 (defn -parent-children-transformer [parent children transformer method options]
   (let [parent-transformer (-value-transformer transformer parent method options)
         child-transformers (into [] (keep #(-transformer % transformer method options)) children)
-        child-transformer (when (seq child-transformers) (apply -comp (rseq child-transformers)))]
+        mapper (if (= :encode method) identity rseq)
+        child-transformer (when (seq child-transformers) (apply -comp (mapper child-transformers)))]
     (-intercepting parent-transformer child-transformer)))

.. which gives "correct" order/results for this case but doesn't solve the whole problem.

ikitommi commented 10 months ago

Documenting to myself, current order on decode:

(let [log (atom [])
      log! (fn [s p] (fn [x] (swap! log conj [(:i (m/properties s)) p]) x))]
  (m/decode
   [:map {:i 0}
    [:x {:i 1} [:int {:i 2}]]
    [:y {:i 3} [:string {:i 4}]]
    [:z {:i 5} [:maybe {:i 6} [:and {:i 7}
                               [:keyword {:i 8}]
                               [:or {:i 9}
                                [:string {:i 10}]
                                [:qualified-keyword {:i 11}]]]]]]
   {:x 1
    :y "kikka"
    :z :kikka/kukka}
   (mt/transformer
    {:default-decoder {:compile (fn [schema _]
                                  {:enter (log! schema :enter)
                                   :leave (log! schema :leave)})}}))
  @log)
;[[0 :enter]
; [1 :enter]
; [2 :enter]
; [2 :leave]
; [1 :leave]
; [3 :enter]
; [4 :enter]
; [4 :leave]
; [3 :leave]
; [5 :enter]
; [6 :enter]
; [7 :enter]
; [8 :enter]
; [8 :leave]
; [9 :enter]
; [10 :enter]
; [10 :leave]
; [11 :enter]
; [11 :leave]
; [9 :leave]
; [7 :leave]
; [6 :leave]
; [5 :leave]
; [0 :leave]]