diff --git a/modules/reitit-malli/src/reitit/coercion/malli.cljc b/modules/reitit-malli/src/reitit/coercion/malli.cljc index 84f709c2..5c781d1c 100644 --- a/modules/reitit-malli/src/reitit/coercion/malli.cljc +++ b/modules/reitit-malli/src/reitit/coercion/malli.cljc @@ -12,43 +12,63 @@ (defrecord Coercer [decoder encoder validator explainer]) (def string-transformer - mt/string-transformer) + (mt/transformer + mt/strip-extra-keys-transformer + mt/string-transformer)) (def json-transformer - mt/json-transformer) + (mt/transformer + mt/strip-extra-keys-transformer + mt/json-transformer)) (def default-transformer - (mt/transformer {:name :default})) + mt/strip-extra-keys-transformer) (defmulti coerce-response? identity :default ::default) (defmethod coerce-response? ::default [_] true) -(defn- -coercer [schema type transformers f] +(defn- -coercer [schema type transformers f opts] (if schema - (let [->coercer (fn [t] (if t (->Coercer (m/decoder schema t) - (m/encoder schema t) - (m/validator schema) - (m/explainer schema)))) + (let [->coercer (fn [t] (if t (->Coercer (m/decoder schema opts t) + (m/encoder schema opts t) + (m/validator schema opts) + (m/explainer schema opts)))) {:keys [formats default]} (transformers type) default-coercer (->coercer default) - format-coercers (->> (for [[f t] formats] [f (->coercer t)]) (into {})) - get-coercer (if (seq format-coercers) - (fn [format] (or (get format-coercers format) default-coercer)) - (constantly default-coercer))] - (if default-coercer - (fn [value format] - (if-let [coercer (get-coercer format)] - (let [transform (f coercer) - validator (:validator coercer) - transformed (transform value)] - (if (validator transformed) - transformed - (let [explainer (:explainer coercer) - errors (explainer transformed)] + format-coercers (some->> (for [[f t] formats] [f (->coercer t)]) (keep second) (seq) (into {})) + get-coercer (cond format-coercers (fn [format] (or (get format-coercers format) default-coercer)) + default-coercer (constantly default-coercer))] + (if get-coercer + (if (= f :decode) + ;; transform -> validate + (fn [value format] + (if-let [coercer (get-coercer format)] + (let [transform (:decoder coercer) + validator (:validator coercer) + transformed (transform value)] + (if (validator transformed) + transformed + (let [explainer (:explainer coercer) + errors (explainer transformed)] + (coercion/map->CoercionError + {:schema schema + :value value + :transformed transformed + :errors errors})))) + value)) + ;; validate -> transform + (fn [value format] + (if-let [coercer (get-coercer format)] + (let [transform (:encoder coercer) + validator (:validator coercer) + explainer (:explainer coercer)] + (if (validator value) + (transform value) (coercion/map->CoercionError {:schema schema - :errors errors})))) - value)))))) + :value value + :errors (explainer value)}))) + value))))))) ;; ;; swagger @@ -86,44 +106,48 @@ :transformers {:body {:default default-transformer :formats {"application/json" json-transformer}} :string {:default string-transformer} - :response {:default default-transformer}}}) + :response {:default default-transformer + :formats {"application/json" json-transformer}}} + ;; malli options + :options nil}) -(defn create [{:keys [transformers coerce-response?] :as opts}] - ^{:type ::coercion/coercion} - (reify coercion/Coercion - (-get-name [_] :malli) - (-get-options [_] opts) - (-get-apidocs [_ specification {:keys [parameters responses]}] - (case specification - :swagger (merge - (if parameters - {:parameters - (->> (for [[in schema] parameters - parameter (extract-parameter in schema)] - parameter) - (into []))}) - (if responses - {:responses - (into - (empty responses) - (for [[status response] responses] - [status (as-> response $ - (set/rename-keys $ {:body :schema}) - (update $ :description (fnil identity "")) - (if (:schema $) - (update $ :schema swagger/transform {:type :schema}) - $))]))})) - (throw - (ex-info - (str "Can't produce Schema apidocs for " specification) - {:type specification, :coercion :schema})))) - (-compile-model [_ model _] (m/schema model)) - (-open-model [_ schema] schema) - (-encode-error [_ error] error) - (-request-coercer [_ type schema] - (-coercer schema type transformers :decoder)) - (-response-coercer [_ schema] - (if (coerce-response? schema) - (-coercer schema :response transformers :encoder))))) +(defn create [opts] + (let [{:keys [transformers coerce-response? options] :as opts} (merge default-options opts)] + ^{:type ::coercion/coercion} + (reify coercion/Coercion + (-get-name [_] :malli) + (-get-options [_] opts) + (-get-apidocs [_ specification {:keys [parameters responses]}] + (case specification + :swagger (merge + (if parameters + {:parameters + (->> (for [[in schema] parameters + parameter (extract-parameter in schema)] + parameter) + (into []))}) + (if responses + {:responses + (into + (empty responses) + (for [[status response] responses] + [status (as-> response $ + (set/rename-keys $ {:body :schema}) + (update $ :description (fnil identity "")) + (if (:schema $) + (update $ :schema swagger/transform {:type :schema}) + $))]))})) + (throw + (ex-info + (str "Can't produce Schema apidocs for " specification) + {:type specification, :coercion :schema})))) + (-compile-model [_ model _] (m/schema model)) + (-open-model [_ schema] schema) + (-encode-error [_ error] error) + (-request-coercer [_ type schema] + (-coercer schema type transformers :decode options)) + (-response-coercer [_ schema] + (if (coerce-response? schema) + (-coercer schema :response transformers :encode options)))))) (def coercion (create default-options)) diff --git a/modules/reitit-spec/src/reitit/coercion/spec.cljc b/modules/reitit-spec/src/reitit/coercion/spec.cljc index 41e64fe6..bac09a25 100644 --- a/modules/reitit-spec/src/reitit/coercion/spec.cljc +++ b/modules/reitit-spec/src/reitit/coercion/spec.cljc @@ -19,6 +19,9 @@ st/strip-extra-keys-transformer st/json-transformer)) +(def strip-extra-keys-transformer + st/strip-extra-keys-transformer) + (def no-op-transformer (reify st/Transformer @@ -72,7 +75,7 @@ (def default-options {:coerce-response? coerce-response? - :transformers {:body {:default no-op-transformer + :transformers {:body {:default strip-extra-keys-transformer :formats {"application/json" json-transformer}} :string {:default string-transformer} :response {:default no-op-transformer}}}) diff --git a/test/cljc/reitit/coercion_test.cljc b/test/cljc/reitit/coercion_test.cljc index 2cdda88c..93b067ec 100644 --- a/test/cljc/reitit/coercion_test.cljc +++ b/test/cljc/reitit/coercion_test.cljc @@ -36,8 +36,8 @@ (is (= {:path {:keyword :abba, :number 1}, :query nil} (coercion/coerce! m)))) (let [m (r/match-by-path r "/schema/1/abba")] - (is (= {:path {:keyword :abba, :number 1}, :query {:int 10, :ints [1,2,3], :map {1 1}}} - (coercion/coerce! (assoc m :query-params {"int" "10", "ints" ["1" "2" "3"], "map" {:1 "1"}})))))) + (is (= {:path {:keyword :abba, :number 1}, :query {:int 10, :ints [1, 2, 3], :map {1 1, 2 2}}} + (coercion/coerce! (assoc m :query-params {"int" "10", "ints" ["1" "2" "3"], "map" {:1 "1", "2" "2"}})))))) (testing "throws with invalid input" (let [m (r/match-by-path r "/schema/kikka/abba")] (is (thrown? ExceptionInfo (coercion/coerce! m)))))) @@ -48,20 +48,21 @@ (is (= {:path {:keyword :abba, :number 1}, :query nil} (coercion/coerce! m)))) (let [m (r/match-by-path r "/malli/1/abba")] - (is (= {:path {:keyword :abba, :number 1}, :query {:int 10, :ints [1,2,3], :map {1 1}}} - (coercion/coerce! (assoc m :query-params {"int" "10", "ints" ["1" "2" "3"], "map" {:1 "1"}})))))) + (is (= {:path {:keyword :abba, :number 1}, :query {:int 10, :ints [1, 2, 3], :map {1 1, 2 2}}} + (coercion/coerce! (assoc m :query-params {"int" "10", "ints" ["1" "2" "3"], "map" {:1 "1", "2" "2"}})))))) (testing "throws with invalid input" (let [m (r/match-by-path r "/malli/kikka/abba")] (is (thrown? ExceptionInfo (coercion/coerce! m)))))) + ;; TODO: :map-of fails with string-keys (testing "spec-coercion" (testing "succeeds" (let [m (r/match-by-path r "/spec/1/abba")] (is (= {:path {:keyword :abba, :number 1}, :query nil} (coercion/coerce! m)))) (let [m (r/match-by-path r "/schema/1/abba")] - (is (= {:path {:keyword :abba, :number 1}, :query {:int 10, :ints [1,2,3], :map {1 1}}} - (coercion/coerce! (assoc m :query-params {"int" "10", "ints" ["1" "2" "3"], "map" {:1 "1"}})))))) + (is (= {:path {:keyword :abba, :number 1}, :query {:int 10, :ints [1, 2, 3], :map {1 1, #_#_2 2}}} + (coercion/coerce! (assoc m :query-params {"int" "10", "ints" ["1" "2" "3"], "map" {:1 "1"}, #_#_"2" "2"})))))) (testing "throws with invalid input" (let [m (r/match-by-path r "/spec/kikka/abba")] (is (thrown? ExceptionInfo (coercion/coerce! m)))))) diff --git a/test/cljc/reitit/ring_coercion_test.cljc b/test/cljc/reitit/ring_coercion_test.cljc index 494d3c36..73b3d338 100644 --- a/test/cljc/reitit/ring_coercion_test.cljc +++ b/test/cljc/reitit/ring_coercion_test.cljc @@ -17,22 +17,44 @@ {:keys [b]} :body {:keys [c]} :form {:keys [d]} :header - {:keys [e]} :path} :parameters}] + {:keys [e]} :path :as parameters} :parameters}] + ;; extra keys are stripped off + (assert (every? #{0 1} (map (comp count val) parameters))) + (if (= 666 a) {:status 500 :body {:evil true}} {:status 200 - :body {:total (+ a b c d e)}})) + :body {:total (+ (or a 101) b c d e)}})) -(def valid-request +(def valid-request1 {:uri "/api/plus/5" :request-method :get + :muuntaja/request {:format "application/json"} :query-params {"a" "1"} :body-params {:b 2} :form-params {:c 3} :headers {"d" "4"}}) -(def invalid-request +(def valid-request2 + {:uri "/api/plus/5" + :request-method :get + :muuntaja/request {:format "application/json"} + :query-params {} + :body-params {:b 2} + :form-params {:c 3} + :headers {"d" "4"}}) + +(def valid-request3 + {:uri "/api/plus/5" + :request-method :get + :muuntaja/request {:format "application/edn"} + :query-params {"a" "1", "EXTRA" "VALUE"} + :body-params {:b 2, :EXTRA "VALUE"} + :form-params {:c 3, :EXTRA "VALUE"} + :headers {"d" "4", "EXTRA" "VALUE"}}) + +(def invalid-request1 {:uri "/api/plus/5" :request-method :get}) @@ -68,16 +90,22 @@ (testing "all good" (is (= {:status 200 :body {:total 15}} - (app valid-request))) + (app valid-request1))) + (is (= {:status 200 + :body {:total 115}} + (app valid-request2))) + (is (= {:status 200 + :body {:total 15}} + (app valid-request3))) (is (= {:status 500 :body {:evil true}} - (app (assoc-in valid-request [:query-params "a"] "666"))))) + (app (assoc-in valid-request1 [:query-params "a"] "666"))))) (testing "invalid request" (is (thrown-with-msg? ExceptionInfo #"Request coercion failed" - (app invalid-request)))) + (app invalid-request1)))) (testing "invalid response" (is (thrown-with-msg? @@ -93,10 +121,10 @@ (testing "all good" (is (= {:status 200 :body {:total 15}} - (app valid-request)))) + (app valid-request1)))) (testing "invalid request" - (let [{:keys [status body]} (app invalid-request) + (let [{:keys [status body]} (app invalid-request1) problems (:problems body)] (is (= 1 (count problems))) (is (= 400 status)))) @@ -111,7 +139,7 @@ (ring/router ["/api" ["/plus/:e" - {:get {:parameters {:query {:a s/Int} + {:get {:parameters {:query {(s/optional-key :a) s/Int} :body {:b s/Int} :form {:c s/Int} :header {:d s/Int} @@ -129,16 +157,23 @@ (testing "all good" (is (= {:status 200 :body {:total 15}} - (app valid-request))) + (app valid-request1))) + (is (= {:status 200 + :body {:total 115}} + (app valid-request2))) (is (= {:status 500 :body {:evil true}} - (app (assoc-in valid-request [:query-params "a"] "666"))))) + (app (assoc-in valid-request1 [:query-params "a"] "666"))))) (testing "invalid request" (is (thrown-with-msg? ExceptionInfo #"Request coercion failed" - (app invalid-request)))) + (app invalid-request1))) + (is (thrown-with-msg? + ExceptionInfo + #"Request coercion failed" + (app valid-request3)))) (testing "invalid response" (is (thrown-with-msg? @@ -154,10 +189,10 @@ (testing "all good" (is (= {:status 200 :body {:total 15}} - (app valid-request)))) + (app valid-request1)))) (testing "invalid request" - (let [{:keys [status]} (app invalid-request)] + (let [{:keys [status]} (app invalid-request1)] (is (= 400 status)))) (testing "invalid response" @@ -170,7 +205,7 @@ (ring/router ["/api" ["/plus/:e" - {:get {:parameters {:query [:map [:a int?]] + {:get {:parameters {:query [:map [:a {:optional true} int?]] :body [:map [:b int?]] :form [:map [:c int?]] :header [:map [:d int?]] @@ -188,16 +223,22 @@ (testing "all good" (is (= {:status 200 :body {:total 15}} - (app valid-request))) + (app valid-request1))) + (is (= {:status 200 + :body {:total 115}} + (app valid-request2))) + (is (= {:status 200 + :body {:total 15}} + (app valid-request3))) (is (= {:status 500 :body {:evil true}} - (app (assoc-in valid-request [:query-params "a"] "666"))))) + (app (assoc-in valid-request1 [:query-params "a"] "666"))))) (testing "invalid request" (is (thrown-with-msg? ExceptionInfo #"Request coercion failed" - (app invalid-request)))) + (app invalid-request1)))) (testing "invalid response" (is (thrown-with-msg? @@ -213,10 +254,10 @@ (testing "all good" (is (= {:status 200 :body {:total 15}} - (app valid-request)))) + (app valid-request1)))) (testing "invalid request" - (let [{:keys [status]} (app invalid-request)] + (let [{:keys [status]} (app invalid-request1)] (is (= 400 status)))) (testing "invalid response" @@ -249,11 +290,11 @@ (testing "json coercion" (let [e2e #(-> (request "application/json" (ByteArrayInputStream. (j/write-value-as-bytes %))) (app) :body (slurp) (j/read-value (j/object-mapper {:decode-key-fn true})))] - (is (= data-json (e2e data-edn))) - (is (= data-json (e2e data-json))))) + (is (= data-json (e2e (assoc data-edn :EXTRA "VALUE")))) + (is (= data-json (e2e (assoc data-json :EXTRA "VALUE")))))) (testing "edn coercion" (let [e2e #(-> (request "application/edn" (pr-str %)) (app) :body slurp (read-string))] - (is (= data-edn (e2e data-edn))) + (is (= data-edn (e2e (assoc data-edn :EXTRA "VALUE")))) (is (thrown? ExceptionInfo (e2e data-json))))))))