diff --git a/doc/ring/coercion.md b/doc/ring/coercion.md index 342f1020..c450b43c 100644 --- a/doc/ring/coercion.md +++ b/doc/ring/coercion.md @@ -37,7 +37,7 @@ Coercion can be attached to route data under `:coercion` key. There can be multi Parameters are defined in route data under `:parameters` key. It's value should be a map of parameter `:type` -> Coercion Schema. -Responses are defined in route data under `:responses` key. It's value should be a map of http status code to a map which can contain `:body` key with Coercion Schema as value. +Responses are defined in route data under `:responses` key. It's value should be a map of http status code to a map which can contain `:body` key with Coercion Schema as value. Additionally, the key `:default` specifies the coercion for other status codes. Below is an example with [Plumatic Schema](https://github.com/plumatic/schema). It defines schemas for `:query`, `:body` and `:path` parameters and for http 200 response `:body`. @@ -54,7 +54,8 @@ Handlers can access the coerced parameters via the `:parameters` key in the requ :parameters {:query {:x s/Int} :body {:y s/Int} :path {:z s/Int}} - :responses {200 {:body {:total PositiveInt}}} + :responses {200 {:body {:total PositiveInt}} + :default {:body {:error s/Str}}} :handler (fn [{:keys [parameters]}] (let [total (+ (-> parameters :query :x) (-> parameters :body :y) @@ -206,6 +207,14 @@ is: rrc/coerce-response-middleware]}}))) ``` +The resolution logic for response coercers is: +1. Get the response status, or `:default` from the `:responses` map +2. From this map, get use the first of these to coerce: + 1. `:content :schema` + 2. `:content :default :schema` + 3. `:body` +3. If nothing was found, do not coerce + ## Pretty printing spec errors Spec problems are exposed as is in request & response coercion errors. Pretty-printers like [expound](https://github.com/bhb/expound) can be enabled like this: diff --git a/doc/ring/compiling_middleware.md b/doc/ring/compiling_middleware.md index f9404cc9..270d231f 100644 --- a/doc/ring/compiling_middleware.md +++ b/doc/ring/compiling_middleware.md @@ -27,8 +27,8 @@ To demonstrate the two approaches, below is the response coercion middleware wri coercion (-> match :data :coercion) opts (-> match :data :opts)] (if (and coercion responses) - (let [coercers (response-coercers coercion responses opts)] - (coerce-response coercers request response)) + (let [coercer (response-coercer coercion responses opts)] + (coercer request response)) response))) ([request respond raise] (let [method (:request-method request) @@ -37,8 +37,8 @@ To demonstrate the two approaches, below is the response coercion middleware wri coercion (-> match :data :coercion) opts (-> match :data :opts)] (if (and coercion responses) - (let [coercers (response-coercers coercion responses opts)] - (handler request #(respond (coerce-response coercers request %)))) + (let [coercer (response-coercer coercion responses opts)] + (handler request #(respond (coercer request %)))) (handler request respond raise)))))) ``` @@ -60,13 +60,13 @@ To demonstrate the two approaches, below is the response coercion middleware wri :spec ::rs/responses :compile (fn [{:keys [coercion responses]} opts] (if (and coercion responses) - (let [coercers (coercion/response-coercers coercion responses opts)] + (let [coercer (coercion/response-coercer coercion responses opts)] (fn [handler] (fn ([request] - (coercion/coerce-response coercers request (handler request))) + (coercer request (handler request))) ([request respond raise] - (handler request #(respond (coercion/coerce-response coercers request %)) raise)))))))}) + (handler request #(respond (coercer request %)) raise)))))))}) ``` It has 50% less code, it's much easier to reason about and is much faster. diff --git a/examples/openapi/src/example/server.clj b/examples/openapi/src/example/server.clj index 6c4ae3e7..618a469a 100644 --- a/examples/openapi/src/example/server.clj +++ b/examples/openapi/src/example/server.clj @@ -69,7 +69,7 @@ {:status 200 :body {:color :red :pineapple true}})} - :post {:summary "Create a pizza | Multiple content-types, multiple examples" + :post {:summary "Create a pizza | Multiple content-types, multiple examples | Default response schema" :request {:description "Create a pizza using json or EDN" :content {"application/json" {:schema [:map [:color :keyword] @@ -83,10 +83,16 @@ :pineapple false})}}}}} :responses {200 {:description "Success" :content {:default {:schema [:map [:success :boolean]] - :example {:success true}}}}} + :example {:success true}}}} + :default {:description "Not success" + :content {:default {:schema [:map [:error :string]] + :example {:error "error"}}}}} :handler (fn [_request] - {:status 200 - :body {:success true}})}}] + (if (< (Math/random) 0.5) + {:status 200 + :body {:success true}} + {:status 500 + :body {:error "an error happened"}}))}}] ["/contact" diff --git a/modules/reitit-core/src/reitit/coercion.cljc b/modules/reitit-core/src/reitit/coercion.cljc index ad96102b..7a3d4458 100644 --- a/modules/reitit-core/src/reitit/coercion.cljc +++ b/modules/reitit-core/src/reitit/coercion.cljc @@ -130,29 +130,6 @@ (request-coercion-failed! result coercion value in request serialize-failed-result) result))))))) -(defn extract-response-format-default [request _] - (-> request :muuntaja/response :format)) - -(defn response-coercer [coercion {:keys [content body]} {:keys [extract-response-format serialize-failed-result] - :or {extract-response-format extract-response-format-default}}] - (if coercion - (let [format->coercer (some->> (concat (when body - [[:default (-response-coercer coercion body)]]) - (for [[format {:keys [schema]}] content, :when schema] - [format (-response-coercer coercion schema)])) - (filter second) (seq) (into (array-map)))] - (when format->coercer - (fn [request response] - (let [format (extract-response-format request response) - value (:body response) - coercer (or (format->coercer format) - (format->coercer :default) - -identity-coercer) - result (coercer value format)] - (if (error? result) - (response-coercion-failed! result coercion value request response serialize-failed-result) - result))))))) - (defn encode-error [data] (-> data (dissoc :request :response) @@ -165,12 +142,6 @@ (impl/fast-assoc acc k (coercer request))) {} coercers)) -(defn coerce-response [coercers request response] - (if response - (if-let [coercer (or (coercers (:status response)) (coercers :default))] - (impl/fast-assoc response :body (coercer request response)) - response))) - (defn request-coercers ([coercion parameters opts] (some->> (for [[k v] parameters, :when v] @@ -181,13 +152,42 @@ rcs (request-coercers coercion parameters (cond-> opts route-request (assoc ::skip #{:body})))] (if (and crc rcs) (into crc (vec rcs)) (or crc rcs))))) -(defn response-coercers [coercion responses opts] - (some->> (for [[status model] responses] - (do - (when-not (int? status) - (throw (ex-info "Response status must be int" {:status status}))) - [status (response-coercer coercion model opts)])) - (filter second) (seq) (into {}))) +(defn extract-response-format-default [request _] + (-> request :muuntaja/response :format)) + +(defn -format->coercer [coercion {:keys [content body]} _opts] + (->> (concat (when body + [[:default (-response-coercer coercion body)]]) + (for [[format {:keys [schema]}] content, :when schema] + [format (-response-coercer coercion schema)])) + (filter second) (into (array-map)))) + +(defn response-coercer [coercion responses {:keys [extract-response-format serialize-failed-result] + :or {extract-response-format extract-response-format-default} + :as opts}] + (when coercion + (let [status->format->coercer + (into {} + (for [[status model] responses] + (do + (when-not (or (= :default status) (int? status)) + (throw (ex-info "Response status must be int or :default" {:status status}))) + [status (-format->coercer coercion model opts)])))] + (when-not (every? empty? (vals status->format->coercer)) ;; fast path: return nil if there are no models to coerce + (fn [request response] + (let [format->coercer (or (status->format->coercer (:status response)) + (status->format->coercer :default)) + format (extract-response-format request response) + coercer (or (format->coercer format) + (format->coercer :default))] + (if-not coercer + response + (let [value (:body response) + coerced (coercer (:body response) format) + result (if (error? coerced) + (response-coercion-failed! coerced coercion value request response serialize-failed-result) + coerced)] + (impl/fast-assoc response :body result))))))))) (defn -compile-parameters [data coercion] (impl/path-update data [[[:parameters any?] #(-compile-model coercion % nil)]])) diff --git a/modules/reitit-http/src/reitit/http/coercion.cljc b/modules/reitit-http/src/reitit/http/coercion.cljc index 4807f3a3..c8165979 100644 --- a/modules/reitit-http/src/reitit/http/coercion.cljc +++ b/modules/reitit-http/src/reitit/http/coercion.cljc @@ -41,11 +41,11 @@ (not responses) {} ;; mount :else - (if-let [coercers (coercion/response-coercers coercion responses opts)] + (if-let [coercer (coercion/response-coercer coercion responses opts)] {:leave (fn [ctx] (let [request (:request ctx) response (:response ctx) - response (coercion/coerce-response coercers request response)] + response (coercer request response)] (assoc ctx :response response)))} {})))}) diff --git a/modules/reitit-ring/src/reitit/ring/coercion.cljc b/modules/reitit-ring/src/reitit/ring/coercion.cljc index c870c180..3223432c 100644 --- a/modules/reitit-ring/src/reitit/ring/coercion.cljc +++ b/modules/reitit-ring/src/reitit/ring/coercion.cljc @@ -58,13 +58,13 @@ (not responses) {} ;; mount :else - (if-let [coercers (coercion/response-coercers coercion responses opts)] + (if-let [coercer (coercion/response-coercer coercion responses opts)] (fn [handler] (fn ([request] - (coercion/coerce-response coercers request (handler request))) + (coercer request (handler request))) ([request respond raise] - (handler request #(respond (coercion/coerce-response coercers request %)) raise)))) + (handler request #(respond (coercer request %)) raise)))) {})))}) (def coerce-exceptions-middleware diff --git a/test/cljc/reitit/openapi_test.clj b/test/cljc/reitit/openapi_test.clj index 6ad6c215..11d3fd85 100644 --- a/test/cljc/reitit/openapi_test.clj +++ b/test/cljc/reitit/openapi_test.clj @@ -69,9 +69,9 @@ :responses {200 {:description "success" :body {:total int?}} 500 {:description "fail"} - 504 {:description "default" - :content {:default {:schema {:error string?}}} - :body {:masked string?}}} + :default {:description "default" + :content {:default {:schema {:error string?}}} + :body {:masked string?}}} :handler (fn [{{{:keys [z]} :path xs :body} :parameters}] {:status 200, :body {:total (+ (reduce + xs) z)}})}}]] @@ -99,9 +99,9 @@ :responses {200 {:description "success" :body [:map [:total int?]]} 500 {:description "fail"} - 504 {:description "default" - :content {:default {:schema {:error string?}}} - :body {:masked string?}}} + :default {:description "default" + :content {:default {:schema {:error string?}}} + :body {:masked string?}}} :handler (fn [{{{:keys [z]} :path xs :body} :parameters}] {:status 200, :body {:total (+ (reduce + xs) z)}})}}]] @@ -129,9 +129,9 @@ :responses {200 {:description "success" :body {:total s/Int}} 500 {:description "fail"} - 504 {:description "default" - :content {:default {:schema {:error s/Str}}} - :body {:masked s/Str}}} + :default {:description "default" + :content {:default {:schema {:error s/Str}}} + :body {:masked s/Str}}} :handler (fn [{{{:keys [z]} :path xs :body} :parameters}] {:status 200, :body {:total (+ (reduce + xs) z)}})}}]]] @@ -206,10 +206,10 @@ 400 {:content {"application/json" {:schema {:type "string"}}} :description "kosh"} 500 {:description "fail"} - 504 {:description "default" - :content {"application/json" {:schema {:properties {"error" {:type "string"}} - :required ["error"] - :type "object"}}}}} + :default {:description "default" + :content {"application/json" {:schema {:properties {"error" {:type "string"}} + :required ["error"] + :type "object"}}}}} :summary "plus with body"}} "/api/malli/plus/{z}" {:get {:parameters [{:in "query" :name :x @@ -250,11 +250,11 @@ 400 {:description "kosh" :content {"application/json" {:schema {:type "string"}}}} 500 {:description "fail"} - 504 {:description "default" - :content {"application/json" {:schema {:additionalProperties false - :properties {:error {:type "string"}} - :required [:error] - :type "object"}}}}} + :default {:description "default" + :content {"application/json" {:schema {:additionalProperties false + :properties {:error {:type "string"}} + :required [:error] + :type "object"}}}}} :summary "plus with body"}} "/api/schema/plus/{z}" {:get {:parameters [{:in "query" :name "x" @@ -302,11 +302,11 @@ 400 {:description "kosh" :content {"application/json" {:schema {:type "string"}}}} 500 {:description "fail"} - 504 {:description "default" - :content {"application/json" {:schema {:additionalProperties false - :properties {"error" {:type "string"}} - :required ["error"] - :type "object"}}}}} + :default {:description "default" + :content {"application/json" {:schema {:additionalProperties false + :properties {"error" {:type "string"}} + :required ["error"] + :type "object"}}}}} :summary "plus with body"}}}}] (is (= expected spec)) (is (= nil (validate spec)))))) diff --git a/test/cljc/reitit/ring_coercion_test.cljc b/test/cljc/reitit/ring_coercion_test.cljc index 91fa5135..cf8364a8 100644 --- a/test/cljc/reitit/ring_coercion_test.cljc +++ b/test/cljc/reitit/ring_coercion_test.cljc @@ -680,6 +680,74 @@ (call (request "application/json" "application/transit" {:request :json :response :json})))))))))))) +#?(:clj + (deftest response-coercion-test + (doseq [[coercion schema-200 schema-default] + [[malli/coercion + [:map [:a :int]] + [:map [:b :int]]] + [schema/coercion + {:a s/Int} + {:b s/Int}] + [spec/coercion + {:a int?} + {:b int?}]]] + (testing (str coercion) + (let [app (ring/ring-handler + (ring/router + ["/foo" {:post {:responses {200 {:content {:default {:schema schema-200}}} + 201 {:content {"application/edn" {:schema schema-200}}} + 202 {:description "status code and content-type explicitly mentioned, but no :schema" + :content {"application/edn" {} + "application/json" {}}} + :default {:content {"application/json" {:schema schema-default}}}} + :handler (fn [req] + {:status (-> req :body-params :status) + :body (-> req :body-params :response)})}}] + {:validate reitit.ring.spec/validate + :data {:middleware [rrc/coerce-request-middleware + rrc/coerce-response-middleware] + :coercion coercion}})) + call (fn [request] + (try + (app request) + (catch ExceptionInfo e + (select-keys (ex-data e) [:type :in])))) + request (fn [body] + {:request-method :post + :uri "/foo" + :muuntaja/request {:format "application/json"} + :muuntaja/response {:format (:format body "application/json")} + :body-params body})] + (testing "explicit response schema" + (is (= {:status 200 :body {:a 1}} + (call (request {:status 200 :response {:a 1}}))) + "valid response") + (is (= {:type :reitit.coercion/response-coercion, :in [:response :body]} + (call (request {:status 200 :response {:b 1}}))) + "invalid response") + (is (= {:type :reitit.coercion/response-coercion, :in [:response :body]} + (call (request {:status 200 :response {:b 1} :format "application/edn"}))) + "invalid response, different content-type")) + (testing "explicit response schema, but for the wrong content-type" + (is (= {:status 201 :body "anything goes!"} + (call (request {:status 201 :response "anything goes!"}))) + "no coercion applied")) + (testing "response config without :schema" + (is (= {:status 202 :body "anything goes!"} + (call (request {:status 202 :response "anything goes!"}))) + "no coercion applied")) + (testing "default response schema" + (is (= {:status 300 :body {:b 2}} + (call (request {:status 300 :response {:b 2}}))) + "valid response") + (is (= {:type :reitit.coercion/response-coercion, :in [:response :body]} + (call (request {:status 300 :response {:a 2}}))) + "invalid response") + (is (= {:status 300 :body "anything goes!"} + (call (request {:status 300 :response "anything goes!" :format "application/edn"}))) + "no coercion applied due to content-type"))))))) + #?(:clj (deftest muuntaja-test (let [app (ring/ring-handler