Implement malli-based coercion

[ikitommi]

• Adds support for malli-based coercion
• Strips extra keys on all EDN & Transit request bodies with clojure.spec (a MINOR bump?)

TODO

• encode on way back
• strip-extra-keys on both ways?
• example
• docs

Current defaults:

; defaults (closed + strip extras)
(reitit.malli.coercion/create)

;; open schemas, strip extra keys
(reitit.malli.coercion/create {:compile identity})

;; open schemas, don't strip extra keys
(reitit.malli.coercion/create {:compile identity, :strip-extra-keys false})

comments welcome on options & defaults

[rschmukler]

+1 for striping extra keys both ways. Also I suggest we do that as the default transformer so that users can override it if the want to.

[rschmukler]

Just testing this out on my own project. A few things of note:

1. Right now it's impossible to pass options to malli directly, so for example, a custom registry can't be done. I recommend adding malli-opts to the root options.

2. Coercion and validation needs to happen in inverse order on response encoding. ie. We want to first ensure that the response is valid, and then transform it - right now we transform and the validate (which makes sense for decode).

3. I frequently find myself writing nested maps, eg. {:error {:message string?}} - it might be nice if users could provide a custom function to map their body to the schema, allowing for this kind of data-spec short-hand syntax. Then again, maybe this should just be something that the user writes a function for and calls in their reitit map. eg. (error-message :message string?). The downside of this is that the definitions become less homoiconic as the response shapes.

[ikitommi]

1. added :options key to coercion options

2. changed the order

3. short-cut syntaxes are root of many evil, but adding a extension fn to support this would add the evil to user-space. Converting a spec app to malli-based: 3b53efc

Added also an option :error-keys to control what kind of errors are created, oob support for :humanized:

[ikitommi]

stripping extra keys on return by default for all schemas... not sure if that is a good property.

[rschmukler]

Looks good! One thing I'd like to suggest is (and perhaps this is more of a malli issue) that errors be in a consistent shape. Right now errors are a single error string in the case that there is only one, and a vector if there are many. This forces the work of always remembering to handle the differing shapes onto the client (in this case, the web client that consumes the error response JSON).

In general I think it's easier to consistently handle a more complex shape (eg. a collection) than having to handle both a complex shape and a single shape.

As I said, this might be more of a malli issue, but seeing it in the reitit web response made me recall it.

Re: stipping by default - I think it's a good default. Without default stripping a careless developer could accidentally send sensitive data (eg. :user/encrypted-password) without intending to. If someone wants to override the behavior, they specify their own :transformers.

[ikitommi]

Thanks for the comments.

• humanized errors will be changed in: Wrap humamized erros always in vectors malli#155

• The stripping-extra-keys on responses... not happy either way as changing a transformer introduces a lot of boilerplate, this should be resolved before releasing this. Possibilities:

1) By default, only strip extra keys from :closed schemas

• Schemas will have :closed property, effecting validation.
• coercion could have an option :close-schemas to close all (api) schemas. This can easily be set programmatically with malli.

2) Helper option in coercion

• :strip-extra-keys to easily set the stripping on/off (without needing to re-create all the transformers)

3) Butt-naked

• The current way, one needs to re-wire the transformers to enable & disable stripping.

[rschmukler]

My personal taste would be to lean towards #1. Making closed schemas a first-class citizen in malli seems most useful as it allows the validators / encoders / decoders to be used at the serialization boundaries of an application, which would be great. Similarly, adding the option (:close-schemas) also feels quite useful for ad-hoc generation of closed variants of schemas.

We could also have a function (m.utils/closed schema) which returns a new version of a schema with {:closed true} set in the properties.

[ikitommi]

Let's go with 1, related PR on malli: metosin/malli#156

[ikitommi]

Default options are now:

(def default-options
  {:transformers {:body {:default default-transformer-provider
                         :formats {"application/json" json-transformer-provider}}
                  :string {:default string-transformer-provider}
                  :response {:default default-transformer-provider}}
   ;; set of keys to include in error messages
   :error-keys #{:type :coercion :in :schema :value :errors :humanized #_:transformed}
   ;; schema identity function
   :compile mu/closed-schema
   ;; strip-extra-keys (effects only default transformers!)
   :strip-extra-keys true
   ;; add default values
   :default-values true
   ;; malli options
   :options nil})

we have an internal review of malli on friday, small tuning possible.