diff --git a/README.md b/README.md index 7cad9a20..9bf269e9 100644 --- a/README.md +++ b/README.md @@ -392,14 +392,16 @@ Authorized access to guarded route: Routers can be configured via options. Options allow things like [`clojure.spec`](https://clojure.org/about/spec) validation for meta-data and fast, compiled handlers. The following options are available for the `reitit.core/router`: - | key | description | - | -----------|-------------| - | `:path` | Base-path for routes (default `""`) - | `: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`) - | `: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 + | key | description | + | -------------|-------------| + | `:path` | Base-path for routes (default `""`) + | `: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`) + | `: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 + | `: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 ## Special thanks diff --git a/perf-test/clj/reitit/opensensors_routing_test.clj b/perf-test/clj/reitit/opensensors_routing_test.clj index d25b7942..5487e043 100644 --- a/perf-test/clj/reitit/opensensors_routing_test.clj +++ b/perf-test/clj/reitit/opensensors_routing_test.clj @@ -120,7 +120,7 @@ ["/v1/orgs/:org-id/errors" {:handler handler, :name :test/route17}] ["/v1/public/orgs/:org-id" {:handler handler, :name :test/route18}] ["/v1/orgs/:org-id/invitations" {:handler handler, :name :test/route19}] - ["/v2/public/messages/dataset/bulk" {:handler handler, :name :test/route20}] + #_["/v2/public/messages/dataset/bulk" {:handler handler, :name :test/route20}] #_["/v1/users/:user-id/devices/bulk" {:handler handler, :name :test/route21}] ["/v1/users/:user-id/device-errors" {:handler handler, :name :test/route22}] ["/v2/login" {:handler handler, :name :test/route23}] @@ -202,13 +202,13 @@ "whoami" :test/route41 "login" :test/route51} "v2/" {"whoami" :test/route1 - ["users/" :user-id "/"] {"datasets" :test/route2 - "devices" :test/route25 - "topics" {"/bulk" :test/route29 - "" :test/route45}} + ["users/" :user-id] {"/datasets" :test/route2 + "/devices" :test/route25 + "/topics" {"/bulk" :test/route29 + "" :test/route45}} "public/" {["projects/" :project-id] {"/datasets" :test/route3 "" :test/route27} - "messages/dataset/bulk" :test/route20 + #_#_"messages/dataset/bulk" :test/route20 ["datasets/" :dataset-id] :test/route28 ["messages/dataset/" :dataset-id] :test/route53} ["datasets/" :dataset-id] :test/route11 @@ -264,11 +264,11 @@ "/v2/" {"whoami" [:test/route1] ["users/" user-id] {"/datasets" [:test/route2 user-id] "/devices" [:test/route25 user-id] - "topics/" {"bulk" [:test/route29 user-id] + "/topics" {"/bulk" [:test/route29 user-id] "" [:test/route45 user-id]}} "public/" {["projects/" project-id] {"/datasets" [:test/route3 project-id] "" [:test/route27 project-id]} - "messages/dataset/bulk" [:test/route20] + #_#_"messages/dataset/bulk" [:test/route20] ["datasets/" dataset-id] [:test/route28 dataset-id] ["messages/dataset/" dataset-id] [:test/route53 dataset-id]} ["datasets/" dataset-id] [:test/route11 dataset-id] @@ -344,7 +344,7 @@ (context "/projects/:project-id" [] (ANY "/datasets" [] {:name :test/route3} handler) (ANY "/" [] {:name :test/route27} handler)) - (ANY "/messages/dataset/bulk" [] {:name :test/route20} handler) + #_(ANY "/messages/dataset/bulk" [] {:name :test/route20} handler) (ANY "/datasets/:dataset-id" [] {:name :test/route28} handler) (ANY "/messages/dataset/:dataset-id" [] {:name :test/route53} handler)) (ANY "/datasets/:dataset-id" [] {:name :test/route11} handler) @@ -376,7 +376,7 @@ ["/v1/orgs/:org-id/errors" :get handler :route-name :test/route17] ["/v1/public/orgs/:org-id" :get handler :route-name :test/route18] ["/v1/orgs/:org-id/invitations" :get handler :route-name :test/route19] - ["/v2/public/messages/dataset/bulk" :get handler :route-name :test/route20] + #_["/v2/public/messages/dataset/bulk" :get handler :route-name :test/route20] #_["/v1/users/:user-id/devices/bulk" :get handler :route-name :test/route21] ["/v1/users/:user-id/device-errors" :get handler :route-name :test/route22] ["/v2/login" :get handler :route-name :test/route23] @@ -493,12 +493,12 @@ 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 10% - (bench! routes true "pedestal" pedestal-f) ;; 2737ns 11% - (bench! routes true "reitit-ring" reitit-ring-f) ;; 2845ns 11% - (bench! routes true "compojure-api" compojure-api-f) ;; 10215ns 41% - (bench! routes true "bidi" bidi-f) ;; 19298ns 77% - (bench! routes true "ataraxy" ataraxy-f) ;; 24950ns 100% + (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 )) diff --git a/src/reitit/core.cljc b/src/reitit/core.cljc index 6ee1c63a..003c4040 100644 --- a/src/reitit/core.cljc +++ b/src/reitit/core.cljc @@ -1,5 +1,6 @@ (ns reitit.core (:require [meta-merge.core :refer [meta-merge]] + [clojure.string :as str] [reitit.impl :as impl #?@(:cljs [:refer [Route]])]) #?(:clj (:import (reitit.impl Route)))) @@ -60,16 +61,38 @@ (cond->> (->> (walk data opts) (map-meta merge-meta)) coerce (into [] (keep #(coerce % opts))))) +;; This whole function might be more efficient and easier to understand with transducers. +(defn conflicting-routes [routes] + (some->> + (loop [[r & rest] routes, acc {}] + (if (seq rest) + (let [conflicting (set (keep #(if (impl/conflicting-routes? r %) %) rest))] + (recur rest (update acc r (fnil (comp set concat) #{}) conflicting))) + acc)) + (filter (comp seq second)) + (seq) + (into {}))) + +(defn throw-on-conflicts! [conflicts] + (throw + (ex-info + (apply str "router contains conflicting routes:\n\n" + (mapv + (fn [[[path] vals]] + (str " " path "\n-> " (str/join "\n-> " (mapv first vals)) "\n\n")) + conflicts)) + {:conflicts conflicts}))) + (defn name-lookup [[_ {:keys [name]}] opts] (if name #{name})) (defn find-names [routes opts] - (into [] (keep #(-> % second :name) routes))) + (into [] (keep #(-> % second :name)) routes)) (defn compile-route [[p m :as route] {:keys [compile] :as opts}] [p m (if compile (compile route opts))]) -(defprotocol Routing +(defprotocol Router (router-type [this]) (routes [this]) (options [this]) @@ -97,10 +120,11 @@ {:lookup name-lookup :expand expand :coerce (fn [route _] route) - :compile (fn [[_ {:keys [handler]}] _] handler)}) + :compile (fn [[_ {:keys [handler]}] _] handler) + :conflicts throw-on-conflicts!}) (defn linear-router - "Creates a [[LinearRouter]] from resolved routes and optional + "Creates a LinearRouter from resolved routes and optional expanded options. See [[router]] for available options" ([routes] (linear-router routes {})) @@ -118,7 +142,7 @@ [[] {}] compiled) lookup (impl/fast-map lookup)] (reify - Routing + Router (router-type [_] :linear-router) (routes [_] @@ -146,11 +170,11 @@ ([routes] (lookup-router routes {})) ([routes opts] - (when-let [route (some impl/contains-wilds? (map first routes))] + (when-let [wilds (seq (filter impl/wild-route? routes))] (throw (ex-info - (str "can't create LookupRouter with wildcard routes: " route) - {:route route + (str "can't create LookupRouter with wildcard routes: " wilds) + {:wilds wilds :routes routes}))) (let [compiled (map #(compile-route % opts) routes) names (find-names routes opts) @@ -162,7 +186,7 @@ lookup)]) [{} {}] compiled) data (impl/fast-map data) lookup (impl/fast-map lookup)] - (reify Routing + (reify Router (router-type [_] :lookup-router) (routes [_] @@ -180,23 +204,68 @@ (if-let [match (impl/fast-get lookup name)] (match params))))))) +(defn mixed-router + "Creates two routers: [[lookup-router]] for static routes and + [[linear-router]] for wildcard routes. All routes should be + non-conflicting. Takes resolved routes and optional + expanded options. See [[router]] for options." + ([routes] + (mixed-router routes {})) + ([routes opts] + (let [{linear true, lookup false} (group-by impl/wild-route? routes) + linear-router (linear-router linear opts) + lookup-router (lookup-router lookup opts) + names (find-names routes opts)] + (reify Router + (router-type [_] + :mixed-router) + (routes [_] + routes) + (options [_] + opts) + (route-names [_] + names) + (match-by-path [_ path] + (or (match-by-path lookup-router path) + (match-by-path linear-router path))) + (match-by-name [_ name] + (or (match-by-name lookup-router name) + (match-by-name linear-router name))) + (match-by-name [_ name params] + (or (match-by-name lookup-router name params) + (match-by-name linear-router name params))))))) + (defn router "Create a [[Router]] from raw route data and optionally an options map. If routes contain wildcards, a [[LinearRouter]] is used, otherwise a [[LookupRouter]]. The following options are available: - | key | description | - | -----------|-------------| - | `:path` | Base-path for routes (default `\"\"`) - | `: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`) - | `: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" + | key | description | + | -------------|-------------| + | `:path` | Base-path for routes (default `\"\"`) + | `: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`) + | `: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 + | `: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] (router data {})) ([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)))) + (let [{:keys [router] :as opts} (meta-merge default-router-options opts) + routes (resolve-routes data opts) + conflicting (conflicting-routes routes) + wilds? (some impl/wild-route? routes) + all-wilds? (every? impl/wild-route? routes) + router (cond + router router + (not wilds?) lookup-router + all-wilds? linear-router + (not conflicting) mixed-router + :else linear-router)] + + (when-let [conflicts (:conflicts opts)] + (when conflicting (conflicts conflicting))) + + (router routes opts)))) diff --git a/src/reitit/impl.cljc b/src/reitit/impl.cljc index 0e95c253..217a48fb 100644 --- a/src/reitit/impl.cljc +++ b/src/reitit/impl.cljc @@ -121,6 +121,28 @@ :matcher #(if (= path %) {}) :handler handler}))) +(defn segments [path] + (let [ss (-> (str/split path #"/") rest vec)] + (if (str/ends-with? path "/") + (conj ss "") ss))) + +(defn- catch-all? [segment] + (= \* (first segment))) + +(defn wild-route? [[path]] + (contains-wilds? path)) + +(defn conflicting-routes? [[p1 :as route1] [p2 :as route2]] + (loop [[s1 & ss1] (segments p1) + [s2 & ss2] (segments p2)] + (cond + (= s1 s2 nil) true + (or (nil? s1) (nil? s2)) false + (or (catch-all? s1) (catch-all? s2)) true + (or (wild? s1) (wild? s2)) (recur ss1 ss2) + (not= s1 s2) false + :else (recur ss1 ss2)))) + (defn path-for [^Route route params] (if-let [required (:params route)] (if (every? #(contains? params %) required) diff --git a/test/cljc/reitit/core_test.cljc b/test/cljc/reitit/core_test.cljc index ba8c0dbf..d292fc23 100644 --- a/test/cljc/reitit/core_test.cljc +++ b/test/cljc/reitit/core_test.cljc @@ -1,13 +1,13 @@ (ns reitit.core-test - (:require [clojure.test :refer [deftest testing is]] - [reitit.core :as reitit #?@(:cljs [:refer [Match]])]) + (:require [clojure.test :refer [deftest testing is are]] + [reitit.core :as reitit #?@(:cljs [:refer [Match Router]])]) #?(:clj - (:import (reitit.core Match) + (:import (reitit.core Match Router) (clojure.lang ExceptionInfo)))) (deftest reitit-test - (testing "linear router" + (testing "linear-router" (let [router (reitit/router ["/api" ["/ipa" ["/:size" ::beer]]])] (is (= :linear-router (reitit/router-type router))) (is (= [["/api/ipa/:size" {:name ::beer}]] @@ -40,7 +40,7 @@ #"^missing path-params for route /api/ipa/:size: \#\{:size\}$" (reitit/match-by-name! router ::beer)))))) - (testing "lookup router" + (testing "lookup-router" (let [router (reitit/router ["/api" ["/ipa" ["/large" ::beer]]])] (is (= :lookup-router (reitit/router-type router))) (is (= [["/api/ipa/large" {:name ::beer}]] @@ -106,6 +106,13 @@ (is handler) (is (= "ok" (handler))))))) + (testing "custom router" + (let [router (reitit/router ["/ping"] {:router (fn [_ _] + (reify Router + (reitit/router-type [_] + ::custom)))})] + (is (= ::custom (reitit/router-type router))))) + (testing "bide sample" (let [routes [["/auth/login" :auth/login] ["/auth/recovery/token/:token" :auth/recovery] @@ -138,3 +145,54 @@ :path "/api/user/1/2" :params {:id "1", :sub-id "2"}}) (reitit/match-by-path router "/api/user/1/2")))))) + +(deftest conflicting-routes-test + (are [conflicting? data] + (let [routes (reitit/resolve-routes data {}) + conflicts (-> routes (reitit/resolve-routes {}) (reitit/conflicting-routes))] + (if conflicting? (seq conflicts) (nil? conflicts))) + + true [["/a"] + ["/a"]] + + true [["/a"] + ["/:b"]] + + true [["/a"] + ["/*b"]] + + true [["/a/1/2"] + ["/*b"]] + + false [["/a"] + ["/a/"]] + + false [["/a"] + ["/a/1"]] + + false [["/a"] + ["/a/:b"]] + + false [["/a"] + ["/a/*b"]] + + true [["/v2/public/messages/dataset/bulk"] + ["/v2/public/messages/dataset/:dataset-id"]]) + + (testing "all conflicts are returned" + (is (= {["/a" {}] #{["/*d" {}] ["/:b" {}]}, + ["/:b" {}] #{["/c" {}] ["/*d" {}]}, + ["/c" {}] #{["/*d" {}]}} + (-> [["/a"] ["/:b"] ["/c"] ["/*d"]] + (reitit/resolve-routes {}) + (reitit/conflicting-routes))))) + + (testing "router with conflicting routes" + (testing "throws by default" + (is (thrown-with-msg? + ExceptionInfo + #"router contains conflicting routes" + (reitit/router + [["/a"] ["/a"]])))) + (testing "can be configured to ignore" + (is (not (nil? (reitit/router [["/a"] ["/a"]] {:conflicts (constantly nil)})))))))