Merge pull request '2.0: Improve performance, usability.' (#1) from 2.0 into master

Reviewed-on: https://git.nmm.ee/asko/ruuter/pulls/1

1 files changed, 339 insertions, 97 deletions

diff --git a/test/ruuter/core_test.cljc b/test/ruuter/core_test.cljc
index 3b7d478..36c998d 100644
--- a/test/ruuter/core_test.cljc
+++ b/test/ruuter/core_test.cljc
@@ -4,103 +4,345 @@
   #?(:cljs (:require [cljs.test :refer-macros [deftest testing is]]
                      [ruuter.core :as ruuter])))
 
-(deftest path+uri->path-params-test
-  (let [testfn #'ruuter/path+uri->path-params]
-    (testing "No params returns an empty map"
-      (is (= {}
-             (testfn "/hello/world" "/hello/world"))))
-    (testing "Having a param returns a map accordingly"
-      (is (= {:who "world"}
-             (testfn "/hello/:who" "/hello/world"))))
-    (testing "Multiple params returns a map accordingly"
-      (is (= {:who "world"
-              :why "because"}
-             (testfn "/hello/:who/:why" "/hello/world/because"))))
-    (testing "Multiple params, but one is optional"
-      (is (= {:who "world"}
-             (testfn "/hello/:who/:why?" "/hello/world")))
-      (is (= {:who "world"
-              :why "because"}
-             (testfn "/hello/:who/:why?" "/hello/world/because"))))
-    (testing "Multiple params, but all are optional"
-      (is (= {:who "world"
-              :why "because"}
-             (testfn "hello/:who?/:why?" "/hello/world/because")))
-      (is (= {:who "world"}
-             (testfn "/hello/:who?/:why?" "/hello/world"))))
-    (testing "Wildcard param"
-      (is (= {:everything "this/means/literally/everything"}
-             (testfn "/hello/:everything*" "/hello/this/means/literally/everything"))))
-    (testing "Normal params and wildcard param in the end"
-      (is (= {:id "123"
-              :path "foo.txt"}
-             (testfn "/user/:id/file/:path*" "/user/123/file/foo.txt")))
-      (is (= {:id "123"
-              :path "a/b/c/foo.txt"}
-             (testfn "/user/:id/file/:path*" "/user/123/file/a/b/c/foo.txt")))
-      (is (= {:id "123"
-              :path "a/b/c/foo.txt"
-              :sub-path "b/c/foo.txt"}
-             (testfn "/user/:id/file/:path*/:sub-path*" "/user/123/file/a/b/c/foo.txt"))))))
+;; ── Required Param Tests ────────────────────────────────────────────────
+
+(deftest no-params-test
+  (testing "No params returns an empty params map"
+    (let [params (atom nil)]
+      (ruuter/route [{:path "/hello/world" :method :get
+                      :response (fn [req] (reset! params (:params req)) {:status 200 :body ""})}]
+                    {:uri "/hello/world" :request-method :get})
+      (is (= {} @params)))))
+
+(deftest single-param-test
+  (testing "Single required param"
+    (let [params (atom nil)]
+      (ruuter/route [{:path "/hello/:who" :method :get
+                      :response (fn [req] (reset! params (:params req)) {:status 200 :body ""})}]
+                    {:uri "/hello/world" :request-method :get})
+      (is (= {:who "world"} @params))))
+
+  (testing "Multiple required params"
+    (let [params (atom nil)]
+      (ruuter/route [{:path "/hello/:who/:why" :method :get
+                      :response (fn [req] (reset! params (:params req)) {:status 200 :body ""})}]
+                    {:uri "/hello/world/because" :request-method :get})
+      (is (= {:who "world" :why "because"} @params)))))
+
+;; ── Optional Param Tests ───────────────────────────────────────────────
+
+(deftest optional-param-test
+  (testing "Required and optional param — both present"
+    (let [params (atom nil)]
+      (ruuter/route [{:path "/hello/:who/:why?" :method :get
+                      :response (fn [req] (reset! params (:params req)) {:status 200 :body ""})}]
+                    {:uri "/hello/world/because" :request-method :get})
+      (is (= {:who "world" :why "because"} @params))))
+
+  (testing "Required and optional param — optional absent"
+    (let [params (atom nil)]
+      (ruuter/route [{:path "/hello/:who/:why?" :method :get
+                      :response (fn [req] (reset! params (:params req)) {:status 200 :body ""})}]
+                    {:uri "/hello/world" :request-method :get})
+      (is (= {:who "world"} @params))))
+
+  (testing "Multiple optional params — all present"
+    (let [params (atom nil)]
+      (ruuter/route [{:path "/hello/:who?/:why?" :method :get
+                      :response (fn [req] (reset! params (:params req)) {:status 200 :body ""})}]
+                    {:uri "/hello/world/because" :request-method :get})
+      (is (= {:who "world" :why "because"} @params))))
+
+  (testing "Multiple optional params — only first present"
+    (let [params (atom nil)]
+      (ruuter/route [{:path "/hello/:who?/:why?" :method :get
+                      :response (fn [req] (reset! params (:params req)) {:status 200 :body ""})}]
+                    {:uri "/hello/world" :request-method :get})
+      (is (= {:who "world"} @params)))))
+
+;; ── Wildcard Param Tests ───────────────────────────────────────────────
+
+(deftest wildcard-param-test
+  (testing "Wildcard param"
+    (let [params (atom nil)]
+      (ruuter/route [{:path "/hello/:everything*" :method :get
+                      :response (fn [req] (reset! params (:params req)) {:status 200 :body ""})}]
+                    {:uri "/hello/this/means/literally/everything" :request-method :get})
+      (is (= {:everything "this/means/literally/everything"} @params))))
+
+  (testing "Normal params and wildcard param"
+    (let [params (atom nil)]
+      (ruuter/route [{:path "/user/:id/file/:path*" :method :get
+                      :response (fn [req] (reset! params (:params req)) {:status 200 :body ""})}]
+                    {:uri "/user/123/file/foo.txt" :request-method :get})
+      (is (= {:id "123" :path "foo.txt"} @params))))
+
+  (testing "Normal params and wildcard with deep path"
+    (let [params (atom nil)]
+      (ruuter/route [{:path "/user/:id/file/:path*" :method :get
+                      :response (fn [req] (reset! params (:params req)) {:status 200 :body ""})}]
+                    {:uri "/user/123/file/a/b/c/foo.txt" :request-method :get})
+      (is (= {:id "123" :path "a/b/c/foo.txt"} @params)))))
+
+;; ── Route Matching Tests ───────────────────────────────────────────────
 
 (deftest match-route-test
-  (let [testfn #'ruuter/match-route]
-    (testing "Find a route that exists"
-      (is (= {:path "/hello"
-              :method :get
-              :response {:status 200
-                         :body "Hello."}}
-             (testfn [{:path "/hello"
-                       :method :get
-                       :response {:status 200
-                                  :body "Hello."}}] "/hello" :get))))
-    (testing "No route found"
-      (is (= nil
-             (testfn [] "/hello" :get))))))
-
-(deftest route+req->response-test
-  (let [testfn #'ruuter/route+req->response]
-    (testing "Returns a map when the response is a direct map"
-      (is (= {:status 200
-              :body "Hello."}
-             (testfn {:path "/hello"
-                      :response {:status 200
-                                 :body "Hello."}}
-                     {:uri "/hello"}))))
-    (testing "Returns a map via a fn when the response is a fn"
-      (is (= {:status 200
-              :body "Hello, world."}
-             (testfn {:path "/hello/:who"
+  (testing "Finds a matching route"
+    (is (= {:status 200 :body "Hello."}
+           (ruuter/route [{:path "/hello" :method :get
+                           :response {:status 200 :body "Hello."}}]
+                         {:uri "/hello" :request-method :get}))))
+
+  (testing "No route found returns 404"
+    (is (= {:status 404 :body "Not found."}
+           (ruuter/route [] {:uri "/hello" :request-method :get}))))
+
+  (testing "Method mismatch returns 404"
+    (is (= {:status 404 :body "Not found."}
+           (ruuter/route [{:path "/hello" :method :post
+                           :response {:status 200 :body "Hello."}}]
+                         {:uri "/hello" :request-method :get}))))
+
+  (testing "Root path matches"
+    (is (= {:status 200 :body "root"}
+           (ruuter/route [{:path "/" :method :get
+                           :response {:status 200 :body "root"}}]
+                         {:uri "/" :request-method :get})))))
+
+;; ── Response Tests ─────────────────────────────────────────────────────
+
+(deftest response-test
+  (testing "Returns a map when the response is a direct map"
+    (is (= {:status 200 :body "Hello."}
+           (ruuter/route [{:path "/hello" :method :get
+                           :response {:status 200 :body "Hello."}}]
+                         {:uri "/hello" :request-method :get}))))
+
+  (testing "Returns a map via fn when the response is a fn"
+    (is (= {:status 200 :body "Hello, world."}
+           (ruuter/route [{:path "/hello/:who" :method :get
+                           :response (fn [req]
+                                       {:status 200
+                                        :body (str "Hello, " (:who (:params req)) ".")})}]
+                         {:uri "/hello/world" :request-method :get}))))
+
+  (testing "Returns 404 when route is not found"
+    (is (= {:status 404 :body "Not found."}
+           (ruuter/route [] {:uri "/hello" :request-method :get}))))
+
+  (testing "Returns combined :params map"
+    (let [params (atom nil)]
+      (ruuter/route [{:path "/hello/:who" :method :get
+                      :response (fn [req]
+                                  (reset! params (:params req))
+                                  {:status 200 :body ""})}]
+                    {:uri "/hello/world"
+                     :request-method :get
+                     :params {:some-params :from-elsewhere}})
+      (is (= {:who "world" :some-params :from-elsewhere} @params))))
+
+  (testing "External params overwrite route params"
+    (let [params (atom nil)]
+      (ruuter/route [{:path "/hello/:who" :method :get
                       :response (fn [req]
-                                  {:status 200
-                                   :body (str "Hello, " (:who (:params req)) ".")})}
-                     {:uri "/hello/world"}))))
-    (testing "Returns an error map when route is invalid"
-      (is (= {:status 404
-              :body "Not found."}
-             (testfn nil {:uri "/hello"}))))
-
-    (testing "Returns a combined :params map"
-      (let [params (atom nil)]
-        (testfn {:path "/hello/:who"
-                 :response (fn [req]
-                             (reset! params (:params req))
-                             {:status 200
-                              :body ""})}
-                {:uri "/hello/world"
-                 :params {:some-params :from-elsewhere}})
-        (is (= {:who "world"
-                :some-params :from-elsewhere}
-               @params))))
-
-    (testing "Overwrites :params if needed"
-      (let [params (atom nil)]
-        (testfn {:path "/hello/:who"
-                 :response (fn [req]
-                             (reset! params (:params req))
-                             {:status 200
-                              :body ""})}
-                {:uri "/hello/world"
-                 :params {:who "overwritten"}})
-        (is (= {:who "overwritten"}
-               @params))))))
+                                  (reset! params (:params req))
+                                  {:status 200 :body ""})}]
+                    {:uri "/hello/world"
+                     :request-method :get
+                     :params {:who "overwritten"}})
+      (is (= {:who "overwritten"} @params)))))
+
+;; ── Best-Match / Specificity Tests ─────────────────────────────────────
+
+(deftest literal-beats-param-test
+  (testing "Literal beats param regardless of route order"
+    (is (= {:status 200 :body "literal"}
+           (ruuter/route [{:path "/hello/:who" :method :get
+                           :response {:status 200 :body "param"}}
+                          {:path "/hello/world" :method :get
+                           :response {:status 200 :body "literal"}}]
+                         {:uri "/hello/world" :request-method :get}))))
+
+  (testing "Literal beats param — reversed order"
+    (is (= {:status 200 :body "literal"}
+           (ruuter/route [{:path "/hello/world" :method :get
+                           :response {:status 200 :body "literal"}}
+                          {:path "/hello/:who" :method :get
+                           :response {:status 200 :body "param"}}]
+                         {:uri "/hello/world" :request-method :get})))))
+
+(deftest param-specificity-test
+  (testing "Required param beats optional param"
+    (is (= {:status 200 :body "required"}
+           (ruuter/route [{:path "/hello/:who?" :method :get
+                           :response {:status 200 :body "optional"}}
+                          {:path "/hello/:who" :method :get
+                           :response {:status 200 :body "required"}}]
+                         {:uri "/hello/world" :request-method :get}))))
+
+  (testing "Required param beats wildcard"
+    (is (= {:status 200 :body "param"}
+           (ruuter/route [{:path "/hello/:catch*" :method :get
+                           :response {:status 200 :body "wildcard"}}
+                          {:path "/hello/:who" :method :get
+                           :response {:status 200 :body "param"}}]
+                         {:uri "/hello/world" :request-method :get}))))
+
+  (testing "Param match still works when no literal matches"
+    (is (= {:status 200 :body "param"}
+           (ruuter/route [{:path "/hello/world" :method :get
+                           :response {:status 200 :body "literal"}}
+                          {:path "/hello/:who" :method :get
+                           :response {:status 200 :body "param"}}]
+                         {:uri "/hello/clojure" :request-method :get})))))
+
+(deftest nested-specificity-test
+  (testing "More specific nested path beats less specific"
+    (is (= {:status 200 :body "specific"}
+           (ruuter/route [{:path "/api/:resource" :method :get
+                           :response {:status 200 :body "generic"}}
+                          {:path "/api/users" :method :get
+                           :response {:status 200 :body "specific"}}]
+                         {:uri "/api/users" :request-method :get}))))
+
+  (testing "Deeper literal path beats shallow param"
+    (is (= {:status 200 :body "deep-literal"}
+           (ruuter/route [{:path "/api/:resource/:id" :method :get
+                           :response {:status 200 :body "params"}}
+                          {:path "/api/users/me" :method :get
+                           :response {:status 200 :body "deep-literal"}}]
+                         {:uri "/api/users/me" :request-method :get}))))
+
+  (testing "Wildcard is last resort"
+    (is (= {:status 200 :body "wildcard"}
+           (ruuter/route [{:path "/hello/world" :method :get
+                           :response {:status 200 :body "literal"}}
+                          {:path "/hello/:who" :method :get
+                           :response {:status 200 :body "param"}}
+                          {:path "/hello/:catch*" :method :get
+                           :response {:status 200 :body "wildcard"}}]
+                         {:uri "/hello/a/b/c" :request-method :get})))))
+
+;; ── Not-Found Fallback Tests ───────────────────────────────────────────
+
+(deftest not-found-test
+  (testing "Custom :not-found route is used when no match"
+    (is (= {:status 404 :body "custom 404"}
+           (ruuter/route [{:path "/hello" :method :get
+                           :response {:status 200 :body "hi"}}
+                          {:path :not-found
+                           :response {:status 404 :body "custom 404"}}]
+                         {:uri "/nope" :request-method :get}))))
+
+  (testing "Custom :not-found route with fn response"
+    (is (= {:status 404 :body "Not found: /nope"}
+           (ruuter/route [{:path :not-found
+                           :response (fn [req]
+                                       {:status 404
+                                        :body (str "Not found: " (:uri req))})}]
+                         {:uri "/nope" :request-method :get}))))
+
+  (testing "Built-in 404 when no :not-found route"
+    (is (= {:status 404 :body "Not found."}
+           (ruuter/route [{:path "/hello" :method :get
+                           :response {:status 200 :body "hi"}}]
+                         {:uri "/nope" :request-method :get})))))
+
+;; ── Compile-Routes Tests ───────────────────────────────────────────────
+
+(deftest compile-routes-test
+  (testing "Pre-compiled routes work the same as raw routes"
+    (let [routes [{:path "/hello/:who" :method :get
+                   :response (fn [req]
+                               {:status 200
+                                :body (str "Hello, " (:who (:params req)))})}
+                  {:path :not-found
+                   :response {:status 404 :body "custom 404"}}]
+          compiled (ruuter/compile-routes routes)]
+      (is (= {:status 200 :body "Hello, world"}
+             (ruuter/route compiled {:uri "/hello/world" :request-method :get})))
+      (is (= {:status 404 :body "custom 404"}
+             (ruuter/route compiled {:uri "/nope" :request-method :get}))))))
+
+;; ── Edge Case Tests ────────────────────────────────────────────────────
+
+(deftest trie-node-reuse-test
+  (testing "Two routes sharing same param slot — trie reuses param node"
+    (let [routes [{:path "/users/:id/profile" :method :get
+                   :response {:status 200 :body "profile"}}
+                  {:path "/users/:id/settings" :method :get
+                   :response {:status 200 :body "settings"}}]]
+      (is (= {:status 200 :body "profile"}
+             (ruuter/route routes {:uri "/users/42/profile" :request-method :get})))
+      (is (= {:status 200 :body "settings"}
+             (ruuter/route routes {:uri "/users/42/settings" :request-method :get})))))
+
+  (testing "Two routes sharing same optional slot — trie reuses optional node"
+    (let [routes [{:path "/search/:q?/page" :method :get
+                   :response {:status 200 :body "page"}}
+                  {:path "/search/:q?/sort" :method :get
+                   :response {:status 200 :body "sort"}}]]
+      (is (= {:status 200 :body "page"}
+             (ruuter/route routes {:uri "/search/foo/page" :request-method :get})))
+      (is (= {:status 200 :body "sort"}
+             (ruuter/route routes {:uri "/search/foo/sort" :request-method :get}))))))
+
+(deftest edge-cases-test
+  (testing "Trailing slash on root"
+    (is (= {:status 200 :body "root"}
+           (ruuter/route [{:path "/" :method :get
+                           :response {:status 200 :body "root"}}]
+                         {:uri "/" :request-method :get}))))
+
+  (testing "Multiple HTTP methods on same path"
+    (let [routes [{:path "/items" :method :get
+                   :response {:status 200 :body "list"}}
+                  {:path "/items" :method :post
+                   :response {:status 201 :body "created"}}]]
+      (is (= {:status 200 :body "list"}
+             (ruuter/route routes {:uri "/items" :request-method :get})))
+      (is (= {:status 201 :body "created"}
+             (ruuter/route routes {:uri "/items" :request-method :post})))))
+
+  (testing "Many overlapping routes — best match wins"
+    (let [routes [{:path "/:catch*" :method :get
+                   :response {:status 200 :body "catch-all"}}
+                  {:path "/api/:resource" :method :get
+                   :response {:status 200 :body "api-resource"}}
+                  {:path "/api/users" :method :get
+                   :response {:status 200 :body "api-users"}}
+                  {:path "/api/users/:id" :method :get
+                   :response {:status 200 :body "api-user-id"}}
+                  {:path "/api/users/me" :method :get
+                   :response {:status 200 :body "api-users-me"}}]]
+      (is (= {:status 200 :body "api-users"}
+             (ruuter/route routes {:uri "/api/users" :request-method :get})))
+      (is (= {:status 200 :body "api-users-me"}
+             (ruuter/route routes {:uri "/api/users/me" :request-method :get})))
+      (is (= {:status 200 :body "api-user-id"}
+             (ruuter/route routes {:uri "/api/users/42" :request-method :get})))
+      (is (= {:status 200 :body "api-resource"}
+             (ruuter/route routes {:uri "/api/posts" :request-method :get})))
+      (is (= {:status 200 :body "catch-all"}
+             (ruuter/route routes {:uri "/random/deep/path" :request-method :get}))))))
+
+(deftest wildcard-reuse-test
+  (testing "Multiple wildcards at same position in different routes"
+    (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}))))
+
+  (testing "Non-map non-fn response returns 404"
+    (is (= {:status 404 :body "Not found."}
+           (ruuter/route [{:path "/hello" :method :get
+                           :response "just a string"}]
+                         {:uri "/hello" :request-method :get})))))