From 8bbf2eb78cfec44e929e008b589788631847498b Mon Sep 17 00:00:00 2001 From: Tommi Reiman Date: Sun, 3 Dec 2017 21:07:41 +0200 Subject: [PATCH 1/5] Polish tests --- test/cljc/reitit/middleware_test.cljc | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/test/cljc/reitit/middleware_test.cljc b/test/cljc/reitit/middleware_test.cljc index 9fc40a4a..ebb39533 100644 --- a/test/cljc/reitit/middleware_test.cljc +++ b/test/cljc/reitit/middleware_test.cljc @@ -105,6 +105,12 @@ (dotimes [_ 10] (is (= :request (app :request)))))))))) +(defn create-app [router] + (let [h (middleware/middleware-handler router)] + (fn [path] + (if-let [f (h path)] + (f []))))) + (deftest middleware-handler-test (testing "all paths should have a handler" @@ -125,12 +131,7 @@ ["/ping" handler] ["/admin" {:middleware [[mw :admin]]} ["/ping" handler]]]]) - ->app (fn [router] - (let [h (middleware/middleware-handler router)] - (fn [path] - (if-let [f (h path)] - (f []))))) - app (->app router)] + app (create-app router)] (testing "not found" (is (= nil (app "/favicon.ico")))) @@ -152,7 +153,7 @@ ["/api" {:name ::api :middleware [mw1 mw2 mw3 mw2] :handler handler}]) - app (->app router)] + app (create-app router)] (is (= [::mw1 ::mw3 :ok ::mw3 ::mw1] (app "/api"))) From 368850b6ab0bd8159d2a4a95db75b678b134e4e4 Mon Sep 17 00:00:00 2001 From: Tommi Reiman Date: Sun, 3 Dec 2017 21:09:05 +0200 Subject: [PATCH 2/5] :reitit.ring.middleware/transform! --- .../src/reitit/ring/middleware.cljc | 5 ++-- test/cljc/reitit/middleware_test.cljc | 24 +++++++++++++++++++ 2 files changed, 27 insertions(+), 2 deletions(-) diff --git a/modules/reitit-ring/src/reitit/ring/middleware.cljc b/modules/reitit-ring/src/reitit/ring/middleware.cljc index 8846efcc..5abcb57e 100644 --- a/modules/reitit-ring/src/reitit/ring/middleware.cljc +++ b/modules/reitit-ring/src/reitit/ring/middleware.cljc @@ -78,9 +78,10 @@ (defn compile-result ([route opts] (compile-result route opts nil)) - ([[path {:keys [middleware handler] :as data}] opts scope] + ([[path {:keys [middleware handler] :as data}] + {:keys [::transform] :or {transform identity} :as opts} scope] (ensure-handler! path data scope) - (let [middleware (expand middleware data opts)] + (let [middleware (transform (expand middleware data opts))] (map->Endpoint {:handler (compile-handler middleware handler) :middleware middleware diff --git a/test/cljc/reitit/middleware_test.cljc b/test/cljc/reitit/middleware_test.cljc index ebb39533..5a68c03a 100644 --- a/test/cljc/reitit/middleware_test.cljc +++ b/test/cljc/reitit/middleware_test.cljc @@ -188,3 +188,27 @@ (is (= [::mw1 ::mw3 ::mw4 ::mw5 :ok ::mw5 ::mw4 ::mw3 ::mw1] (chain1 []))) (is (= [::mw1 ::mw3 ::mw4 :ok ::mw4 ::mw3 ::mw1] (chain2 [])))))) +(deftest middleware-transform-test + (let [wrap (fn [handler value] + #(handler (conj % value))) + debug-mw {:name ::debug, :wrap #(wrap % ::debug)} + create (fn [options] + (create-app + (middleware/router + ["/ping" {:middleware [{:name ::olipa, :wrap #(wrap % ::olipa)} + {:name ::kerran, :wrap #(wrap % ::kerran)} + {:name ::avaruus, :wrap #(wrap % ::avaruus)}] + :handler #(conj % :ok)}] + options)))] + + (testing "by default, all middleware are applied in order" + (let [app (create nil)] + (is (= [::olipa ::kerran ::avaruus :ok] (app "/ping"))))) + + (testing "middleware can be re-ordered" + (let [app (create {::middleware/transform (partial sort-by :name)})] + (is (= [::avaruus ::kerran ::olipa :ok] (app "/ping"))))) + + (testing "adding debug middleware between middleware" + (let [app (create {::middleware/transform #(interleave % (repeat debug-mw))})] + (is (= [::olipa ::debug ::kerran ::debug ::avaruus ::debug :ok] (app "/ping"))))))) From ba78008d900bcb87590cf1f1b43d70daebdaf7c7 Mon Sep 17 00:00:00 2001 From: Tommi Reiman Date: Mon, 4 Dec 2017 08:35:58 +0200 Subject: [PATCH 3/5] expand the transformed mw --- .../src/reitit/ring/middleware.cljc | 2 +- test/cljc/reitit/ring_test.cljc | 63 ++++++++++++------- 2 files changed, 42 insertions(+), 23 deletions(-) diff --git a/modules/reitit-ring/src/reitit/ring/middleware.cljc b/modules/reitit-ring/src/reitit/ring/middleware.cljc index 5abcb57e..e8fc7669 100644 --- a/modules/reitit-ring/src/reitit/ring/middleware.cljc +++ b/modules/reitit-ring/src/reitit/ring/middleware.cljc @@ -81,7 +81,7 @@ ([[path {:keys [middleware handler] :as data}] {:keys [::transform] :or {transform identity} :as opts} scope] (ensure-handler! path data scope) - (let [middleware (transform (expand middleware data opts))] + (let [middleware (expand (transform (expand middleware data opts)) data opts)] (map->Endpoint {:handler (compile-handler middleware handler) :middleware middleware diff --git a/test/cljc/reitit/ring_test.cljc b/test/cljc/reitit/ring_test.cljc index 6e3591e1..ed258c0c 100644 --- a/test/cljc/reitit/ring_test.cljc +++ b/test/cljc/reitit/ring_test.cljc @@ -10,15 +10,9 @@ (defn mw [handler name] (fn ([request] - (-> request - (update ::mw (fnil conj []) name) - (handler) - (update :body (fnil conj []) name))) + (handler (update request ::mw (fnil conj []) name))) ([request respond raise] - (handler - (update request ::mw (fnil conj []) name) - #(respond (update % :body (fnil conj []) name)) - raise)))) + (handler (update request ::mw (fnil conj []) name) respond raise)))) (defn handler ([{:keys [::mw]}] @@ -37,14 +31,14 @@ (testing "ring-handler" (let [api-mw #(mw % :api) router (ring/router - [["/api" {:middleware [api-mw]} - ["/all" handler] - ["/get" {:get handler}] - ["/users" {:middleware [[mw :users]] - :get handler - :post {:handler handler - :middleware [[mw :post]]} - :handler handler}]]]) + ["/api" {:middleware [api-mw]} + ["/all" handler] + ["/get" {:get handler}] + ["/users" {:middleware [[mw :users]] + :get handler + :post {:handler handler + :middleware [[mw :post]]} + :handler handler}]]) app (ring/ring-handler router)] (testing "router can be extracted" @@ -54,31 +48,31 @@ (is (= nil (app {:uri "/favicon.ico"})))) (testing "catch all handler" - (is (= {:status 200, :body [:api :ok :api]} + (is (= {:status 200, :body [:api :ok]} (app {:uri "/api/all" :request-method :get})))) (testing "just get handler" - (is (= {:status 200, :body [:api :ok :api]} + (is (= {:status 200, :body [:api :ok]} (app {:uri "/api/get" :request-method :get}))) (is (= nil (app {:uri "/api/get" :request-method :post})))) (testing "expanded method handler" - (is (= {:status 200, :body [:api :users :ok :users :api]} + (is (= {:status 200, :body [:api :users :ok]} (app {:uri "/api/users" :request-method :get})))) (testing "method handler with middleware" - (is (= {:status 200, :body [:api :users :post :ok :post :users :api]} + (is (= {:status 200, :body [:api :users :post :ok]} (app {:uri "/api/users" :request-method :post})))) (testing "fallback handler" - (is (= {:status 200, :body [:api :users :ok :users :api]} + (is (= {:status 200, :body [:api :users :ok]} (app {:uri "/api/users" :request-method :put})))) (testing "3-arity" (let [result (atom nil) respond (partial reset! result), raise ::not-called] (app {:uri "/api/users" :request-method :post} respond raise) - (is (= {:status 200, :body [:api :users :post :ok :post :users :api]} + (is (= {:status 200, :body [:api :users :post :ok]} @result)))))) (testing "named routes" @@ -160,3 +154,28 @@ (is (= nil (respond))) (is (= ::nil (raise))))))) +(deftest middleware-transform-test + (let [middleware (fn [name] {:name name, :wrap #(mw % name)}) + request {:uri "/api/avaruus" :request-method :get} + create (fn [options] + (ring/ring-handler + (ring/router + ["/api" {:middleware [(middleware :olipa)]} + ["/avaruus" {:middleware [(middleware :kerran)] + :get {:handler handler + :middleware [(middleware :avaruus)]}}]] + options)))] + + (testing "by default, all middleware are applied in order" + (let [app (create nil)] + (is (= {:status 200, :body [:olipa :kerran :avaruus :ok]} + (app request))))) + + (testing "middleware can be re-ordered" + (let [app (create {::middleware/transform (partial sort-by :name)})] + (is (= {:status 200, :body [:avaruus :kerran :olipa :ok]} + (app request))))) + + (testing "adding debug middleware between middleware" + (let [app (create {::middleware/transform #(interleave % (repeat (middleware "debug")))})] + (is (= {:status 200, :body [:olipa "debug" :kerran "debug" :avaruus "debug" :ok]} (app request))))))) From 33260396d2fa0d77708728293467c4b8562dc3f5 Mon Sep 17 00:00:00 2001 From: Tommi Reiman Date: Mon, 4 Dec 2017 08:36:11 +0200 Subject: [PATCH 4/5] Update docs --- doc/ring/data_driven_middleware.md | 79 ++++++++++++++++++++++++------ 1 file changed, 63 insertions(+), 16 deletions(-) diff --git a/doc/ring/data_driven_middleware.md b/doc/ring/data_driven_middleware.md index f4477621..8b9f59d3 100644 --- a/doc/ring/data_driven_middleware.md +++ b/doc/ring/data_driven_middleware.md @@ -1,23 +1,25 @@ # Data-driven Middleware -Ring [defines middleware](https://github.com/ring-clojure/ring/wiki/Concepts#middleware) as a function of type `handler & args => request => response`. It's easy to undrstand and enables great performance. Still, in the end - the middleware-chain is just a opaque function, making things like documentation and debugging hard. +Ring [defines middleware](https://github.com/ring-clojure/ring/wiki/Concepts#middleware) as a function of type `handler & args => request => response`. It's relatively easy to understand and enables good performance. Downside is that the middleware-chain is just a opaque function, making things like debugging and composition hard. It's too easy to apply the middleware in wrong order. -Reitit does things bit differently: +Reitit defines middleware as data: -1. Middleware is defined as a vector (of middleware) enabling the chain to be malipulated before turned into the runtime middleware function. -2. Middleware can be defined as first-class data entries +1. Middleware can be defined as first-class data entries +2. Middleware can be defined as a [duct-style](https://github.com/duct-framework/duct/wiki/Configuration) vector (of middleware) +4. Middleware can be optimized & [compiled](compiling_middleware.md) againt an endpoint +3. Middleware chain can be transformed by the router -### Middleware as data +## Middleware as data All values in the `:middleware` vector in the route data are coerced into `reitit.ring.middleware/Middleware` Records with using the `reitit.ring.middleware/IntoMiddleware` Protocol. By default, functions, maps and `Middleware` records are allowed. Records can have arbitrary keys, but the following keys have a special purpose: -| key | description | -| ------------|-------------| -| `:name` | Name of the middleware as a qualified keyword (optional) -| `:wrap` | The actual middleware function of `handler & args => request => response` -| `:gen-wrap` | Middleware function generation function, see [compiling middleware](compiling_middleware.md). +| key | description | +| ---------------|-------------| +| `:name` | Name of the middleware as a qualified keyword (optional) +| `:wrap` | The actual middleware function of `handler & args => request => response` +| `:gen-wrap` | Middleware function generation function, see [compiling middleware](compiling_middleware.md). Middleware Records are accessible in their raw form in the compiled route results, thus available for inventories, creating api-docs etc. @@ -27,7 +29,7 @@ For the actual request processing, the Records are unwrapped into normal functio The following produce identical middleware runtime function. -#### Function +### Function ```clj (defn wrap [handler id] @@ -47,7 +49,7 @@ The following produce identical middleware runtime function. :wrap wrap})) ``` -#### Map +### Map ```clj (def wrap3 @@ -56,7 +58,9 @@ The following produce identical middleware runtime function. :wrap wrap}) ``` -### Using Middleware +## Using Middleware + +`:middleware` is merged to endpoints by the `router`. ```clj (require '[reitit.ring :as ring]) @@ -72,18 +76,61 @@ The following produce identical middleware runtime function. :handler handler}}]]))) ``` -All the middleware are called correctly: +All the middleware are applied correctly: ```clj (app {:request-method :get, :uri "/api/ping"}) ; {:status 200, :body [1 2 3 :handler]} ``` -### Future +## Compiling middleware + +Middleware can be optimized against an endpoint using [middleware compilation](compiling_middleware.md). + +## Transforming the middleware chain + +There is an extra option in ring-router (actually, in the undelaying middleware-router): `:reitit.ring.middleware/transform` to transform the middleware chain per endpoint. It sees the vector of compiled middleware and should return a new vector of middleware. + +#### Adding debug middleware between all other middleware + +```clj +(def app + (ring/ring-handler + (ring/router + ["/api" {:middleware [[wrap 1] [wrap2 2]]} + ["/ping" {:get {:middleware [[wrap3 3]] + :handler handler}}]] + {::middleware/transform #(interleave % (repeat [wrap :debug]))}))) +``` + +``` +(app {:request-method :get, :uri "/api/ping"}) +; {:status 200, :body [1 :debug 2 :debug 3 :debug :handler]} +``` + +#### Reversing the middleware chain + +```clj +(def app + (ring/ring-handler + (ring/router + ["/api" {:middleware [[wrap 1] [wrap2 2]]} + ["/ping" {:get {:middleware [[wrap3 3]] + :handler handler}}]] + {::middleware/transform reverse)}))) +``` + +``` +(app {:request-method :get, :uri "/api/ping"}) +; {:status 200, :body [3 2 1 :handler]} +``` + +## Roadmap for middleware Some things bubblin' under: -* Hooks to manipulate the `:middleware` chain before compilation +* Re-package all useful middleware into (optimized) data-driven Middleware + * just package or a new community-repo with rehosting stuffm? * Support `Keyword` expansion into Middleware, enabling external Middleware Registries (duct/integrant/macchiato -style) * Support Middleware dependency resolution with new keys `:requires` and `:provides`. Values are set of top-level keys of the request. e.g. * `InjectUserIntoRequestMiddleware` requires `#{:session}` and provides `#{:user}` From 7167c76af8eff2925477a73c639f38edbe5edc9f Mon Sep 17 00:00:00 2001 From: Tommi Reiman Date: Mon, 4 Dec 2017 08:49:15 +0200 Subject: [PATCH 5/5] Cleanup example --- test/cljc/reitit/ring_test.cljc | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/test/cljc/reitit/ring_test.cljc b/test/cljc/reitit/ring_test.cljc index ed258c0c..04667c89 100644 --- a/test/cljc/reitit/ring_test.cljc +++ b/test/cljc/reitit/ring_test.cljc @@ -155,7 +155,11 @@ (is (= ::nil (raise))))))) (deftest middleware-transform-test - (let [middleware (fn [name] {:name name, :wrap #(mw % name)}) + (let [middleware (fn [name] {:name name + :wrap (fn [handler] + (fn [request] + (handler (update request ::mw (fnil conj []) name))))}) + handler (fn [{:keys [::mw]}] {:status 200 :body (conj mw :ok)}) request {:uri "/api/avaruus" :request-method :get} create (fn [options] (ring/ring-handler