s/or causes inconsistent results with st/coerce

[kelveden]

Consider that I define the following contrived specs:

(require '[clojure.spec.alpha :as s])
(require '[spec-tools.core :as st])

(s/def ::keyword keyword?)
(s/def ::int int?)
(s/def ::date inst?)
(s/def ::s (s/or :x (s/keys :req-un [::keyword ::int])
                 :y (s/keys :req-un [::keyword ::date])))

Now we can try some coercions:

(st/coerce ::s {:date "2020-02-22"} st/json-transformer)
=> {:date #inst"2020-02-22T00:00:00.000-00:00"}

(st/coerce ::s {:keyword "a"} st/json-transformer)
=> {:keyword :a}

So far so good: the values in the maps are coerced to "json friendly" form; but...

(st/coerce ::s {:keyword "a" :date dt} st/json-transformer)
=> {:keyword :a, :date "2020-02-22"}

i.e. why is the :date value now not coerced to an inst?

[kelveden]

Note that if ::s is defined instead as

(s/def ::s (s/keys :req-un [::keyword ::date]))

then the coercion works as expected:

(st/coerce ::s {:keyword "a" :date dt} st/json-transformer)
=> {:keyword :a, :date #inst"2020-02-22T00:00:00.000-00:00"}

[kelveden]

Also, if the two s/or clauses are swapped around; i.e.

(s/def ::s (s/or :y (s/keys :req-un [::keyword ::date])
                 :x (s/keys :req-un [::keyword ::int])))

then the coercion also works as expected:

(st/coerce ::s {:keyword "a" :date dt} st/json-transformer)
=> {:keyword :a, :date #inst"2020-02-22T00:00:00.000-00:00"}

[kelveden]

Note that a workaround (of sorts) for this problem is simply to apply the coercion twice; i.e.

(st/coerce ::s
  (st/coerce ::s {:keyword "a" :date dt} st/json-transformer)
  st/json-transformer)
=> {:keyword :a, :date #inst"2020-02-22T00:00:00.000-00:00"}

This works because the result of the first coercion causes the value for :keyword to be coerced a keyword and, therefore, is ignored by the second coercion.

[ivarref]

Thanks for a fine project!
This issue is also hitting us. Will there be a new release for this soon?
Thanks!

[edmondkong]

I am also wondering when is the next release that will include this patch.

Otherwise, I'd like to thank the developers of this project as well. The coercion and data-specs features makes using spec a much more intuitive experience!

[biiwide]

Hello, I think have encountered a variation of this issue through Reitit. Using s/or with s/keys specs and the strip-extra-keys-transformer. Here is an example of what I am encountering:

(require '[clojure.spec.alpha :as s]
         '[spec-tools.core :as st])

(s/def ::keyword keyword?)
(s/def ::int int?)
(s/def ::date inst?)
(s/def ::s (s/merge
             (s/keys :req-un [::keyword])
             (s/or :x (s/keys :req-un [::int])
                   :y (s/keys :req-un [::date]))))

(st/coerce ::s {:keyword "a" :date "2020-02-02"} st/strip-extra-keys-transformer)
=> {}

(st/coerce ::s
           {:keyword "a" :int "1"}
           (st/type-transformer
             st/strip-extra-keys-transformer
             st/json-transformer))
=> {}

I encountered this because the default coercion settings in reitit.coercion.spec define the json-transformer as:

(def json-transformer
  (st/type-transformer
    st/strip-extra-keys-transformer
    st/json-transformer))

I am using:

• [metosin/reitit "0.5.12"]
• [metosin/reitit-spec "0.5.12"]
• [metosin/spec-tools "0.10.5"]
• [org.clojure/clojure "1.10.3"]
• • [org.clojure/core.specs.alpha "0.2.56"]
• • [org.clojure/spec.alpha "0.2.194"]

[JoaquinIglesiasTurina]

I've taken a look at the issue of reitit's strict-json-transfomer and st/coerce yielding strange results.
The issue is also reported at reitit's issue 494 and spec-tools' issue 255.

The issue spawns from the walk :or method at spec-tools core. Since it is reducing over all the items (different specs within the spec/or), it is essentialy coercing to a given item and the next given item and so forth and so on.
If a strict-json-transformer is used, it is possible that you pass some correct data, some item in the reduction will strip all its content, and by the time you are coercing to the actual s/or item you want to coerce, you are dealing with an empty map.

Following @ikitommi 's comment at issue 178, I have implemented a toy solution:

(defmethod walk :or [{:keys [::parse/items]} value accept options]
  (defn walk-or-helper
    "for a particular `item` of the spec, this function coerces that `value` 
     into that `item` and returns the coerced value if `valid?`"
    [item]
    (let [transformed (accept item value options)
          valid? (some-> item :spec (s/valid? transformed))]
      {:transformed transformed :valid? valid?}))
   ;; iterate above function, return valid value or the last one or original value
  (let [valid-items (for [item items]
                      (walk-or-helper item))]
    (or (-> (filter #(:valid? % ) valid-items) (last) :transformed)
        (:transformed (last valid-items))
        value)))

Issues with the proposed solution

Now, I think that needs some more thought, and it also fails some tests.
The main problem I have is I fail to understand under which circumstances those failed tests yield a result that I would expect. To be crystal clear: I would like confirmation that the test I am going to mention do indeed constitute an expected result. I do not care for the fact that I don't understand why that would be an expected result, only want to know if the change of those results is acceptable.

Testing Composed
In that particular test, the value under keys ["keys" ::c2] gets coerced to a keyword. Nowhere in the spec is that requested. And it seems to me that coercing some value string to a keyword is odd. Is this the expected result of coercion?

Smaller changes
This and this do not pass with the proposed solution, but would work if changing the order of the arguments.

[kelveden]

I've just had an opportunity to revisit this issue and it looks like it has been fixed - I can't replicate with version 0.10.5 of spec-tools.

So, closing.

[opqdonut]

there's still some remaining weirdness, see #255