Allow path conflict resolution with :conflicting

This commit is contained in:
Ilmo Raunio 2019-11-09 01:11:55 +02:00 committed by Tommi Reiman
parent 5fc86737b8
commit 8a86701902
6 changed files with 92 additions and 32 deletions

View file

@ -2,7 +2,7 @@
We should fail fast if a router contains conflicting paths or route names. 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 ## Path Conflicts
@ -25,14 +25,14 @@ Creating router with defaults:
(r/router routes) (r/router routes)
; CompilerException clojure.lang.ExceptionInfo: Router contains conflicting route paths: ; CompilerException clojure.lang.ExceptionInfo: Router contains conflicting route paths:
; ;
; /:user-id/orders ; -> /:user-id/orders
; -> /public/*path ; -> /public/*path
; -> /bulk/:bulk-id ; -> /bulk/:bulk-id
; ;
; /bulk/:bulk-id ; -> /bulk/:bulk-id
; -> /:version/status ; -> /:version/status
; ;
; /public/*path ; -> /public/*path
; -> /:version/status ; -> /:version/status
; ;
``` ```
@ -43,29 +43,45 @@ To ignore the conflicts:
(r/router (r/router
routes routes
{:conflicts nil}) {:conflicts nil})
; => #object[reitit.core$linear_router$reify] ; => #object[reitit.core$quarantine_router$reify
``` ```
To just log the conflicts: To just log the conflicts:
```clj ```clj
(require '[reitit.exception :as exception])
(r/router (r/router
routes routes
{:conflicts (fn [conflicts] {:conflicts (fn [conflicts]
(println (r/path-conflicts-str conflicts)))}) (println (exception/format-exception :path-conflicts nil conflicts)))})
; Router contains conflicting route paths: ; Router contains conflicting route paths:
; ;
; /:user-id/orders ; -> /:user-id/orders
; -> /public/*path ; -> /public/*path
; -> /bulk/:bulk-id ; -> /bulk/:bulk-id
; ;
; /bulk/:bulk-id ; -> /bulk/:bulk-id
; -> /:version/status ; -> /:version/status
; ;
; /public/*path ; -> /public/*path
; -> /:version/status ; -> /: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 ## Name conflicts

View file

@ -381,7 +381,9 @@
:else mixed-router)] :else mixed-router)]
(when-let [conflicts (:conflicts opts)] (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 (when name-conflicting
(exception/fail! :name-conflicts name-conflicting)) (exception/fail! :name-conflicts name-conflicting))

View file

@ -26,11 +26,18 @@
(str message (if data (str "\n\n" (pr-str data))))) (str message (if data (str "\n\n" (pr-str data)))))
(defmethod format-exception :path-conflicts [_ _ conflicts] (defmethod format-exception :path-conflicts [_ _ conflicts]
(apply str "Router contains conflicting route paths:\n\n" (letfn [(resolve-str [path route-data]
(mapv (str (if (:conflicting route-data) " " "-> ")
(fn [[[path] vals]] path " " (not-empty (select-keys route-data [:conflicting]))))]
(str " " path "\n-> " (str/join "\n-> " (mapv first vals)) "\n\n")) (apply str "Router contains conflicting route paths:\n\n"
conflicts))) (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] (defmethod format-exception :name-conflicts [_ _ conflicts]
(apply str "Router contains conflicting route names:\n\n" (apply str "Router contains conflicting route names:\n\n"

View file

@ -85,6 +85,15 @@
routes) routes)
(not-empty))) (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] (defn conflicting-paths [conflicts]
(->> (for [[p pc] conflicts] (->> (for [[p pc] conflicts]
(conj (map first pc) (first p))) (conj (map first pc) (first p)))

View file

@ -270,22 +270,29 @@
[:group [:group
(text "Router contains conflicting route paths:") (text "Router contains conflicting route paths:")
[:break] [:break] [:break] [:break]
(into (letfn [(path-report [path route-data]
[:group] [:span (color :grey
(mapv (if (:conflicting route-data) " " "-> ")
(fn [[[path] vals]] path
[:group " ")
[:span " " (text path)] (edn (not-empty (select-keys route-data [:conflicting])))])]
[:break] (into
(into [:group]
[:group] (mapv
(map (fn [[[path route-data] vals]]
(fn [p] [:span (color :grey "-> " p) [:break]]) [:group
(mapv first vals))) (path-report path route-data)
[:break]]) (into
conflicts)) [: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") [: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})] (edn {:conflicts nil} {:margin 3})]
[:break] [:break]
(color :white "https://cljdoc.org/d/metosin/reitit/CURRENT/doc/basics/route-conflicts") (color :white "https://cljdoc.org/d/metosin/reitit/CURRENT/doc/basics/route-conflicts")

View file

@ -337,7 +337,26 @@
#"Router contains conflicting route paths" #"Router contains conflicting route paths"
(r/router (r/router
[["/a"] ["/a"]])))) [["/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}))))))) (is (not (nil? (r/router [["/a"] ["/a"]] {:conflicts nil})))))))
(testing "name conflicts" (testing "name conflicts"