From a208f7df6ca795269c4c983b302ecb4e27f0154b Mon Sep 17 00:00:00 2001 From: Tommi Reiman Date: Sun, 13 Aug 2017 14:56:28 +0300 Subject: [PATCH 1/6] Remove println --- test/cljc/reitit/core_test.cljc | 1 - 1 file changed, 1 deletion(-) diff --git a/test/cljc/reitit/core_test.cljc b/test/cljc/reitit/core_test.cljc index 2da36f71..b93efa1f 100644 --- a/test/cljc/reitit/core_test.cljc +++ b/test/cljc/reitit/core_test.cljc @@ -89,7 +89,6 @@ (is (= 2 @compile-times)))))) (testing "default compile" (let [router (reitit/router ["/ping" (constantly "ok")])] - (println (reitit/match-by-path router "/ping")) (let [{:keys [handler]} (reitit/match-by-path router "/ping")] (is handler) (is (= "ok" (handler))))))) From fa37e3e198927906dc83eac2374d742a8c6e3cf5 Mon Sep 17 00:00:00 2001 From: Tommi Reiman Date: Sun, 13 Aug 2017 15:44:49 +0300 Subject: [PATCH 2/6] Initial sketch for a ring-router --- src/reitit/ring.cljc | 43 +++++++++++++++++++++ test/cljc/reitit/ring_test.cljc | 64 ++++++++++++++++++++++++++++++++ test/cljs/reitit/doo_runner.cljs | 6 ++- 3 files changed, 111 insertions(+), 2 deletions(-) create mode 100644 src/reitit/ring.cljc create mode 100644 test/cljc/reitit/ring_test.cljc diff --git a/src/reitit/ring.cljc b/src/reitit/ring.cljc new file mode 100644 index 00000000..ac078877 --- /dev/null +++ b/src/reitit/ring.cljc @@ -0,0 +1,43 @@ +(ns reitit.ring + (:require [reitit.core :as reitit])) + +(defprotocol ExpandMiddleware + (expand-middleware [this])) + +(extend-protocol ExpandMiddleware + + #?(:clj clojure.lang.APersistentVector + :cljs cljs.core.PersistentVector) + (expand-middleware [[f & args]] + (fn [handler] + (apply f handler args))) + + #?(:clj clojure.lang.Fn + :cljs function) + (expand-middleware [this] this) + + nil + (expand-middleware [_])) + +(defn compile-handler [[path {:keys [middleware handler] :as meta}]] + (when-not handler + (throw (ex-info + (str "path '" path "' doesn't have a :handler defined") + {:path path, :meta meta}))) + (let [wrap (->> middleware + (keep identity) + (map expand-middleware) + (apply comp identity))] + (wrap handler))) + +(defn router [data] + (reitit/router data {:compile compile-handler})) + +(defn ring-handler [router] + (fn + ([request] + (if-let [match (reitit/match-by-path router (:uri request))] + ((:handler match) request))) + ([request respond raise] + (if-let [match (reitit/match-by-path router (:uri request))] + ((:handler match) request respond raise))))) diff --git a/test/cljc/reitit/ring_test.cljc b/test/cljc/reitit/ring_test.cljc new file mode 100644 index 00000000..cbf95424 --- /dev/null +++ b/test/cljc/reitit/ring_test.cljc @@ -0,0 +1,64 @@ +(ns reitit.ring-test + (:require [clojure.test :refer [deftest testing is are]] + [reitit.core :as reitit] + [reitit.ring :as ring]) + #?(: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)))) + +(deftest ring-test + + (testing "all paths should have a handler" + (is (thrown-with-msg? + ExceptionInfo + #"^path '/ping' doesn't have a :handler defined$" + (ring/router ["/ping"])))) + + (testing "ring-handler" + (let [api-mw #(mw % :api) + handler (fn handle + ([{:keys [::mw]}] + {:status 200 :body (conj mw :ok)}) + ([request respond raise] + (respond (handle request)))) + app (ring/ring-handler + (ring/router + [["/ping" handler] + ["/api" {:middleware [api-mw]} + ["/ping" handler] + ["/admin" {:middleware [[mw :admin]]} + ["/ping" handler]]]]))] + + (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 "not found" + (is (= nil (app {:uri "/favicon.ico"})))) + + (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/cljs/reitit/doo_runner.cljs b/test/cljs/reitit/doo_runner.cljs index cc33a68e..ff3bd70e 100644 --- a/test/cljs/reitit/doo_runner.cljs +++ b/test/cljs/reitit/doo_runner.cljs @@ -1,7 +1,9 @@ (ns reitit.doo-runner (:require [doo.runner :refer-macros [doo-tests]] - reitit.core-test)) + reitit.core-test + reitit.ring-test)) (enable-console-print!) -(doo-tests 'reitit.core-test) +(doo-tests 'reitit.core-test + 'reitit.ring-test) From 93447fdc71e88ffcb44a9642c5e46c370540d2d3 Mon Sep 17 00:00:00 2001 From: Tommi Reiman Date: Mon, 14 Aug 2017 08:04:20 +0300 Subject: [PATCH 3/6] coerce & compile take opts, resolved in router --- src/reitit/core.cljc | 32 ++++++++++++++++---------------- test/cljc/reitit/core_test.cljc | 4 ++-- 2 files changed, 18 insertions(+), 18 deletions(-) diff --git a/src/reitit/core.cljc b/src/reitit/core.cljc index 2ef79975..2c039192 100644 --- a/src/reitit/core.cljc +++ b/src/reitit/core.cljc @@ -58,11 +58,11 @@ (defn resolve-routes [data {:keys [coerce] :as opts}] (cond-> (->> (walk data opts) (map-meta merge-meta)) - coerce (->> (mapv (partial coerce)) + coerce (->> (mapv #(coerce % opts)) (filterv identity)))) -(defn compile-route [compile [p m :as route]] - [p m (if compile (compile route))]) +(defn compile-route [[p m :as route] {:keys [compile] :as opts}] + [p m (if compile (compile route opts))]) (defprotocol Routing (routes [this]) @@ -72,8 +72,9 @@ (defrecord Match [template meta path handler params]) (def default-router-options - {:coerce identity - :compile (comp :handler second)}) + {:expand expand + :coerce (fn [route _] route) + :compile (fn [[_ {:keys [handler]}] _] handler)}) (defrecord LinearRouter [routes data lookup] Routing @@ -91,13 +92,12 @@ ((lookup name) params))) (defn linear-router - "Creates a [[LinearRouter]] from routes and optional options. - See [[router]] for available options" + "Creates a [[LinearRouter]] from resolved routes and optional + expanded options. See [[router]] for available options" ([routes] (linear-router routes {})) ([routes opts] - (let [{:keys [compile]} (meta-merge default-router-options opts) - compiled (map (partial compile-route compile) routes) + (let [compiled (map #(compile-route % opts) routes) [data lookup] (reduce (fn [[data lookup] [p {:keys [name] :as meta} handler]] (let [route (impl/create [p meta handler])] @@ -119,8 +119,8 @@ ((lookup name) params))) (defn lookup-router - "Creates a [[LookupRouter]] from routes and optional options. - See [[router]] for available options" + "Creates a [[LookupRouter]] from resolved routes and optional + expanded options. See [[router]] for available options" ([routes] (lookup-router routes {})) ([routes opts] @@ -130,8 +130,7 @@ (str "can't create LookupRouter with wildcard routes: " route) {:route route :routes routes}))) - (let [{:keys [compile]} (meta-merge default-router-options opts) - compiled (map (partial compile-route compile) routes) + (let [compiled (map #(compile-route % opts) routes) [data lookup] (reduce (fn [[data lookup] [p {:keys [name] :as meta} handler]] [(assoc data p (->Match p meta p handler {})) @@ -151,11 +150,12 @@ | `:routes` | Initial resolved routes (default `[]`) | `:meta` | Initial expanded route-meta vector (default `[]`) | `:expand` | Function `arg => meta` to expand route arg to route meta-data (default `reitit.core/expand`) - | `:coerce` | Function `[path meta] => [path meta]` to coerce resolved route, can throw or return `nil` (default `identity`) - | `:compile` | Function `[path meta] => handler` to compile a route handler (default `(comp :handler second)`)" + | `:coerce` | Function `[path meta] opts => [path meta]` to coerce resolved route, can throw or return `nil` + | `:compile` | Function `[path meta] opts => handler` to compile a route handler" ([data] (router data {})) ([data opts] - (let [routes (resolve-routes data opts)] + (let [opts (meta-merge default-router-options opts) + routes (resolve-routes data opts)] ((if (some impl/contains-wilds? (map first routes)) linear-router lookup-router) routes opts)))) diff --git a/test/cljc/reitit/core_test.cljc b/test/cljc/reitit/core_test.cljc index b93efa1f..4eb57e69 100644 --- a/test/cljc/reitit/core_test.cljc +++ b/test/cljc/reitit/core_test.cljc @@ -58,10 +58,10 @@ (testing "route coercion & compilation" (testing "custom compile" (let [compile-times (atom 0) - coerce (fn [[path meta]] + coerce (fn [[path meta] _] (if-not (:invalid? meta) [path (assoc meta :path path)])) - compile (fn [[path meta]] + compile (fn [[path meta] _] (swap! compile-times inc) (constantly path)) router (reitit/router From fe19b57b010b539c5fc763365ad5da13b2ffec7a Mon Sep 17 00:00:00 2001 From: Tommi Reiman Date: Mon, 14 Aug 2017 08:35:14 +0300 Subject: [PATCH 4/6] Ring simple-router & method-router --- src/reitit/ring.cljc | 90 ++++++++++++++++----- test/cljc/reitit/core_test.cljc | 2 +- test/cljc/reitit/ring_test.cljc | 138 +++++++++++++++++++++++--------- 3 files changed, 174 insertions(+), 56 deletions(-) diff --git a/src/reitit/ring.cljc b/src/reitit/ring.cljc index ac078877..681f3fad 100644 --- a/src/reitit/ring.cljc +++ b/src/reitit/ring.cljc @@ -1,5 +1,6 @@ (ns reitit.ring - (:require [reitit.core :as reitit])) + (:require [meta-merge.core :refer [meta-merge]] + [reitit.core :as reitit])) (defprotocol ExpandMiddleware (expand-middleware [this])) @@ -19,25 +20,78 @@ nil (expand-middleware [_])) -(defn compile-handler [[path {:keys [middleware handler] :as meta}]] - (when-not handler +(defn- ensure-handler! [path meta method] + (when-not (:handler meta) (throw (ex-info - (str "path '" path "' doesn't have a :handler defined") - {:path path, :meta meta}))) - (let [wrap (->> middleware - (keep identity) - (map expand-middleware) - (apply comp identity))] - (wrap handler))) + (str "path \"" path "\" doesn't have a :handler defined" + (if method (str " for method " method))) + {:path path, :method method, :meta meta})))) -(defn router [data] +(defn- compose-middleware [middleware] + (->> middleware + (keep identity) + (map expand-middleware) + (apply comp identity))) + +(defn- compile-handler + ([route opts] + (compile-handler route opts nil)) + ([[path {:keys [middleware handler] :as meta}] _ method] + (ensure-handler! path meta method) + ((compose-middleware middleware) handler))) + +(defn simple-router [data] (reitit/router data {:compile compile-handler})) (defn ring-handler [router] - (fn - ([request] - (if-let [match (reitit/match-by-path router (:uri request))] - ((:handler match) request))) - ([request respond raise] - (if-let [match (reitit/match-by-path router (:uri request))] - ((:handler match) request respond raise))))) + (with-meta + (fn + ([request] + (if-let [match (reitit/match-by-path router (:uri request))] + ((:handler match) request))) + ([request respond raise] + (if-let [match (reitit/match-by-path router (:uri request))] + ((:handler match) request respond raise)))) + {::router router})) + +(defn get-router [handler] + (some-> handler meta ::router)) + +(def http-methods #{:get :head :patch :delete :options :post :put}) +(defrecord MethodHandlers [get head patch delete options post put]) + +(defn- group-keys [meta] + (reduce-kv + (fn [[top childs] k v] + (if (http-methods k) + [top (assoc childs k v)] + [(assoc top k v) childs])) [{} {}] meta)) + +(defn coerce-method-handler [[path meta] {:keys [expand]}] + [path (reduce + (fn [acc method] + (if (contains? acc method) + (update acc method expand) + acc)) meta http-methods)]) + +(defn compile-method-handler [[path meta] opts] + (let [[top childs] (group-keys meta)] + (if-not (seq childs) + (compile-handler [path meta] opts) + (let [handlers (map->MethodHandlers + (reduce-kv + #(assoc %1 %2 (compile-handler [path (meta-merge top %3)] opts %2)) + {} childs)) + default-handler (if (:handler top) (compile-handler [path meta] opts)) + resolved-handler (fn [method] (or (method handlers) default-handler))] + (fn + ([request] + (if-let [handler (resolved-handler (:request-method request))] + (handler request))) + ([request respond raise] + (if-let [handler (resolved-handler (:request-method request))] + (handler request respond raise)))))))) + +(defn method-router [data] + (reitit/router data {:coerce coerce-method-handler + :compile compile-method-handler})) diff --git a/test/cljc/reitit/core_test.cljc b/test/cljc/reitit/core_test.cljc index 4eb57e69..9489bab4 100644 --- a/test/cljc/reitit/core_test.cljc +++ b/test/cljc/reitit/core_test.cljc @@ -1,5 +1,5 @@ (ns reitit.core-test - (:require [clojure.test :refer [deftest testing is are]] + (:require [clojure.test :refer [deftest testing is]] [reitit.core :as reitit #?@(:cljs [:refer [Match LinearRouter LookupRouter]])]) #?(:clj (:import (reitit.core Match LinearRouter LookupRouter) diff --git a/test/cljc/reitit/ring_test.cljc b/test/cljc/reitit/ring_test.cljc index cbf95424..60eb92a2 100644 --- a/test/cljc/reitit/ring_test.cljc +++ b/test/cljc/reitit/ring_test.cljc @@ -1,6 +1,5 @@ (ns reitit.ring-test - (:require [clojure.test :refer [deftest testing is are]] - [reitit.core :as reitit] + (:require [clojure.test :refer [deftest testing is]] [reitit.ring :as ring]) #?(:clj (:import (clojure.lang ExceptionInfo)))) @@ -20,45 +19,110 @@ (deftest ring-test - (testing "all paths should have a handler" - (is (thrown-with-msg? - ExceptionInfo - #"^path '/ping' doesn't have a :handler defined$" - (ring/router ["/ping"])))) + (testing "simple-router" - (testing "ring-handler" - (let [api-mw #(mw % :api) - handler (fn handle - ([{:keys [::mw]}] - {:status 200 :body (conj mw :ok)}) - ([request respond raise] - (respond (handle request)))) - app (ring/ring-handler - (ring/router - [["/ping" handler] - ["/api" {:middleware [api-mw]} - ["/ping" handler] - ["/admin" {:middleware [[mw :admin]]} - ["/ping" handler]]]]))] + (testing "all paths should have a handler" + (is (thrown-with-msg? + ExceptionInfo + #"path \"/ping\" doesn't have a :handler defined" + (ring/simple-router ["/ping"])))) - (testing "normal handler" - (is (= {:status 200, :body [:ok]} - (app {:uri "/ping"})))) + (testing "ring-handler" + (let [api-mw #(mw % :api) + handler (fn handle + ([{:keys [::mw]}] + {:status 200 :body (conj mw :ok)}) + ([request respond raise] + (respond (handle request)))) + router (ring/simple-router + [["/ping" handler] + ["/api" {:middleware [api-mw]} + ["/ping" handler] + ["/admin" {:middleware [[mw :admin]]} + ["/ping" handler]]]]) + app (ring/ring-handler router)] - (testing "with middleware" - (is (= {:status 200, :body [:api :ok :api]} - (app {:uri "/api/ping"})))) + (testing "router can be extracted" + (is (= router (ring/get-router app)))) - (testing "with nested middleware" - (is (= {:status 200, :body [:api :admin :ok :admin :api]} - (app {:uri "/api/admin/ping"})))) + (testing "not found" + (is (= nil (app {:uri "/favicon.ico"})))) - (testing "not found" - (is (= nil (app {:uri "/favicon.ico"})))) + (testing "normal handler" + (is (= {:status 200, :body [:ok]} + (app {:uri "/ping"})))) - (testing "3-arity" - (let [result (atom nil) - respond (partial reset! result), raise ::not-called] - (app {:uri "/api/admin/ping"} respond raise) + (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]} - @result))))))) + (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))))))) + + (testing "method-router" + + (testing "all paths should have a handler" + (is (thrown-with-msg? + ExceptionInfo + #"path \"/ping\" doesn't have a :handler defined for method :get" + (ring/method-router ["/ping" {:get {}}])))) + + (testing "ring-handler" + (let [api-mw #(mw % :api) + handler (fn handle + ([{:keys [::mw]}] + {:status 200 :body (conj mw :ok)}) + ([request respond raise] + (respond (handle request)))) + router (ring/method-router + [["/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" + (is (= router (ring/get-router app)))) + + (testing "not found" + (is (= nil (app {:uri "/favicon.ico"})))) + + (testing "catch all handler" + (is (= {:status 200, :body [:api :ok :api]} + (app {:uri "/api/all" :request-method :get})))) + + (testing "just get handler" + (is (= {:status 200, :body [:api :ok :api]} + (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]} + (app {:uri "/api/users" :request-method :get})))) + + (testing "method handler with middleware" + (is (= {:status 200, :body [:api :users :post :ok :post :users :api]} + (app {:uri "/api/users" :request-method :post})))) + + (testing "fallback handler" + (is (= {:status 200, :body [:api :users :ok :users :api]} + (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]} + @result)))))))) From d21f051868f339e55b70116fac8d83ff6442a350 Mon Sep 17 00:00:00 2001 From: Tommi Reiman Date: Mon, 14 Aug 2017 09:41:48 +0300 Subject: [PATCH 5/6] Review fix by @miikka --- src/reitit/core.cljc | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/reitit/core.cljc b/src/reitit/core.cljc index 2c039192..d0056ba2 100644 --- a/src/reitit/core.cljc +++ b/src/reitit/core.cljc @@ -56,10 +56,8 @@ {} x)) (defn resolve-routes [data {:keys [coerce] :as opts}] - (cond-> (->> (walk data opts) - (map-meta merge-meta)) - coerce (->> (mapv #(coerce % opts)) - (filterv identity)))) + (cond->> (->> (walk data opts) (map-meta merge-meta)) + coerce (into [] (keep #(coerce % opts))))) (defn compile-route [[p m :as route] {:keys [compile] :as opts}] [p m (if compile (compile route opts))]) From cf06a1757d4efa33c4818212a86ff88df16f9e56 Mon Sep 17 00:00:00 2001 From: Tommi Reiman Date: Mon, 14 Aug 2017 09:50:12 +0300 Subject: [PATCH 6/6] Cleanup tests --- test/cljc/reitit/ring_test.cljc | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/test/cljc/reitit/ring_test.cljc b/test/cljc/reitit/ring_test.cljc index 60eb92a2..4a7a1783 100644 --- a/test/cljc/reitit/ring_test.cljc +++ b/test/cljc/reitit/ring_test.cljc @@ -17,6 +17,12 @@ #(respond (update % :body (fnil conj []) name)) raise)))) +(defn handler + ([{:keys [::mw]}] + {:status 200 :body (conj mw :ok)}) + ([request respond raise] + (respond (handler request)))) + (deftest ring-test (testing "simple-router" @@ -29,11 +35,6 @@ (testing "ring-handler" (let [api-mw #(mw % :api) - handler (fn handle - ([{:keys [::mw]}] - {:status 200 :body (conj mw :ok)}) - ([request respond raise] - (respond (handle request)))) router (ring/simple-router [["/ping" handler] ["/api" {:middleware [api-mw]} @@ -77,11 +78,6 @@ (testing "ring-handler" (let [api-mw #(mw % :api) - handler (fn handle - ([{:keys [::mw]}] - {:status 200 :body (conj mw :ok)}) - ([request respond raise] - (respond (handle request)))) router (ring/method-router [["/api" {:middleware [api-mw]} ["/all" handler]