Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

clojure.spec.alpha/merge breaks dynamic conforming #90

Closed
arttuka opened this issue Nov 17, 2017 · 5 comments
Closed

clojure.spec.alpha/merge breaks dynamic conforming #90

arttuka opened this issue Nov 17, 2017 · 5 comments

Comments

@arttuka
Copy link
Contributor

arttuka commented Nov 17, 2017

With this spec:

(require '[clojure.spec.alpha :as s])

(s/def ::kw1 st/keyword?)
(s/def ::kw2 st/keyword?)
(s/def ::map (s/merge (s/keys :req-un [::kw1])
                      (s/keys :req-un [::kw2])))

and this input :

(def input {:kw1 "kw1",
            :kw2 "kw2"})

in the conformed map, the value of :kw1 is the string "kw1"and the value of :kw2 is the keyword :kw2:

(require '[spec-tools.core :as st])
(st/conform ::map input st/json-conforming)
=> {:kw1 "kw1"
    :kw2 :kw2}

The reason is that clojure.spec.alpha/merge separately conforms each merged spec, then merges the results, so the unconformed value of :kw1 from the latter result overwrites the conformed value from the former result.

@slipset
Copy link
Contributor

slipset commented Feb 12, 2018

Could it be the same thing as I'm seeing?

(defn string->clj-time [_ s]
  (if (string? s)
    (f/parse (:date-time f/formatters) s)
    s))

(def doesn't-work-conforming
  (spec-tools/type-conforming
   {:clj-time string->clj-time}))

(s/def ::named (s/keys :req-un [::name]))

(s/def ::created (spec-tools/spec date-time? {:type :clj-time}))
(s/def ::timestamped (s/keys :req-un [::created]))

(s/def ::broken-thing (s/merge ::timestamped ::named))

(s/def ::working-thing (s/merge ::named ::timestamped))

user> (spec-tools/conform ::broken-thing {:name "foo" :created (str (clj-time.core/now)) } doesn't-work-conforming)
;; => {:name "foo", :created "2018-02-12T21:39:20.084Z"}
user> (spec-tools/conform ::working-thing {:name "foo" :created (str (clj-time.core/now)) } doesn't-work-conforming)
;; => {:name "foo",
 :created
 #object[org.joda.time.DateTime 0x3491879 "2018-02-12T21:39:28.413Z"]}

Yes, it seems to be. I applied the stuff in the pr and problem went away.

@ikitommi
Copy link
Member

As Alex said, "This is working as designed" but a new JIRA can be created. Please :) I can't see why why it works like it does today.

Meanwhile, let's just merge the PR?

@slipset
Copy link
Contributor

slipset commented Feb 13, 2018

There might be a regression here wrt s/or. I’ll have a look during today and try to come up with a small repro.

@slipset
Copy link
Contributor

slipset commented Feb 13, 2018

I cannot seem to reproduce the problem I had with s/or, so I guess this could be merged.
Also, will you cut a release of spec-tools, lein complains about using snapshots in uberjars.

@ikitommi
Copy link
Member

It's now spec-tools.core/merge and in [metosin/spec-tools "0.6.1"]. Thanks @arttuka!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants