From df38a0de05ed602b5326f747f71510f9f68d29fb Mon Sep 17 00:00:00 2001 From: Tommi Reiman Date: Sat, 9 Feb 2019 15:29:25 +0200 Subject: [PATCH] Test bracket syntax + fix trie conflicting rules --- modules/reitit-core/src/reitit/impl.cljc | 44 ++-------------- modules/reitit-core/src/reitit/trie.cljc | 64 +++++++++++++++++++++--- test/cljc/reitit/core_test.cljc | 51 +++++++++++++++---- 3 files changed, 102 insertions(+), 57 deletions(-) diff --git a/modules/reitit-core/src/reitit/impl.cljc b/modules/reitit-core/src/reitit/impl.cljc index 38727802..9f400df2 100644 --- a/modules/reitit-core/src/reitit/impl.cljc +++ b/modules/reitit-core/src/reitit/impl.cljc @@ -34,45 +34,6 @@ coll coll)) -(defn- -slice-start [[p1 :as p1s] [p2 :as p2s]] - (let [-split (fn [p] - (if-let [i (and p (str/index-of p "/"))] - [(subs p 0 i) (subs p i)] - [p])) - -slash (fn [cp p] - (cond - (not (string? cp)) [cp] - (and (string? cp) (not= (count cp) (count p))) [(subs p (count cp))] - (and (string? p) (not cp)) (-split p))) - -postcut (fn [[p :as pps]] - (let [i (and p (str/index-of p "/"))] - (if (and i (pos? i)) - (concat [(subs p 0 i) (subs p i)] (rest pps)) - pps))) - -tailcut (fn [cp [p :as ps]] (concat (-slash cp p) (rest ps)))] - (if (or (nil? p1) (nil? p2)) - [(-postcut p1s) (-postcut p2s)] - (let [cp (and (string? p1) (string? p2) (trie/common-prefix p1 p2))] - [(-tailcut cp p1s) (-tailcut cp p2s)])))) - -(defn- -slice-end [x xs] - (let [i (if (string? x) (str/index-of x "/"))] - (if (and (number? i) (pos? i)) - (concat [(subs x i)] xs) - xs))) - -(defn conflicting-routes? [route1 route2] - (loop [parts1 (-> route1 first parse :path-parts) - parts2 (-> route2 first parse :path-parts)] - (let [[[s1 & ss1] [s2 & ss2]] (-slice-start parts1 parts2)] - (cond - (= s1 s2 nil) true - (or (nil? s1) (nil? s2)) false - (or (trie/catch-all? s1) (trie/catch-all? s2)) true - (or (trie/wild? s1) (trie/wild? s2)) (recur (-slice-end s1 ss1) (-slice-end s2 ss2)) - (not= s1 s2) false - :else (recur ss1 ss2))))) - (defn walk [raw-routes {:keys [path data routes expand] :or {data [], routes []} :as opts}] @@ -108,11 +69,14 @@ (cond->> (->> (walk raw-routes opts) (map-data merge-data)) coerce (into [] (keep #(coerce % opts))))) +(defn conflicting-routes? [route1 route2] + (trie/conflicting-paths? (first route1) (first route2))) + (defn path-conflicting-routes [routes] (-> (into {} (comp (map-indexed (fn [index route] [route (into #{} - (filter #(conflicting-routes? route %)) + (filter (partial conflicting-routes? route)) (subvec routes (inc index)))])) (filter (comp seq second))) routes) diff --git a/modules/reitit-core/src/reitit/trie.cljc b/modules/reitit-core/src/reitit/trie.cljc index c7f83269..78d57db1 100644 --- a/modules/reitit-core/src/reitit/trie.cljc +++ b/modules/reitit-core/src/reitit/trie.cljc @@ -1,5 +1,5 @@ (ns reitit.trie - (:refer-clojure :exclude [compile -assoc!]) + (:refer-clojure :exclude [compile]) (:require [clojure.string :as str]) #?(:clj (:import [reitit Trie Trie$Match Trie$Matcher] (java.net URLDecoder)))) @@ -18,9 +18,9 @@ (depth [this]) (length [this])) -(defn -assoc! [match k v] - (let [params (or (:path-params match) (transient {}))] - (assoc match :path-params (assoc! params k v)))) +(defn assoc-path-param [match k v] + (let [params (:path-params match)] + (assoc match :path-params (assoc params k v)))) ;; https://stackoverflow.com/questions/8033655/find-longest-common-prefix (defn common-prefix [s1 s2] @@ -71,6 +71,54 @@ (defn normalize [s] (-> s (split-path) (join-path))) +;; +;; Conflict Resolution +;; + +(defn- -slice-start [[p1 :as p1s] [p2 :as p2s]] + (let [-split (fn [p] + (if-let [i (and p (str/index-of p "/"))] + [(subs p 0 i) (subs p i)] + [p])) + -slash (fn [cp p] + (cond + (not (string? cp)) [cp] + (and (string? cp) (not= (count cp) (count p))) [(subs p (count cp))] + (and (string? p) (not cp)) (-split p))) + -postcut (fn [[p :as pps]] + (let [i (and p (str/index-of p "/"))] + (if (and i (pos? i)) + (concat [(subs p 0 i) (subs p i)] (rest pps)) + pps))) + -tailcut (fn [cp [p :as ps]] (concat (-slash cp p) (rest ps)))] + (if (or (nil? p1) (nil? p2)) + [(-postcut p1s) (-postcut p2s)] + (if-let [cp (and (string? p1) (string? p2) (common-prefix p1 p2))] + [(-tailcut cp p1s) (-tailcut cp p2s)] + [p1s p2s])))) + +(defn- -slice-end [x xs] + (let [i (if (string? x) (str/index-of x "/"))] + (if (and (number? i) (pos? i)) + (concat [(subs x i)] xs) + xs))) + +(defn conflicting-paths? [path1 path2] + (loop [parts1 (split-path path1) + parts2 (split-path path2)] + (let [[[s1 & ss1] [s2 & ss2]] (-slice-start parts1 parts2)] + (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 (-slice-end s1 ss1) (-slice-end s2 ss2)) + (not= s1 s2) false + :else (recur ss1 ss2))))) + +;; +;; Creating Tries +;; + (defn- -node [m] (map->Node (merge {:children {}, :wilds {}, :catch-all {}} m))) @@ -120,7 +168,7 @@ node'))) #?(:cljs - (defn decode! [path start end percent?] + (defn decode [path start end percent?] (if percent? (js/decodeURIComponent (subs path start end)) path))) (defn data-matcher [data] @@ -157,11 +205,11 @@ (loop [percent? false, j i] (if (= max j) (if-let [match (match matcher max max path)] - (-assoc! match key (decode! path i max percent?))) + (assoc-path-param match key (decode path i max percent?))) (let [c ^char (get path j)] (condp = c end (if-let [match (match matcher j max path)] - (-assoc! match key (decode! path i j percent?))) + (assoc-path-param match key (decode path i j percent?))) \% (recur true (inc j)) (recur percent? (inc j)))))))) (view [_] [key (view matcher)]) @@ -173,7 +221,7 @@ :cljs (let [match (->Match data nil)] (reify Matcher (match [_ i max path] - (if (< i max) (-assoc! match key (decode! path i max true)))) + (if (< i max) (assoc-path-param match key (decode path i max true)))) (view [_] [key [data]]) (depth [_] 1) (length [_]))))) diff --git a/test/cljc/reitit/core_test.cljc b/test/cljc/reitit/core_test.cljc index 19e061e8..75a88c56 100644 --- a/test/cljc/reitit/core_test.cljc +++ b/test/cljc/reitit/core_test.cljc @@ -79,15 +79,48 @@ ["/abba/:dabba/boo" ::boo] ["/:jabba/:dabba/:doo/:daa/*foo" ::wild]] {:router r}) - matches #(-> router (r/match-by-path %) :data :name)] - (is (= ::abba (matches "/abba"))) - (is (= ::abba2 (matches "/abba/1"))) - (is (= ::jabba2 (matches "/abba/2"))) - (is (= ::doo (matches "/abba/1/doo"))) - (is (= ::boo (matches "/abba/1/boo"))) - (is (= ::baa (matches "/abba/dabba/boo/baa"))) - (is (= ::boo (matches "/abba/dabba/boo"))) - (is (= ::wild (matches "/olipa/kerran/avaruus/vaan/ei/toista/kertaa"))))) + by-path #(-> router (r/match-by-path %) :data :name)] + (is (= ::abba (by-path "/abba"))) + (is (= ::abba2 (by-path "/abba/1"))) + (is (= ::jabba2 (by-path "/abba/2"))) + (is (= ::doo (by-path "/abba/1/doo"))) + (is (= ::boo (by-path "/abba/1/boo"))) + (is (= ::baa (by-path "/abba/dabba/boo/baa"))) + (is (= ::boo (by-path "/abba/dabba/boo"))) + (is (= ::wild (by-path "/olipa/kerran/avaruus/vaan/ei/toista/kertaa"))))) + + (testing "bracket-params" + (let [router (r/router + [["/{abba}" ::abba] + ["/abba/1" ::abba2] + ["/{jabba}/2" ::jabba2] + ["/{abba}/{dabba}/doo" ::doo] + ["/abba/dabba/boo/baa" ::baa] + ["/abba/{dabba}/boo" ::boo] + ["/{a/jabba}/{a.b/dabba}/{a.b.c/doo}/{a.b.c.d/daa}/{*foo/bar}" ::wild] + ["/files/file-{name}.html" ::html] + ["/files/file-{name}.json" ::json] + ["/files/file-{name}-large.json" ::large]] + {:router r}) + by-path #(-> router (r/match-by-path %) ((juxt (comp :name :data) :path-params)))] + (is (= [::abba {:abba "abba"}] (by-path "/abba"))) + (is (= [::abba2 {}] (by-path "/abba/1"))) + (is (= [::jabba2 {:jabba "abba"}] (by-path "/abba/2"))) + (is (= [::doo {:abba "abba", :dabba "1"}] (by-path "/abba/1/doo"))) + (is (= [::boo {:dabba "1"}] (by-path "/abba/1/boo"))) + (is (= [::baa {}] (by-path "/abba/dabba/boo/baa"))) + (is (= [::boo {:dabba "dabba"}] (by-path "/abba/dabba/boo"))) + (is (= [::wild {:a/jabba "olipa" + :a.b/dabba "kerran" + :a.b.c/doo "avaruus" + :a.b.c.d/daa "vaan" + :foo/bar "ei/toista/kertaa"}] + (by-path "/olipa/kerran/avaruus/vaan/ei/toista/kertaa"))) + (is (= [::html {:name "10"}] (by-path "/files/file-10.html"))) + (is (= [::json {:name "10"}] (by-path "/files/file-10.json"))) + ;(is (= [::large {:name "10"}] (by-path "/files/file-10-large.json"))) + + )) (testing "empty path segments" (let [router (r/router