diff --git a/doc/basics/route_conflicts.md b/doc/basics/route_conflicts.md index 4f6a02c9..8b2a1089 100644 --- a/doc/basics/route_conflicts.md +++ b/doc/basics/route_conflicts.md @@ -2,7 +2,7 @@ We should fail fast if a router contains conflicting paths or route names. -When a `Router` is created via `reitit.core/router`, both path and route name conflicts are checked automatically. By default, in case of conflict, an `ex-info` is thrown with a descriptive message. In some (legacy api) cases, path conflicts should be allowed and one can override the path conflict resolution via `:conflicts` router option. +When a `Router` is created via `reitit.core/router`, both path and route name conflicts are checked automatically. By default, in case of conflict, an `ex-info` is thrown with a descriptive message. In some (legacy api) cases, path conflicts should be allowed and one can override the path conflict resolution via `:conflicts` router option or via `:conflicting` route data. ## Path Conflicts @@ -25,14 +25,14 @@ Creating router with defaults: (r/router routes) ; CompilerException clojure.lang.ExceptionInfo: Router contains conflicting route paths: ; -; /:user-id/orders +; -> /:user-id/orders ; -> /public/*path ; -> /bulk/:bulk-id ; -; /bulk/:bulk-id +; -> /bulk/:bulk-id ; -> /:version/status ; -; /public/*path +; -> /public/*path ; -> /:version/status ; ``` @@ -43,29 +43,45 @@ To ignore the conflicts: (r/router routes {:conflicts nil}) -; => #object[reitit.core$linear_router$reify] +; => #object[reitit.core$quarantine_router$reify ``` To just log the conflicts: ```clj +(require '[reitit.exception :as exception]) + (r/router routes {:conflicts (fn [conflicts] - (println (r/path-conflicts-str conflicts)))}) + (println (exception/format-exception :path-conflicts nil conflicts)))}) ; Router contains conflicting route paths: ; -; /:user-id/orders +; -> /:user-id/orders ; -> /public/*path ; -> /bulk/:bulk-id ; -; /bulk/:bulk-id +; -> /bulk/:bulk-id ; -> /:version/status ; -; /public/*path +; -> /public/*path ; -> /:version/status ; -; => #object[reitit.core$linear_router$reify] +; => #object[reitit.core$quarantine_router$reify] +``` + +Alternatively, you can ignore conflicting paths individually via `:conflicting` in route data: + +```clj +(def routes + [["/ping"] + ["/:user-id/orders" {:conflicting true}] + ["/bulk/:bulk-id" {:conflicting true}] + ["/public/*path" {:conflicting true}] + ["/:version/status" {:conflicting true}]]) +; => #'user/routes +(r/router routes) +; => #object[reitit.core$quarantine_router$reify] ``` ## Name conflicts diff --git a/modules/reitit-core/src/reitit/core.cljc b/modules/reitit-core/src/reitit/core.cljc index 26f61ab5..a582badb 100644 --- a/modules/reitit-core/src/reitit/core.cljc +++ b/modules/reitit-core/src/reitit/core.cljc @@ -381,7 +381,9 @@ :else mixed-router)] (when-let [conflicts (:conflicts opts)] - (when path-conflicting (conflicts path-conflicting))) + (when-let [conflict-report (impl/unresolved-conflicts + path-conflicting)] + (conflicts conflict-report))) (when name-conflicting (exception/fail! :name-conflicts name-conflicting)) diff --git a/modules/reitit-core/src/reitit/exception.cljc b/modules/reitit-core/src/reitit/exception.cljc index 81a6c693..ff848910 100644 --- a/modules/reitit-core/src/reitit/exception.cljc +++ b/modules/reitit-core/src/reitit/exception.cljc @@ -26,11 +26,18 @@ (str message (if data (str "\n\n" (pr-str data))))) (defmethod format-exception :path-conflicts [_ _ conflicts] - (apply str "Router contains conflicting route paths:\n\n" - (mapv - (fn [[[path] vals]] - (str " " path "\n-> " (str/join "\n-> " (mapv first vals)) "\n\n")) - conflicts))) + (letfn [(resolve-str [path route-data] + (str (if (:conflicting route-data) " " "-> ") + path " " (not-empty (select-keys route-data [:conflicting]))))] + (apply str "Router contains conflicting route paths:\n\n" + (mapv + (fn [[[path route-data] vals]] + (str (resolve-str path route-data) + "\n" + (str/join "\n" (mapv (fn [[path route-data]] + (resolve-str path route-data)) vals)) + "\n\n")) + conflicts)))) (defmethod format-exception :name-conflicts [_ _ conflicts] (apply str "Router contains conflicting route names:\n\n" diff --git a/modules/reitit-core/src/reitit/impl.cljc b/modules/reitit-core/src/reitit/impl.cljc index 0fddc92b..c676a49d 100644 --- a/modules/reitit-core/src/reitit/impl.cljc +++ b/modules/reitit-core/src/reitit/impl.cljc @@ -85,6 +85,15 @@ routes) (not-empty))) +(defn unresolved-conflicts [path-conflicting] + (-> (into {} + (remove (fn [[[_ route-data] conflicts]] + (and (:conflicting route-data) + (every? (comp :conflicting second) + conflicts))) + path-conflicting)) + (not-empty))) + (defn conflicting-paths [conflicts] (->> (for [[p pc] conflicts] (conj (map first pc) (first p))) diff --git a/modules/reitit-dev/src/reitit/dev/pretty.cljc b/modules/reitit-dev/src/reitit/dev/pretty.cljc index dfec05e4..7a60664c 100644 --- a/modules/reitit-dev/src/reitit/dev/pretty.cljc +++ b/modules/reitit-dev/src/reitit/dev/pretty.cljc @@ -270,22 +270,29 @@ [:group (text "Router contains conflicting route paths:") [:break] [:break] - (into - [:group] - (mapv - (fn [[[path] vals]] - [:group - [:span " " (text path)] - [:break] - (into - [:group] - (map - (fn [p] [:span (color :grey "-> " p) [:break]]) - (mapv first vals))) - [:break]]) - conflicts)) + (letfn [(path-report [path route-data] + [:span (color :grey + (if (:conflicting route-data) " " "-> ") + path + " ") + (edn (not-empty (select-keys route-data [:conflicting])))])] + (into + [:group] + (mapv + (fn [[[path route-data] vals]] + [:group + (path-report path route-data) + (into + [:group] + (map + (fn [[path route-data]] (path-report path route-data)) + vals)) + [:break]]) + conflicts))) [:span (text "Either fix the conflicting paths or disable the conflict resolution") - [:break] (text "by setting a router option: ") [:break] [:break] + [:break] (text "by setting route data for conflicting route: ") [:break] [:break] + (edn {:conflicting true} {:margin 3}) + [:break] (text "or by setting a router option: ") [:break] [:break] (edn {:conflicts nil} {:margin 3})] [:break] (color :white "https://cljdoc.org/d/metosin/reitit/CURRENT/doc/basics/route-conflicts") diff --git a/test/cljc/reitit/core_test.cljc b/test/cljc/reitit/core_test.cljc index b29fc18c..2c6f6458 100644 --- a/test/cljc/reitit/core_test.cljc +++ b/test/cljc/reitit/core_test.cljc @@ -337,7 +337,26 @@ #"Router contains conflicting route paths" (r/router [["/a"] ["/a"]])))) - (testing "can be configured to ignore" + (testing "can be configured to ignore with route data" + (are [paths expected] + (let [router (r/router paths)] + (is (not (nil? router))) + (is (= expected (r/router-name router)))) + [["/a" {:conflicting true}] + ["/a" {:conflicting true}]] :quarantine-router + [["/a" {:conflicting true}] + ["/:b" {:conflicting true}] + ["/c" {:conflicting true}] + ["/*d" {:conflicting true}]] :quarantine-router) + (testing "unmarked path conflicts throw" + (are [paths] + (is (thrown-with-msg? + ExceptionInfo + #"Router contains conflicting route paths" + (r/router paths))) + [["/a"] ["/a" {:conflicting true}]] + [["/a" {:conflicting true}] ["/a"]]))) + (testing "can be configured to ignore with router option" (is (not (nil? (r/router [["/a"] ["/a"]] {:conflicts nil}))))))) (testing "name conflicts"