Merge pull request #196 from metosin/polished-dev-tools

Polished dev tools

CHANGELOG.md

@@ -4,6 +4,18 @@
 
 * `segment-router` doesn't accept empty segments as path-parameters, fixes [#181](https://github.com/metosin/reitit/issues/181).
 * path-params are decoded correctly with `r/match-by-name`, fixes [#192](https://github.com/metosin/reitit/issues/192).
+* new `:quarantine-router`, which is uses by default if there are any path conflicts: uses internally `:mixed-router` for non-conflicting routes and `:linear-router` for conflicting routes.
+
+```clj
+(-> [["/joulu/kinkku"]   ;; linear router
+     ["/joulu/:torttu"]  ;; linear router
+     ["/tonttu/:id"]     ;; segment-router
+     ["/manna/puuro"]    ;; lookup-router
+     ["/sinappi/silli"]] ;; lookup-router
+    (r/router {:conflicts nil})
+    (r/router-name))
+; => :quarantine-router
+```
 
 * updated deps:
 

doc/advanced/different_routers.md

@@ -9,6 +9,7 @@ Reitit ships with several different implementations for the `Router` protocol, o
 | `:lookup-router`              | Fast router, uses hash-lookup to resolve the route. Valid if no paths have path or catch-all parameters and there are no [Route conflicts](../basics/route_conflicts.md).
 | `:single-static-path-router`  | Super fast router: string-matches a route. Valid only if there is one static route.
 | `:mixed-router`               | Contains two routers: `:segment-router` for wildcard routes and a `:lookup-router` or `:single-static-path-router` for static routes. Valid only if there are no [Route conflicts](../basics/route_conflicts.md).
+| `:quarantine-router`          | Contains two routers: `:mixed-router` for non-conflicting routes and a `:linear-router` for conflicting routes.
 
 The router name can be asked from the router:
 

examples/ring-swagger/src/example/server.clj

@@ -61,7 +61,7 @@
                             {:status 200
                              :body {:total (+ x y)}})}}]]]
 
-      {:reitit.middleware/transform dev/print-request-diffs
+      {;;:reitit.middleware/transform dev/print-request-diffs
        :data {:coercion reitit.coercion.spec/coercion
               :muuntaja m/instance
               :middleware [;; query-params & form-params

modules/reitit-core/src/reitit/core.cljc

@@ -77,6 +77,12 @@
     (seq)
     (into {})))
 
+(defn conflicting-paths [conflicts]
+  (->> (for [[p pc] conflicts]
+         (conj (map first pc) (first p)))
+       (apply concat)
+       (set)))
+
 (defn path-conflicts-str [conflicts]
   (apply str "Router contains conflicting route paths:\n\n"
          (mapv
@@ -167,7 +173,7 @@
 
 (defn linear-router
   "Creates a linear-router from resolved routes and optional
-  expanded options. See [[router]] for available options"
+  expanded options. See [[router]] for available options."
   ([compiled-routes]
    (linear-router compiled-routes {}))
   ([compiled-routes opts]
@@ -212,7 +218,7 @@
 
 (defn lookup-router
   "Creates a lookup-router from resolved routes and optional
-  expanded options. See [[router]] for available options"
+  expanded options. See [[router]] for available options."
   ([compiled-routes]
    (lookup-router compiled-routes {}))
   ([compiled-routes opts]
@@ -257,7 +263,7 @@
 
 (defn segment-router
   "Creates a special prefix-tree style segment router from resolved routes and optional
-  expanded options. See [[router]] for available options"
+  expanded options. See [[router]] for available options."
   ([compiled-routes]
    (segment-router compiled-routes {}))
   ([compiled-routes opts]
@@ -301,7 +307,7 @@
 
 (defn single-static-path-router
   "Creates a fast router of 1 static route(s) and optional
-  expanded options. See [[router]] for available options"
+  expanded options. See [[router]] for available options."
   ([compiled-routes]
    (single-static-path-router compiled-routes {}))
   ([compiled-routes opts]
@@ -373,6 +379,42 @@
          (or (match-by-name static-router name path-params)
              (match-by-name wildcard-router name path-params)))))))
 
+(defn quarantine-router
+  "Creates two routers: [[mixed-router]] for non-conflicting routes
+  and [[linear-router]] for conflicting routes. Takes resolved routes
+  and optional expanded options. See [[router]] for options."
+  ([compiled-routes]
+   (quarantine-router compiled-routes {}))
+  ([compiled-routes opts]
+   (let [conflicting-paths (-> compiled-routes path-conflicting-routes conflicting-paths)
+         conflicting? #(contains? conflicting-paths (first %))
+         {conflicting true, non-conflicting false} (group-by conflicting? compiled-routes)
+         linear-router (linear-router conflicting opts)
+         mixed-router (mixed-router non-conflicting opts)
+         names (find-names compiled-routes opts)
+         routes (uncompile-routes compiled-routes)]
+     ^{:type ::router}
+     (reify Router
+       (router-name [_]
+         :quarantine-router)
+       (routes [_]
+         routes)
+       (compiled-routes [_]
+         compiled-routes)
+       (options [_]
+         opts)
+       (route-names [_]
+         names)
+       (match-by-path [_ path]
+         (or (match-by-path mixed-router path)
+             (match-by-path linear-router path)))
+       (match-by-name [_ name]
+         (or (match-by-name mixed-router name)
+             (match-by-name linear-router name)))
+       (match-by-name [_ name path-params]
+         (or (match-by-name mixed-router name path-params)
+             (match-by-name linear-router name path-params)))))))
+
 (defn router
   "Create a [[Router]] from raw route data and optionally an options map.
   Selects implementation based on route details. The following options
@@ -403,7 +445,7 @@
          router (cond
                   router router
                   (and (= 1 (count compiled-routes)) (not wilds?)) single-static-path-router
-                  path-conflicting linear-router
+                  path-conflicting quarantine-router
                   (not wilds?) lookup-router
                   all-wilds? segment-router
                   :else mixed-router)]

modules/reitit-middleware/src/reitit/ring/middleware/dev.clj

@@ -9,26 +9,32 @@
       (assoc :width 70)
       (update :color-scheme merge {:middleware [:blue]})))
 
-(defn diff-doc [x y z]
+(defn diff-doc [name previous current]
   [:group
-   [:span "--- Middleware " (if z (color/document printer :middleware (str z " "))) "---" :break :break]
+   [:span "--- Middleware " (if name (color/document printer :middleware (str name " "))) "---" :break :break]
-   [:nest (printer/format-doc (if x (ddiff/diff x y) y) printer)]
+   [:nest (printer/format-doc (if previous (ddiff/diff previous current) current) printer)]
    :break])
 
 (defn polish [request]
   (dissoc request ::r/match ::r/router ::original ::previous))
 
+(defn printed-request [name {:keys [::original ::previous] :as request}]
+  (printer/print-doc (diff-doc name (polish previous) (polish request)) printer)
+  (-> request
+      (update ::original (fnil identity request))
+      (assoc ::previous request)))
+
 (defn print-diff-middleware
   ([]
    (print-diff-middleware nil))
   ([{:keys [name]}]
-   {:name ::debug
+   {:name ::diff
     :wrap (fn [handler]
-            (fn [{:keys [::original ::previous] :as request}]
+            (fn
-              (printer/print-doc (diff-doc (polish previous) (polish request) name) printer)
+              ([request]
-              (handler (-> request
+               (handler (printed-request name request)))
-                           (update ::original (fnil identity request))
+              ([request respond raise]
-                           (assoc ::previous request)))))}))
+               (handler (printed-request name request) respond raise))))}))
 
 (defn print-request-diffs
   "A middleware chain transformer that adds a request-diff printer between all middleware"

test/cljc/reitit/core_test.cljc

@@ -98,7 +98,8 @@
 
       r/linear-router :linear-router
       r/segment-router :segment-router
-      r/mixed-router :mixed-router))
+      r/mixed-router :mixed-router
+      r/quarantine-router :quarantine-router))
 
   (testing "routers handling static paths"
     (are [r name]
@@ -134,7 +135,8 @@
       r/single-static-path-router :single-static-path-router
       r/linear-router :linear-router
       r/segment-router :segment-router
-      r/mixed-router :mixed-router))
+      r/mixed-router :mixed-router
+      r/quarantine-router :quarantine-router))
 
   (testing "nil routes are stripped"
     (is (= [] (r/routes (r/router nil))))