Merge pull request #53 from metosin/middleware-games

Middleware games

doc/ring/data_driven_middleware.md

@@ -1,20 +1,22 @@
 # 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.
+1. Middleware can be defined as first-class data entries
-2. 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).
@@ -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}`

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 (expand (transform (expand middleware data opts)) data opts)]
      (map->Endpoint
        {:handler (compile-handler middleware handler)
         :middleware middleware

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]
+          app (create-app router)]
-                  (let [h (middleware/middleware-handler router)]
-                    (fn [path]
-                      (if-let [f (h path)]
-                        (f [])))))
-          app (->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")))
 
@@ -187,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")))))))

test/cljc/reitit/ring_test.cljc

@@ -10,15 +10,9 @@
 (defn mw [handler name]
   (fn
     ([request]
-     (-> request
+     (handler (update request ::mw (fnil conj []) name)))
-         (update ::mw (fnil conj []) name)
-         (handler)
-         (update :body (fnil conj []) name)))
     ([request respond raise]
-     (handler
+     (handler (update request ::mw (fnil conj []) name) respond raise))))
-       (update request ::mw (fnil conj []) name)
-       #(respond (update % :body (fnil conj []) name))
-       raise))))
 
 (defn handler
   ([{:keys [::mw]}]
@@ -37,14 +31,14 @@
   (testing "ring-handler"
     (let [api-mw #(mw % :api)
           router (ring/router
-                   [["/api" {:middleware [api-mw]}
+                   ["/api" {:middleware [api-mw]}
                     ["/all" handler]
                     ["/get" {:get handler}]
                     ["/users" {:middleware [[mw :users]]
                                :get handler
                                :post {:handler handler
                                       :middleware [[mw :post]]}
-                                :handler handler}]]])
+                               :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,32 @@
         (is (= nil (respond)))
         (is (= ::nil (raise)))))))
 
+(deftest middleware-transform-test
+  (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
+                   (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)))))))