metosin / reitit

A fast data-driven routing library for Clojure/Script
https://cljdoc.org/d/metosin/reitit/
Eclipse Public License 1.0
1.41k stars 255 forks source link

Provide documentation and example of response coercion using spec #297

Open johnmcconnell opened 5 years ago

johnmcconnell commented 5 years ago

I am attempting to decode clj-time datetime objects as a json string. For example:

(clj-time.core/now) #=> #clj-time/date-time "2019-06-16T01:56:14.670Z

I have tried using spec transforms to automatically coerce my response to a string format for json encoding. For example:

  (s/def ::created-at
    (st/spec
     {:spec (s/spec :clj-time.spec/date-time)
      :swagger/type "string"
      :swagger/format "date-time"
      :decode/string (fn [_ y]
                       (clj-time.coerce/to-string y))
      :encode/string clj-time.coerce/from-string}))

However, this does not seem to work as expected. I have included a small test case below.

  (require '[reitit.core :as r])

  (s/def ::created-at
    (st/spec
     {:spec (s/spec :clj-time.spec/date-time)
      :swagger/type "string"
      :swagger/format "date-time"
      :decode/string (fn [_ y]
                       (clj-time.coerce/to-string y))
      :encode/string clj-time.coerce/from-string}))

  (def test-router
    (r/router
     ["/:created-at" {:name ::user-view
                                  :coercion reitit.coercion.spec/coercion
                                  :parameters {:path {:created-at (s/spec ::created-at)}}}]
     {:compile coercion/compile-request-coercers}))

  (coercion/coerce!
   (r/match-by-path test-router "/2019-06-20T04:46:58.968Z"))

#=> 
Unhandled clojure.lang.ExceptionInfo
   Request coercion failed: #reitit.coercion.CoercionError{:spec #Spec{:form
   (clojure.spec.alpha/keys :req-un [:spec$28727/created-at]), :type :map,
   :leaf? false}, :problems #:clojure.spec.alpha{:problems ({:path
   [:created-at], :pred :clojure.spec.alpha/unknown, :val
   "2019-06-20T04:46:58.968Z", :via [:spec$28727/created-at], :in
   [:created-at]}), :spec #Spec{:form (clojure.spec.alpha/keys :req-un
   [:spec$28727/created-at]), :type :map, :leaf? false}, :value {:created-at
   "2019-06-20T04:46:58.968Z"}}}
   {:spec
    {:spec
     #object[clojure.spec.alpha$map_spec_impl$reify__1931 0x16d0ed64 "clojure.spec.alpha$map_spec_impl$reify__1931@16d0ed64"],
     :form (clojure.spec.alpha/keys :req-un [:spec$28727/created-at]),
     :type :map,
     :spec-tools.parse/key->spec {:created-at :spec$28727/created-at},
     :spec-tools.parse/keys #{:created-at},
     :spec-tools.parse/keys-req #{:created-at},
     :leaf? false},
    :problems
    #:clojure.spec.alpha{:problems
                         ({:path [:created-at],
                           :pred :clojure.spec.alpha/unknown,
                           :val "2019-06-20T04:46:58.968Z",
                           :via [:spec$28727/created-at],
                           :in [:created-at]}),
                         :spec
                         {:spec
                          #object[clojure.spec.alpha$map_spec_impl$reify__1931 0x16d0ed64 "clojure.spec.alpha$map_spec_impl$reify__1931@16d0ed64"],
                          :form
                          (clojure.spec.alpha/keys
                           :req-un
                           [:spec$28727/created-at]),
                          :type :map,
                          :spec-tools.parse/key->spec
                          {:created-at :spec$28727/created-at},
                          :spec-tools.parse/keys #{:created-at},
                          :spec-tools.parse/keys-req #{:created-at},
                          :leaf? false},
                         :value {:created-at "2019-06-20T04:46:58.968Z"}},
    :type :reitit.coercion/request-coercion,
    :coercion <<:spec>>,
    :value {:created-at "2019-06-20T04:46:58.968Z"},
    :in [:request :path-params],
    :request
    {:template "/:created-at",
     :data
     {:name :api.users.api.v1.handlers/user-view,
      :coercion <<:spec>>,
      :parameters
      {:path
       {:created-at
        {:spec
         #object[clojure.spec.alpha$spec_impl$reify__1987 0x7d901ad6 "clojure.spec.alpha$spec_impl$reify__1987@7d901ad6"],
         :form (clojure.spec.alpha/spec :clj-time.spec/date-time),
         :type nil,
         :swagger/type "string",
         :swagger/format "date-time",
         :decode/string #function[api.users.api.v1.handlers/fn--28721/fn--28722],
         :encode/string #function[clj-time.coerce/from-string],
         :leaf? true}}}},
     :result {:path #function[reitit.coercion/request-coercer/fn--18933]},
     :path-params {:created-at "2019-06-20T04:46:58.968Z"},
     :path "/2019-06-20T04:46:58.968Z"}}
johnmcconnell commented 5 years ago

Clearly using decode/string and encode/string is incorrect. I have tried encode/json and decode/json as well.

  (s/def ::created-at
    (st/spec
     {:spec (s/spec :clj-time.spec/date-time)
      :swagger/type "string"
      :swagger/format "date-time"
      :decode/string (fn [_ y]
                       (clj-time.coerce/to-string y))
      :encode/string clj-time.coerce/from-string}))
johnmcconnell commented 5 years ago

Figuring out to run:

(require '[reitit.coercion :as coercion])
(coercion/-get-options reitit.coercion.spec/coercion)

helped

ikitommi commented 5 years ago

encode should be a 2-arity fn, not sure if that's the root cause thou:

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

(def spec
  (st/spec
    {:spec (s/and int? #(> % 10))
     :swagger/type "integer"
     :decode/string (fn [_ y]
                      (Long/parseLong y))
     :encode/string (fn [_ y]
                      (str y))}))

(as-> "11" $
      (doto $ prn)
      (st/decode spec $ st/string-transformer)
      (doto $ prn)
      (st/encode spec $ st/string-transformer)
      (doto $ prn))
; "11"
; 11
; => "11"

when this works, I think we should add a custom transformation into the examples and link it in the docs.

johnmcconnell commented 5 years ago

This test example you provided works

  (def my-spec
    (st/spec
     {:spec (s/and int? #(> % 10))
      :swagger/type "string"
      :decode/string (fn [_ y]
                       (Long/parseLong y))
      :encode/string (fn [_ y]
                       (str y))}))

  (def test-router
    (r/router
     ["/:id" {:name ::user-view
                                  :coercion reitit.coercion.spec/coercion
                                  :parameters {:path {:id my-spec}}}]
     {:compile coercion/compile-request-coercers}))

  (def x (r/match-by-path test-router "/11"))
  (def q (coercion/coerce! x))
  (do q) => {:path {:id 11}}

This can be ignored However, this test case still fails

  (def created-at
    (st/spec
     {:spec (s/spec :clj-time.spec/date-time)
      :swagger/type "string"
      :swagger/format "date-time"
      :decode/string (fn [_ y]
                       (clj-time.coerce/to-string y))
      :encode/string (fn [_ y]
                       (clj-time.coerce/from-string y))}))

  (def test-router
    (r/router
     ["/:id" {:name ::user-view
                                  :coercion reitit.coercion.spec/coercion
                                  :parameters {:path {:id created-at}}}]
     {:compile coercion/compile-request-coercers}))

  (def x (r/match-by-path test-router "/2019-06-20T04:46:58.968Z"))
  (def q (coercion/coerce! x))

=> Request coercion failed: #reitit.coercion.CoercionError{:spec #Spec{:form
   (clojure.spec.alpha/keys :req-un [:spec$28036/created-at]), :type :map,
   :leaf? false}, :problems #:clojure.spec.alpha{:problems ({:path
   [:created-at], :pred :clojure.spec.alpha/unknown, :val
   "2019-06-20T04:46:58.968Z", :via [:spec$28036/created-at], :in
   [:created-at]}), :spec #Spec{:form (clojure.spec.alpha/keys :req-un
   [:spec$28036/created-at]), :type :map, :leaf? false}, :value {:created-at
   "2019-06-20T04:46:58.968Z"}}}
   {:spec
    {:spec
     #object[clojure.spec.alpha$map_spec_impl$reify__1931 0x3ad23b6b "clojure.spec.alpha$map_spec_impl$reify__1931@3ad23b6b"],
     :form (clojure.spec.alpha/keys :req-un [:spec$28036/created-at]),
     :type :map,
     :spec-tools.parse/key->spec {:created-at :spec$28036/created-at},
     :spec-tools.parse/keys #{:created-at},
     :spec-tools.parse/keys-req #{:created-at},
     :leaf? false},
    :problems
johnmcconnell commented 5 years ago

I think now this is probably a bug with spec tools on how to resolve transformers.

^ This is incorrect it is resolved below. Ignore this comment and its details below

Smallest failing test case:

(def created-at
  (st/spec
   {:spec (s/spec clj-time.types/date-time?)
    :swagger/type "string"
    :swagger/format "date-time"
    :swagger/example (clj-time.core/now)
    :decode/string (fn [_ y]
                     (clj-time.coerce/to-string y))
    :encode/string (fn [_ y]
                     (clj-time.coerce/from-string y))}))

(as-> "2019-06-20T04:55:55.968Z" $
  (doto $ prn)
  (st/decode created-at $ st/string-transformer)
  (doto $ prn)
  (st/encode created-at $ st/string-transformer)
  (doto $ prn)) => :clojure.spec.alpha/invalid

Output:

"2019-06-20T04:55:55.968Z"
:clojure.spec.alpha/invalid
:clojure.spec.alpha/invalid
johnmcconnell commented 5 years ago

This can be ignored Passing test case 1:

(s/def ::t-spec-2
  (s/and
   #(int? %)))

(def created-at-2
  (st/spec
   {:spec ::t-spec-2
    :decode/string (constantly 5)
    :encode/string (constantly "20")}))

(as-> "0" $
  (doto $ prn)
  (st/decode created-at-2 $ st/string-transformer)
  (doto $ prn)
  (st/encode created-at-2 $ st/string-transformer)
  (doto $ prn)) => "20"

Passing test case 2:

(s/def ::t-spec-1
  (s/and
   #(instance? BaseDateTime %)
   #(= (.getZone ^BaseDateTime %) DateTimeZone/UTC)))

(def created-at-1
  (st/spec
   {:spec ::t-spec-1
    (st/spec
     {:spec :clj-time.spec/date-time
      :decode/string (fn [_ y]
                        (clj-time.coerce/from-string y))
      :encode/string (fn [_ y]
                        (clj-time.coerce/to-string y))
}))

(as-> "2019-06-20T04:55:55.968Z" $
  (doto $ prn)
  (st/decode created-at-1 $ st/string-transformer)
  (doto $ prn)
  (st/encode created-at-1 $ st/string-transformer)
  (doto $ prn)) => "2019-06-16T01:56:14.670Z"
ikitommi commented 5 years ago

I think your decode & encode functions have swapped places. Decode is from string->edn, encode from edn->string

johnmcconnell commented 5 years ago

Coercion is happening at the spec-tools level but not on the reitit level.

(def integer-inc
  (st/spec
   {:spec int?
    :swagger/type "string"
    :decode/string (fn [_ y]
                     (inc (Long/parseLong y)))
    :encode/string (fn [_ y]
                     (str (inc y)))
    :decode/json (spy
                  (fn [_ y]
                    (inc (Long/parseLong y))))
    :encode/json (spy
                  (fn [_ y]
                    (str (inc y))))
    }))

(defn routes
  [^DatastoreOptions ds-opts]
  ["/example"
   ["/api"
    {:get {:responses {200 {:body {:value integer-inc}}}
           :handler (fn [{params :parameters}]
                      {:status 200
                       :body
                       {:value 1}})}}]])

Middleware diff output

--- :response---

  {:body {:value 1}, :status 200}

--- :response :reitit.ring.middleware.exception/exception ---

  {:body {:value 1}, :status 200}

--- :response :reitit.ring.coercion/coerce-request ---

  {:body {:value 1}, :status 200}

--- :response :reitit.ring.coercion/coerce-response --- <---- Why was it not incremented and coerced here

  {:body {:value 1}, :status 200}

--- :response :reitit.ring.middleware.muuntaja/format-request ---

  {:body {:value 1}, :status 200}

--- :response :reitit.ring.middleware.exception/exception ---

  {:body {:value 1}, :status 200}

--- :response :reitit.ring.middleware.muuntaja/format-response ---

  {:body {:value 1}, :status 200}

--- :response :reitit.ring.middleware.muuntaja/format-negotiate ---

  {:body -{:value 1} +#<java.io.ByteArrayInputStream@66c16a7b>,
   :status 200,
   +:headers {"Content-Type" "application/json; charset=utf-8"}}
johnmcconnell commented 5 years ago

I think your decode & encode functions have swapped places. Decode is from string->edn, encode from edn->string

Yes that was a bug. I think there is still something going on though.

johnmcconnell commented 5 years ago

Passing Test Case:

  (def integer-inc
    (st/spec
     {:spec int?
      :swagger/type "string"
      :decode/string (fn [_ y]
                       (inc (Long/parseLong y)))
      :encode/string (fn [_ y]
                       (str (inc y)))
      :decode/json (fn [_ y]
                      (inc (Long/parseLong y)))
      :encode/json (fn [_ y]
                      (str (inc y)))
      }))

  (def test-router
    (r/router
     ["/:data" {:name ::user-view
                :coercion reitit.coercion.spec/coercion
                :parameters {:path {:data integer-inc}}}]
     {:compile coercion/compile-request-coercers}))

  (def x (r/match-by-path test-router "/1"))
  (coercion/coerce! x) => {:path {:data 2}}
johnmcconnell commented 5 years ago

It looks like response coercion is turned off for reitit.coercion.spec: https://github.com/metosin/reitit/blob/0.3.9/modules/reitit-spec/src/reitit/coercion/spec.cljc#L78

Also, plugging in the json-transfomer via:

              :coercion (let [opts (assoc-in
                                    reitit.coercion.spec/default-options
                                    [:transformers :response :default]
                                    reitit.coercion.spec/json-transformer)]
                          (reitit.coercion.spec/create opts))

appears to be coercing in the wrong direction

johnmcconnell commented 5 years ago

Maybe the simplest approach is to create a response-transformer and associate it with the default response transformer.

Then you could have a spec like:

  (def multiply-response-by-10-spec
    (st/spec
     {:spec int?
      :swagger/type "string"
      :decode/clj-response (fn [_ x] (* i 10))}))

(defn routes
  [^DatastoreOptions ds-opts]
  ["/example"
   ["/api"
    {:get {:responses {200 {:body {:value multiply-response-by-10-spec}}}
           :handler (fn [{params :parameters}]
                      {:status 200
                       :body
                       {:value 1}})}}]])

The actual http response body would look like:

{:value 10}
johnmcconnell commented 5 years ago

FYI, most of these problems were my expectations of the system (and I had a few bugs as well).

It was not clear to me that there was no response coercion. Maybe that should be documented somehow. Maybe that was the real trouble.

johnmcconnell commented 5 years ago

Also, maybe there should be a debugging function to run a value through the coercion middleware, under different circumstances (:request, :response, etc...)

kkazuo commented 5 years ago

Thanks! This helps me a lot.

This is my workaround of custom response body transformer. Maybe not a expected library usage, but it works for me now.

(ns xxx
  (:require
   [reitit.coercion.spec :as spec-coercion]
   [spec-tools.core :as st]
   [spec-tools.transform :as stt]
   [clojure.spec.alpha :as s]))

;; Define response transformer
(def response-transformer
  (st/type-transformer
   {:name            :response
    :decoders        stt/string-type-decoders
    :encoders        stt/any->any
    :default-encoder stt/any->any}))

;; Use this coercion instead of spec-coercion/coercion
(def coercion
  (-> spec-coercion/default-options
      (assoc-in [:transformers :response :default] response-transformer)
      spec-coercion/create))

;; Use response transformer
(s/def ::date
  (st/spec
   {:name            "date"
    :swagger/type    "string"
    :swagger/format  "date"
    :decode/json     string->date
    :decode/string   string->date
    :decode/response date->string})) ;; <-- yes decode, not encode
;; date->string is my custom java.time.OffsetDateTime formatter