Fork me on GitHub
#malli
<
2019-11-24
>
ikitommi14:11:16

about closed/open schemas, comments welcome on this: https://github.com/metosin/malli/issues/31#issuecomment-557892061

👌 4
rschmukler17:11:24

This feature is currently blocking me, and I'm relatively familiar with malli's code at this point (lots of tracing to figure out how to implement the above - if you had ideas on what you were thinking in terms of implementation for #120 I could potentially submit a PR

rschmukler17:11:58

One possible solution that comes to mind is to just extend the Schema protocol to have a -parent method and then invoke things as appropriate

rschmukler17:11:35

Also, regarding the public m/parent function, perhaps we could also specify passing in a name to traverse up parents until one with the provided name is found

rschmukler17:11:57

eg (m/parent schema :map)

ikitommi17:11:11

@rschmukler I think the -parent is the way to go, but it might need some bigger refactoring on internals: when a Schema is created, the -father is not yet available, as we are creating the childs from the mothers “constructor”, so it needs to be a Delay or similar value. Derefon that while creating a child would either return nil or block indefinitely, depending on the implementation. So, would have to see the code after the change to know if that’s a good value for adding the (potential) complexity in.

ikitommi17:11:32

if that is blocking you, you could add a custom decoder on the :map schema already?

rschmukler17:11:14

@ikitommi I initially went down that path but then I had to do the key tracking to invoke the child schemas appropriately

rschmukler17:11:38

I was actually wondering if we might be better of implementing the coercions as a postwalk instead of a prewalk

ikitommi17:11:05

(m/decode 
  [:map {:decode/my-thing (fn [schema opts] ...do...thing...)} [:a int?] [:b int?]]
  {"a_kikka" 12, "b_kikka" 32}
  (mt/transformer {:name :my-thing))

rschmukler17:11:56

The issue becomes that if the map renames a key, the child schemas don't get mapped

rschmukler17:11:48

(because the map now has a new key at a different path, and the transformers for the values are resolved via key, after the key is renamed

rschmukler17:11:24

(Hence why I potentially suggest doing a postwalk instead of a prewalk for coercions) - is there any reason that it's currently implemented as a prewalk?

rschmukler17:11:07

@ikitommi I have another PR that I'm going to submit that might be a simpler solution for what I'm trying to achieve - feel free to reject it

👍 4
ikitommi18:11:16

It has to be prewalk, good example:

(m/decode
  [:multi {:dispatch :type
           :decode/string '(constantly #(update % :type keyword))}
   [:sized [:map [:type [:= :sized] [:size int?]]]
   [:human [:map [:type [:= :human]] [:name string?] [:address [:map [:country keyword?]]]]]]
  {:type "human"
   :name "Tiina"
   :age "98"
   :address {:country "finland"
             :street "this is an extra key"}}
  (mt/transformer mt/strip-extra-keys-transformer mt/string-transformer))
;{:type :human
; :name "Tiina"
; :address {:country :finland}}

ikitommi18:11:56

there will be two-way transformers using interceptos, one could put all the stuff into :leave to get postwalk

rschmukler18:11:22

@ikitommi Interceptors is a great idea

rschmukler18:11:53

@ikitommi are you open to applying the map transformer after the key and value transformer for the :map schema?

rschmukler18:11:11

ie. Ideally I'd like to be able to have:

(is (= {:x_key "true", :y 1} (m/decode schema {:x true :y "1"}
                                             (transform/transformer
                                              {:name :custom
                                               :decoders
                                               {:map
                                                (constantly (fn [map]
                                                              (if-not (contains? map :x)
                                                                map
                                                                (-> map
                                                                    (assoc :x_key (:x map))
                                                                    (dissoc :x)))))
                                                'boolean? (constantly str)}}))))

rschmukler18:11:00

Actually this feels inconsistent - perhaps just implementing the interceptors is the way to go

ikitommi20:11:47

@rschmukler what is your schema in the example?

rschmukler20:11:07

[:map [:x boolean? :y int?]]

rschmukler20:11:25

(although lets assume that I composed with transformer/json-transformer so that the number decodes correctly

rschmukler20:11:26

I realize that I can always pop an escape hatch and write a :custom/decode function but since I want to apply it on every map, it seems like a transformer is the idiomatic way to go

rschmukler20:11:05

I think interceptors solve everything so I'm trying my hand at implementing them right now

ikitommi20:11:24

but, isn’t that m/encode you are doing?

ikitommi20:11:26

decode should end up in valid values against the schema (eg. JSON->Schema), encode is Schema->JSON.

rschmukler20:11:12

That's a buggy example haha

rschmukler20:11:16

(mine above)

rschmukler20:11:46

It is indeed an encode

rschmukler20:11:20

But, none the less, I believe after the key is renamed during the encode, the subsequent value transformers will fail because they look up by the original key name

rschmukler20:11:29

(ie. (if-let [entry (find m k)] (assoc m k (t (val entry))) I believe is the code in the map schema)

ikitommi20:11:42

true that. interceptors are needed

rschmukler20:11:56

Yeah, hoping to have a PR today

ikitommi20:11:22

of interceptors? that’s a big one.

rschmukler20:11:46

Right now I'm having it so that -transformer and -value-transformer return {:enter _ :exit _} maps

rschmukler20:11:05

and then I'm updating the schema impls to use those on children as needed

rschmukler20:11:34

(defn- ->interceptor
  "Utility function to convert a transformer into an interceptor. Works with transformers
  that are already interceptors, as well as sequences of transformers"
  [transformer]
  (cond
   (fn? transformer) {:enter transformer :exit nil}
   (and (map? transformer)
        (or (contains? transformer :enter)
            (contains? transformer :exit))) transformer

   (coll? transformer) (reduce
                        (fn [{:keys [enter exit]} {new-enter :enter new-exit :exit}]
                          (let [enter (if (and enter new-enter)
                                        (comp enter new-enter)
                                        (or enter new-enter))
                                exit (if (and exit new-exit)
                                       (comp exit new-exit)
                                       (or exit new-exit))]
                            {:enter enter :exit exit}))
                        (map ->interceptor transformer))
   (nil? transformer) nil
   :else (throw (ex-info "Invalid transformer. Must be a function, sequence, or interceptor map"
                         {:value transformer}))))

rschmukler20:11:06

I think that solves it, converting all transformers into an interceptor, even allowing for the vectors that you included

rschmukler20:11:38

that map call should be a keep

rschmukler20:11:05

Then, like I said, basically all Schema impls need to be aware of :enter and :exit when returning their own interceptor (with :enter and :exit) themselves

ikitommi20:11:14

awesome, looking forward to the PR. (`:exit`=> :leave would be coherent with interceptor libs)

rschmukler20:11:26

and then, I think encoder / decoder / encode / decode just need to call (comp exit enter)

rschmukler20:11:35

Ah, yes, will rename the key, thank you!

rschmukler20:11:59

Do you want me to rename the -transformer and -value-transformer fns to -interceptor and -value-interceptor?

rschmukler20:11:03

(no opinions here)