From 066f5752c2b62ebce044da690528e33ece753578 Mon Sep 17 00:00:00 2001 From: Tommi Reiman Date: Mon, 21 Aug 2017 20:44:16 +0300 Subject: [PATCH] Return all conflicts for IO --- README.md | 2 +- src/reitit/core.cljc | 27 ++++++++++++++++----------- test/cljc/reitit/core_test.cljc | 18 ++++++++++++------ 3 files changed, 29 insertions(+), 18 deletions(-) diff --git a/README.md b/README.md index ca5a865f..817b1126 100644 --- a/README.md +++ b/README.md @@ -400,7 +400,7 @@ Routers can be configured via options. Options allow things like [`clojure.spec` | `: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!`)" + | `:conflicts` | Function of `{route #{route}} => side-effect` to handle conflicting routes (default `reitit.core/throw-on-conflicts!`)" ## Special thanks diff --git a/src/reitit/core.cljc b/src/reitit/core.cljc index a17bfb2e..3417f931 100644 --- a/src/reitit/core.cljc +++ b/src/reitit/core.cljc @@ -60,17 +60,22 @@ (cond->> (->> (walk data opts) (map-meta merge-meta)) coerce (into [] (keep #(coerce % opts))))) -(defn first-conflicting-routes [routes] - (loop [[r & rest] routes] - (if (seq rest) - (or (some #(if (impl/conflicting-routes? r %) [r %]) rest) - (recur rest))))) +(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! [routes] +(defn throw-on-conflicts! [conflicts] (throw (ex-info - (str "router contains conflicting routes: " routes) - {:routes routes}))) + (str "router contains conflicting routes: " conflicts) + {:conflicts conflicts}))) (defn name-lookup [[_ {:keys [name]}] opts] (if name #{name})) @@ -206,14 +211,14 @@ | `: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!`)" + | `:conflicts` | Function of `{route #{route}} => side-effect` to handle conflicting routes (default `reitit.core/throw-on-conflicts!`)" ([data] (router data {})) ([data opts] (let [opts (meta-merge default-router-options opts) routes (resolve-routes data opts)] (when-let [conflicts (:conflicts opts)] - (when-let [conflicting-routes (first-conflicting-routes routes)] - (conflicts conflicting-routes))) + (when-let [conflicting (conflicting-routes routes)] + (conflicts conflicting))) ((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 0195bcda..6034767d 100644 --- a/test/cljc/reitit/core_test.cljc +++ b/test/cljc/reitit/core_test.cljc @@ -139,12 +139,11 @@ :params {:id "1", :sub-id "2"}}) (reitit/match-by-path router "/api/user/1/2")))))) -(deftest first-conflicting-routes-test +(deftest conflicting-routes-test (are [conflicting? data] - (let [routes (reitit/resolve-routes data {})] - (= (if conflicting? routes) - (reitit/first-conflicting-routes - (reitit/resolve-routes routes {})))) + (let [routes (reitit/resolve-routes data {}) + conflicts (-> routes (reitit/resolve-routes {}) (reitit/conflicting-routes))] + (if conflicting? (seq conflicts) (nil? conflicts))) true [["/a"] ["/a"]] @@ -173,6 +172,14 @@ 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? @@ -186,4 +193,3 @@ (reitit/router [["/a"] ["/a"]] {:conflicts (constantly nil)}))))))) -