diff options
| -rw-r--r-- | src/ruuter/core.cljc | 42 | ||||
| -rw-r--r-- | test/ruuter/core_test.cljc | 107 |
2 files changed, 137 insertions, 12 deletions
diff --git a/src/ruuter/core.cljc b/src/ruuter/core.cljc index c1c5644..c2e05a1 100644 --- a/src/ruuter/core.cljc +++ b/src/ruuter/core.cljc @@ -62,6 +62,18 @@ :wildcard nil ;; {:param-name kw, :leaves [...]} or nil :leaves []}) ;; routes that terminate here [{:method :get :response fn :path str}] +(defn- throw-conflict + "Throws an exception when two routes define different parameter names at the + same trie position." + [slot-type existing-name new-name path] + (let [msg (str "ruuter: conflicting " slot-type " parameter names at same position — :" + (name existing-name) " and :" (name new-name) + ". Route: " path)] + (throw (ex-info msg {:slot-type slot-type + :existing-name existing-name + :new-name new-name + :path path})))) + (defn- insert-route "Inserts a single route into the trie, returning the updated trie." [trie segments leaf] @@ -76,22 +88,28 @@ (assoc-in trie [:children value] child')) :param - (let [existing (:param trie) - child (if existing (:node existing) (empty-node)) - child' (insert-route child remaining leaf)] - (assoc trie :param {:param-name value :node child'})) + (let [existing (:param trie)] + (when (and existing (not= (:param-name existing) value)) + (throw-conflict "param" (:param-name existing) value (:path leaf))) + (let [child (if existing (:node existing) (empty-node)) + child' (insert-route child remaining leaf)] + (assoc trie :param {:param-name value :node child'}))) :optional - (let [existing (:optional trie) - child (if existing (:node existing) (empty-node)) - child' (insert-route child remaining leaf)] - (assoc trie :optional {:param-name value :node child'})) + (let [existing (:optional trie)] + (when (and existing (not= (:param-name existing) value)) + (throw-conflict "optional" (:param-name existing) value (:path leaf))) + (let [child (if existing (:node existing) (empty-node)) + child' (insert-route child remaining leaf)] + (assoc trie :optional {:param-name value :node child'}))) :wildcard - (let [existing (:wildcard trie) - leaves (if existing (:leaves existing) []) - leaves' (conj leaves leaf)] - (assoc trie :wildcard {:param-name value :leaves leaves'})))))) + (let [existing (:wildcard trie)] + (when (and existing (not= (:param-name existing) value)) + (throw-conflict "wildcard" (:param-name existing) value (:path leaf))) + (let [leaves (if existing (:leaves existing) []) + leaves' (conj leaves leaf)] + (assoc trie :wildcard {:param-name value :leaves leaves'}))))))) (defn compile-routes "Compiles a vector of route maps into a trie structure for efficient diff --git a/test/ruuter/core_test.cljc b/test/ruuter/core_test.cljc index 9f47212..d9150e6 100644 --- a/test/ruuter/core_test.cljc +++ b/test/ruuter/core_test.cljc @@ -348,3 +348,110 @@ (ruuter/route [{:path "/hello" :method :get :response "just a string"}] {:uri "/hello" :request-method :get}))))) + +;; -- Conflicting Parameter Name Tests -- + +#?(:clj + (deftest conflicting-param-names-test + (testing "Conflicting :param names — throws exception" + (is (thrown? clojure.lang.ExceptionInfo + (ruuter/route [{:path "/user/:id" :method :get + :response (fn [req] {:status 200 :body "first"})} + {:path "/user/:x" :method :get + :response (fn [req] {:status 200 :body "second"})}] + {:uri "/user/42" :request-method :get})))) + + (testing "Conflicting :param names across methods — throws exception" + (is (thrown? clojure.lang.ExceptionInfo + (ruuter/route [{:path "/user/:id" :method :get + :response (fn [req] {:status 200 :body "get"})} + {:path "/user/:x" :method :post + :response (fn [req] {:status 200 :body "post"})}] + {:uri "/user/42" :request-method :get}))))) + + :cljs + (deftest conflicting-param-names-test + (testing "Conflicting :param names — throws exception" + (is (thrown? ExceptionInfo + (ruuter/route [{:path "/user/:id" :method :get + :response (fn [req] {:status 200 :body "first"})} + {:path "/user/:x" :method :get + :response (fn [req] {:status 200 :body "second"})}] + {:uri "/user/42" :request-method :get})))) + + (testing "Conflicting :param names across methods — throws exception" + (is (thrown? ExceptionInfo + (ruuter/route [{:path "/user/:id" :method :get + :response (fn [req] {:status 200 :body "get"})} + {:path "/user/:x" :method :post + :response (fn [req] {:status 200 :body "post"})}] + {:uri "/user/42" :request-method :get})))))) + +#?(:clj + (deftest conflicting-optional-names-test + (testing "Conflicting :optional names — throws exception" + (is (thrown? clojure.lang.ExceptionInfo + (ruuter/route [{:path "/search/:q?" :method :get + :response (fn [req] {:status 200 :body "first"})} + {:path "/search/:query?" :method :get + :response (fn [req] {:status 200 :body "second"})}] + {:uri "/search/clojure" :request-method :get}))))) + + :cljs + (deftest conflicting-optional-names-test + (testing "Conflicting :optional names — throws exception" + (is (thrown? ExceptionInfo + (ruuter/route [{:path "/search/:q?" :method :get + :response (fn [req] {:status 200 :body "first"})} + {:path "/search/:query?" :method :get + :response (fn [req] {:status 200 :body "second"})}] + {:uri "/search/clojure" :request-method :get})))))) + +#?(:clj + (deftest conflicting-wildcard-names-test + (testing "Conflicting :wildcard names — throws exception" + (is (thrown? clojure.lang.ExceptionInfo + (ruuter/route [{:path "/files/:path*" :method :get + :response (fn [req] {:status 200 :body "first"})} + {:path "/files/:glob*" :method :get + :response (fn [req] {:status 200 :body "second"})}] + {:uri "/files/a/b/c" :request-method :get})))) + + (testing "Same wildcard name with different methods — no conflict" + (is (= {:status 200 :body "get-files"} + (ruuter/route [{:path "/files/:path*" :method :get + :response {:status 200 :body "get-files"}} + {:path "/files/:path*" :method :delete + :response {:status 200 :body "delete-files"}}] + {:uri "/files/a/b/c" :request-method :get}))) + (is (= {:status 200 :body "delete-files"} + (ruuter/route [{:path "/files/:path*" :method :get + :response {:status 200 :body "get-files"}} + {:path "/files/:path*" :method :delete + :response {:status 200 :body "delete-files"}}] + {:uri "/files/a/b/c" :request-method :delete}))))) + + :cljs + (deftest conflicting-wildcard-names-test + (testing "Conflicting :wildcard names — throws exception" + (is (thrown? ExceptionInfo + (ruuter/route [{:path "/files/:path*" :method :get + :response (fn [req] {:status 200 :body "first"})} + {:path "/files/:glob*" :method :get + :response (fn [req] {:status 200 :body "second"})}] + {:uri "/files/a/b/c" :request-method :get})))) + + (testing "Same wildcard name with different methods — no conflict" + (is (= {:status 200 :body "get-files"} + (ruuter/route [{:path "/files/:path*" :method :get + :response {:status 200 :body "get-files"}} + {:path "/files/:path*" :method :delete + :response {:status 200 :body "delete-files"}}] + {:uri "/files/a/b/c" :request-method :get}))) + (is (= {:status 200 :body "delete-files"} + (ruuter/route [{:path "/files/:path*" :method :get + :response {:status 200 :body "get-files"}} + {:path "/files/:path*" :method :delete + :response {:status 200 :body "delete-files"}}] + {:uri "/files/a/b/c" :request-method :delete})))))) + |