spec-tools/explain-data differs from spec/explain-data when using transformer

[mping]

When using a transformer, it seems that if the spec fails, the transformer stops midway. I guess that the transformer should finalize transformation and only after it should try to validate the spec.

Picking on the spec-tools example:

(s/def ::name string?)
(s/def ::birthdate spec/inst?)

(s/def ::languages
  (s/coll-of
    (s/and spec/keyword? #{:clj :cljs})
    :into #{}))

(s/def ::user
  (s/keys
    :req-un [::name ::languages ::age]
    :opt-un [::birthdate]))

;; vanilla, only 1 problem
(s/explain-data ::user  {:name "foo" :age 24 :lanxguages [:clj] :birthdate (java.util.Date.)})

;; reports problems with ::languages and :birthdate too
(st/explain-data ::user  
                 {:name "foo" :age "24" :lanxguages ["clj"] :birthdate "1968-01-02T15:04:05Z"}
                 st/string-transformer)

;; without languages typo, is ok
(st/explain-data ::user
                 {:name "foo" :age "24" :languages ["clj"] :birthdate "1968-01-02T15:04:05Z"}
                 st/string-transformer)

;;EDIT apparently, coercing before explain works
(st/explain-data
  ::user
  (st/coerce ::user {:name "foo" :age "24" :lanxguages ["clj"] :birthdate "1968-01-02T15:04:05Z"} st/string-transformer)
  st/string-transformer)

[mping]

I believe the problem is in compojure.api.coercion.spec/coerce-request implementation of Coercion protocol.

I think st/conform should be called upon the coerced value, not the original one.

Here's the code, with my comments:

(coerce-request [_ spec value type format _]
    (let [spec (maybe-memoized-specify spec)
          type-options (options type)]
      (if-let [transformer (or (get (get type-options :formats) format)
                               (get type-options :default))]
        (let [coerced (st/coerce spec value transformer)]
          (if (s/valid? spec coerced)
            coerced
            ;; st/conform should be called with "coerced" instead of "value"?
            (let [conformed (st/conform spec value transformer)]
              (if (s/invalid? conformed)
                ;; st/explain-data should be called with "coerced" instead of "value"?
                (let [problems (st/explain-data spec value transformer)]
                  (cc/map->CoercionError
                    {:spec spec
                     :problems problems}))
                (s/unform spec conformed)))))
        value)))

Looking at schema coercion, it seems that the behaviour is what I expect:

(let [coerce (memoized-coercer schema matcher)
              coerced (coerce value)]
          (if (su/error? coerced)
            (let [errors (su/error-val coerced)] ;;error-val is called on top of coerced val
              (cc/map->CoercionError
                {:schema schema
                 :errors errors}))
            coerced))

Let me know if this is correct, I'll be happy to submit a PR.

EDIT: a PR in the correct repo, seems I got lost while debugging: https://github.com/metosin/compojure-api/issues/409

[lfborjas]

Fwiw, I ended up in this thread after several hours of troubleshooting my specs. It seems to violate the principle of least surprise that coerce will give up halfway and not decode otherwise perfectly valid values, making things like explain-data return more problems than there actually are. My use case boils down to:

• Define a data-spec with ds/spec, with explicit decode/string and decode/json entries.
• st/conform and st/explain work fine (i.e. the former will transform all values in a valid object, and the latter will only report actual problems in invalid ones)
• When using st/coerce and st/explain-data on invalid objects, however, the behavior is way different: st/coerce will only coerce some values in e.g. a map spec/data-spec, and st/explain-data will also only transform some values and leave others that would be valid if transformation had gotten to them, thus wrongly reporting more problems than there are. It boggles the mind that one needs to do (st/explain-data spec (st/coerce spec value transformers) transformers) to get "the right thing" -- it's not obvious, and it feels wrong.

Maybe (likely!) this is user error, maybe this is an inherent shortcoming of spec.alpha that spec-tools is trying its best to improve on, but, dang, it really makes one doubt relying on spec for "parsing" raw data into domain data in a declarative way if these non-obvious time-sinks are lurking along the way -- perhaps such is the fate of the early adopter!

Edit: that "explain the coerced" workaround doesn't even work for more complex cases: I have a data spec that references other data specs and even though the workaround is effective for the individual components, it breaks down again for their composition. Sad day, I may need to re-write my specs using vanilla clojure.spec because I'm sure I'm somehow misusing spec-tools's fancier constructs, just wanted to update my comment in case any passerby think I found a robust solution 😅

[ikitommi]

If you could give a code snipplet that doesn't work, I could try at least to figure out what goes wrong with it. Spec should support data transformations itself, have pushed a patch some years ago, with no luck. Spec2 might help, but the latest comment is that data transformations are still out of scope. Spec-tools tries to do the best that can be done from outside, but there are issues and the codebase related to coercion/conform is already quite complex.

malli will have first class two-way data transformation, but surely, will try to fix this here too. Just need a broken sample to start digging.

cheers.

[ikitommi]

btw, here's how reitit tries to do it: https://github.com/metosin/reitit/blob/master/modules/reitit-spec/src/reitit/coercion/spec.cljc#L120-L132

[lfborjas]

Thanks for the link and background! That and the compojure sample linked above helped me figure some stuff out, as I said, I think it was pretty much me misunderstanding how data specs work and compose with each other, vs. vanilla s/keys specs. I rewrote most of my specs using regular spec "syntax" and left the top-level data-specs alone, and the coerce-then-explain-data trick now works reliably for non-conforming values. Will cook up a simplified sample later today once I've gotten the code cleaned up.

On a separate, more personal note: thanks for the work on all these libraries, I'm a huge fan of Reitit--recently rewrote our frontend routing to use it, to take advantage of the notion of controllers, and we use the heck out of it for our swagger-enabled backend API; even dug a little bit and made it so that our SPA's routes are in a cljc file so the backend knows which routes are legit and which it should fail fast for with a 404. If I sound frustrated in the previous comments, it's more at the inherently mercurial nature of spec in clojure in general. From the activity in the core bug tracker, it seems there's a bit of friction in the various directions people are exploring, but I'm hopeful we'll end up with something very elegant and powerful after these travails, and not have to downgrade our codebases to Haskell/Elm to get that sweet sweet typing 😬

[anyayunli]

will cook up a simplified sample later today once I've gotten the code cleaned up.

Do we have a simplified sample now, as I found this part is very confusing.