Merge pull request #60 from metosin/coercion-specs

Coercion specs

README.md

@@ -64,21 +64,22 @@ A Ring routing app with input & output coercion using [data-specs](https://githu
 ```clj
 (require '[reitit.ring :as ring])
 (require '[reitit.coercion.spec])
-(require '[reitit.ring.coercion-middleware :as mw])
+(require '[reitit.ring.coercion :as rrc])
 
 (def app
   (ring/ring-handler
     (ring/router
       ["/api"
-       ["/math" {:get {:coercion reitit.coercion.spec/coercion
+       ["/math" {:get {:parameters {:query {:x int?, :y int?}}
-                       :parameters {:query {:x int?, :y int?}}
                        :responses {200 {:schema {:total pos-int?}}}
                        :handler (fn [{{{:keys [x y]} :query} :parameters}]
                                   {:status 200
                                    :body {:total (+ x y)}})}}]]
-      {:data {:middleware [mw/coerce-exceptions-middleware
+      ;; router data effecting all routes
-                           mw/coerce-request-middleware
+      {:data {:coercion reitit.coercion.spec/coercion
-                           mw/coerce-response-middleware]}})))
+              :middleware [rrc/coerce-exceptions-middleware
+                           rrc/coerce-request-middleware
+                           rrc/coerce-response-middleware]}})))
 ```
 
 Valid request:

doc/ring/coercion.md

@@ -46,7 +46,7 @@ The coerced parameters can be read under `:parameters` key in the request.
 
 ## Coercion Middleware
 
-Defining a coercion for a route data doesn't do anything, as it's just data. We have to attach some code to apply the actual coercion. We can use the middleware from `reitit.ring.coercion-middleware`:
+Defining a coercion for a route data doesn't do anything, as it's just data. We have to attach some code to apply the actual coercion. We can use the middleware from `reitit.ring.coercion`:
 
 * `coerce-request-middleware` for the parameter coercion
 * `coerce-response-middleware` for the response coercion
@@ -57,7 +57,7 @@ Defining a coercion for a route data doesn't do anything, as it's just data. We
 Here's an full example for applying coercion with Reitit, Ring and Schema:
 
 ```clj
-(require '[reitit.ring.coercion-middleware :as mw])
+(require '[reitit.ring.coercion :as rrc])
 (require '[reitit.coercion.schema])
 (require '[reitit.ring :as ring])
 (require '[schema.core :as s])
@@ -84,9 +84,9 @@ Here's an full example for applying coercion with Reitit, Ring and Schema:
                                                      (-> parameters :path :z))]
                                         {:status 200
                                          :body {:total total}}))}}]]
-      {:data {:middleware [mw/coerce-exceptions-middleware
+      {:data {:middleware [rrc/coerce-exceptions-middleware
-                           mw/coerce-request-middleware
+                           rrc/coerce-request-middleware
-                           mw/coerce-response-middleware]}})))
+                           rrc/coerce-response-middleware]}})))
 ```
 
 Valid request:

doc/ring/compiling_middleware.md

@@ -47,15 +47,17 @@ To demonstrate the two approaches, below are response coercion middleware writte
 * Route information is provided via a closure
 * Pre-compiled coercers
 * Mounts only if `:coercion` and `:responses` are defined for the route
+* Also defines spec for the route data `:responses` for the [route data validation](route_data_validation.md).
 
 ```clj
-(require '[reitit.middleware :as middleware])
+(require '[reitit.spec :as rs])
 
 (def coerce-response-middleware
   "Middleware for pluggable response coercion.
   Expects a :coercion of type `reitit.coercion/Coercion`
   and :responses from route data, otherwise does not mount."
   {:name ::coerce-response
+   :spec ::rs/responses
    :compile (fn [{:keys [coercion responses]} opts]
               (if (and coercion responses)
                 (let [coercers (coercion/response-coercers coercion responses opts)]

examples/just-coercion-with-ring/src/example/server.clj

@@ -1,7 +1,7 @@
 (ns example.server
   (:require [ring.adapter.jetty :as jetty]
             [reitit.middleware :as middleware]
-            [reitit.ring.coercion-middleware :as coercion-middleware]))
+            [reitit.ring.coercion :as rrc]))
 
 (defonce ^:private server (atom nil))
 
@@ -10,9 +10,9 @@
 ;; to be set with :extract-request-format and extract-response-format
 (defn wrap-coercion [handler resource]
   (middleware/chain
-    [coercion-middleware/coerce-request-middleware
+    [rrc/coerce-request-middleware
-     coercion-middleware/coerce-response-middleware
+     rrc/coerce-response-middleware
-     coercion-middleware/coerce-exceptions-middleware]
+     rrc/coerce-exceptions-middleware]
     handler
     resource))
 

examples/ring-example/src/example/server.clj

@@ -3,7 +3,7 @@
             [ring.middleware.params]
             [muuntaja.middleware]
             [reitit.ring :as ring]
-            [reitit.ring.coercion-middleware :as coercion-middleware]
+            [reitit.ring.coercion :as rrc]
             [example.dspec]
             [example.schema]
             [example.spec]))
@@ -18,9 +18,9 @@
        example.spec/routes]
       {:data {:middleware [ring.middleware.params/wrap-params
                            muuntaja.middleware/wrap-format
-                           coercion-middleware/coerce-exceptions-middleware
+                           rrc/coerce-exceptions-middleware
-                           coercion-middleware/coerce-request-middleware
+                           rrc/coerce-request-middleware
-                           coercion-middleware/coerce-response-middleware]}})))
+                           rrc/coerce-response-middleware]}})))
 
 (defn restart []
   (swap! server (fn [x]

modules/reitit-core/src/reitit/spec.cljc

@@ -1,7 +1,6 @@
 (ns reitit.spec
   (:require [clojure.spec.alpha :as s]
             [clojure.spec.gen.alpha :as gen]
-            [clojure.string :as str]
             [reitit.core :as reitit]))
 
 ;;
@@ -71,9 +70,42 @@
         :ret ::router)
 
 ;;
-;; Route data validator
+;; coercion
 ;;
 
+(s/def :reitit.core.coercion/kw-map (s/map-of keyword? any?))
+
+(s/def :reitit.core.coercion/query :reitit.core.coercion/kw-map)
+(s/def :reitit.core.coercion/body :reitit.core.coercion/kw-map)
+(s/def :reitit.core.coercion/form :reitit.core.coercion/kw-map)
+(s/def :reitit.core.coercion/header :reitit.core.coercion/kw-map)
+(s/def :reitit.core.coercion/path :reitit.core.coercion/kw-map)
+(s/def :reitit.core.coercion/parameters
+  (s/keys :opt-un [:reitit.core.coercion/query
+                   :reitit.core.coercion/body
+                   :reitit.core.coercion/form
+                   :reitit.core.coercion/header
+                   :reitit.core.coercion/path]))
+
+(s/def ::parameters
+  (s/keys :opt-un [:reitit.core.coercion/parameters]))
+
+(s/def :reitit.core.coercion/status
+  (s/or :number number? :default #{:default}))
+(s/def :reitit.core.coercion/schema any?)
+(s/def :reitit.core.coercion/description string?)
+(s/def :reitit.core.coercion/response
+  (s/keys :opt-un [:reitit.core.coercion/schema
+                   :reitit.core.coercion/description]))
+(s/def :reitit.core.coercion/responses
+  (s/map-of :reitit.core.coercion/status :reitit.core.coercion/response))
+
+(s/def ::responses
+  (s/keys :opt-un [:reitit.core.coercion/responses]))
+
+;;
+;; Route data validator
+;;
 
 (defrecord Problem [path scope data spec problems])
 

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

@@ -1,5 +1,6 @@
-(ns reitit.ring.coercion-middleware
+(ns reitit.ring.coercion
   (:require [reitit.coercion :as coercion]
+            [reitit.spec :as rs]
             [reitit.impl :as impl]))
 
 (defn handle-coercion-exception [e respond raise]
@@ -22,6 +23,7 @@
   Expects a :coercion of type `reitit.coercion/Coercion`
   and :parameters from route data, otherwise does not mount."
   {:name ::coerce-request
+   :spec ::rs/parameters
    :compile (fn [{:keys [coercion parameters]} opts]
               (if (and coercion parameters)
                 (let [coercers (coercion/request-coercers coercion parameters opts)]
@@ -39,6 +41,7 @@
   Expects a :coercion of type `reitit.coercion/Coercion`
   and :responses from route data, otherwise does not mount."
   {:name ::coerce-response
+   :spec ::rs/responses
    :compile (fn [{:keys [coercion responses]} opts]
               (if (and coercion responses)
                 (let [coercers (coercion/response-coercers coercion responses opts)]

perf-test/clj/reitit/coercion_perf_test.clj

@@ -8,7 +8,7 @@
             [muuntaja.core :as m]
             [muuntaja.format.jsonista :as jsonista-format]
             [jsonista.core :as j]
-            [reitit.ring.coercion-middleware :as coercion-middleware]
+            [reitit.ring.coercion :as rrc]
             [reitit.coercion.spec :as spec]
             [reitit.coercion.schema :as schema]
             [reitit.coercion :as coercion]
@@ -36,14 +36,14 @@
     (s/def ::k (s/keys :req-un [::x ::y]))
 
     (let [spec (spec/into-spec {:x int?, :y int?} ::jeah)
-          coercers (#'coercion-middleware/request-coercers spec/coercion {:body spec})
+          coercers (#'rrc/request-coercers spec/coercion {:body spec})
           params {:x "1", :y "2"}
           request {:body-params {:x "1", :y "2"}}]
 
       ;; 4600ns
       (bench!
         "coerce-parameters"
-        (#'coercion-middleware/coerce-request-middleware coercers request))
+        (#'rrc/coerce-request-middleware coercers request))
 
       ;; 2700ns
       (bench!
@@ -102,24 +102,24 @@
           app (ring/ring-handler
                 (ring/router
                   routes
-                  {:data {:middleware [coercion-middleware/coerce-request-middleware]
+                  {:data {:middleware [rrc/coerce-request-middleware]
                           :coercion coercion}}))
           app2 (ring/ring-handler
                  (ring/router
                    routes
-                   {:data {:middleware [coercion-middleware/coerce-request-middleware]
+                   {:data {:middleware [rrc/coerce-request-middleware]
                            :coercion coercion}}))
           app3 (ring/ring-handler
                  (ring/router
                    routes
-                   {:data {:middleware [coercion-middleware/coerce-request-middleware
+                   {:data {:middleware [rrc/coerce-request-middleware
-                                        coercion-middleware/wrap-coerce-response]
+                                        rrc/wrap-coerce-response]
                            :coercion coercion}}))
           app4 (ring/ring-handler
                  (ring/router
                    routes
-                   {:data {:middleware [coercion-middleware/coerce-request-middleware
+                   {:data {:middleware [rrc/coerce-request-middleware
-                                        coercion-middleware/coerce-response-middleware]
+                                        rrc/coerce-response-middleware]
                            :coercion coercion}}))
           req {:request-method :get
                :uri "/api/ping"
@@ -156,8 +156,8 @@
                      :get {:handler (fn [{{{:keys [x y]} :body} :parameters}]
                                       {:status 200
                                        :body {:total (+ x y)}})}}]]
-          {:data {:middleware [coercion-middleware/coerce-request-middleware
+          {:data {:middleware [rrc/coerce-request-middleware
-                               coercion-middleware/coerce-response-middleware]
+                               rrc/coerce-response-middleware]
                   :coercion spec/coercion}})))
 
     (app
@@ -205,8 +205,8 @@
                                             (let [body (-> request :parameters :body)]
                                               {:status 200, :body {:result (+ (:x body) (:y body))}}))}}]
                 {:data {:middleware [[mm/wrap-format m]
-                                     coercion-middleware/coerce-request-middleware
+                                     rrc/coerce-request-middleware
-                                     coercion-middleware/coerce-response-middleware]
+                                     rrc/coerce-response-middleware]
                         :coercion schema/coercion}}))
         request {:request-method :post
                  :uri "/plus"
@@ -229,8 +229,8 @@
                                  :handler (fn [request]
                                             (let [body (-> request :parameters :body)]
                                               {:status 200, :body {:result (+ (:x body) (:y body))}}))}}]
-                {:data {:middleware [coercion-middleware/coerce-request-middleware
+                {:data {:middleware [rrc/coerce-request-middleware
-                                     coercion-middleware/coerce-response-middleware]
+                                     rrc/coerce-response-middleware]
                         :coercion schema/coercion}}))
         request {:request-method :post
                  :uri "/plus"
@@ -253,8 +253,8 @@
                                  :handler (fn [request]
                                             (let [body (-> request :parameters :body)]
                                               {:status 200, :body {:result (+ (:x body) (:y body))}}))}}]
-                {:data {:middleware [coercion-middleware/coerce-request-middleware
+                {:data {:middleware [rrc/coerce-request-middleware
-                                     coercion-middleware/coerce-response-middleware]
+                                     rrc/coerce-response-middleware]
                         :coercion spec/coercion}}))
         request {:request-method :post
                  :uri "/plus"
@@ -282,8 +282,8 @@
                                  :handler (fn [request]
                                             (let [body (-> request :parameters :body)]
                                               {:status 200, :body {:result (+ (:x body) (:y body))}}))}}]
-                {:data {:middleware [coercion-middleware/coerce-request-middleware
+                {:data {:middleware [rrc/coerce-request-middleware
-                                     coercion-middleware/coerce-response-middleware]
+                                     rrc/coerce-response-middleware]
                         :coercion spec/coercion}}))
         request {:request-method :post
                  :uri "/plus"

test/cljc/reitit/coercion_test.cljc

@@ -48,4 +48,3 @@
           (is (= nil (coercion/coerce! m))))
         (let [m (r/match-by-path r "/none/kikka/abba")]
           (is (= nil (coercion/coerce! m))))))))
-

test/cljc/reitit/core_test.cljc

@@ -16,7 +16,7 @@
             (is (= name (r/router-name router)))
             (is (= [["/api/ipa/:size" {:name ::beer} nil]]
                    (r/routes router)))
-            (is (= true (map? (r/options router))))
+            (is (map? (r/options router)))
             (is (= (r/map->Match
                      {:template "/api/ipa/:size"
                       :data {:name ::beer}
@@ -39,7 +39,7 @@
                         :required #{:size}
                         :params nil})
                      (r/match-by-name router ::beer)))
-              (is (= true (r/partial-match? (r/match-by-name router ::beer))))
+              (is (r/partial-match? (r/match-by-name router ::beer)))
               (is (thrown-with-msg?
                     ExceptionInfo
                     #"^missing path-params for route /api/ipa/:size -> \#\{:size\}$"
@@ -72,7 +72,7 @@
         (is (= name (r/router-name router)))
         (is (= [["/api/ipa/large" {:name ::beer} nil]]
                (r/routes router)))
-        (is (= true (map? (r/options router))))
+        (is (map? (r/options router)))
         (is (= (r/map->Match
                  {:template "/api/ipa/large"
                   :data {:name ::beer}

test/cljc/reitit/ring_coercion_test.cljc

@@ -2,7 +2,7 @@
   (:require [clojure.test :refer [deftest testing is]]
             [schema.core :as s]
             [reitit.ring :as ring]
-            [reitit.ring.coercion-middleware :as coercion-middleware]
+            [reitit.ring.coercion :as rrc]
             [reitit.coercion.spec :as spec]
             [reitit.coercion.schema :as schema])
   #?(:clj
@@ -53,8 +53,8 @@
                              :coercion spec/coercion}})))]
 
     (testing "withut exception handling"
-      (let [app (create [coercion-middleware/coerce-request-middleware
+      (let [app (create [rrc/coerce-request-middleware
-                         coercion-middleware/coerce-response-middleware])]
+                         rrc/coerce-response-middleware])]
 
         (testing "all good"
           (is (= {:status 200
@@ -74,9 +74,9 @@
                 (app invalid-request2))))))
 
     (testing "with exception handling"
-      (let [app (create [coercion-middleware/coerce-exceptions-middleware
+      (let [app (create [rrc/coerce-exceptions-middleware
-                         coercion-middleware/coerce-request-middleware
+                         rrc/coerce-request-middleware
-                         coercion-middleware/coerce-response-middleware])]
+                         rrc/coerce-response-middleware])]
 
         (testing "all good"
           (is (= {:status 200
@@ -108,8 +108,8 @@
                              :coercion schema/coercion}})))]
 
     (testing "withut exception handling"
-      (let [app (create [coercion-middleware/coerce-request-middleware
+      (let [app (create [rrc/coerce-request-middleware
-                         coercion-middleware/coerce-response-middleware])]
+                         rrc/coerce-response-middleware])]
 
         (testing "all good"
           (is (= {:status 200
@@ -129,9 +129,9 @@
                 (app invalid-request2))))
 
         (testing "with exception handling"
-          (let [app (create [coercion-middleware/coerce-exceptions-middleware
+          (let [app (create [rrc/coerce-exceptions-middleware
-                             coercion-middleware/coerce-request-middleware
+                             rrc/coerce-request-middleware
-                             coercion-middleware/coerce-response-middleware])]
+                             rrc/coerce-response-middleware])]
 
             (testing "all good"
               (is (= {:status 200

test/cljc/reitit/ring_spec_test.cljc

@@ -2,6 +2,8 @@
   (:require [clojure.test :refer [deftest testing is]]
             [reitit.ring :as ring]
             [reitit.ring.spec :as rrs]
+            [reitit.ring.coercion :as rrc]
+            [reitit.coercion.spec]
             [clojure.spec.alpha :as s]
             [reitit.core :as r])
   #?(:clj
@@ -12,9 +14,9 @@
 
 (deftest route-data-validation-test
   (testing "validation is turned off by default"
-    (is (true? (r/router?
+    (is (r/router?
-                 (r/router
+          (r/router
-                   ["/api" {:handler "identity"}])))))
+            ["/api" {:handler "identity"}]))))
 
   (testing "with default spec validates :name, :handler and :middleware"
     (is (thrown-with-msg?
@@ -40,11 +42,11 @@
             {:validate rrs/validate-spec!}))))
 
   (testing "spec can be overridden"
-    (is (true? (r/router?
+    (is (r/router?
-                 (ring/router
+          (ring/router
-                   ["/api" {:handler "identity"}]
+            ["/api" {:handler "identity"}]
-                   {:spec (s/spec any?)
+            {:spec (s/spec any?)
-                    :validate rrs/validate-spec!}))))
+             :validate rrs/validate-spec!})))
 
     (testing "predicates are not allowed"
       (is (thrown-with-msg?
@@ -56,15 +58,15 @@
                :validate rrs/validate-spec!})))))
 
   (testing "middleware can contribute to specs"
-    (is (true? (r/router?
+    (is (r/router?
-                 (ring/router
+          (ring/router
-                   ["/api" {:get {:handler identity
+            ["/api" {:get {:handler identity
-                                  :roles #{:admin}}}]
+                           :roles #{:admin}}}]
-                   {:validate rrs/validate-spec!
+            {:validate rrs/validate-spec!
-                    :data {:middleware [{:spec (s/keys :opt-un [::roles])
+             :data {:middleware [{:spec (s/keys :opt-un [::roles])
-                                         :wrap (fn [handler]
+                                  :wrap (fn [handler]
-                                                 (fn [request]
+                                          (fn [request]
-                                                   (handler request)))}]}}))))
+                                            (handler request)))}]}})))
     (is (thrown-with-msg?
           ExceptionInfo
           #"Invalid route data"
@@ -76,3 +78,49 @@
                                   :wrap (fn [handler]
                                           (fn [request]
                                             (handler request)))}]}})))))
+
+(deftest coercion-spec-test
+  (is (r/router?
+        (ring/router
+          ["/api"
+           ["/plus/:e"
+            {:get {:parameters {:query {:a string?}
+                                :body {:b string?}
+                                :form {:c string?}
+                                :header {:d string?}
+                                :path {:e string?}}
+                   :responses {200 {:schema {:total pos-int?}}}
+                   :handler identity}}]]
+          {:data {:middleware [rrc/coerce-exceptions-middleware
+                               rrc/coerce-request-middleware
+                               rrc/coerce-response-middleware]
+                  :coercion reitit.coercion.spec/coercion}
+           :validate rrs/validate-spec!})))
+
+  (is (thrown-with-msg?
+        ExceptionInfo
+        #"Invalid route data"
+        (ring/router
+          ["/api"
+           ["/plus/:e"
+            {:get {:parameters {:query {"a" string?}}
+                   :handler identity}}]]
+          {:data {:middleware [rrc/coerce-exceptions-middleware
+                               rrc/coerce-request-middleware
+                               rrc/coerce-response-middleware]
+                  :coercion reitit.coercion.spec/coercion}
+           :validate rrs/validate-spec!})))
+
+  (is (thrown-with-msg?
+        ExceptionInfo
+        #"Invalid route data"
+        (ring/router
+          ["/api"
+           ["/plus/:e"
+            {:get {:responses {"200" {}}
+                   :handler identity}}]]
+          {:data {:middleware [rrc/coerce-exceptions-middleware
+                               rrc/coerce-request-middleware
+                               rrc/coerce-response-middleware]
+                  :coercion reitit.coercion.spec/coercion}
+           :validate rrs/validate-spec!}))))

test/cljc/reitit/spec_test.cljc

@@ -16,7 +16,7 @@
 
     (testing "route-data"
       (are [data]
-        (is (= true (r/router? (r/router data))))
+        (is (r/router? (r/router data)))
 
         ["/api" {}]
 
@@ -45,12 +45,12 @@
            ["/ipa"]])))
 
     (testing "routes conform to spec (can't spec protocol functions)"
-      (is (= true (s/valid? ::rs/routes (r/routes (r/router ["/ping"]))))))
+      (is (s/valid? ::rs/routes (r/routes (r/router ["/ping"])))))
 
     (testing "options"
 
       (are [opts]
-        (is (= true (r/router? (r/router ["/api"] opts))))
+        (is (r/router? (r/router ["/api"] opts)))
 
         {:path "/"}
         {:data {}}
@@ -78,8 +78,8 @@
 
 (deftest route-data-validation-test
   (testing "validation is turned off by default"
-    (is (true? (r/router? (r/router
+    (is (r/router? (r/router
-                            ["/api" {:handler "identity"}])))))
+                     ["/api" {:handler "identity"}]))))
 
   (testing "with default spec validates :name and :handler"
     (is (thrown-with-msg?
@@ -96,7 +96,35 @@
             {:validate rs/validate-spec!}))))
 
   (testing "spec can be overridden"
-    (is (true? (r/router? (r/router
+    (is (r/router? (r/router
-                            ["/api" {:handler "identity"}]
+                     ["/api" {:handler "identity"}]
-                            {:spec any?
+                     {:spec any?
-                             :validate rs/validate-spec!}))))))
+                      :validate rs/validate-spec!})))))
+
+(deftest parameters-test
+  (is (s/valid?
+        ::rs/parameters
+        {:parameters {:query {:a string?}
+                      :body {:b string?}
+                      :form {:c string?}
+                      :header {:d string?}
+                      :path {:e string?}}}))
+
+  (is (not (s/valid?
+             ::rs/parameters
+             {:parameters {:header {"d" string?}}})))
+
+  (is (s/valid?
+        ::rs/responses
+        {:responses {200 {:description "ok", :schema string?}
+                     400 {:description "fail"}
+                     500 {:schema string?}
+                     :default {}}}))
+
+  (is (not (s/valid?
+             ::rs/responses
+             {:responses {"200" {:description "ok", :schema string?}}})))
+
+  (is (not (s/valid?
+             ::rs/responses
+             {:responses {200 {:description :ok, :schema string?}}}))))