optimized http-coercion

CHANGELOG.md

@@ -25,10 +25,14 @@ We use [Break Versioning][breakver]. The version numbers follow a `<major>.<mino
   * `:validate` - boolean to indicate whether validation is enabled (true)
   * `:enabled` - boolean to indicate whether coercion (and validation) is enabled (true)
 
-###  `reitit-middleware`
+###  `reitit-ring`
 
 * Coercion middleware will not to mount if the selected `:coercion` is not enabled for the given `:parameters`, e.g. "just api-docs"
 
+###  `reitit-http`
+
+* Coercion interceptor will not to mount if the selected `:coercion` is not enabled for the given `:parameters`, e.g. "just api-docs"
+
 ## 0.5.1 (2020-05-18)
 
 ```clj

modules/reitit-http/src/reitit/http/coercion.cljc

@@ -18,12 +18,13 @@
                 (not parameters) {}
                 ;; mount
                 :else
-                (let [coercers (coercion/request-coercers coercion parameters opts)]
+                (if-let [coercers (coercion/request-coercers coercion parameters opts)]
                   {:enter (fn [ctx]
                             (let [request (:request ctx)
                                   coerced (coercion/coerce-request coercers request)
                                   request (impl/fast-assoc request :parameters coerced)]
-                              (assoc ctx :request request)))})))})
+                              (assoc ctx :request request)))}
+                  {})))})
 
 (defn coerce-response-interceptor
   "Interceptor for pluggable response coercion.
@@ -40,12 +41,13 @@
                 (not responses) {}
                 ;; mount
                 :else
-                (let [coercers (coercion/response-coercers coercion responses opts)]
+                (if-let [coercers (coercion/response-coercers coercion responses opts)]
                   {:leave (fn [ctx]
                             (let [request (:request ctx)
                                   response (:response ctx)
                                   response (coercion/coerce-response coercers request response)]
-                              (assoc ctx :response response)))})))})
+                              (assoc ctx :response response)))}
+                  {})))})
 
 (defn coerce-exceptions-interceptor
   "Interceptor for handling coercion exceptions.

test/clj/reitit/http_coercion_test.clj

@@ -7,10 +7,21 @@
             [reitit.coercion.schema :as schema]
             [muuntaja.interceptor]
             [jsonista.core :as j]
-            [reitit.interceptor.sieppari :as sieppari])
+            [reitit.interceptor.sieppari :as sieppari]
+            [reitit.coercion.malli :as malli]
+            [reitit.core :as r])
   (:import (clojure.lang ExceptionInfo)
            (java.io ByteArrayInputStream)))
 
+(defn mounted-interceptor [app path method]
+  (->> app
+       (http/get-router)
+       (r/compiled-routes)
+       (filter (comp (partial = path) first))
+       (first) (last) method :interceptors
+       (filter #(->> (select-keys % [:enter :leave :error]) (vals) (keep identity) (seq)))
+       (mapv :name)))
+
 (defn handler [{{{:keys [a]} :query
                  {:keys [b]} :body
                  {:keys [c]} :form
@@ -22,7 +33,7 @@
     {:status 200
      :body {:total (+ a b c d e)}}))
 
-(def valid-request
+(def valid-request1
   {:uri "/api/plus/5"
    :request-method :get
    :query-params {"a" "1"}
@@ -30,7 +41,25 @@
    :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})
 
@@ -67,16 +96,16 @@
         (testing "all good"
           (is (= {:status 200
                   :body {:total 15}}
-                 (app valid-request)))
+                 (app valid-request1)))
           (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?
@@ -92,10 +121,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"
@@ -127,16 +156,16 @@
         (testing "all good"
           (is (= {:status 200
                   :body {:total 15}}
-                 (app valid-request)))
+                 (app valid-request1)))
           (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?
@@ -152,16 +181,262 @@
             (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"
               (let [{:keys [status]} (app invalid-request2)]
                 (is (= 500 status))))))))))
 
+(deftest malli-coercion-test
+  (let [create (fn [interceptors]
+                 (http/ring-handler
+                   (http/router
+                     ["/api"
+
+                      ["/validate" {:summary "just validation"
+                                    :coercion (reitit.coercion.malli/create {:transformers {}})
+                                    :post {:parameters {:body [:map [:x int?]]}
+                                           :responses {200 {:body [:map [:x int?]]}}
+                                           :handler (fn [req]
+                                                      {:status 200
+                                                       :body (-> req :parameters :body)})}}]
+
+                      ["/no-op" {:summary "no-operation"
+                                 :coercion (reitit.coercion.malli/create {:transformers {}, :validate false})
+                                 :post {:parameters {:body [:map [:x int?]]}
+                                        :responses {200 {:body [:map [:x int?]]}}
+                                        :handler (fn [req]
+                                                   {:status 200
+                                                    :body (-> req :parameters :body)})}}]
+
+                      ["/skip" {:summary "skip"
+                                :coercion (reitit.coercion.malli/create {:enabled false})
+                                :post {:parameters {:body [:map [:x int?]]}
+                                       :responses {200 {:body [:map [:x int?]]}}
+                                       :handler (fn [req]
+                                                  {:status 200
+                                                   :body (-> req :parameters :body)})}}]
+
+                      ["/or" {:post {:summary "accepts either of two map schemas"
+                                     :parameters {:body [:or [:map [:x int?]] [:map [:y int?]]]}
+                                     :responses {200 {:body [:map [:msg string?]]}}
+                                     :handler (fn [{{{:keys [x]} :body} :parameters}]
+                                                {:status 200
+                                                 :body {:msg (if x "you sent x" "you sent y")}})}}]
+
+                      ["/plus/:e" {:get {:parameters {:query [:map [:a {:optional true} int?]]
+                                                      :body [:map [:b int?]]
+                                                      :form [:map [:c [int? {:default 3}]]]
+                                                      :header [:map [:d int?]]
+                                                      :path [:map [:e int?]]}
+                                         :responses {200 {:body [:map [:total pos-int?]]}
+                                                     500 {:description "fail"}}
+                                         :handler handler}}]]
+                     {:data {:interceptors interceptors
+                             :coercion malli/coercion}})
+                   {:executor sieppari/executor}))]
+
+    (testing "withut exception handling"
+      (let [app (create [(rrc/coerce-request-interceptor)
+                         (rrc/coerce-response-interceptor)])]
+
+        (testing "all good"
+          (is (= {:status 200
+                  :body {:total 15}}
+                 (app valid-request1)))
+          #_(is (= {:status 200
+                  :body {:total 115}}
+                 (app valid-request2)))
+          (is (= {:status 200
+                  :body {:total 15}}
+                 (app valid-request3)))
+          (testing "default values work"
+            (is (= {:status 200
+                    :body {:total 15}}
+                   (app (update valid-request3 :form-params dissoc :c)))))
+          (is (= {:status 500
+                  :body {:evil true}}
+                 (app (assoc-in valid-request1 [:query-params "a"] "666")))))
+
+        (testing "invalid request"
+          (is (thrown-with-msg?
+                ExceptionInfo
+                #"Request coercion failed"
+                (app invalid-request1))))
+
+        (testing "invalid response"
+          (is (thrown-with-msg?
+                ExceptionInfo
+                #"Response coercion failed"
+                (app invalid-request2))))))
+
+    (testing "with exception handling"
+      (let [app (create [(rrc/coerce-exceptions-interceptor)
+                         (rrc/coerce-request-interceptor)
+                         (rrc/coerce-response-interceptor)])]
+
+        (testing "just validation"
+          (is (= 400 (:status (app {:uri "/api/validate"
+                                    :request-method :post
+                                    :muuntaja/request {:format "application/edn"}
+                                    :body-params 123}))))
+          (is (= [:reitit.http.coercion/coerce-exceptions
+                  :reitit.http.coercion/coerce-request
+                  :reitit.http.coercion/coerce-response
+                  :reitit.interceptor/handler]
+                 (mounted-interceptor app "/api/validate" :post))))
+
+        (testing "no tranformation & validation"
+          (is (= 123 (:body (app {:uri "/api/no-op"
+                                  :request-method :post
+                                  :muuntaja/request {:format "application/edn"}
+                                  :body-params 123}))))
+          (is (= [:reitit.http.coercion/coerce-exceptions
+                  :reitit.http.coercion/coerce-request
+                  :reitit.http.coercion/coerce-response
+                  :reitit.interceptor/handler]
+                 (mounted-interceptor app "/api/no-op" :post))))
+
+        (testing "skipping coercion"
+          (is (= nil (:body (app {:uri "/api/skip"
+                                  :request-method :post
+                                  :muuntaja/request {:format "application/edn"}
+                                  :body-params 123}))))
+
+          (is (= [:reitit.http.coercion/coerce-exceptions
+                  :reitit.interceptor/handler]
+                 (mounted-interceptor app "/api/skip" :post))))
+
+        (testing "or #407"
+          (is (= {:status 200
+                  :body {:msg "you sent x"}}
+                 (app {:uri "/api/or"
+                       :request-method :post
+                       :body-params {:x 1}}))))
+
+        (testing "all good"
+          (is (= {:status 200
+                  :body {:total 15}}
+                 (app valid-request1))))
+
+        (testing "invalid request"
+          (let [{:keys [status]} (app invalid-request1)]
+            (is (= 400 status))))
+
+        (testing "invalid response"
+          (let [{:keys [status]} (app invalid-request2)]
+            (is (= 500 status))))))
+
+    (testing "open & closed schemas"
+      (let [endpoint (fn [schema]
+                       {:get {:parameters {:body schema}
+                              :responses {200 {:body schema}}
+                              :handler (fn [{{:keys [body]} :parameters}]
+                                         {:status 200, :body (assoc body :response true)})}})
+            ->app (fn [options]
+                    (http/ring-handler
+                      (http/router
+                        ["/api"
+                         ["/default" (endpoint [:map [:x int?]])]
+                         ["/closed" (endpoint [:map {:closed true} [:x int?]])]
+                         ["/open" (endpoint [:map {:closed false} [:x int?]])]]
+                        {:data {:interceptors [(rrc/coerce-exceptions-interceptor)
+                                               (rrc/coerce-request-interceptor)
+                                               (rrc/coerce-response-interceptor)]
+                                :coercion (malli/create options)}})
+                      {:executor sieppari/executor}))
+            ->request (fn [uri] {:uri (str "/api/" uri)
+                                 :request-method :get
+                                 :muuntaja/request {:format "application/json"}
+                                 :body-params {:x 1, :request true}})]
+
+        (testing "with defaults"
+          (let [app (->app nil)]
+
+            (testing "default: keys are stripped"
+              (is (= {:status 200, :body {:x 1}}
+                     (app (->request "default")))))
+
+            (testing "closed: keys are stripped"
+              (is (= {:status 200, :body {:x 1}}
+                     (app (->request "closed")))))
+
+            (testing "open: keys are NOT stripped"
+              (is (= {:status 200, :body {:x 1, :request true, :response true}}
+                     (app (->request "open")))))))
+
+        (testing "when schemas are not closed"
+          (let [app (->app {:compile (fn [v _] v)})]
+
+            (testing "default: keys are stripped"
+              (is (= {:status 200, :body {:x 1}}
+                     (app (->request "default")))))
+
+            (testing "closed: keys are stripped"
+              (is (= {:status 200, :body {:x 1}}
+                     (app (->request "closed")))))
+
+            (testing "open: keys are NOT stripped"
+              (is (= {:status 200, :body {:x 1, :request true, :response true}}
+                     (app (->request "open")))))))
+
+        (testing "when schemas are not closed and extra keys are not stripped"
+          (let [app (->app {:compile (fn [v _] v) :strip-extra-keys false})]
+            (testing "default: keys are NOT stripped"
+              (is (= {:status 200, :body {:x 1, :request true, :response true}}
+                     (app (->request "default")))))
+
+            (testing "closed: FAILS for extra keys"
+              (is (= 400 (:status (app (->request "closed"))))))
+
+            (testing "open: keys are NOT stripped"
+              (is (= {:status 200, :body {:x 1, :request true, :response true}}
+                     (app (->request "open")))))))))
+
+    (testing "sequence schemas"
+      (let [app (http/ring-handler
+                  (http/router
+                    ["/ping" {:get {:parameters {:body [:vector [:map [:message string?]]]}
+                                    :responses {200 {:body [:vector [:map [:pong string?]]]}
+                                                501 {:body [:vector [:map [:error string?]]]}}
+                                    :handler (fn [{{[{:keys [message]}] :body} :parameters :as req}]
+                                               (condp = message
+                                                 "ping" {:status 200
+                                                         :body [{:pong message}]}
+                                                 "fail" {:status 501
+                                                         :body [{:error "fail"}]}
+                                                 {:status 200
+                                                  :body {:invalid "response"}}))}}]
+                    {:data {:interceptors [(rrc/coerce-exceptions-interceptor)
+                                           (rrc/coerce-request-interceptor)
+                                           (rrc/coerce-response-interceptor)]
+                            :coercion malli/coercion}})
+                  {:executor sieppari/executor})
+            ->request (fn [body]
+                        {:uri "/ping"
+                         :request-method :get
+                         :muuntaja/request {:format "application/json"}
+                         :body-params body})]
+
+        (testing "succesfull request"
+          (let [{:keys [status body]} (app (->request [{:message "ping"}]))]
+            (is (= 200 status))
+            (is (= [{:pong "ping"}] body)))
+
+          (testing "succesfull failure"
+            (let [{:keys [status body]} (app (->request [{:message "fail"}]))]
+              (is (= 501 status))
+              (is (= [{:error "fail"}] body))))
+
+          (testing "failed response"
+            (let [{:keys [status body]} (app (->request [{:message "kosh"}]))]
+              (is (= 500 status))
+              (is (= :reitit.coercion/response-coercion (:type body))))))))))
+
 (deftest muuntaja-test
   (let [app (http/ring-handler
               (http/router

test/cljc/reitit/ring_coercion_test.cljc

@@ -14,15 +14,12 @@
      (:import (clojure.lang ExceptionInfo)
               (java.io ByteArrayInputStream))))
 
-(defn middleware-name [{:keys [wrap name]}]
-  (or name (-> wrap str symbol)))
-
 (defn mounted-middleware [app path method]
   (->> app
        (ring/get-router)
        (r/compiled-routes)
        (filter (comp (partial = path) first))
-       (first) (last) method :middleware (filter :wrap) (mapv middleware-name)))
+       (first) (last) method :middleware (filter :wrap) (mapv :name)))
 
 (defn handler [{{{:keys [a]} :query
                  {:keys [b]} :body