plumatic / schema

Clojure(Script) library for declarative data description and validation
Other
2.4k stars 256 forks source link

Proposal: allow maps to have s/precondition-key for use in s/cond-pre #428

Closed shaunlebron closed 2 years ago

shaunlebron commented 2 years ago

Background

My team uses s/either for maps and noticed it was deprecated. I studied the repo to find out why (and in the process I added a Schema Quick Ref to the wiki for newcomers).

I learned that s/cond-pre relies on hidden preconditions belonging to all core schemas, so maps can’t be used there since they all share the same precondition map?.

Proposal

Add s/precondition-key for use in map specs. Each precondition-key found in a map is appended as a precondition on the map schema. They are also treated as required keys.

Example

The wiki mentions a Shape schema, which we can rewrite with this proposal:

Before:

(s/defschema Shape
  "A schema for shapes: squares, rectangles, and circles"
  (s/conditional
    #(= (:type %) :square) {:type (s/eq :square) :side s/Num}
    #(= (:type %) :rectangle) {:type (s/eq :rectangle) :width s/Num :height s/Num}
    #(= (:type %) :circle) {:type (s/eq :circle) :radius s/Num}))

After:

(s/defschema Shape
  "A schema for shapes: squares, rectangles, and circles"
  (s/cond-pre
    {(s/precondition-key :type) (s/eq :square) :side s/Num}
    {(s/precondition-key :type) (s/eq :rectangle) :width s/Num :height s/Num}
    {(s/precondition-key :type) (s/eq :circle) :radius s/Num}))

Tests

I added basic tests for s/precondition-key and added more cases to s/cond-pre that use it.

Blind Spots

I think I understand that s/either was deprecated for better efficiency and errors. And I have a general sense that a tagged union should have a proper way to compute its claimed type (tag) without validating everything in it. But I don’t understand why it breaks coercion. So this proposal comes with the caveat that I may be missing the purpose of deprecating it.

frenchy64 commented 2 years ago

Have you considered something like this?

(defn dispatch [k & cases]
  (assert (-> cases count even?))
  (apply conditional
         (mapcat (fn [[d s]]
                   [#(= d (get % k))
                    (if (map? s)
                      (assoc s k (eq d))
                      ;; see also schema-tools.core/merge for handling this case
                      s)])
                 (partition 2 cases))))

Applied to your example:

(comment
  (defschema Shape
    (dispatch :type
              :square {:side Num}
              :rectangle {:width Num :height Num}
              :circle {:radius Num}))
  (validate
    Shape
    {:type :square :side 1})
  )
w01fe commented 2 years ago

Thanks for writing this up!

I agree with @frenchy64 that it would be preferable to build something on top of the existing conditional support, unless there is something I'm missing that is gained by adding this additional complexity into the core of the library. Can you say more about why you chose this approach?

As for your question about coercion, the problem with coercion is that you need to be able to figure out what kind of thing something is supposed to be in order to coerce it to that thing. With either you would have to attempt coercion on each branch and take the first which succeeds; this might be okay in some cases, but seems like it could produce unexpected behavior or performance problems in others.

frenchy64 commented 2 years ago

TIL about schema-refined.core/dispatch-on which seems more general than my example.

shaunlebron commented 2 years ago

the problem with coercion is that you need to be able to figure out what kind of thing something is supposed to be in order to coerce it to that thing. With either you would have to attempt coercion on each branch and take the first which succeeds; this might be okay in some cases, but seems like it could produce unexpected behavior or performance problems in others.

Thanks @w01fe, this is very clear now 👍

TIL about schema-refined.core/dispatch-on

Thanks for the reference! That’ll do. Closing now 👋