Merge pull request #263 from metosin/north

Closed spec validation demoed in Clojure/North (WIP)

CHANGELOG.md

@@ -17,11 +17,15 @@ We use [Break Versioning][breakver]. The version numbers follow a `<major>.<mino
 * Updated dependencies:
 
 ```clj
-[metosin/spec-tools "0.9.1"] is available but we use "0.9.0"
+[metosin/spec-tools "0.9.2-alpha1"] is available but we use "0.9.0"
 [metosin/muuntaja "0.6.4"] is available but we use "0.6.3"
 [lambdaisland/deep-diff "0.0-47"] is available but we use "0.0-25"
 ```
 
+### `reitit-core`
+
+* new options `:reitit.spec/wrap` to wrap top-level route data specs when spec validation is enabled. Using `spec-tools.spell/closed` closes top-level specs. 
+
 ### `reitit.pedestal`
 
 * Automatically coerce Sieppari-style 1-arity `:error` handlers into Pedestal-style 2-arity `:error` handlers. Thanks to [Mathieu MARCHANDISE](https://github.com/vielmath).

examples/http-swagger/src/example/server.clj

@@ -9,8 +9,11 @@
             [reitit.http.interceptors.muuntaja :as muuntaja]
             [reitit.http.interceptors.exception :as exception]
             [reitit.http.interceptors.multipart :as multipart]
+            [reitit.http.spec :as spec]
             [reitit.http.interceptors.dev :as dev]
             [reitit.interceptor.sieppari :as sieppari]
+            [reitit.dev.pretty :as pretty]
+            [spec-tools.spell :as spell]
             [ring.adapter.jetty :as jetty]
             [aleph.http :as client]
             [muuntaja.core :as m]
@@ -72,7 +75,7 @@
                               "https://randomuser.me/api/"
                               {:query-params {:seed seed, :results results}})
                             :body
-                            (partial m/decode m/instance "application/json")
+                            (partial m/decode "application/json")
                             :results
                             (fn [results]
                               {:status 200
@@ -109,10 +112,15 @@
                             {:status 200
                              :body {:total (- x y)}})}}]]]
 
-      {;;:reitit.interceptor/transform dev/print-context-diffs
+      {;:reitit.interceptor/transform dev/print-context-diffs ;; pretty context diffs
+       :validate spec/validate ;; enable spec validation for route data
+       :reitit.spec/wrap spell/closed ;; strict top-level validation (alpha)
+       :exception pretty/exception
        :data {:coercion spec-coercion/coercion
               :muuntaja m/instance
-              :interceptors [;; query-params & form-params
+              :interceptors [;; swagger feature
+                             swagger/swagger-feature
+                             ;; query-params & form-params
                              (parameters/parameters-interceptor)
                              ;; content-negotiation
                              (muuntaja/format-negotiate-interceptor)

modules/reitit-core/java-src/reitit/Trie.java

@@ -291,8 +291,8 @@ public class Trie {
                             staticMatcher("/auth/",
                                     linearMatcher(
                                             Arrays.asList(
-                                                    staticMatcher("login", dataMatcher(null, 1)),
-                                                    staticMatcher("recovery", dataMatcher(null, 2))), true))), true);
+                                                    staticMatcher("login", dataMatcher(PersistentArrayMap.EMPTY, 1)),
+                                                    staticMatcher("recovery", dataMatcher(PersistentArrayMap.EMPTY, 2))), true))), true);
     System.err.println(matcher);
     System.out.println(lookup(matcher, "/auth/login"));
     System.out.println(lookup(matcher, "/auth/recovery"));

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

@@ -39,7 +39,8 @@
 
 (s/def ::name keyword?)
 (s/def ::handler fn?)
-(s/def ::default-data (s/keys :opt-un [::name ::handler]))
+(s/def ::no-doc boolean?)
+(s/def ::default-data (s/keys :opt-un [::name ::handler ::no-doc]))
 
 ;;
 ;; router
@@ -75,6 +76,8 @@
 ;; coercion
 ;;
 
+(s/def :reitit.core.coercion/coercion any?)
+
 (s/def :reitit.core.coercion/model any?)
 
 (s/def :reitit.core.coercion/query :reitit.core.coercion/model)
@@ -90,7 +93,8 @@
                    :reitit.core.coercion/path]))
 
 (s/def ::parameters
-  (s/keys :opt-un [:reitit.core.coercion/parameters]))
+  (s/keys :opt-un [:reitit.core.coercion/coercion
+                   :reitit.core.coercion/parameters]))
 
 (s/def :reitit.core.coercion/status
   (s/or :number number? :default #{:default}))
@@ -103,7 +107,8 @@
   (s/map-of :reitit.core.coercion/status :reitit.core.coercion/response))
 
 (s/def ::responses
-  (s/keys :opt-un [:reitit.core.coercion/responses]))
+  (s/keys :opt-un [:reitit.core.coercion/coercion
+                   :reitit.core.coercion/responses]))
 
 ;;
 ;; Route data validator
@@ -111,14 +116,14 @@
 
 (defrecord Problem [path scope data spec problems])
 
-(defn validate-route-data [routes spec]
+(defn validate-route-data [routes wrap spec]
   (some->> (for [[p d _] routes]
-             (when-let [problems (and spec (s/explain-data spec d))]
+             (when-let [problems (and spec (s/explain-data (wrap spec) d))]
                (->Problem p nil d spec problems)))
            (keep identity) (seq) (vec)))
 
-(defn validate [routes {:keys [spec] :or {spec ::default-data}}]
-  (when-let [problems (validate-route-data routes spec)]
+(defn validate [routes {:keys [spec ::wrap] :or {spec ::default-data, wrap identity}}]
+  (when-let [problems (validate-route-data routes wrap spec)]
     (exception/fail!
       ::invalid-route-data
       {:problems problems})))

modules/reitit-dev/project.clj

@@ -9,5 +9,6 @@
   :parent-project {:path "../../project.clj"
                    :inherit [:deploy-repositories :managed-dependencies]}
   :dependencies [[metosin/reitit-core]
+                 [com.bhauman/spell-spec]
                  [expound]
                  [fipp]])

modules/reitit-dev/src/reitit/dev/pretty.cljc

@@ -3,6 +3,9 @@
             [clojure.spec.alpha :as s]
             [reitit.exception :as exception]
             [arrangement.core]
+    ;; spell-spec
+            [spec-tools.spell :as spell]
+            [spell-spec.expound]
     ;; expound
             [expound.ansi]
             [expound.alpha]
@@ -178,7 +181,7 @@
     (if (and (not= 1 line))
       (let [file-name (str/replace file #"(.*?)\.\S[^\.]+" "$1")
             target-name (name target)
-            ns (str (subs target-name 0 (str/index-of target-name (str "user" "$"))) file-name)]
+            ns (str (subs target-name 0 (or (str/index-of target-name (str file-name "$")) 0)) file-name)]
         (str ns ":" line))
       "repl")
     (catch #?(:clj Exception, :cljs js/Error) _
@@ -316,12 +319,12 @@
    (into
      [:group]
      (map
-       (fn [{:keys [data path spec]}]
+       (fn [{:keys [data path spec scope]}]
          [:group
           [:span (color :grey "-- On route -----------------------")]
           [:break]
           [:break]
-          (text path)
+          (text path) (if scope [:span " " (text scope)])
           [:break]
           [:break]
           (-> (s/explain-data spec data)

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

@@ -2,8 +2,7 @@
   (:require [meta-merge.core :refer [meta-merge]]
             [reitit.interceptor :as interceptor]
             [reitit.ring :as ring]
-            [reitit.core :as r]
-            [reitit.impl :as impl]))
+            [reitit.core :as r]))
 
 (defrecord Endpoint [data interceptors queue handler path method])
 
@@ -16,6 +15,9 @@
 
 (defn compile-result [[path data] {:keys [::default-options-handler] :as opts}]
   (let [[top childs] (ring/group-keys data)
+        childs (cond-> childs
+                       (and (not (:options childs)) (not (:handler top)) default-options-handler)
+                       (assoc :options {:no-doc true, :handler default-options-handler}))
         compile (fn [[path data] opts scope]
                   (interceptor/compile-result [path data] opts scope))
         ->endpoint (fn [p d m s]
@@ -29,12 +31,7 @@
                       (fn [acc method]
                         (cond-> acc
                                 any? (assoc method (->endpoint path data method nil))))
-                      (ring/map->Methods
-                        {:options
-                         (if default-options-handler
-                           (->endpoint path (assoc data
-                                              :handler default-options-handler
-                                              :no-doc true) :options nil))})
+                      (ring/map->Methods {})
                       ring/http-methods))]
     (if-not (seq childs)
       (->methods true top)

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

@@ -11,7 +11,13 @@
   {:name ::coerce-request
    :spec ::rs/parameters
    :compile (fn [{:keys [coercion parameters]} opts]
-              (if (and coercion parameters)
+              (cond
+                ;; no coercion, skip
+                (not coercion) nil
+                ;; just coercion, don't mount
+                (not parameters) {}
+                ;; mount
+                :else
                 (let [coercers (coercion/request-coercers coercion parameters opts)]
                   {:enter (fn [ctx]
                             (let [request (:request ctx)
@@ -27,7 +33,13 @@
   {:name ::coerce-response
    :spec ::rs/responses
    :compile (fn [{:keys [coercion responses]} opts]
-              (if (and coercion responses)
+              (cond
+                ;; no coercion, skip
+                (not coercion) nil
+                ;; just coercion, don't mount
+                (not responses) {}
+                ;; mount
+                :else
                 (let [coercers (coercion/response-coercers coercion responses opts)]
                   {:leave (fn [ctx]
                             (let [request (:request ctx)

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

@@ -12,15 +12,15 @@
 (s/def ::interceptors (s/coll-of (partial satisfies? interceptor/IntoInterceptor)))
 
 (s/def ::data
-  (s/keys :opt-un [::rs/handler ::rs/name ::interceptors]))
+  (s/keys :opt-un [::rs/handler ::rs/name ::rs/no-doc ::interceptors]))
 
 ;;
 ;; Validator
 ;;
 
 (defn validate
-  [routes {:keys [spec] :or {spec ::data}}]
-  (when-let [problems (rrs/validate-route-data routes :interceptors spec)]
+  [routes {:keys [spec ::rs/wrap] :or {spec ::data, wrap identity}}]
+  (when-let [problems (rrs/validate-route-data routes :interceptors wrap spec)]
     (exception/fail!
-      ::invalid-route-data
+      ::rs/invalid-route-data
       {:problems problems})))

modules/reitit-interceptors/src/reitit/http/interceptors/multipart.clj

@@ -11,6 +11,9 @@
 (s/def ::bytes bytes?)
 (s/def ::size int?)
 
+(s/def ::multipart :reitit.core.coercion/model)
+(s/def ::parameters (s/keys :opt-un [::multipart]))
+
 (def temp-file-part
   "Spec for file param created by ring.middleware.multipart-params.temp-file store."
   (st/spec
@@ -41,6 +44,7 @@
    (multipart-interceptor nil))
   ([options]
    {:name ::multipart
+    :spec ::parameters
     :compile (fn [{:keys [parameters coercion]} opts]
                (if-let [multipart (:multipart parameters)]
                  (let [parameter-coercion {:multipart (coercion/->ParameterCoercion

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

@@ -30,6 +30,9 @@
 
 (defn compile-result [[path data] {:keys [::default-options-handler] :as opts}]
   (let [[top childs] (group-keys data)
+        childs (cond-> childs
+                       (and (not (:options childs)) (not (:handler top)) default-options-handler)
+                       (assoc :options {:no-doc true, :handler default-options-handler}))
         ->endpoint (fn [p d m s]
                      (-> (middleware/compile-result [p d] opts s)
                          (map->Endpoint)
@@ -40,12 +43,7 @@
                       (fn [acc method]
                         (cond-> acc
                                 any? (assoc method (->endpoint path data method nil))))
-                      (map->Methods
-                        {:options
-                         (if default-options-handler
-                           (->endpoint path (assoc data
-                                              :handler default-options-handler
-                                              :no-doc true) :options nil))})
+                      (map->Methods {})
                       http-methods))]
     (if-not (seq childs)
       (->methods true top)

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

@@ -25,7 +25,13 @@
   {:name ::coerce-request
    :spec ::rs/parameters
    :compile (fn [{:keys [coercion parameters]} opts]
-              (if (and coercion parameters)
+              (cond
+                ;; no coercion, skip
+                (not coercion) nil
+                ;; just coercion, don't mount
+                (not parameters) {}
+                ;; mount
+                :else
                 (let [coercers (coercion/request-coercers coercion parameters opts)]
                   (fn [handler]
                     (fn
@@ -43,7 +49,13 @@
   {:name ::coerce-response
    :spec ::rs/responses
    :compile (fn [{:keys [coercion responses]} opts]
-              (if (and coercion responses)
+              (cond
+                ;; no coercion, skip
+                (not coercion) nil
+                ;; just coercion, don't mount
+                (not responses) {}
+                ;; mount
+                :else
                 (let [coercers (coercion/response-coercers coercion responses opts)]
                   (fn [handler]
                     (fn

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

@@ -9,10 +9,19 @@
 ;;
 
 (s/def ::middleware (s/coll-of #(satisfies? middleware/IntoMiddleware %)))
+(s/def ::get map?)
+(s/def ::head map?)
+(s/def ::post map?)
+(s/def ::put map?)
+(s/def ::delete map?)
+(s/def ::connect map?)
+(s/def ::options map?)
+(s/def ::trace map?)
+(s/def ::patch map?)
+
 
 (s/def ::data
-  (s/keys :req-un [::rs/handler]
-          :opt-un [::rs/name ::middleware]))
+  (s/keys :opt-un [::rs/handler ::rs/name ::rs/no-doc ::middleware]))
 
 ;;
 ;; Validator
@@ -26,21 +35,21 @@
        :invalid non-specs}))
   (s/merge-spec-impl (vec specs) (vec specs) nil))
 
-(defn validate-route-data [routes key spec]
+(defn validate-route-data [routes key wrap spec]
   (->> (for [[p _ c] routes
              [method {:keys [data] :as endpoint}] c
              :when endpoint
              :let [target (key endpoint)
                    component-specs (seq (keep :spec target))
                    specs (keep identity (into [spec] component-specs))
-                   spec (merge-specs specs)]]
+                   spec (wrap (merge-specs specs))]]
          (when-let [problems (and spec (s/explain-data spec data))]
            (rs/->Problem p method data spec problems)))
        (keep identity) (seq)))
 
 (defn validate
-  [routes {:keys [spec] :or {spec ::data}}]
-  (when-let [problems (validate-route-data routes :middleware spec)]
+  [routes {:keys [spec ::rs/wrap] :or {spec ::data, wrap identity}}]
+  (when-let [problems (validate-route-data routes :middleware wrap spec)]
     (exception/fail!
-      ::invalid-route-data
+      ::rs/invalid-route-data
       {:problems problems})))

perf-test/clj/reitit/go_perf_test.clj

@@ -296,8 +296,7 @@
 (def app
   (ring/ring-handler
     (ring/router
-      (reduce (partial add h) [] routes)
-      {::trie/parameters trie/record-parameters})
+      (reduce (partial add h) [] routes))
     (ring/create-default-handler)
     {:inject-match? false, :inject-router? false}))
 
@@ -319,6 +318,7 @@
   ;; 140µs (java-segment-router)
   ;;  60ns (java-segment-router, no injects)
   ;;  55ns (trie-router, no injects)
+  ;;  54µs (trie-router, quick-pam)
   ;;  54ns (trie-router, no injects, optimized)
   (let [req (map->Req {:request-method :get, :uri "/user/repos"})]
     (title "static")
@@ -337,6 +337,7 @@
   ;; 273ns (trie-router, no injects, direct-data)
   ;; 256ns (trie-router, pre-defined parameters)
   ;; 237ns (trie-router, single-sweep wild-params)
+  ;; 226µs (trie-router, quick-pam)
   ;; 191ns (trie-router, record parameters)
   (let [req (map->Req {:request-method :get, :uri "/repos/julienschmidt/httprouter/stargazers"})]
     (title "param")
@@ -354,6 +355,7 @@
   ;;  63µs (trie-router, no injects, switch-case) - 124µs (clojure)
   ;;  63µs (trie-router, no injects, direct-data)
   ;;  54µs (trie-router, non-transient params)
+  ;;  50µs (trie-router, quick-pam)
   ;;  49µs (trie-router, pre-defined parameters)
   (let [requests (mapv route->req routes)]
     (title "all")

perf-test/clj/reitit/request_perf.cljc

@@ -0,0 +1,96 @@
+(ns reitit.request-perf
+  (:require [criterium.core :as cc]
+            [reitit.perf-utils :refer :all]
+            [potemkin :as p]))
+
+(set! *warn-on-reflection* true)
+
+;;
+;; start repl with `lein perf repl`
+;; perf measured with the following setup:
+;;
+;; Model Name:            MacBook Pro
+;; Model Identifier:      MacBookPro113
+;; Processor Name:        Intel Core i7
+;; Processor Speed:       2,5 GHz
+;; Number of Processors:  1
+;; Total Number of Cores: 4
+;; L2 Cache (per Core):   256 KB
+;; L3 Cache:              6 MB
+;; Memory:                16 GB
+;;
+
+(defprotocol RawRequest
+  (-uri [this])
+  (-request-method [this])
+  (-path-params [this]))
+
+(p/def-derived-map
+  ZeroCopyRequest
+  [raw]
+  :uri (-uri raw)
+  :request-method (-request-method raw)
+  :path-params (-path-params raw))
+
+(defprotocol RingRequest
+  (get-uri [this])
+  (get-request-method [this])
+  (get-path-params [this]))
+
+(defn ring-request [raw]
+  {:uri (-uri raw)
+   :request-method (-request-method raw)
+   :path-params (-path-params raw)})
+
+(defn record-request [raw]
+  (->RecordRequest (-uri raw) (-request-method raw) (-path-params raw)))
+
+(defrecord RawRingRequest [raw]
+  RingRequest
+  (get-uri [_] (-uri raw))
+  (get-request-method [_] (-request-method raw))
+  (get-path-params [_] (-path-params raw)))
+
+(def raw
+  (reify
+    RawRequest
+    (-uri [_] "/ping")
+    (-request-method [_] :get)
+    (-path-params [_] {:a 1})))
+
+(defn bench-all! []
+
+  ;; 530ns
+  (title "potemkin zero-copy")
+  (assert (= :get (:request-method (->ZeroCopyRequest raw))))
+  (cc/quick-bench
+    (let [req (->ZeroCopyRequest raw)]
+      (dotimes [_ 10]
+        (:request-method req))))
+
+  ;; 73ns
+  (title "map copy-request")
+  (assert (= :get (:request-method (ring-request raw))))
+  (cc/quick-bench
+    (let [req (ring-request raw)]
+      (dotimes [_ 10]
+        (:request-method req))))
+
+  ;; 7ns
+  (title "record copy-request")
+  (assert (= :get (:request-method (record-request raw))))
+  (cc/quick-bench
+    (let [req (record-request raw)]
+      (dotimes [_ 10]
+        (:request-method req))))
+
+  ;; 7ns
+  (title "request protocols")
+  (assert (= :get (get-request-method (->RawRingRequest raw))))
+  (cc/quick-bench
+    (let [req (->RawRingRequest raw)]
+      (dotimes [_ 10]
+        (get-request-method req)))))
+
+(comment
+  (bench-all!))

project.clj

@@ -27,7 +27,7 @@
                          [metosin/reitit-sieppari "0.3.1"]
                          [metosin/reitit-pedestal "0.3.1"]
                          [metosin/ring-swagger-ui "2.2.10"]
-                         [metosin/spec-tools "0.9.1"]
+                         [metosin/spec-tools "0.9.2-alpha1"]
                          [metosin/schema-tools "0.11.0"]
                          [metosin/muuntaja "0.6.4"]
                          [metosin/jsonista "0.2.2"]
@@ -37,6 +37,7 @@
                          [fipp "0.6.17" :exclusions [org.clojure/core.rrb-vector]]
                          [expound "0.7.2"]
                          [lambdaisland/deep-diff "0.0-47"]
+                         [com.bhauman/spell-spec "0.1.1"]
                          [ring/ring-core "1.7.1"]
 
                          [io.pedestal/pedestal.service "0.5.5"]]
@@ -69,7 +70,7 @@
 
                    :java-source-paths ["modules/reitit-core/java-src"]
 
-                   :dependencies [[org.clojure/clojure "1.10.0"]
+                   :dependencies [[org.clojure/clojure "1.10.1-beta2"]
                                   [org.clojure/clojurescript "1.10.520"]
 
                                   ;; modules dependencies
@@ -80,6 +81,7 @@
                                   [metosin/jsonista]
                                   [lambdaisland/deep-diff]
                                   [meta-merge]
+                                  [com.bhauman/spell-spec]
                                   [expound]
                                   [fipp]
 

test/cljc/reitit/ring_spec_test.cljc

@@ -21,13 +21,13 @@
   (testing "with default spec validates :name, :handler and :middleware"
     (is (thrown-with-msg?
           ExceptionInfo
-          #":reitit.ring.spec/invalid-route-data"
+          #"Invalid route data"
           (ring/router
             ["/api" {:handler "identity"}]
             {:validate rrs/validate})))
     (is (thrown-with-msg?
           ExceptionInfo
-          #":reitit.ring.spec/invalid-route-data"
+          #"Invalid route data"
           (ring/router
             ["/api" {:handler identity
                      :name "kikka"}]
@@ -36,7 +36,7 @@
   (testing "all endpoints are validated"
     (is (thrown-with-msg?
           ExceptionInfo
-          #":reitit.ring.spec/invalid-route-data"
+          #"Invalid route data"
           (ring/router
             ["/api" {:patch {:handler "identity"}}]
             {:validate rrs/validate}))))
@@ -69,7 +69,7 @@
                                             (handler request)))}]}})))
     (is (thrown-with-msg?
           ExceptionInfo
-          #":reitit.ring.spec/invalid-route-data"
+          #"Invalid route data"
           (ring/router
             ["/api" {:get {:handler identity
                            :roles #{:adminz}}}]
@@ -113,7 +113,7 @@
 
   (is (thrown-with-msg?
         ExceptionInfo
-        #":reitit.ring.spec/invalid-route-data"
+        #"Invalid route data"
         (ring/router
           ["/api"
            ["/plus/:e"