diff --git a/src/reitit/core.cljc b/src/reitit/core.cljc index beab043e..faf0e60f 100644 --- a/src/reitit/core.cljc +++ b/src/reitit/core.cljc @@ -89,9 +89,12 @@ (defn find-names [routes opts] (into [] (keep #(-> % second :name)) routes)) -(defn compile-route [[p m :as route] {:keys [compile] :as opts}] +(defn- compile-route [[p m :as route] {:keys [compile] :as opts}] [p m (if compile (compile route opts))]) +(defn- compile-routes [routes opts] + (into [] (keep #(compile-route % opts) routes))) + (defprotocol Router (router-name [this]) (routes [this]) @@ -132,7 +135,7 @@ ([routes] (linear-router routes {})) ([routes opts] - (let [compiled (map #(compile-route % opts) routes) + (let [compiled (compile-routes routes opts) names (find-names routes opts) [data lookup] (reduce (fn [[data lookup] [p {:keys [name] :as meta} result]] @@ -149,7 +152,7 @@ (router-name [_] :linear-router) (routes [_] - routes) + compiled) (options [_] opts) (route-names [_] @@ -179,7 +182,7 @@ (str "can't create LookupRouter with wildcard routes: " wilds) {:wilds wilds :routes routes}))) - (let [compiled (map #(compile-route % opts) routes) + (let [compiled (compile-routes routes opts) names (find-names routes opts) [data lookup] (reduce (fn [[data lookup] [p {:keys [name] :as meta} result]] @@ -193,7 +196,7 @@ (router-name [_] :lookup-router) (routes [_] - routes) + compiled) (options [_] opts) (route-names [_] diff --git a/src/reitit/middleware.cljc b/src/reitit/middleware.cljc index a817b2f8..86dea874 100644 --- a/src/reitit/middleware.cljc +++ b/src/reitit/middleware.cljc @@ -2,10 +2,11 @@ (:require [meta-merge.core :refer [meta-merge]] [reitit.core :as reitit])) -(defprotocol ExpandMiddleware - (expand-middleware [this meta opts])) +(defprotocol IntoMiddleware + (into-middleware [this meta opts])) -(defrecord Middleware [name wrap create]) +(defrecord Middleware [name wrap]) +(defrecord Endpoint [meta handler middleware]) (defn create [{:keys [name gen wrap] :as m}] (when-not name @@ -18,40 +19,42 @@ (str "Middleware can't both :wrap and :gen defined " m) m))) (map->Middleware m)) -(extend-protocol ExpandMiddleware +(extend-protocol IntoMiddleware #?(:clj clojure.lang.APersistentVector :cljs cljs.core.PersistentVector) - (expand-middleware [[f & args] meta opts] - (if-let [mw (expand-middleware f meta opts)] - (fn [handler] - (apply mw handler args)))) + (into-middleware [[f & args] meta opts] + (if-let [{:keys [wrap] :as mw} (into-middleware f meta opts)] + (assoc mw :wrap #(apply wrap % args)))) #?(:clj clojure.lang.Fn :cljs function) - (expand-middleware [this _ _] this) + (into-middleware [this _ _] + (map->Middleware + {:wrap this})) #?(:clj clojure.lang.PersistentArrayMap :cljs cljs.core.PersistentArrayMap) - (expand-middleware [this meta opts] - (expand-middleware (create this) meta opts)) + (into-middleware [this meta opts] + (into-middleware (create this) meta opts)) #?(:clj clojure.lang.PersistentHashMap :cljs cljs.core.PersistentHashMap) - (expand-middleware [this meta opts] - (expand-middleware (create this) meta opts)) + (into-middleware [this meta opts] + (into-middleware (create this) meta opts)) Middleware - (expand-middleware [{:keys [wrap gen]} meta opts] - (if gen + (into-middleware [{:keys [wrap gen] :as this} meta opts] + (if-not gen + this (if-let [wrap (gen meta opts)] - (fn [handler & args] - (apply wrap handler args))) - (fn [handler & args] - (apply wrap handler args)))) + (map->Middleware + (-> this + (dissoc :gen) + (assoc :wrap wrap)))))) nil - (expand-middleware [_ _ _])) + (into-middleware [_ _ _])) (defn- ensure-handler! [path meta scope] (when-not (:handler meta) @@ -61,23 +64,44 @@ (merge {:path path, :meta meta} (if scope {:scope scope})))))) -(defn compose-middleware [middleware meta opts] +(defn expand [middleware meta opts] (->> middleware - (keep identity) - (map #(expand-middleware % meta opts)) - (keep identity) - (apply comp identity))) + (keep #(into-middleware % meta opts)) + (into []))) -(defn compile-handler +(defn compile-handler [middleware handler] + ((apply comp identity (keep :wrap middleware)) handler)) + +(compile-handler + [(map->Middleware + {:wrap + (fn [handler] + (fn [request] + (handler request)))})] identity) + +(defn compile-result ([route opts] - (compile-handler route opts nil)) + (compile-result route opts nil)) ([[path {:keys [middleware handler] :as meta}] opts scope] (ensure-handler! path meta scope) - ((compose-middleware middleware meta opts) handler))) + (let [middleware (expand middleware meta opts)] + (map->Endpoint + {:handler (compile-handler middleware handler) + :middleware middleware + :meta meta})))) (defn router ([data] (router data nil)) ([data opts] - (let [opts (meta-merge {:compile compile-handler} opts)] + (let [opts (meta-merge {:compile compile-result} opts)] (reitit/router data opts)))) + +(defn middleware-handler [router] + (with-meta + (fn [path] + (some->> path + (reitit/match-by-path router) + :result + :handler)) + {::router router})) diff --git a/test/cljc/reitit/core_test.cljc b/test/cljc/reitit/core_test.cljc index 15c75b64..0540d68c 100644 --- a/test/cljc/reitit/core_test.cljc +++ b/test/cljc/reitit/core_test.cljc @@ -9,8 +9,8 @@ (testing "linear-router" (let [router (reitit/router ["/api" ["/ipa" ["/:size" ::beer]]])] - (is (= [["/api/ipa/:size" {:name ::beer}]] (is (= :linear-router (reitit/router-name router))) + (is (= [["/api/ipa/:size" {:name ::beer} nil]] (reitit/routes router))) (is (= true (map? (reitit/options router)))) (is (= (reitit/map->Match @@ -43,8 +43,8 @@ (testing "lookup-router" (let [router (reitit/router ["/api" ["/ipa" ["/large" ::beer]]])] - (is (= [["/api/ipa/large" {:name ::beer}]] (is (= :lookup-router (reitit/router-name router))) + (is (= [["/api/ipa/large" {:name ::beer} nil]] (reitit/routes router))) (is (= true (map? (reitit/options router)))) (is (= (reitit/map->Match @@ -97,7 +97,8 @@ ["/api/pong" {:name ::pong :path "/api/pong", :roles #{:admin}}]] - (reitit/routes router)))) + (map butlast (reitit/routes router))))) + (testing "route match contains compiled handler" (is (= 2 @compile-times)) (let [{:keys [result]} (reitit/match-by-path router "/api/pong")] diff --git a/test/cljc/reitit/middleware_test.cljc b/test/cljc/reitit/middleware_test.cljc index 6c24f5c3..58e39498 100644 --- a/test/cljc/reitit/middleware_test.cljc +++ b/test/cljc/reitit/middleware_test.cljc @@ -6,25 +6,6 @@ #?(: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 records" @@ -46,78 +27,93 @@ :wrap identity :gen (constantly identity)})))) - (testing ":wrap" + (testing "middleware" (let [calls (atom 0) - data {:name ::test - :wrap (fn [handler value] - (swap! calls inc) - (fn [request] - [value request]))}] + wrap (fn [handler value] + (swap! calls inc) + (fn [request] + [value request])) + ->app (fn [ast handler] + (middleware/compile-handler + (middleware/expand ast :meta {}) + handler))] + + (testing "as middleware function" + (reset! calls 0) + (let [app (->app [[#(wrap % :value)]] identity)] + (dotimes [_ 10] + (is (= [:value :request] (app :request))) + (is (= 1 @calls))))) + + (testing "as middleware vector" + (reset! calls 0) + (let [app (->app [[wrap :value]] identity)] + (dotimes [_ 10] + (is (= [:value :request] (app :request))) + (is (= 1 @calls))))) (testing "as map" (reset! calls 0) - (let [app ((middleware/compose-middleware [data] :meta {}) identity :value)] + (let [app (->app [[{:wrap #(wrap % :value)}]] identity)] (dotimes [_ 10] (is (= [:value :request] (app :request))) (is (= 1 @calls))))) - (testing "direct" + (testing "as map vector" (reset! calls 0) - (let [app ((middleware/compose-middleware [(middleware/create data)] :meta {}) identity :value)] + (let [app (->app [[{:wrap wrap} :value]] identity)] (dotimes [_ 10] (is (= [:value :request] (app :request))) (is (= 1 @calls))))) - (testing "vector" + (testing "as Middleware" (reset! calls 0) - (let [app ((middleware/compose-middleware [[(middleware/create data) :value]] :meta {}) identity)] + (let [app (->app [[(middleware/create {:wrap #(wrap % :value)})]] identity)] + (dotimes [_ 10] + (is (= [:value :request] (app :request))) + (is (= 1 @calls))))) + + (testing "as Middleware vector" + (reset! calls 0) + (let [app (->app [[(middleware/create {:wrap wrap}) :value]] identity)] (dotimes [_ 10] (is (= [:value :request] (app :request))) (is (= 1 @calls))))))) - (testing ":gen" + (testing "compiled Middleware" (let [calls (atom 0) - data {:name ::test - :gen (fn [meta _] + mw {:gen (fn [meta _] + (swap! calls inc) + (fn [handler value] (swap! calls inc) - (fn [handler value] - (swap! calls inc) - (fn [request] - [meta value request])))}] + (fn [request] + [meta value request])))} + ->app (fn [ast handler] + (middleware/compile-handler + (middleware/expand ast :meta {}) + handler))] (testing "as map" (reset! calls 0) - (let [app ((middleware/compose-middleware [data] :meta {}) identity :value)] + (let [app (->app [[mw :value]] identity)] (dotimes [_ 10] (is (= [:meta :value :request] (app :request))) (is (= 2 @calls))))) - (testing "direct" + (testing "as Middleware" (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))) + (let [app (->app [[(middleware/create mw) :value]] identity)] (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))) + (let [app (->app [{:gen (constantly nil)} + {:gen (constantly nil)}] identity)] (dotimes [_ 10] (is (= :request (app :request)))))))))) -(deftest middleware-router-test +(deftest middleware-handler-test (testing "all paths should have a handler" (is (thrown-with-msg? @@ -125,40 +121,59 @@ #"path \"/ping\" doesn't have a :handler defined" (middleware/router ["/ping"])))) - (testing "ring-handler" - (let [api-mw #(mw % :api) + (testing "middleware-handler" + (let [mw (fn [handler value] + (fn [request] + (conj (handler (conj request value)) value))) + api-mw #(mw % :api) + handler #(conj % :ok) 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))))] + ->app (fn [router] + (let [h (middleware/middleware-handler router)] + (fn [path] + (if-let [f (h path)] + (f []))))) + app (->app router)] (testing "not found" - (is (= nil (app {:uri "/favicon.ico"})))) + (is (= nil (app "/favicon.ico")))) (testing "normal handler" - (is (= {:status 200, :body [:ok]} - (app {:uri "/ping"})))) + (is (= [:ok] (app "/ping")))) (testing "with middleware" - (is (= {:status 200, :body [:api :ok :api]} - (app {:uri "/api/ping"})))) + (is (= [:api :ok :api] (app "/api/ping")))) (testing "with nested middleware" - (is (= {:status 200, :body [:api :admin :ok :admin :api]} - (app {:uri "/api/admin/ping"})))) + (is (= [:api :admin :ok :admin :api] (app "/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))))))) + (testing ":gen middleware can be unmounted at creation-time" + (let [mw1 {:name ::mw1, :gen (constantly #(mw % ::mw1))} + mw2 {:name ::mw2, :gen (constantly nil)} + mw3 {:name ::mw3, :wrap #(mw % ::mw3)} + router (middleware/router + ["/api" {:name ::api + :middleware [mw1 mw2 mw3 mw2] + :handler handler}]) + app (->app router)] + + (is (= [::mw1 ::mw3 :ok ::mw3 ::mw1] (app "/api"))) + + (testing "routes contain list of actually applied mw" + (is (= [::mw1 ::mw3] (->> (reitit/routes router) + first + last + :middleware + (map :name))))) + + (testing "match contains list of actually applied mw" + (is (= [::mw1 ::mw3] (->> "/api" + (reitit/match-by-path router) + :result + :middleware + (map :name))))))))))