metosin / reitit

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

Spec coercion gives :val nil. #533

Open zendevil opened 2 years ago

zendevil commented 2 years ago

I have the rrc/coerce-request-middleware middleware turned on.

Here's the handler:

["/reset-password" {:post {:parameters {:body ::spec/reset-password-params}
                                                :handler reset-password-handler
                                                :coercion coercion/coercion}}]

Here's the error that I'm getting:

          + {:spec
             "(spec-tools.core/spec {:spec (clojure.spec.alpha/keys :req-un [:lms.auth.spec/resetToken :lms.auth.spec/newPassword :lms.auth.spec/email]), :type :map, :leaf? false})",
             :problems
             [{:path [],
               :pred "clojure.core/map?",
               :val nil,
               :via [:lms.auth.spec/reset-password-params],
               :in []}],
             :type :reitit.coercion/request-coercion,
             :coercion :spec,
             :value nil,
             :in [:request :body-params]}

When making the request I'm sending two params: {:email "foobar@foobar.com" :password "password"}

Why am I getting the spec error?

Deraen commented 2 years ago

The code here looks OK.

Could be a problem with body params middleware (muuntaja) or the middleware ordering.

Check example: https://github.com/metosin/reitit/blob/master/examples/ring-spec-swagger/src/example/server.clj#L92-L109

The middleware order is important. Muuntaja has to read the request body before Reitit can coerce the parameters.

zendevil commented 2 years ago

Here's our middleware. Does anything look wrong here to you?

(def all-middleware
  [wrap-salesforce-hack
   #(wrap-defaults % (assoc-in site-defaults [:security :anti-forgery] false))
   #(wrap-cors %
               :access-control-allow-origin [#"http://localhost:3000"]
               :access-control-allow-methods [:get :put :post :delete])
   wrap-datomic
   wrap-auth-token
   wrap-json-params
   parameters/parameters-middleware
   muuntaja/format-negotiate-middleware
   muuntaja/format-response-middleware
   exception/exception-middleware
   muuntaja/format-request-middleware
   multipart/multipart-middleware
   wrap-json-response
   wrap-keyword-params
   wrap-json-exceptions
   wrap-no-op
   wrap-reload
   ring.middleware.params/wrap-params
   swagger/swagger-feature
   rrc/coerce-exceptions-middleware
   rrc/coerce-request-middleware
   rrc/coerce-response-middleware])
Deraen commented 2 years ago

Well, you have both Muuntaja format middlewares and wrap-json-params/response. No idea what happens, but it doesn't make much sense as Muuntaja should handle the json requests and responses. Use either Muuntaja or json middlewares, not both.

Http request body stream can be read only once, so if ring-json-params reads the body, Muuntaja can't read the body.

zendevil commented 2 years ago

I actually copy pasted the exact middleware you pointed to, but I'm getting the same spec error. Maybe something about how the router is defined that's causing the problem?:

(def all-middleware
  [;; swagger feature
   swagger/swagger-feature
   ;; query-params & form-params
   parameters/parameters-middleware
   ;; content-negotiation
   muuntaja/format-negotiate-middleware
   ;; encoding response body
   muuntaja/format-response-middleware
   ;; exception handling
   exception/exception-middleware
   ;; decoding request body
   muuntaja/format-request-middleware
   ;; coercing response bodys
   coercion/coerce-response-middleware
   ;; coercing request parameters
   coercion/coerce-request-middleware
   ;; multipart
   multipart/multipart-middleware])

(def app
  (ring/ring-handler

   (ring/router
    ["" {:swagger {:id ::math}}
     ["/api-docs/*" {:no-doc true
                     :get (swagger-ui/create-swagger-ui-handler)}]
     ["/swagger.json"
      {:get {:no-doc true
             :swagger {:info {:title "Clojure LMS Api", :version "0.1.0"}}
             :handler (swagger/create-swagger-handler)}}]

     ["/api"
      message-handlers
      assigned-classes-handlers
      service-types-handlers
      locations-handlers
      schools-handlers
      files-handlers
      users-handlers
      sessions-handlers
      roles-handlers
      auth-handlers
      schools-handlers
      subjects-handlers
      misc-handlers]]
    {:conflicts nil

     :data {:middleware all-middleware
            :coercion spec/coercion
            ;;:muuntaja muuntaja/instance
            :compile coercion/compile-request-coercers
            :swagger {:produces #{"application/json"
                                  "application/edn"
                                  "application/transit+json"}
                      :consumes #{"application/json"
                                  "application/edn"
                                  "application/transit+json"}}}
       ;; TODO: these should work by default!
     ;;:extract-request-format (comp :format :muuntaja/request)
     ;;:extract-response-format (comp :format :muuntaja/response)
     })
   (ring/routes
    (swagger-ui/create-swagger-ui-handler
     {:path "/", :url "/swagger.json"})
    (ring/create-default-handler))))
zendevil commented 2 years ago

We're sending the request using peridot like so:

(-> session
      (request "/api/reset-password"
               :content-type "application/json"
               :request-method :post
               :body (cc/generate-string
                      {:email email
                       :newPassword newPassword
                       :resetToken token}
                      )))