From 4e22fd2f53389dc424fa400f5b20dc4141bb549c Mon Sep 17 00:00:00 2001 From: Tommi Reiman Date: Wed, 30 Aug 2017 08:14:06 +0300 Subject: [PATCH 1/5] Allow middleware to be compiled (fixes #26) Match :handler => :result --- README.md | 16 ++-- src/reitit/core.cljc | 26 +++--- src/reitit/impl.cljc | 8 +- src/reitit/middleware.cljc | 45 +++++++-- src/reitit/ring.cljc | 3 +- test/cljc/reitit/core_test.cljc | 12 +-- test/cljc/reitit/middleware_test.cljc | 129 ++++++++++++++++++++++++++ test/cljc/reitit/ring_test.cljc | 46 +-------- 8 files changed, 197 insertions(+), 88 deletions(-) create mode 100644 test/cljc/reitit/middleware_test.cljc diff --git a/README.md b/README.md index f73c3178..c24c9bfb 100644 --- a/README.md +++ b/README.md @@ -114,7 +114,7 @@ Route names: ; #Match{:template "/api/user/:id" ; :meta {:name :user/user} ; :path "/api/user/1" -; :handler nil +; :result nil ; :params {:id "1"}} ``` @@ -124,7 +124,7 @@ Route names: (reitit/match-by-name router ::user) ; #PartialMatch{:template "/api/user/:id", ; :meta {:name :user/user}, -; :handler nil, +; :result nil, ; :params nil, ; :required #{:id}} @@ -139,7 +139,7 @@ Only a partial match. Let's provide the path-parameters: ; #Match{:template "/api/user/:id" ; :meta {:name :user/user} ; :path "/api/user/1" -; :handler nil +; :result nil ; :params {:id "1"}} ``` @@ -201,13 +201,13 @@ Path-based routing: ; :interceptors [::api ::admin] ; :roles #{:root}} ; :path "/api/admin/root" -; :handler nil +; :result nil ; :params {}} ``` On match, route meta-data is returned and can interpreted by the application. -Routers also support meta-data compilation enabling things like fast [Ring](https://github.com/ring-clojure/ring) or [Pedestal](http://pedestal.io/) -style handlers. Compilation results are found under `:handler` in the match. See [configuring routers](#configuring-routers) for details. +Routers also support meta-data compilation enabling things like fast [Ring](https://github.com/ring-clojure/ring) or [Pedestal](http://pedestal.io/) -style handlers. Compilation results are found under `:result` in the match. See [configuring routers](#configuring-routers) for details. ## Route conflicts @@ -225,10 +225,10 @@ Route trees should not have multiple routes that match to a single (request) pat ; /:user-id/orders ; -> /public/*path ; -> /bulk/:bulk-id -; +; ; /bulk/:bulk-id ; -> /:version/status -; +; ; /public/*path ; -> /:version/status ; @@ -424,7 +424,7 @@ Routers can be configured via options. Options allow things like [`clojure.spec` | `:meta` | Initial expanded route-meta vector (default `[]`) | `:expand` | Function of `arg opts => meta` to expand route arg to route meta-data (default `reitit.core/expand`) | `:coerce` | Function of `route opts => route` to coerce resolved route, can throw or return `nil` - | `:compile` | Function of `route opts => handler` to compile a route handler + | `:compile` | Function of `route opts => result` to compile a route handler | `:conflicts` | Function of `{route #{route}} => side-effect` to handle conflicting routes (default `reitit.core/throw-on-conflicts!`) | `:router` | Function of `routes opts => router` to override the actual router implementation diff --git a/src/reitit/core.cljc b/src/reitit/core.cljc index 003c4040..ae1282fb 100644 --- a/src/reitit/core.cljc +++ b/src/reitit/core.cljc @@ -46,7 +46,7 @@ (if (seq childs) (walk-many (str pacc path) macc childs) [[(str pacc path) macc]]))))] - (walk-one path meta data))) + (walk-one path (mapv identity meta) data))) (defn map-meta [f routes] (mapv #(update % 1 f) routes)) @@ -100,8 +100,8 @@ (match-by-path [this path]) (match-by-name [this name] [this name params])) -(defrecord Match [template meta handler params path]) -(defrecord PartialMatch [template meta handler params required]) +(defrecord Match [template meta result params path]) +(defrecord PartialMatch [template meta result params required]) (defn partial-match? [x] (instance? PartialMatch x)) @@ -132,11 +132,11 @@ (let [compiled (map #(compile-route % opts) routes) names (find-names routes opts) [data lookup] (reduce - (fn [[data lookup] [p {:keys [name] :as meta} handler]] - (let [{:keys [params] :as route} (impl/create [p meta handler]) + (fn [[data lookup] [p {:keys [name] :as meta} result]] + (let [{:keys [params] :as route} (impl/create [p meta result]) f #(if-let [path (impl/path-for route %)] - (->Match p meta handler % path) - (->PartialMatch p meta handler % params))] + (->Match p meta result % path) + (->PartialMatch p meta result % params))] [(conj data route) (if name (assoc lookup name f) lookup)])) [[] {}] compiled) @@ -155,7 +155,7 @@ (reduce (fn [acc ^Route route] (if-let [params ((:matcher route) path)] - (reduced (->Match (:path route) (:meta route) (:handler route) params path)))) + (reduced (->Match (:path route) (:meta route) (:result route) params path)))) nil data)) (match-by-name [_ name] (if-let [match (impl/fast-get lookup name)] @@ -179,10 +179,10 @@ (let [compiled (map #(compile-route % opts) routes) names (find-names routes opts) [data lookup] (reduce - (fn [[data lookup] [p {:keys [name] :as meta} handler]] - [(assoc data p (->Match p meta handler {} p)) + (fn [[data lookup] [p {:keys [name] :as meta} result]] + [(assoc data p (->Match p meta result {} p)) (if name - (assoc lookup name #(->Match p meta handler % p)) + (assoc lookup name #(->Match p meta result % p)) lookup)]) [{} {}] compiled) data (impl/fast-map data) lookup (impl/fast-map lookup)] @@ -244,10 +244,10 @@ | -------------|-------------| | `:path` | Base-path for routes (default `\"\"`) | `:routes` | Initial resolved routes (default `[]`) - | `:meta` | Initial expanded route-meta vector (default `[]`) + | `:meta` | Initial route meta (default `{}`) | `:expand` | Function of `arg opts => meta` to expand route arg to route meta-data (default `reitit.core/expand`) | `:coerce` | Function of `route opts => route` to coerce resolved route, can throw or return `nil` - | `:compile` | Function of `route opts => handler` to compile a route handler + | `:compile` | Function of `route opts => result` to compile a route handler | `:conflicts` | Function of `{route #{route}} => side-effect` to handle conflicting routes (default `reitit.core/throw-on-conflicts!`) | `:router` | Function of `routes opts => router` to override the actual router implementation" ([data] diff --git a/src/reitit/impl.cljc b/src/reitit/impl.cljc index 217a48fb..be1e610c 100644 --- a/src/reitit/impl.cljc +++ b/src/reitit/impl.cljc @@ -101,15 +101,15 @@ ;; Routing (c) Metosin ;; -(defrecord Route [path matcher parts params meta handler]) +(defrecord Route [path matcher parts params meta result]) -(defn create [[path meta handler]] +(defn create [[path meta result]] (if (contains-wilds? path) (as-> (parse-path path) $ (assoc $ :path-re (path-regex $)) (merge $ {:path path :matcher (path-matcher $) - :handler handler + :result result :meta meta}) (dissoc $ :path-re :path-constraints) (update $ :path-params set) @@ -119,7 +119,7 @@ (map->Route {:path path :meta meta :matcher #(if (= path %) {}) - :handler handler}))) + :result result}))) (defn segments [path] (let [ss (-> (str/split path #"/") rest vec)] diff --git a/src/reitit/middleware.cljc b/src/reitit/middleware.cljc index 0dd97da7..853fc50d 100644 --- a/src/reitit/middleware.cljc +++ b/src/reitit/middleware.cljc @@ -1,24 +1,45 @@ (ns reitit.middleware (:require [meta-merge.core :refer [meta-merge]] - [reitit.core :as reitit])) + [reitit.core :as reitit]) + #?(:clj + (:import (clojure.lang IFn AFn)))) (defprotocol ExpandMiddleware - (expand-middleware [this opts])) + (expand-middleware [this meta opts])) + +(defrecord MiddlewareGenerator [f args] + IFn + (invoke [_] + (f nil nil)) + (invoke [_ meta] + (f meta nil)) + (invoke [_ meta opts] + (f meta opts)) + #?(:clj + (applyTo [this args] + (AFn/applyToHelper this args)))) (extend-protocol ExpandMiddleware #?(:clj clojure.lang.APersistentVector :cljs cljs.core.PersistentVector) - (expand-middleware [[f & args] _] - (fn [handler] - (apply f handler args))) + (expand-middleware [[f & args] meta opts] + (if-let [mw (expand-middleware f meta opts)] + (fn [handler] + (apply mw handler args)))) #?(:clj clojure.lang.Fn :cljs function) - (expand-middleware [this _] this) + (expand-middleware [this _ _] this) + + MiddlewareGenerator + (expand-middleware [this meta opts] + (if-let [mw (this meta opts)] + (fn [handler & args] + (apply mw handler args)))) nil - (expand-middleware [_ _])) + (expand-middleware [_ _ _])) (defn- ensure-handler! [path meta scope] (when-not (:handler meta) @@ -28,18 +49,22 @@ (merge {:path path, :meta meta} (if scope {:scope scope})))))) -(defn compose-middleware [middleware opts] +(defn compose-middleware [middleware meta opts] (->> middleware (keep identity) - (map #(expand-middleware % opts)) + (map #(expand-middleware % meta opts)) + (keep identity) (apply comp identity))) +(defn gen [f & args] + (->MiddlewareGenerator f args)) + (defn compile-handler ([route opts] (compile-handler route opts nil)) ([[path {:keys [middleware handler] :as meta}] opts scope] (ensure-handler! path meta scope) - ((compose-middleware middleware opts) handler))) + ((compose-middleware middleware meta opts) handler))) (defn router ([data] diff --git a/src/reitit/ring.cljc b/src/reitit/ring.cljc index 2badd934..5ff83a98 100644 --- a/src/reitit/ring.cljc +++ b/src/reitit/ring.cljc @@ -60,6 +60,5 @@ ([data] (router data nil)) ([data opts] - (let [opts (meta-merge {:coerce coerce-handler - :compile compile-handler} opts)] + (let [opts (meta-merge {:coerce coerce-handler, :compile compile-handler} opts)] (reitit/router data opts)))) diff --git a/test/cljc/reitit/core_test.cljc b/test/cljc/reitit/core_test.cljc index d292fc23..5dfe38f2 100644 --- a/test/cljc/reitit/core_test.cljc +++ b/test/cljc/reitit/core_test.cljc @@ -96,15 +96,15 @@ (reitit/routes router)))) (testing "route match contains compiled handler" (is (= 2 @compile-times)) - (let [{:keys [handler]} (reitit/match-by-path router "/api/pong")] - (is handler) - (is (= "/api/pong" (handler))) + (let [{:keys [result]} (reitit/match-by-path router "/api/pong")] + (is result) + (is (= "/api/pong" (result))) (is (= 2 @compile-times)))))) (testing "default compile" (let [router (reitit/router ["/ping" (constantly "ok")])] - (let [{:keys [handler]} (reitit/match-by-path router "/ping")] - (is handler) - (is (= "ok" (handler))))))) + (let [{:keys [result]} (reitit/match-by-path router "/ping")] + (is result) + (is (= "ok" (result))))))) (testing "custom router" (let [router (reitit/router ["/ping"] {:router (fn [_ _] diff --git a/test/cljc/reitit/middleware_test.cljc b/test/cljc/reitit/middleware_test.cljc new file mode 100644 index 00000000..3c57b2aa --- /dev/null +++ b/test/cljc/reitit/middleware_test.cljc @@ -0,0 +1,129 @@ +(ns reitit.middleware-test + (:require [clojure.test :refer [deftest testing is]] + [reitit.middleware :as middleware] + [clojure.set :as set] + [reitit.core :as reitit]) + #?(:clj + (:import (clojure.lang ExceptionInfo)))) + +(defn mw [handler name] + (fn + ([request] + (-> request + (update ::mw (fnil conj []) name) + (handler) + (update :body (fnil conj []) name))) + ([request respond raise] + (handler + (update request ::mw (fnil conj []) name) + #(respond (update % :body (fnil conj []) name)) + raise)))) + +(defn handler + ([{:keys [::mw]}] + {:status 200 :body (conj mw :ok)}) + ([request respond raise] + (respond (handler request)))) + +(deftest expand-middleware-test + (testing "middleware generators" + (let [calls (atom 0)] + + (testing "record generator" + (reset! calls 0) + (let [syntax [(middleware/gen + (fn [meta _] + (swap! calls inc) + (fn [handler value] + (swap! calls inc) + (fn [request] + [meta value request]))))] + app ((middleware/compose-middleware syntax :meta {}) identity :value)] + (dotimes [_ 10] + (is (= [:meta :value :request] (app :request))) + (is (= 2 @calls))))) + + (testing "middleware generator as function" + (reset! calls 0) + (let [syntax (middleware/gen + (fn [meta _] + (swap! calls inc) + (fn [handler value] + (swap! calls inc) + (fn [request] + [meta value request])))) + app ((syntax :meta nil) identity :value)] + (dotimes [_ 10] + (is (= [:meta :value :request] (app :request))) + (is (= 2 @calls))))) + + (testing "generator vector" + (reset! calls 0) + (let [syntax [[(middleware/gen + (fn [meta _] + (swap! calls inc) + (fn [handler value] + (swap! calls inc) + (fn [request] + [meta value request])))) :value]] + app ((middleware/compose-middleware syntax :meta {}) identity)] + (is (= [:meta :value :request] (app :request))) + (dotimes [_ 10] + (is (= [:meta :value :request] (app :request))) + (is (= 2 @calls))))) + + (testing "generator can return nil" + (reset! calls 0) + (let [syntax [[(middleware/gen + (fn [meta _])) :value]] + app ((middleware/compose-middleware syntax :meta {}) identity)] + (is (= :request (app :request))) + (dotimes [_ 10] + (is (= :request (app :request))))))))) + + +(deftest middleware-router-test + + (testing "all paths should have a handler" + (is (thrown-with-msg? + ExceptionInfo + #"path \"/ping\" doesn't have a :handler defined" + (middleware/router ["/ping"])))) + + (testing "ring-handler" + (let [api-mw #(mw % :api) + router (middleware/router + [["/ping" handler] + ["/api" {:middleware [api-mw]} + ["/ping" handler] + ["/admin" {:middleware [[mw :admin]]} + ["/ping" handler]]]]) + app (fn + ([{:keys [uri] :as request}] + (if-let [handler (:result (reitit/match-by-path router uri))] + (handler request))) + ([{:keys [uri] :as request} respond raise] + (if-let [handler (:result (reitit/match-by-path router uri))] + (handler request respond raise))))] + + (testing "not found" + (is (= nil (app {:uri "/favicon.ico"})))) + + (testing "normal handler" + (is (= {:status 200, :body [:ok]} + (app {:uri "/ping"})))) + + (testing "with middleware" + (is (= {:status 200, :body [:api :ok :api]} + (app {:uri "/api/ping"})))) + + (testing "with nested middleware" + (is (= {:status 200, :body [:api :admin :ok :admin :api]} + (app {:uri "/api/admin/ping"})))) + + (testing "3-arity" + (let [result (atom nil) + respond (partial reset! result), raise ::not-called] + (app {:uri "/api/admin/ping"} respond raise) + (is (= {:status 200, :body [:api :admin :ok :admin :api]} + @result))))))) diff --git a/test/cljc/reitit/ring_test.cljc b/test/cljc/reitit/ring_test.cljc index 641c40a7..8029d55f 100644 --- a/test/cljc/reitit/ring_test.cljc +++ b/test/cljc/reitit/ring_test.cljc @@ -26,50 +26,6 @@ ([request respond raise] (respond (handler request)))) -(deftest middleware-router-test - - (testing "all paths should have a handler" - (is (thrown-with-msg? - ExceptionInfo - #"path \"/ping\" doesn't have a :handler defined" - (middleware/router ["/ping"])))) - - (testing "ring-handler" - (let [api-mw #(mw % :api) - router (middleware/router - [["/ping" handler] - ["/api" {:middleware [api-mw]} - ["/ping" handler] - ["/admin" {:middleware [[mw :admin]]} - ["/ping" handler]]]]) - app (ring/ring-handler router)] - - (testing "router can be extracted" - (is (= router (ring/get-router app)))) - - (testing "not found" - (is (= nil (app {:uri "/favicon.ico"})))) - - (testing "normal handler" - (is (= {:status 200, :body [:ok]} - (app {:uri "/ping"})))) - - (testing "with middleware" - (is (= {:status 200, :body [:api :ok :api]} - (app {:uri "/api/ping"})))) - - (testing "with nested middleware" - (is (= {:status 200, :body [:api :admin :ok :admin :api]} - (app {:uri "/api/admin/ping"})))) - - (testing "3-arity" - (let [result (atom nil) - respond (partial reset! result), raise ::not-called] - (app {:uri "/api/admin/ping"} respond raise) - (is (= {:status 200, :body [:api :admin :ok :admin :api]} - @result))))))) - - (deftest ring-router-test (testing "all paths should have a handler" @@ -142,7 +98,7 @@ (testing "only top-level route names are matched" (is (= [::all ::get ::users] - (reitit/route-names router)))) + (reitit/route-names router)))) (testing "all named routes can be matched" (doseq [name (reitit/route-names router)] From 86acee9098e6771a63cbae46f602aee769f4d67b Mon Sep 17 00:00:00 2001 From: Tommi Reiman Date: Wed, 30 Aug 2017 08:16:21 +0300 Subject: [PATCH 2/5] Polish perf tests --- .../clj/reitit/opensensors_routing_test.clj | 64 +++++++++++-------- perf-test/clj/reitit/perf_test.clj | 17 ++--- perf-test/clj/reitit/perf_utils.clj | 16 +++++ 3 files changed, 57 insertions(+), 40 deletions(-) create mode 100644 perf-test/clj/reitit/perf_utils.clj diff --git a/perf-test/clj/reitit/opensensors_routing_test.clj b/perf-test/clj/reitit/opensensors_routing_test.clj index 5487e043..a5ee7a89 100644 --- a/perf-test/clj/reitit/opensensors_routing_test.clj +++ b/perf-test/clj/reitit/opensensors_routing_test.clj @@ -1,5 +1,7 @@ (ns reitit.opensensors-routing-test (:require [clojure.test :refer [deftest testing is]] + [criterium.core :as cc] + [reitit.perf-utils :refer :all] [cheshire.core :as json] [clojure.string :as str] [reitit.core :as reitit] @@ -16,13 +18,20 @@ [io.pedestal.http.route.router :as pedestal] [io.pedestal.http.route :as route])) -(defn raw-title [color s] - (println (str color (apply str (repeat (count s) "#")) "\u001B[0m")) - (println (str color s "\u001B[0m")) - (println (str color (apply str (repeat (count s) "#")) "\u001B[0m"))) - -(def title (partial raw-title "\u001B[35m")) -(def suite (partial raw-title "\u001B[32m")) +;; +;; start repl with `lein perf repl` +;; perf measured with the following setup: +;; +;; Model Name: MacBook Pro +;; Model Identifier: MacBookPro11,3 +;; 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 +;; ;; ;; extract sample routes @@ -77,15 +86,6 @@ avg (int (/ (reduce + times) (count times)))] [% avg]) urls))) -(defn bench [routes no-paths?] - (let [routes (mapv (fn [[path name]] - (if no-paths? - [(str/replace path #"\:" "") name] - [path name])) routes) - router (reitit/router routes)] - (doseq [[path time] (bench-routes routes #(reitit/match-by-path router %))] - (println path "\t" time)))) - (defn bench [routes no-paths?] (let [routes (mapv (fn [[path name]] (if no-paths? @@ -471,7 +471,7 @@ (if-not match (println route))))) -(defn bench! [routes verbose? name f] +(defn bench!! [routes verbose? name f] (System/gc) (println) (suite name) @@ -493,14 +493,23 @@ compojure-api-f #(opensensors-compojure-api-routes {:uri % :request-method :get}) pedestal-f #(pedestal/find-route opensensors-pedestal-routes {:path-info % :request-method :get})] - (bench! routes true "reitit" reitit-f) ;; 2538ns -> 2028ns - (bench! routes true "reitit-ring" reitit-ring-f) ;; 2845ns -> 2299ns - (bench! routes true "pedestal" pedestal-f) ;; 2737ns - (bench! routes true "compojure-api" compojure-api-f) ;; 9823ns - (bench! routes true "bidi" bidi-f) ;; 16716ns - (bench! routes true "ataraxy" ataraxy-f) ;; 24467ns + ;; 2538ns -> 2028ns + (bench!! routes true "reitit" reitit-f) - )) + ;; 2845ns -> 2299ns + (bench!! routes true "reitit-ring" reitit-ring-f) + + ;; 2737ns + (bench!! routes true "pedestal" pedestal-f) + + ;; 9823ns + (bench!! routes true "compojure-api" compojure-api-f) + + ;; 16716ns + (bench!! routes true "bidi" bidi-f) + + ;; 24467ns + (bench!! routes true "ataraxy" ataraxy-f))) (comment (bench-rest!)) @@ -548,16 +557,15 @@ ;; 125ns ;; 62ns (fast-map) - (bench! routes false "reitit" reitit-f) + (bench!! routes false "reitit" reitit-f) ;; 272ns ;; 219ns (fast-assoc) ;; 171ns (fast-map) - (bench! routes false "reitit-ring" reitit-ring-f) + (bench!! routes false "reitit-ring" reitit-ring-f) ;; 172ns - (bench! routes false "pedestal" pedestal-f))) + (bench!! routes false "pedestal" pedestal-f))) (comment (bench-cqrs!)) - diff --git a/perf-test/clj/reitit/perf_test.clj b/perf-test/clj/reitit/perf_test.clj index abf917ab..279d9fe2 100644 --- a/perf-test/clj/reitit/perf_test.clj +++ b/perf-test/clj/reitit/perf_test.clj @@ -1,6 +1,7 @@ (ns reitit.perf-test (:require [criterium.core :as cc] [reitit.core :as reitit] + [reitit.perf-utils :refer :all] [bidi.bidi :as bidi] [compojure.api.sweet :refer [api routes GET]] @@ -27,14 +28,6 @@ ;; Memory: 16 GB ;; -(defn raw-title [color s] - (println (str color (apply str (repeat (count s) "#")) "\u001B[0m")) - (println (str color s "\u001B[0m")) - (println (str color (apply str (repeat (count s) "#")) "\u001B[0m"))) - -(def title (partial raw-title "\u001B[35m")) -(def suite (partial raw-title "\u001B[32m")) - (def bidi-routes ["/" [["auth/login" :auth/login] [["auth/recovery/token/" :token] :auth/recovery] @@ -106,7 +99,7 @@ (call))) ;; 1.0µs (-94%) - ;; 770ns (-95%, -23%) + ;; 690ns (-96%) (title "reitit") (let [call #(reitit/match-by-path reitit-routes "/workspace/1/1")] (assert (call)) @@ -117,7 +110,7 @@ (suite "reverse routing") - ;; 2.2µs (-56%) + ;; 2.0µs (-59%) (title "bidi") (let [call #(bidi/path-for bidi-routes :workspace/page :project "1" :page "1")] (assert (= "/workspace/1/1" (call))) @@ -126,14 +119,14 @@ (title "ataraxy doesn't support reverse routing :(") - ;; 3.8µs (-25%) + ;; 3.8µs (-22%) (title "pedestal - map-tree => prefix-tree") (let [call #(pedestal-url-for :workspace/page :path-params {:project "1" :page "1"})] (assert (= "/workspace/1/1" (call))) (cc/quick-bench (call))) - ;; 5.1µs + ;; 4.9µs (title "compojure-api") (let [call #(routes/path-for* :workspace/page compojure-api-request {:project "1", :page "1"})] (assert (= "/workspace/1/1" (call))) diff --git a/perf-test/clj/reitit/perf_utils.clj b/perf-test/clj/reitit/perf_utils.clj new file mode 100644 index 00000000..14e3c7ce --- /dev/null +++ b/perf-test/clj/reitit/perf_utils.clj @@ -0,0 +1,16 @@ +(ns reitit.perf-utils + (:require [criterium.core :as cc])) + +(defn raw-title [color s] + (println (str color (apply str (repeat (count s) "#")) "\u001B[0m")) + (println (str color s "\u001B[0m")) + (println (str color (apply str (repeat (count s) "#")) "\u001B[0m"))) + +(def title (partial raw-title "\u001B[35m")) +(def suite (partial raw-title "\u001B[32m")) + +(defmacro bench! [name & body] + `(do + (title ~name) + (println ~@body) + (cc/quick-bench ~@body))) From dcd559bf2739a8afad5c083579abd3bf9bb9dc17 Mon Sep 17 00:00:00 2001 From: Tommi Reiman Date: Wed, 30 Aug 2017 08:19:21 +0300 Subject: [PATCH 3/5] Initial Spec request/response coercion (fixes #20) --- perf-test/clj/reitit/coercion_perf_test.clj | 174 ++++++++++++++++++ project.clj | 4 + src/reitit/coercion.cljc | 192 ++++++++++++++++++++ src/reitit/coercion/protocol.cljc | 16 ++ src/reitit/coercion/spec.cljc | 113 ++++++++++++ src/reitit/ring.cljc | 55 ++++-- test/cljc/reitit/coercion_test.cljc | 63 +++++++ 7 files changed, 600 insertions(+), 17 deletions(-) create mode 100644 perf-test/clj/reitit/coercion_perf_test.clj create mode 100644 src/reitit/coercion.cljc create mode 100644 src/reitit/coercion/protocol.cljc create mode 100644 src/reitit/coercion/spec.cljc create mode 100644 test/cljc/reitit/coercion_test.cljc diff --git a/perf-test/clj/reitit/coercion_perf_test.clj b/perf-test/clj/reitit/coercion_perf_test.clj new file mode 100644 index 00000000..bd443498 --- /dev/null +++ b/perf-test/clj/reitit/coercion_perf_test.clj @@ -0,0 +1,174 @@ +(ns reitit.coercion-perf-test + (:require [clojure.test :refer [deftest testing is]] + [criterium.core :as cc] + [reitit.perf-utils :refer :all] + [clojure.spec.alpha :as s] + [spec-tools.core :as st] + + [reitit.core :as reitit] + [reitit.ring :as ring] + [reitit.coercion :as coercion] + [reitit.coercion.spec :as spec] + [reitit.coercion.protocol :as protocol] + [spec-tools.data-spec :as ds])) + +;; +;; start repl with `lein perf repl` +;; perf measured with the following setup: +;; +;; Model Name: MacBook Pro +;; Model Identifier: MacBookPro11,3 +;; 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 +;; + +(comment + (do + (s/def ::x (s/and (s/conformer #(if (string? %) (Long/parseLong %) %) identity) int?)) + (s/def ::y (s/and (s/conformer #(if (string? %) (Long/parseLong %) %) identity) int?)) + (s/def ::k (s/keys :req-un [::x ::y])) + + (let [spec (spec/specify {:x int?, :y int?} ::jeah) + coercers (#'coercion/request-coercers spec/coercion {:body spec}) + params {:x "1", :y "2"} + request {:body-params {:x "1", :y "2"}}] + + ;; 4600ns + (bench! + "coerce-parameters" + (#'coercion/coerce-parameters coercers request)) + + ;; 2700ns + (bench! + "st/conform" + (st/conform + spec + params + spec/string-conforming)) + + ;; 4100ns + (bench! + "st/conform + s/unform" + (s/unform + spec + (st/conform + spec + params + spec/string-conforming))) + + ;; 610ns + (bench! + "s/conform" + (s/conform + ::k + params)) + + ;; 2700ns + (bench! + "s/conform + s/unform" + (s/unform + ::k + (s/conform + ::k + params)))))) + +(defrecord NoOpCoercion [] + protocol/Coercion + (get-name [_] :no-op) + (compile [_ model] model) + (get-apidocs [_ _ {:keys [parameters responses] :as info}]) + (make-open [_ spec] spec) + (encode-error [_ error] error) + (request-coercer [_ type spec] (fn [value format] value)) + (response-coercer [this spec] (protocol/request-coercer this :response spec))) + +(comment + (doseq [coercion [nil (->NoOpCoercion) spec/coercion]] + (suite (str (if coercion (protocol/get-name coercion)))) + (let [routes ["/api" + ["/ping" {:parameters {:body {:x int?, :y int?}} + :responses {200 {:schema {:total pos-int?}}} + :get {:handler (fn [request] + (let [{:keys [x y]} (-> request :parameters :body)] + {:status 200 + :body {:total (+ (or x 0) (or y 0))}}))}}]] + app (ring/ring-handler + (ring/router + routes + {:meta {:middleware [coercion/wrap-coerce-parameters] + :coercion coercion}})) + app2 (ring/ring-handler + (ring/router + routes + {:meta {:middleware [coercion/gen-wrap-coerce-parameters] + :coercion coercion}})) + app3 (ring/ring-handler + (ring/router + routes + {:meta {:middleware [coercion/wrap-coerce-parameters + coercion/wrap-coerce-response] + :coercion coercion}})) + app4 (ring/ring-handler + (ring/router + routes + {:meta {:middleware [coercion/gen-wrap-coerce-parameters + coercion/gen-wrap-coerce-response] + :coercion coercion}})) + req {:request-method :get + :uri "/api/ping" + :body-params {:x 1, :y 2}}] + + ;; 210ns + ;; 1300ns + ;; 7400ns + (bench! "wrap-coerce-parameters" (app req)) + + ;; 170ns (-19%) + ;; 500ns (-62%) + ;; 5400ns (-26%) + (bench! "gen-wrap-coerce-parameters" (app2 req)) + + ;; 340ns + ;; 2400ns + ;; 14800ns + (bench! "wrap-coerce-parameters & responses" (app3 req)) + + ;; 180ns (-47%) + ;; 580ns (-76%) + ;; 8300ns (-44%) + (bench! "gen-wrap-coerce-parameters & responses" (app4 req))))) + +(comment + (do + (require '[reitit.ring :as ring]) + (require '[reitit.coercion :as coercion]) + (require '[reitit.coercion.spec :as spec]) + + (def app + (ring/ring-handler + (ring/router + ["/api" + ["/ping" {:parameters {:body {:x int?, :y int?}} + :responses {200 {:schema {:total pos-int?}}} + :get {:handler (fn [{{{:keys [x y]} :body} :parameters}] + {:status 200 + :body {:total (+ x y)}})}}]] + {:meta {:middleware [coercion/gen-wrap-coerce-parameters + coercion/gen-wrap-coerce-response] + :coercion spec/coercion}}))) + + (app + {:request-method :get + :uri "/api/ping" + :body-params {:x 1, :y 2}}) + ; {:status 200, :body {:total 3}} + + (let [req {:request-method :get + :uri "/api/ping" + :body-params {:x 1, :y 2}}] + (cc/quick-bench (app req))))) diff --git a/project.clj b/project.clj index 97dbaf25..cf5305c2 100644 --- a/project.clj +++ b/project.clj @@ -22,6 +22,10 @@ :jvm-opts ^:replace ["-server"] :dependencies [[org.clojure/clojure "1.9.0-alpha17"] [org.clojure/clojurescript "1.9.660"] + + [metosin/spec-tools "0.3.2"] + [org.clojure/spec.alpha "0.1.123"] + [criterium "0.4.4"] [org.clojure/test.check "0.9.0"] [org.clojure/tools.namespace "0.2.11"] diff --git a/src/reitit/coercion.cljc b/src/reitit/coercion.cljc new file mode 100644 index 00000000..3ddfd129 --- /dev/null +++ b/src/reitit/coercion.cljc @@ -0,0 +1,192 @@ +(ns reitit.coercion + (:require [clojure.walk :as walk] + [spec-tools.core :as st] + [reitit.coercion.protocol :as protocol] + [reitit.middleware :as middleware] + [reitit.ring :as ring] + [reitit.impl :as impl])) + +(defn get-apidocs [coercion spec info] + (protocol/get-apidocs coercion spec info)) + +;; +;; coercer +;; + +(defrecord ParameterCoercion [in style keywordize? open?]) + +(def ring-parameter-coercion + {:query (->ParameterCoercion :query-params :string true true) + :body (->ParameterCoercion :body-params :string false true) + :form (->ParameterCoercion :form-params :string true true) + :header (->ParameterCoercion :header-params :string true true) + :path (->ParameterCoercion :path-params :string true true)}) + +(defn request-coercion-failed! [result coercion value in request] + (throw + (ex-info + (str "Request coercion failed: " (pr-str result)) + (merge + (into {} result) + {:type ::request-coercion + :coercion coercion + :value value + :in [:request in] + :request request})))) + +(defn response-coercion-failed! [result coercion value request response] + (throw + (ex-info + (str "Response coercion failed: " (pr-str result)) + (merge + (into {} result) + {:type ::response-coercion + :coercion coercion + :value value + :in [:response :body] + :request request + :response response})))) + +(defn request-coercer [coercion type model] + (if coercion + (let [{:keys [keywordize? open? in style]} (ring-parameter-coercion type) + transform (comp (if keywordize? walk/keywordize-keys identity) in) + model (if open? (protocol/make-open coercion model) model) + coercer (protocol/request-coercer coercion style model)] + (fn [request] + (let [value (transform request) + format (some-> request :muuntaja/request :format) + result (coercer value format)] + (if (protocol/error? result) + (request-coercion-failed! result coercion value in request) + result)))))) + +(defn- response-format [request response] + (or (-> response :muuntaja/content-type) + (some-> request :muuntaja/response :format))) + +(defn response-coercer [coercion model] + (if coercion + (let [coercer (protocol/response-coercer coercion model)] + (fn [request response] + (let [format (response-format request response) + value (:body response) + result (coercer value format)] + (if (protocol/error? result) + (response-coercion-failed! result coercion value request response) + result)))))) + +;; +;; middleware +;; + +(defn- coerce-parameters [coercers request] + (reduce-kv + (fn [acc k coercer] + (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))))) + +(defn ^:no-doc request-coercers [coercion parameters] + (->> (for [[k v] parameters + :when v] + [k (request-coercer coercion k v)]) + (into {}))) + +(defn ^:no-doc response-coercers [coercion responses] + (->> (for [[status {:keys [schema]}] responses :when schema] + [status (response-coercer coercion schema)]) + (into {}))) + +(defn wrap-coerce-parameters + "Pluggable request coercion middleware. + Expects a :coercion of type `reitit.coercion.protocol/Coercion` + from injected route meta, otherwise does not mount." + [handler] + (fn + ([request] + (let [method (:request-method request) + match (ring/get-match request) + parameters (-> match :result method :meta :parameters) + coercion (-> match :meta :coercion)] + (if coercion + (let [coercers (request-coercers coercion parameters) + coerced (coerce-parameters coercers request)] + (handler (impl/fast-assoc request :parameters coerced))) + (handler request)))) + ([request respond raise] + (let [method (:request-method request) + match (ring/get-match request) + parameters (-> match :result method :meta :parameters) + coercion (-> match :meta :coercion)] + (if coercion + (let [coercers (request-coercers coercion parameters) + coerced (coerce-parameters coercers request)] + (handler (impl/fast-assoc request :parameters coerced) respond raise))))))) + +(def gen-wrap-coerce-parameters + "Generator for pluggable request coercion middleware. + Expects a :coercion of type `reitit.coercion.protocol/Coercion` + from injected route meta, otherwise does not mount." + (middleware/gen + (fn [{:keys [parameters coercion]} _] + (if coercion + (let [coercers (request-coercers coercion parameters)] + (fn [handler] + (fn + ([request] + (let [coerced (coerce-parameters coercers request)] + (handler (impl/fast-assoc request :parameters coerced)))) + ([request respond raise] + (let [coerced (coerce-parameters coercers request)] + (handler (impl/fast-assoc request :parameters coerced) respond raise)))))))))) + +(defn wrap-coerce-response + "Pluggable response coercion middleware. + Expects a :coercion of type `reitit.coercion.protocol/Coercion` + from injected route meta, otherwise does not mount." + [handler] + (fn + ([request] + (let [response (handler request) + method (:request-method request) + match (ring/get-match request) + responses (-> match :result method :meta :responses) + coercion (-> match :meta :coercion)] + (if coercion + (let [coercers (response-coercers coercion responses) + coerced (coerce-response coercers request response)] + (coerce-response coercers request (handler request))) + (handler request)))) + ([request respond raise] + (let [response (handler request) + method (:request-method request) + match (ring/get-match request) + responses (-> match :result method :meta :responses) + coercion (-> match :meta :coercion)] + (if coercion + (let [coercers (response-coercers coercion responses) + coerced (coerce-response coercers request response)] + (handler request #(respond (coerce-response coercers request %)))) + (handler request respond raise)))))) + +(def gen-wrap-coerce-response + "Generator for pluggable response coercion middleware. + Expects a :coercion of type `reitit.coercion.protocol/Coercion` + from injected route meta, otherwise does not mount." + (middleware/gen + (fn [{:keys [responses coercion]} _] + (if coercion + (let [coercers (response-coercers coercion responses)] + (fn [handler] + (fn + ([request] + (coerce-response coercers request (handler request))) + ([request respond raise] + (handler request #(respond (coerce-response coercers request %)) raise))))))))) + diff --git a/src/reitit/coercion/protocol.cljc b/src/reitit/coercion/protocol.cljc new file mode 100644 index 00000000..9e58df3b --- /dev/null +++ b/src/reitit/coercion/protocol.cljc @@ -0,0 +1,16 @@ +(ns reitit.coercion.protocol + (:refer-clojure :exclude [compile])) + +(defprotocol Coercion + (get-name [this]) + (compile [this model]) + (get-apidocs [this model data]) + (make-open [this model]) + (encode-error [this error]) + (request-coercer [this type model]) + (response-coercer [this model])) + +(defrecord CoercionError []) + +(defn error? [x] + (instance? CoercionError x)) diff --git a/src/reitit/coercion/spec.cljc b/src/reitit/coercion/spec.cljc new file mode 100644 index 00000000..461ac1a4 --- /dev/null +++ b/src/reitit/coercion/spec.cljc @@ -0,0 +1,113 @@ +(ns reitit.coercion.spec + (:require [clojure.spec.alpha :as s] + [spec-tools.core :as st #?@(:cljs [:refer [Spec]])] + [spec-tools.data-spec :as ds] + [spec-tools.conform :as conform] + [spec-tools.swagger.core :as swagger] + [reitit.coercion.protocol :as protocol]) + #?(:clj + (:import (spec_tools.core Spec)))) + +(def string-conforming + (st/type-conforming + (merge + conform/string-type-conforming + conform/strip-extra-keys-type-conforming))) + +(def json-conforming + (st/type-conforming + (merge + conform/json-type-conforming + conform/strip-extra-keys-type-conforming))) + +(def default-conforming + ::default) + +(defprotocol Specify + (specify [this name])) + +(extend-protocol Specify + + #?(:clj clojure.lang.PersistentArrayMap + :cljs cljs.core.PersistentArrayMap) + (specify [this name] + (ds/spec name this)) + + #?(:clj clojure.lang.PersistentHashMap + :cljs cljs.core.PersistentHashMap) + (specify [this name] + (ds/spec name this)) + + Spec + (specify [this _] this) + + Object + (specify [this _] + (st/create-spec {:spec this}))) + +;; TODO: proper name! +(def memoized-specify + (memoize #(specify %1 (gensym "spec")))) + +(defmulti coerce-response? identity :default ::default) +(defmethod coerce-response? ::default [_] true) + +(defrecord SpecCoercion [name conforming coerce-response?] + + protocol/Coercion + (get-name [_] name) + + (compile [_ model] + (memoized-specify model)) + + (get-apidocs [_ _ {:keys [parameters responses] :as info}] + (cond-> (dissoc info :parameters :responses) + parameters (assoc + ::swagger/parameters + (into + (empty parameters) + (for [[k v] parameters] + [k memoized-specify]))) + responses (assoc + ::swagger/responses + (into + (empty responses) + (for [[k response] responses] + [k (update response :schema memoized-specify)]))))) + + (make-open [_ spec] spec) + + (encode-error [_ error] + (update error :spec (comp str s/form))) + + (request-coercer [_ type spec] + (let [spec (memoized-specify spec) + {:keys [formats default]} (conforming type)] + (fn [value format] + (if-let [conforming (or (get formats format) default)] + (let [conformed (st/conform spec value conforming)] + (if (s/invalid? conformed) + (let [problems (st/explain-data spec value conforming)] + (protocol/map->CoercionError + {:spec spec + :problems (::s/problems problems)})) + (s/unform spec conformed))) + value)))) + + (response-coercer [this spec] + (if (coerce-response? spec) + (protocol/request-coercer this :response spec)))) + +(def default-options + {:coerce-response? coerce-response? + :conforming {:body {:default default-conforming + :formats {"application/json" json-conforming + "application/msgpack" json-conforming + "application/x-yaml" json-conforming}} + :string {:default string-conforming} + :response {:default default-conforming}}}) + +(defn create [{:keys [conforming coerce-response?]}] + (->SpecCoercion :spec conforming coerce-response?)) + +(def coercion (create default-options)) diff --git a/src/reitit/ring.cljc b/src/reitit/ring.cljc index 5ff83a98..9a6b2019 100644 --- a/src/reitit/ring.cljc +++ b/src/reitit/ring.cljc @@ -5,7 +5,8 @@ [reitit.impl :as impl])) (def http-methods #{:get :head :patch :delete :options :post :put}) -(defrecord MethodHandlers [get head patch delete options post put]) +(defrecord Methods [get head post put delete trace options connect patch any]) +(defrecord Endpoint [meta handler]) (defn- group-keys [meta] (reduce-kv @@ -19,10 +20,27 @@ (fn ([request] (if-let [match (reitit/match-by-path router (:uri request))] - ((:handler match) (impl/fast-assoc request ::match match)))) + (let [method (:request-method request :any) + params (:params match) + result (:result match) + handler (or (-> result method :handler) + (-> result :any :handler))] + (if handler + (handler + (cond-> (impl/fast-assoc request ::match match) + params (impl/fast-assoc :path-params params))))))) ([request respond raise] (if-let [match (reitit/match-by-path router (:uri request))] - ((:handler match) (impl/fast-assoc request ::match match) respond raise)))) + (let [method (:request-method request :any) + params (:params match) + result (:result match) + handler (or (-> result method :handler) + (-> result :any :handler))] + (if handler + (handler + (cond-> (impl/fast-assoc request ::match match) + params (impl/fast-assoc :path-params params)) + respond raise)))))) {::router router})) (defn get-router [handler] @@ -41,20 +59,23 @@ (defn compile-handler [[path meta] opts] (let [[top childs] (group-keys meta)] (if-not (seq childs) - (middleware/compile-handler [path meta] opts) - (let [handlers (map->MethodHandlers - (reduce-kv - #(assoc %1 %2 (middleware/compile-handler - [path (meta-merge top %3)] opts %2)) - {} childs)) - default-handler (if (:handler top) (middleware/compile-handler [path meta] opts))] - (fn - ([request] - (if-let [handler (or ((:request-method request) handlers) default-handler)] - (handler request))) - ([request respond raise] - (if-let [handler (or ((:request-method request) handlers) default-handler)] - (handler request respond raise)))))))) + (map->Methods + {:any (map->Endpoint + {:handler (middleware/compile-handler [path top] opts) + :meta top})}) + (let [any-handler (if (:handler top) (middleware/compile-handler [path meta] opts))] + (reduce-kv + (fn [acc method meta] + (let [meta (meta-merge top meta) + handler (middleware/compile-handler [path meta] opts method)] + (assoc acc method (map->Endpoint + {:handler handler + :meta meta})))) + (map->Methods + {:any (map->Endpoint + {:handler (if (:handler top) (middleware/compile-handler [path meta] opts)) + :meta top})}) + childs))))) (defn router ([data] diff --git a/test/cljc/reitit/coercion_test.cljc b/test/cljc/reitit/coercion_test.cljc new file mode 100644 index 00000000..f194da87 --- /dev/null +++ b/test/cljc/reitit/coercion_test.cljc @@ -0,0 +1,63 @@ +(ns reitit.middleware-test + (:require [clojure.test :refer [deftest testing is]] + [reitit.ring :as ring] + [reitit.coercion :as coercion] + [reitit.coercion.spec :as spec]) + #?(:clj + (:import (clojure.lang ExceptionInfo)))) + +(defn handler + ([{:keys [::mw]}] + {:status 200 :body (conj mw :ok)}) + ([request respond raise] + (respond (handler request)))) + +(deftest coercion-test + (let [app (ring/ring-handler + (ring/router + ["/api" + ["/plus/:e" + {:get {:parameters {:query {:a int?} + :body {:b int?} + :form {:c int?} + :header {:d int?} + :path {:e int?}} + :responses {200 {:schema {:total pos-int?}}} + :handler (fn [{{{:keys [a]} :query + {:keys [b]} :body + {:keys [c]} :form + {:keys [d]} :header + {:keys [e]} :path} :parameters}] + {:status 200 + :body {:total (+ a b c d e)}})}}]] + {:meta {:middleware [coercion/gen-wrap-coerce-parameters + coercion/gen-wrap-coerce-response] + :coercion spec/coercion}}))] + + (testing "all good" + (is (= {:status 200 + :body {:total 15}} + (app {:uri "/api/plus/5" + :request-method :get + :query-params {"a" "1"} + :body-params {:b 2} + :form-params {:c 3} + :header-params {:d 4}})))) + + (testing "invalid request" + (is (thrown-with-msg? + ExceptionInfo + #"Request coercion failed" + (app {:uri "/api/plus/5" + :request-method :get})))) + + (testing "invalid response" + (is (thrown-with-msg? + ExceptionInfo + #"Response coercion failed" + (app {:uri "/api/plus/5" + :request-method :get + :query-params {"a" "1"} + :body-params {:b 2} + :form-params {:c 3} + :header-params {:d -40}})))))) From c7c4013f970d6cc475b831df59b9a8182d04658c Mon Sep 17 00:00:00 2001 From: Tommi Reiman Date: Wed, 30 Aug 2017 13:24:01 +0300 Subject: [PATCH 4/5] README & small fixes --- README.md | 232 ++++++++++++++++---- perf-test/clj/reitit/coercion_perf_test.clj | 2 +- src/reitit/coercion.cljc | 39 ++-- src/reitit/coercion/protocol.cljc | 15 +- src/reitit/coercion/spec.cljc | 2 +- test/cljc/reitit/coercion_test.cljc | 2 +- 6 files changed, 227 insertions(+), 65 deletions(-) diff --git a/README.md b/README.md index c24c9bfb..6de6bb91 100644 --- a/README.md +++ b/README.md @@ -6,6 +6,8 @@ A friendly data-driven router for Clojure(Script). * First-class route meta-data * Generic, not tied to HTTP * [Route conflict resolution](#route-conflicts) +* [Pluggable coercion](#parameter-coercion) ([clojure.spec](https://clojure.org/about/spec)) +* Middleware & Interceptors * Extendable * Fast @@ -68,7 +70,7 @@ Same routes flattened: For routing, a `Router` is needed. Reitit ships with several different router implementations: `:linear-router`, `:lookup-router` and `:mixed-router`, based on the awesome [Pedestal](https://github.com/pedestal/pedestal/tree/master/route) implementation. -`Router` is created with `reitit.core/router`, which takes routes and optional options map as arguments. The route tree gets expanded, optionally coerced and compiled. The actual `Router` implementation is selected based on the route tree or can be selected with the `:router` option. `Router` support both fast path- and name-based lookups. +`Router` is created with `reitit.core/router`, which takes routes and optional options map as arguments. The route tree gets expanded, optionally coerced and compiled. Actual `Router` implementation is selected automatically but can be defined with a `:router` option. `Router` support both path- and name-based lookups. Creating a router: @@ -82,7 +84,7 @@ Creating a router: ["/user/:id" ::user]]])) ``` -`:mixed-router` is created (both static & wild routes are used): +`:mixed-router` is created (both static & wild routes are found): ```clj (reitit/router-type router) @@ -157,52 +159,47 @@ Routes can have arbitrary meta-data. For nested routes, the meta-data is accumul A router based on nested route tree: ```clj -(def router (reitit/router ["/api" {:interceptors [::api]} ["/ping" ::ping] - ["/public/*path" ::resources] - ["/user/:id" {:name ::get-user - :parameters {:id String}} - ["/orders" ::user-orders]] - ["/admin" {:interceptors [::admin] - :roles #{:admin}} - ["/root" {:name ::root - :roles ^:replace #{:root}}] - ["/db" {:name ::db - :interceptors [::db]}]]])) + ["/admin" {:roles #{:admin}} + ["/users" ::users] + ["/db" {:interceptors [::db], :roles ^:replace #{:db-admin}} + ["/:db" {:parameters {:db String}} + ["/drop" ::drop-db] + ["/stats" ::db-stats]]]]])) ``` Resolved route tree: ```clj (reitit/routes router) -; [["/api/ping" {:name :user/ping -; :interceptors [::api]}] -; ["/api/public/*path" {:name :user/resources -; :interceptors [::api]}] -; ["/api/user/:id/orders" {:name :user/user-orders -; :interceptors [::api] -; :parameters {:id String}}] -; ["/api/admin/root" {:name :user/root -; :interceptors [::api ::admin] -; :roles #{:root}}] -; ["/api/admin/db" {:name :user/db -; :interceptors [::api ::admin ::db] -; :roles #{:admin}}]] +; [["/api/ping" {:interceptors [::api] +; :name ::ping}] +; ["/api/admin/users" {:interceptors [::api] +; :roles #{:admin} +; :name ::users}] +; ["/api/admin/db/:db/drop" {:interceptors [::api ::db] +; :roles #{:db-admin} +; :parameters {:db String} +; :name ::drop-db}] +; ["/api/admin/db/:db/stats" {:interceptors [::api ::db] +; :roles #{:db-admin} +; :parameters {:db String} +; :name ::db-stats}]] ``` Path-based routing: ```clj -(reitit/match-by-path router "/api/admin/root") -; #Match{:template "/api/admin/root" -; :meta {:name :user/root -; :interceptors [::api ::admin] -; :roles #{:root}} -; :path "/api/admin/root" +(reitit/match-by-path router "/api/admin/users") +; #Match{:template "/api/admin/users" +; :meta {:interceptors [::api] +; :roles #{:admin} +; :name ::users} ; :result nil -; :params {}} +; :params {} +; :path "/api/admin/users"} ``` On match, route meta-data is returned and can interpreted by the application. @@ -333,8 +330,6 @@ Middleware is applied correctly: ; {:status 200, :body [:api :handler]} ``` -Nested middleware works too: - ```clj (app {:request-method :delete, :uri "/api/admin/db"}) ; {:status 200, :body [:api :admin :db :delete :handler]} @@ -342,7 +337,7 @@ Nested middleware works too: ### Async Ring -Ring-router supports also 3-arity [Async Ring](https://www.booleanknot.com/blog/2016/07/15/asynchronous-ring.html), so it can be used on [Node.js](https://nodejs.org/en/) too. +All built-in middleware provide both the 2 and 3-arity, so they work with [Async Ring](https://www.booleanknot.com/blog/2016/07/15/asynchronous-ring.html) too. ### Meta-data based extensions @@ -397,6 +392,169 @@ Authorized access to guarded route: ; {:status 200, :body "ok"} ``` +## Parameter coercion + +Reitit ships with pluggable parameter coercion via `reitit.coercion.protocol/Coercion` protocol. `reitit.coercion.spec/SpecCoercion` provides implements it for [clojure.spec](https://clojure.org/about/spec) & [data-specs](https://github.com/metosin/spec-tools#data-specs). + +**NOTE**: to use the spec-coercion, one needs to add the following dependencies manually to the project: + +```clj +[org.clojure/clojure "1.9.0-alpha17"] +[org.clojure/spec.alpha "0.1.123"] +[metosin/spec-tools "0.3.2"] +``` + +### Ring request and response coercion + +To use `Coercion` with Ring, one needs to do the following: + +1. Define parameters and responses as data into route meta-data, in format adopted from [ring-swagger](https://github.com/metosin/ring-swagger#more-complete-example): + * `:parameters` map, with submaps for different parameters: `:query`, `:body`, `:form`, `:header` and `:path`. Parameters are defined in the format understood by the `Coercion`. + * `:responses` map, with response status codes as keys (or `:default` for "everything else") with maps with `:schema` and optionally `:description` as values. +2. Define a `Coercion` to route meta-data under `:coercion` +3. Mount request & response coercion middleware to the routes. + +If the request coercion succeeds, the coerced parameters are injected into request under `:parameters`. + +If either request or response coercion fails, an descriptive error is thrown. + +#### Example with data-specs + +```clj +(require '[reitit.ring :as ring]) +(require '[reitit.coercion :as coercion]) +(require '[reitit.coercion.spec :as spec]) + +(def app + (ring/ring-handler + (ring/router + ["/api" + ["/ping" {:parameters {:body {:x int?, :y int?}} + :responses {200 {:schema {:total pos-int?}}} + :get {:handler (fn [{{{:keys [x y]} :body} :parameters}] + {:status 200 + :body {:total (+ x y)}})}}]] + {:meta {:middleware [coercion/gen-wrap-coerce-parameters + coercion/gen-wrap-coerce-response] + :coercion spec/coercion}}))) +``` + + +```clj +(app + {:request-method :get + :uri "/api/ping" + :body-params {:x 1, :y 2}}) +; {:status 200, :body {:total 3}} +``` + +#### Example with specs + +```clj +(require '[reitit.ring :as ring]) +(require '[reitit.coercion :as coercion]) +(require '[reitit.coercion.spec :as spec]) +(require '[clojure.spec.alpha :as s]) +(require '[spec-tools.core :as st]) + +(s/def ::x (st/spec int?)) +(s/def ::y (st/spec int?)) +(s/def ::total int?) +(s/def ::request (s/keys :req-un [::x ::y])) +(s/def ::response (s/keys :req-un [::total])) + +(def app + (ring/ring-handler + (ring/router + ["/api" + ["/ping" {:parameters {:body ::request} + :responses {200 {:schema ::response}} + :get {:handler (fn [{{{:keys [x y]} :body} :parameters}] + {:status 200 + :body {:total (+ x y)}})}}]] + {:meta {:middleware [coercion/gen-wrap-coerce-parameters + coercion/gen-wrap-coerce-response] + :coercion spec/coercion}}))) +``` + +```clj +(app + {:request-method :get + :uri "/api/ping" + :body-params {:x 1, :y 2}}) +; {:status 200, :body {:total 3}} +``` + +## Compiling Middleware + +The [meta-data extensions](#meta-data-based-extensions) are a easy way to extend the system. Routes meta-data can be trasnformed into any shape (records, functions etc.) in route compilation, enabling easy access at request-time. + +Still, we can do better. As we know the exact route interceptor/middleware is linked to, we can pass the (compiled) route information into the interceptor/middleware at creation-time. It can extract and transform relevant data just for it and pass it into the actual request-handler via a closure. We can do all the static local computations forehand, yielding much lighter runtime processing. + +For middleware, there is a helper `reitit.middleware/gen` for this. It takes a function of `route-meta router-opts => middleware` and returns a special record extending the internal middleware protocols so it can be mounted as normal middleware. The compiled middleware can also decide no to mount itsef byt returning `nil`. Why mount `wrap-enforce-roles` if there are no roles required for that route? + +To demonstrate the two approaches, below are response coercion middleware written in both ways (found in `reitit.coercion`): + +### Naive + +* Extracts the compiled route information on every request. + +```clj +(defn wrap-coerce-response + "Pluggable response coercion middleware. + Expects a :coercion of type `reitit.coercion.protocol/Coercion` + and :responeses from route meta, otherwise does not mount." + [handler] + (fn + ([request] + (let [response (handler request) + method (:request-method request) + match (ring/get-match request) + responses (-> match :result method :meta :responses) + coercion (-> match :meta :coercion) + opts (-> match :meta :opts)] + (if (and coercion responses) + (let [coercers (response-coercers coercion responses opts) + coerced (coerce-response coercers request response)] + (coerce-response coercers request (handler request))) + (handler request)))) + ([request respond raise] + (let [response (handler request) + method (:request-method request) + match (ring/get-match request) + responses (-> match :result method :meta :responses) + coercion (-> match :meta :coercion) + opts (-> match :meta :opts)] + (if (and coercion responses) + (let [coercers (response-coercers coercion responses opts) + coerced (coerce-response coercers request response)] + (handler request #(respond (coerce-response coercers request %)))) + (handler request respond raise)))))) +``` + +### Compiled + +* Route information is provided via a closure +* Pre-compiled coercers +* Mounts only if `:coercion` and `:responses` are defined for the route + +```clj +(def gen-wrap-coerce-response + "Generator for pluggable response coercion middleware. + Expects a :coercion of type `reitit.coercion.protocol/Coercion` + and :responses from route meta, otherwise does not mount." + (middleware/gen + (fn [{:keys [responses coercion opts]} _] + (if (and coercion responses) + (let [coercers (response-coercers coercion responses opts)] + (fn [handler] + (fn + ([request] + (coerce-response coercers request (handler request))) + ([request respond raise] + (handler request #(respond (coerce-response coercers request %)) raise))))))))) +``` + ## Merging route-trees *TODO* @@ -405,7 +563,7 @@ Authorized access to guarded route: *TODO* -## Schema, Spec, Swagger & Openapi +## Swagger & Openapi *TODO* diff --git a/perf-test/clj/reitit/coercion_perf_test.clj b/perf-test/clj/reitit/coercion_perf_test.clj index bd443498..61adcb1b 100644 --- a/perf-test/clj/reitit/coercion_perf_test.clj +++ b/perf-test/clj/reitit/coercion_perf_test.clj @@ -80,7 +80,7 @@ (defrecord NoOpCoercion [] protocol/Coercion (get-name [_] :no-op) - (compile [_ model] model) + (compile [_ model _] model) (get-apidocs [_ _ {:keys [parameters responses] :as info}]) (make-open [_ spec] spec) (encode-error [_ error] error) diff --git a/src/reitit/coercion.cljc b/src/reitit/coercion.cljc index 3ddfd129..a9004365 100644 --- a/src/reitit/coercion.cljc +++ b/src/reitit/coercion.cljc @@ -6,7 +6,7 @@ [reitit.ring :as ring] [reitit.impl :as impl])) -(defn get-apidocs [coercion spec info] +#_(defn get-apidocs [coercion spec info] (protocol/get-apidocs coercion spec info)) ;; @@ -61,15 +61,16 @@ (request-coercion-failed! result coercion value in request) result)))))) -(defn- response-format [request response] +#_(defn muuntaja-response-format [request response] (or (-> response :muuntaja/content-type) (some-> request :muuntaja/response :format))) -(defn response-coercer [coercion model] +(defn response-coercer [coercion model {:keys [extract-response-format] + :or {extract-response-format (constantly nil)}}] (if coercion (let [coercer (protocol/response-coercer coercion model)] (fn [request response] - (let [format (response-format request response) + (let [format (extract-response-format request response) value (:body response) result (coercer value format)] (if (protocol/error? result) @@ -98,15 +99,15 @@ [k (request-coercer coercion k v)]) (into {}))) -(defn ^:no-doc response-coercers [coercion responses] +(defn ^:no-doc response-coercers [coercion responses opts] (->> (for [[status {:keys [schema]}] responses :when schema] - [status (response-coercer coercion schema)]) + [status (response-coercer coercion schema opts)]) (into {}))) (defn wrap-coerce-parameters "Pluggable request coercion middleware. Expects a :coercion of type `reitit.coercion.protocol/Coercion` - from injected route meta, otherwise does not mount." + and :parameters from route meta, otherwise does not mount." [handler] (fn ([request] @@ -132,10 +133,10 @@ (def gen-wrap-coerce-parameters "Generator for pluggable request coercion middleware. Expects a :coercion of type `reitit.coercion.protocol/Coercion` - from injected route meta, otherwise does not mount." + and :parameters from route meta, otherwise does not mount." (middleware/gen (fn [{:keys [parameters coercion]} _] - (if coercion + (if (and coercion parameters) (let [coercers (request-coercers coercion parameters)] (fn [handler] (fn @@ -149,7 +150,7 @@ (defn wrap-coerce-response "Pluggable response coercion middleware. Expects a :coercion of type `reitit.coercion.protocol/Coercion` - from injected route meta, otherwise does not mount." + and :responses from route meta, otherwise does not mount." [handler] (fn ([request] @@ -157,9 +158,10 @@ method (:request-method request) match (ring/get-match request) responses (-> match :result method :meta :responses) - coercion (-> match :meta :coercion)] + coercion (-> match :meta :coercion) + opts (-> match :meta :opts)] (if coercion - (let [coercers (response-coercers coercion responses) + (let [coercers (response-coercers coercion responses opts) coerced (coerce-response coercers request response)] (coerce-response coercers request (handler request))) (handler request)))) @@ -168,9 +170,10 @@ method (:request-method request) match (ring/get-match request) responses (-> match :result method :meta :responses) - coercion (-> match :meta :coercion)] + coercion (-> match :meta :coercion) + opts (-> match :meta :opts)] (if coercion - (let [coercers (response-coercers coercion responses) + (let [coercers (response-coercers coercion responses opts) coerced (coerce-response coercers request response)] (handler request #(respond (coerce-response coercers request %)))) (handler request respond raise)))))) @@ -178,11 +181,11 @@ (def gen-wrap-coerce-response "Generator for pluggable response coercion middleware. Expects a :coercion of type `reitit.coercion.protocol/Coercion` - from injected route meta, otherwise does not mount." + and :responses from route meta, otherwise does not mount." (middleware/gen - (fn [{:keys [responses coercion]} _] - (if coercion - (let [coercers (response-coercers coercion responses)] + (fn [{:keys [responses coercion opts]} _] + (if (and coercion responses) + (let [coercers (response-coercers coercion responses opts)] (fn [handler] (fn ([request] diff --git a/src/reitit/coercion/protocol.cljc b/src/reitit/coercion/protocol.cljc index 9e58df3b..aaacb41a 100644 --- a/src/reitit/coercion/protocol.cljc +++ b/src/reitit/coercion/protocol.cljc @@ -2,13 +2,14 @@ (:refer-clojure :exclude [compile])) (defprotocol Coercion - (get-name [this]) - (compile [this model]) - (get-apidocs [this model data]) - (make-open [this model]) - (encode-error [this error]) - (request-coercer [this type model]) - (response-coercer [this model])) + "Pluggable coercion protocol" + (get-name [this] "Keyword name for the coercion") + (compile [this model name] "Compiles a coercion model") + (get-apidocs [this model data] "???") + (make-open [this model] "Returns a new map model which doesn't fail on extra keys") + (encode-error [this error] "Converts error in to a serializable format") + (request-coercer [this type model] "Returns a `value format => value` request coercion function") + (response-coercer [this model] "Returns a `value format => value` response coercion function")) (defrecord CoercionError []) diff --git a/src/reitit/coercion/spec.cljc b/src/reitit/coercion/spec.cljc index 461ac1a4..4d8603e3 100644 --- a/src/reitit/coercion/spec.cljc +++ b/src/reitit/coercion/spec.cljc @@ -57,7 +57,7 @@ protocol/Coercion (get-name [_] name) - (compile [_ model] + (compile [_ model _] (memoized-specify model)) (get-apidocs [_ _ {:keys [parameters responses] :as info}] diff --git a/test/cljc/reitit/coercion_test.cljc b/test/cljc/reitit/coercion_test.cljc index f194da87..20057b8c 100644 --- a/test/cljc/reitit/coercion_test.cljc +++ b/test/cljc/reitit/coercion_test.cljc @@ -1,4 +1,4 @@ -(ns reitit.middleware-test +(ns reitit.coercion-test (:require [clojure.test :refer [deftest testing is]] [reitit.ring :as ring] [reitit.coercion :as coercion] From 76f7f2859127a42df67bfdb4ac9a6a58b5ee2e2b Mon Sep 17 00:00:00 2001 From: Tommi Reiman Date: Mon, 4 Sep 2017 08:24:42 +0300 Subject: [PATCH 5/5] welcome, first class data-driven Middleware. --- README.md | 88 ++++++++++++++---- project.clj | 4 +- src/reitit/coercion.cljc | 52 ++++++----- src/reitit/middleware.cljc | 49 ++++++---- test/cljc/reitit/middleware_test.cljc | 129 ++++++++++++++++---------- 5 files changed, 210 insertions(+), 112 deletions(-) diff --git a/README.md b/README.md index 6de6bb91..12090b7e 100644 --- a/README.md +++ b/README.md @@ -7,7 +7,7 @@ A friendly data-driven router for Clojure(Script). * Generic, not tied to HTTP * [Route conflict resolution](#route-conflicts) * [Pluggable coercion](#parameter-coercion) ([clojure.spec](https://clojure.org/about/spec)) -* Middleware & Interceptors +* both Middleware & Interceptors * Extendable * Fast @@ -233,7 +233,7 @@ Route trees should not have multiple routes that match to a single (request) pat ## Ring -[Ring](https://github.com/ring-clojure/ring)-router adds support for [handlers](https://github.com/ring-clojure/ring/wiki/Concepts#handlers), [middleware](https://github.com/ring-clojure/ring/wiki/Concepts#middleware) and routing based on `:request-method`. +[Ring](https://github.com/ring-clojure/ring)-router adds support for ring [handlers](https://github.com/ring-clojure/ring/wiki/Concepts#handlers), [middleware](https://github.com/ring-clojure/ring/wiki/Concepts#middleware) and routing based on `:request-method`. Ring-router is created with `reitit.ring/router` function. It validates that all paths have a `:handler` defined and expands `:middleware` to create accumulated handlers for all request-methods. `reitit.ring/ring-handler` creates an actual ring handler out of a ring-router. Simple Ring app: @@ -295,6 +295,11 @@ Reverse routing: ### Middleware +`:middleware` should be a vector of either of the following (expanded via the `reitit.middleware/ExpandMiddleware`: + +1. a ring middleware function of `handler -> request -> response` +2. a vector of middleware function (`handler args -> request -> response`) and it's args - actial middleware is created by applying function with handler and args + Let's define some middleware and a handler: ```clj @@ -335,6 +340,50 @@ Middleware is applied correctly: ; {:status 200, :body [:api :admin :db :delete :handler]} ``` +### Middleware Records + +Besides just being opaque functions, middleware can be presented as first-class data entries, `reitit.middleware/Middleware` records. They are created with `reitit.middleware/create` function and must have a `:name` and either `:wrap` or `:gen` key with the actual middleware function or a [middleware generator function](#compiling-middleware). + +When routes are compiled, middleware records are unwrapped into normal middleware functions producing no runtime performance penalty. Thanks to the `ExpandMiddleware` protocol, plain clojure(script) maps can also be used - they get expanded into middleware records. + +The previous middleware re-written as records: + +```clj +(require '[reitit.middleware :as middleware]) + +(def wrap2 + (middleware/create + {:name ::wrap + :description "a nice little mw, takes 1 arg." + :wrap wrap})) + +(def wrap2-api + {:name ::wrap-api + :description "a nice little mw, :api as arg" + :wrap (fn [handler] + (wrap handler :api))}) +``` + +Or as maps: + +```clj +(require '[reitit.middleware :as middleware]) + +(def wrap3 + {:name ::wrap + :description "a nice little mw, takes 1 arg." + :wrap wrap}) + +(def wrap3-api + {:name ::wrap-api + :description "a nice little mw, :api as arg" + :wrap (fn [handler] + (wrap handler :api))}) +``` + + + + ### Async Ring All built-in middleware provide both the 2 and 3-arity, so they work with [Async Ring](https://www.booleanknot.com/blog/2016/07/15/asynchronous-ring.html) too. @@ -399,9 +448,9 @@ Reitit ships with pluggable parameter coercion via `reitit.coercion.protocol/Coe **NOTE**: to use the spec-coercion, one needs to add the following dependencies manually to the project: ```clj -[org.clojure/clojure "1.9.0-alpha17"] +[org.clojure/clojure "1.9.0-alpha19"] [org.clojure/spec.alpha "0.1.123"] -[metosin/spec-tools "0.3.2"] +[metosin/spec-tools "0.3.3"] ``` ### Ring request and response coercion @@ -489,11 +538,11 @@ If either request or response coercion fails, an descriptive error is thrown. The [meta-data extensions](#meta-data-based-extensions) are a easy way to extend the system. Routes meta-data can be trasnformed into any shape (records, functions etc.) in route compilation, enabling easy access at request-time. -Still, we can do better. As we know the exact route interceptor/middleware is linked to, we can pass the (compiled) route information into the interceptor/middleware at creation-time. It can extract and transform relevant data just for it and pass it into the actual request-handler via a closure. We can do all the static local computations forehand, yielding much lighter runtime processing. +Still, we can do better. As we know the exact route interceptor/middleware is linked to, we can pass the (compiled) route information into the interceptor/middleware at creation-time. It can extract and transform relevant data just for it and pass it into the actual request-handler via a closure. We can do all the static local computations forehand, yielding faster runtime processing. -For middleware, there is a helper `reitit.middleware/gen` for this. It takes a function of `route-meta router-opts => middleware` and returns a special record extending the internal middleware protocols so it can be mounted as normal middleware. The compiled middleware can also decide no to mount itsef byt returning `nil`. Why mount `wrap-enforce-roles` if there are no roles required for that route? +To do this we use [middleware records](#middleware-records) `:gen` hook instead of the normal `:wrap`. `:gen` expects a function of `route-meta router-opts => wrap`. Instead of returning the actual middleware function, the middleware record can also decide no to mount itsef byt returning `nil`. Why mount `wrap-enforce-roles` for a route if there are no roles required for it? -To demonstrate the two approaches, below are response coercion middleware written in both ways (found in `reitit.coercion`): +To demonstrate the two approaches, below are response coercion middleware written as normal ring middleware function and as middleware record with `:gen`. The actual codes are from `reitit.coercion`: ### Naive @@ -543,18 +592,21 @@ To demonstrate the two approaches, below are response coercion middleware writte "Generator for pluggable response coercion middleware. Expects a :coercion of type `reitit.coercion.protocol/Coercion` and :responses from route meta, otherwise does not mount." - (middleware/gen - (fn [{:keys [responses coercion opts]} _] - (if (and coercion responses) - (let [coercers (response-coercers coercion responses opts)] - (fn [handler] - (fn - ([request] - (coerce-response coercers request (handler request))) - ([request respond raise] - (handler request #(respond (coerce-response coercers request %)) raise))))))))) + (middleware/create + {:name ::coerce-response + :gen (fn [{:keys [responses coercion opts]} _] + (if (and coercion responses) + (let [coercers (response-coercers coercion responses opts)] + (fn [handler] + (fn + ([request] + (coerce-response coercers request (handler request))) + ([request respond raise] + (handler request #(respond (coerce-response coercers request %)) raise)))))))})) ``` +The `:gen` -version is both much easier to understand but also 2-4x faster on basic perf tests. + ## Merging route-trees *TODO* @@ -577,7 +629,7 @@ Routers can be configured via options. Options allow things like [`clojure.spec` | key | description | | -------------|-------------| - | `:path` | Base-path for routes (default `""`) + | `:path` | Base-path for routes | `:routes` | Initial resolved routes (default `[]`) | `:meta` | Initial expanded route-meta vector (default `[]`) | `:expand` | Function of `arg opts => meta` to expand route arg to route meta-data (default `reitit.core/expand`) diff --git a/project.clj b/project.clj index cf5305c2..6c4926db 100644 --- a/project.clj +++ b/project.clj @@ -20,10 +20,10 @@ [lein-cloverage "1.0.9"] [lein-codox "0.10.3"]] :jvm-opts ^:replace ["-server"] - :dependencies [[org.clojure/clojure "1.9.0-alpha17"] + :dependencies [[org.clojure/clojure "1.9.0-alpha19"] [org.clojure/clojurescript "1.9.660"] - [metosin/spec-tools "0.3.2"] + [metosin/spec-tools "0.3.3"] [org.clojure/spec.alpha "0.1.123"] [criterium "0.4.4"] diff --git a/src/reitit/coercion.cljc b/src/reitit/coercion.cljc index a9004365..c2c0c901 100644 --- a/src/reitit/coercion.cljc +++ b/src/reitit/coercion.cljc @@ -7,7 +7,7 @@ [reitit.impl :as impl])) #_(defn get-apidocs [coercion spec info] - (protocol/get-apidocs coercion spec info)) + (protocol/get-apidocs coercion spec info)) ;; ;; coercer @@ -62,8 +62,8 @@ result)))))) #_(defn muuntaja-response-format [request response] - (or (-> response :muuntaja/content-type) - (some-> request :muuntaja/response :format))) + (or (-> response :muuntaja/content-type) + (some-> request :muuntaja/response :format))) (defn response-coercer [coercion model {:keys [extract-response-format] :or {extract-response-format (constantly nil)}}] @@ -134,18 +134,19 @@ "Generator for pluggable request coercion middleware. Expects a :coercion of type `reitit.coercion.protocol/Coercion` and :parameters from route meta, otherwise does not mount." - (middleware/gen - (fn [{:keys [parameters coercion]} _] - (if (and coercion parameters) - (let [coercers (request-coercers coercion parameters)] - (fn [handler] - (fn - ([request] - (let [coerced (coerce-parameters coercers request)] - (handler (impl/fast-assoc request :parameters coerced)))) - ([request respond raise] - (let [coerced (coerce-parameters coercers request)] - (handler (impl/fast-assoc request :parameters coerced) respond raise)))))))))) + (middleware/create + {:name ::coerce-parameters + :gen (fn [{:keys [parameters coercion]} _] + (if (and coercion parameters) + (let [coercers (request-coercers coercion parameters)] + (fn [handler] + (fn + ([request] + (let [coerced (coerce-parameters coercers request)] + (handler (impl/fast-assoc request :parameters coerced)))) + ([request respond raise] + (let [coerced (coerce-parameters coercers request)] + (handler (impl/fast-assoc request :parameters coerced) respond raise))))))))})) (defn wrap-coerce-response "Pluggable response coercion middleware. @@ -182,14 +183,15 @@ "Generator for pluggable response coercion middleware. Expects a :coercion of type `reitit.coercion.protocol/Coercion` and :responses from route meta, otherwise does not mount." - (middleware/gen - (fn [{:keys [responses coercion opts]} _] - (if (and coercion responses) - (let [coercers (response-coercers coercion responses opts)] - (fn [handler] - (fn - ([request] - (coerce-response coercers request (handler request))) - ([request respond raise] - (handler request #(respond (coerce-response coercers request %)) raise))))))))) + (middleware/create + {:name ::coerce-response + :gen (fn [{:keys [responses coercion opts]} _] + (if (and coercion responses) + (let [coercers (response-coercers coercion responses opts)] + (fn [handler] + (fn + ([request] + (coerce-response coercers request (handler request))) + ([request respond raise] + (handler request #(respond (coerce-response coercers request %)) raise)))))))})) diff --git a/src/reitit/middleware.cljc b/src/reitit/middleware.cljc index 853fc50d..a817b2f8 100644 --- a/src/reitit/middleware.cljc +++ b/src/reitit/middleware.cljc @@ -1,23 +1,22 @@ (ns reitit.middleware (:require [meta-merge.core :refer [meta-merge]] - [reitit.core :as reitit]) - #?(:clj - (:import (clojure.lang IFn AFn)))) + [reitit.core :as reitit])) (defprotocol ExpandMiddleware (expand-middleware [this meta opts])) -(defrecord MiddlewareGenerator [f args] - IFn - (invoke [_] - (f nil nil)) - (invoke [_ meta] - (f meta nil)) - (invoke [_ meta opts] - (f meta opts)) - #?(:clj - (applyTo [this args] - (AFn/applyToHelper this args)))) +(defrecord Middleware [name wrap create]) + +(defn create [{:keys [name gen wrap] :as m}] + (when-not name + (throw + (ex-info + (str "Middleware must have :name defined " m) m))) + (when (and gen wrap) + (throw + (ex-info + (str "Middleware can't both :wrap and :gen defined " m) m))) + (map->Middleware m)) (extend-protocol ExpandMiddleware @@ -32,11 +31,24 @@ :cljs function) (expand-middleware [this _ _] this) - MiddlewareGenerator + #?(:clj clojure.lang.PersistentArrayMap + :cljs cljs.core.PersistentArrayMap) (expand-middleware [this meta opts] - (if-let [mw (this meta opts)] + (expand-middleware (create this) meta opts)) + + #?(:clj clojure.lang.PersistentHashMap + :cljs cljs.core.PersistentHashMap) + (expand-middleware [this meta opts] + (expand-middleware (create this) meta opts)) + + Middleware + (expand-middleware [{:keys [wrap gen]} meta opts] + (if gen + (if-let [wrap (gen meta opts)] + (fn [handler & args] + (apply wrap handler args))) (fn [handler & args] - (apply mw handler args)))) + (apply wrap handler args)))) nil (expand-middleware [_ _ _])) @@ -56,9 +68,6 @@ (keep identity) (apply comp identity))) -(defn gen [f & args] - (->MiddlewareGenerator f args)) - (defn compile-handler ([route opts] (compile-handler route opts nil)) diff --git a/test/cljc/reitit/middleware_test.cljc b/test/cljc/reitit/middleware_test.cljc index 3c57b2aa..6c24f5c3 100644 --- a/test/cljc/reitit/middleware_test.cljc +++ b/test/cljc/reitit/middleware_test.cljc @@ -1,5 +1,5 @@ (ns reitit.middleware-test - (:require [clojure.test :refer [deftest testing is]] + (:require [clojure.test :refer [deftest testing is are]] [reitit.middleware :as middleware] [clojure.set :as set] [reitit.core :as reitit]) @@ -26,61 +26,96 @@ (respond (handler request)))) (deftest expand-middleware-test - (testing "middleware generators" - (let [calls (atom 0)] - (testing "record generator" - (reset! calls 0) - (let [syntax [(middleware/gen - (fn [meta _] + (testing "middleware records" + + (testing ":name is mandatory" + (is (thrown-with-msg? + ExceptionInfo + #"Middleware must have :name defined" + (middleware/create + {:wrap identity + :gen (constantly identity)})))) + + (testing ":wrap & :gen are exclusive" + (is (thrown-with-msg? + ExceptionInfo + #"Middleware can't both :wrap and :gen defined" + (middleware/create + {:name ::test + :wrap identity + :gen (constantly identity)})))) + + (testing ":wrap" + (let [calls (atom 0) + data {:name ::test + :wrap (fn [handler value] (swap! calls inc) - (fn [handler value] - (swap! calls inc) - (fn [request] - [meta value request]))))] - app ((middleware/compose-middleware syntax :meta {}) identity :value)] - (dotimes [_ 10] - (is (= [:meta :value :request] (app :request))) - (is (= 2 @calls))))) + (fn [request] + [value request]))}] - (testing "middleware generator as function" - (reset! calls 0) - (let [syntax (middleware/gen - (fn [meta _] + (testing "as map" + (reset! calls 0) + (let [app ((middleware/compose-middleware [data] :meta {}) identity :value)] + (dotimes [_ 10] + (is (= [:value :request] (app :request))) + (is (= 1 @calls))))) + + (testing "direct" + (reset! calls 0) + (let [app ((middleware/compose-middleware [(middleware/create data)] :meta {}) identity :value)] + (dotimes [_ 10] + (is (= [:value :request] (app :request))) + (is (= 1 @calls))))) + + (testing "vector" + (reset! calls 0) + (let [app ((middleware/compose-middleware [[(middleware/create data) :value]] :meta {}) identity)] + (dotimes [_ 10] + (is (= [:value :request] (app :request))) + (is (= 1 @calls))))))) + + (testing ":gen" + (let [calls (atom 0) + data {:name ::test + :gen (fn [meta _] (swap! calls inc) (fn [handler value] (swap! calls inc) (fn [request] - [meta value request])))) - app ((syntax :meta nil) identity :value)] - (dotimes [_ 10] + [meta value request])))}] + + (testing "as map" + (reset! calls 0) + (let [app ((middleware/compose-middleware [data] :meta {}) identity :value)] + (dotimes [_ 10] + (is (= [:meta :value :request] (app :request))) + (is (= 2 @calls))))) + + (testing "direct" + (reset! calls 0) + (let [app ((middleware/compose-middleware [(middleware/create data)] :meta {}) identity :value)] + (dotimes [_ 10] + (is (= [:meta :value :request] (app :request))) + (is (= 2 @calls))))) + + (testing "vector" + (reset! calls 0) + (let [app ((middleware/compose-middleware [[(middleware/create data) :value]] :meta {}) identity)] (is (= [:meta :value :request] (app :request))) - (is (= 2 @calls))))) - - (testing "generator vector" - (reset! calls 0) - (let [syntax [[(middleware/gen - (fn [meta _] - (swap! calls inc) - (fn [handler value] - (swap! calls inc) - (fn [request] - [meta value request])))) :value]] - app ((middleware/compose-middleware syntax :meta {}) identity)] - (is (= [:meta :value :request] (app :request))) - (dotimes [_ 10] - (is (= [:meta :value :request] (app :request))) - (is (= 2 @calls))))) - - (testing "generator can return nil" - (reset! calls 0) - (let [syntax [[(middleware/gen - (fn [meta _])) :value]] - app ((middleware/compose-middleware syntax :meta {}) identity)] - (is (= :request (app :request))) - (dotimes [_ 10] - (is (= :request (app :request))))))))) + (dotimes [_ 10] + (is (= [:meta :value :request] (app :request))) + (is (= 2 @calls))))) + (testing "nil unmounts the middleware" + (reset! calls 0) + (let [syntax [[(middleware/create + {:name ::test + :gen (fn [meta _])}) :value]] + app ((middleware/compose-middleware syntax :meta {}) identity)] + (is (= :request (app :request))) + (dotimes [_ 10] + (is (= :request (app :request)))))))))) (deftest middleware-router-test