malli icon indicating copy to clipboard operation
malli copied to clipboard

Multischema with Merge can generate wrong schemas

Open hkupty opened this issue 4 years ago • 3 comments

Hi, first of all thanks a lot for malli.

I know this is very specific, but from the outside it might look like there's something wrong in how multi specs are interpreted.

If I define a base schema that contains the shared properties and then define a multi schema extending it by adding just the missing/specific properties, even though it's considered a valid schema, it generates wrong results by mixing dispatch values and properties:

(require '[malli.core :as m]
         '[malli.generator :as mg]
         '[malli.util :as mu])

(def Base (m/schema [:map
                     [:id int?]
                     [:type [:enum :stuff :thing]]]))

(def Multi (m/schema [:multi {:dispatch :type}
                      [:stuff (mu/merge Base [:map [:name string?]])]
                      [:thing (mu/merge Base [:map [:address string?]])]]))

(mg/sample Multi)
;; {:id -1, :type :thing, :address ""}
;; {:id -1, :type :thing, :address "6"}
;; {:id 0, :type :stuff, :name "r"}
;; {:id 0, :type :thing, :name "m"}
;; {:id 2, :type :thing, :address ""}
;; {:id -3, :type :stuff, :name "d"}
;; {:id 0, :type :stuff, :address "2rq"}
;; {:id -1, :type :stuff, :name "yT"}
;; :id 1, :type :thing, :name "S6"}
;; {:id 0, :type :thing, :address "920"}

(map (partial m/validate Multi) (mg/sample Multi)) ;; => (true true false false false true true true true false)

I can solve the problem by overwriting the dispatch property with a single-valued enum:

(def Multi (m/schema [:multi {:dispatch :type}
                      [:stuff (mu/merge Base [:map
                                              [:name string?]
                                              [:type [:enum :stuff]]])]
                      [:thing (mu/merge Base [:map
                                              [:address string?]
                                              [:type [:enum :thing]]])]]))
(mg/sample Multi)
;; {:id 0, :type :stuff, :name ""}
;; {:id 0, :type :thing, :address "q"}
;; {:id 0, :type :stuff, :name ""}
;; {:id 2, :type :stuff, :name "9hn"}
;; {:id 2, :type :stuff, :name ""}
;; {:id 1, :type :thing, :address "sJ1"}
;; {:id 2, :type :stuff, :name "8"}
;; {:id 0, :type :stuff, :name "7esI6c"}
;; {:id -110, :type :stuff, :name "UB"}
;; {:id -7, :type :stuff, :name "WRY"}

(map (partial m/validate Multi) (mg/sample Multi)) ;; => (true true true true true true true true true true)

So even though the problem has a solution, I'm thinking whether there's something to be fixed so the generator behaves normally..

hkupty avatar Apr 02 '20 09:04 hkupty

Hi,

As the :dispatch function can be anything, there is no generic way to merge the :multi dispatch values automatically to the generated child values and the child schemas need to be complete as in you latter example. Instead of :enum, you can also use := here: [:type [:= :thing]]

I can think of few ways to simplify this:

  1. for the common case of :dispatch is a keyword, we could just merge the dispatch value with the dispatch key into the generated values
  2. one could add ad :gen/fmap function that is used to do the common merge for cases not automatically correct for 1.

ikitommi avatar Apr 02 '20 10:04 ikitommi

Even though I think option 1 is slightly "intrusive" (as you modify the resulting schema), it seems a valid approach. It's basically doing why I did manually to fix but under the hood. I'm wondering if that'd be fine for string or int values as well - specially if you use [:= <dispatch value>].

hkupty avatar Apr 02 '20 10:04 hkupty

dispatch value can be anything, but the :dispatch key in :multi should be keyword for the injection to work. Example below:

[:multi {:dispatch :type}
 [:sized [:map [:type any?] [:size int?]]]
  ["human" [:map [:type any?] [:name string?] [:address [:map [:country keyword?]]]]]]]

... but it also could be written as:

[:multi {:dispatch :type}
 [:sized [:map [:type [:= :sized] [:size int?]]]
  ["human" [:map [:type [:= "human"]] [:name string?] [:address [:map [:country keyword?]]]]]]]

and the generators would just work.

ikitommi avatar Apr 02 '20 16:04 ikitommi