Return nil on failure (instead of throwing), eval context with args as provided (instead of nil)

2 files changed, 100 insertions, 36 deletions

diff --git a/src/reverse_routing/core.clj b/src/reverse_routing/core.clj
index 4299eba..1977b2a 100644
--- a/src/reverse_routing/core.clj
+++ b/src/reverse_routing/core.clj
@@ -6,17 +6,17 @@
 (def ^:private ^:dynamic *lookup-route* nil)
 (def ^:private ^:dynamic *root* nil)
 
+(defn ^:private deref-if-var [arg]
+  (if (var? arg)
+    (deref arg)
+    arg))
+
 (defn wrap-reverse-routing [handler & {:keys [root] :or {:root ""}}]
   (fn [request]
     (binding [*lookup-route* (->> handler deref-if-var meta ::lookup)
               *root* root]
       (handler request))))
 
-(defn ^:private deref-if-var [arg]
-  (if (var? arg)
-    (deref arg)
-    arg))
-
 (defn ^:private lookup-route [route & handlers]
   (->> handlers
        (map (comp ::lookup meta deref-if-var))
@@ -33,7 +33,7 @@
   `(let ~bindings (routes ~@handlers)))
 
 (defmacro when-routes [cond & handlers]
-  `(when ~cond (routes ~@handlers)))
+  `(if ~cond (routes ~@handlers) (routes)))
 
 (defmacro defroutes [name & handlers]
   `(def ~name (routes ~@handlers)))
@@ -42,15 +42,16 @@
   (let [string-path (if (vector? path) (first path) path)
         path-keys (vec (:keys (clout.core/route-compile string-path)))
         keylen (count path-keys)
-        lookup-fn `(fn [[route-name# args-count#]]
-                     (let [~args (repeat nil) ;; hacky, but necessary - provide nil values for args
-                           r# (#'lookup-route [route-name# (- args-count# ~keylen)]
-                                              ~@routes)
-                           {uri# :uri, args# :args} r#]
-                       (if r#
-                         (assoc r#
-                           :uri (str ~string-path uri#)
-                           :args (vec (concat ~path-keys args#))))))]
+        lookup-fn `(fn [[route-name# args#]]
+                     (if (>= (count args#) ~keylen)
+                       (let [~args args# 
+                             r# (#'lookup-route [route-name# (vec (drop ~keylen args#))]
+                                                ~@routes)
+                             {uri# :uri, args# :args} r#]
+                         (if r#
+                           (assoc r#
+                             :uri (str ~string-path uri#)
+                             :args (vec (concat ~path-keys args#)))))))]
     `(vary-meta (compojure.core/context ~path ~args
                   (routes ~@routes))
                 assoc ::lookup ~lookup-fn)))
@@ -58,20 +59,21 @@
 (defmacro register-route [route-name [type path args & body :as route]]
   (let [string-path (if (vector? path) (first path) path)
         route-args (:keys (clout.core/route-compile string-path))
-        route-id [route-name (count route-args)]
         routes-map {:uri string-path
                     :type (keyword (string/lower-case (name type)))
                     :args (vec route-args)}]
     `(vary-meta ~route
-                assoc ::lookup (fn [signature#]
-                                 (if (= signature# ~route-id)
+                assoc ::lookup (fn [[name# args#]]
+                                 (if (and (= name# ~route-name)
+                                          (= (count args#) (count ~(vec route-args))))
                                    ~routes-map)))))
 
 (defn url-for [route & arg-values]
-  (let [{:keys [uri type args]} (*lookup-route* [route (count arg-values)])
+  (let [spec (*lookup-route* [route arg-values])
+        {:keys [uri type args]} spec
         root-path *root*]
-    (assert uri)
-    (str root-path
-         (reduce (fn [string [name val]]
-                   (clojure.string/replace string (str name) (str val)))
-                 uri (map vector args arg-values)))))
+    (if spec
+      (str root-path
+           (reduce (fn [string [name val]]
+                     (clojure.string/replace string (str name) (str val)))
+                   uri (map vector args arg-values))))))
diff --git a/test/reverse_routing/core_test.clj b/test/reverse_routing/core_test.clj
index ef2fa93..fb37c94 100644
--- a/test/reverse_routing/core_test.clj
+++ b/test/reverse_routing/core_test.clj
@@ -16,6 +16,21 @@
       :body))
 
 (deftest test-basic-route
+  (let [handler (-> (register-route :user
+                      (GET "/succeed" [id] (url-for :user 10)))
+                    wrap-reverse-routing)]
+    (is (= (make-request handler "/succeed") "/user/10"))))
+
+(-> (routes
+     (register-route :user
+       (GET "/user/:id" [id] (str "user " id)))
+     (GET "/succeed" [] (url-for :user 10))
+     (GET "/fail1" [] (url-for :user))
+     (GET "/fail2" [] (url-for :user 10 20)))
+    wrap-reverse-routing
+    (make-request "/succeed"))
+
+(deftest test-basic-route
   (let [handler (-> (routes
                      (register-route :user
                        (GET "/user/:id" [id] (str "user " id)))
@@ -24,8 +39,25 @@
                      (GET "/fail2" [] (url-for :user 10 20)))
                     wrap-reverse-routing)]
     (is (= (make-request handler "/succeed") "/user/10"))
-    (is (thrown? java.lang.AssertionError (make-request handler "/fail1")))
-    (is (thrown? java.lang.AssertionError (make-request handler "/fail2")))))
+    (is (nil? (make-request handler "/fail1")))
+    (is (nil? (make-request handler "/fail2")))))
+
+(-> (routes
+     (register-route :user
+       (GET "/user/" [id] (str "user list")))
+     (context "/user/:id" []
+       (register-route :user
+         (GET "/" [id] (str "user " id)))
+       (register-route :edit-user
+         (GET "/edit" [id] (str "edit user " id))))
+     (GET "/succeed1" [] (url-for :user))
+     (GET "/succeed2" [] (url-for :user 10))
+     (GET "/succeed3" [] (url-for :edit-user 10))
+     (GET "/fail1" [] (url-for :user 10 20))
+     (GET "/fail2" [] (url-for :edit-user))
+     (GET "/fail3" [] (url-for :edit-user 10 20)))
+    wrap-reverse-routing
+    (make-request "/fail2"))
 
 (deftest test-context-routes
   (let [handler (-> (routes
@@ -47,9 +79,9 @@
     (is (= (make-request handler "/succeed2") "/user/10/"))
     (is (= (make-request handler "/succeed3") "/user/10/edit"))
 
-    (is (thrown? java.lang.AssertionError (make-request handler "/fail1")))
-    (is (thrown? java.lang.AssertionError (make-request handler "/fail2")))
-    (is (thrown? java.lang.AssertionError (make-request handler "/fail3")))))
+    (is (nil? (make-request handler "/fail1")))
+    (is (nil? (make-request handler "/fail2")))
+    (is (nil? (make-request handler "/fail3")))))
 
 (deftest test-context-in-context-routes
   (let [handler (-> (routes
@@ -72,9 +104,9 @@
     (is (= (make-request handler "/succeed2") "/user/10/"))
     (is (= (make-request handler "/succeed3") "/user/10/edit"))
 
-    (is (thrown? java.lang.AssertionError (make-request handler "/fail1")))
-    (is (thrown? java.lang.AssertionError (make-request handler "/fail2")))
-    (is (thrown? java.lang.AssertionError (make-request handler "/fail3")))))
+    (is (nil? (make-request handler "/fail1")))
+    (is (nil? (make-request handler "/fail2")))
+    (is (nil? (make-request handler "/fail3")))))
 
 
 (deftest test-with-rebinding-vars
@@ -86,12 +118,12 @@
                        (GET "/fail" [] (url-for :user 10)))
                       wrap-reverse-routing)]
       (is (= (make-request handler "/succeed") "/user"))
-      (is (thrown? java.lang.AssertionError (make-request handler "/fail")))
+      (is (nil? (make-request handler "/fail")))
 
       (var-set subroutes (register-route :user
                            (GET "/not-user" [] (str "user"))))
       (is (= (make-request handler "/succeed") "/not-user"))
-      (is (thrown? java.lang.AssertionError (make-request handler "/fail"))))))
+      (is (nil? (make-request handler "/fail"))))))
 
 (deftest top-level-is-a-var
   (with-local-vars [bare-handler (routes
@@ -102,5 +134,35 @@
                                   (GET "/fail2" [] (url-for :user 10 20)))]
     (let [handler (wrap-reverse-routing bare-handler)]
       (is (= (make-request handler "/succeed") "/user/10"))
-      (is (thrown? java.lang.AssertionError (make-request handler "/fail1")))
-      (is (thrown? java.lang.AssertionError (make-request handler "/fail2"))))))
+      (is (nil? (make-request handler "/fail1")))
+      (is (nil? (make-request handler "/fail2"))))))
+
+
+(deftest context-requiring-not-nil-value
+  (let [handler (-> (routes
+                     (context "/:type" [type]
+                       (if (not= type nil)
+                         (register-route :get-object
+                           (GET "/:id" [id]
+                             (str [type id])))))
+                     (GET "/succeed" [] (url-for :get-object "user" 10))
+                     (GET "/fail" [] (url-for :get-object nil 10)))
+                    wrap-reverse-routing)]
+    (is (= (make-request handler "/succeed"), "/user/10"))
+    (is (nil? (make-request handler "/fail")))))
+
+(deftest context-with-limited-options
+  (let [valid-type? #{"user"}
+        handler (-> (routes
+                     (context "/:type" [type]
+                       (when-routes (valid-type? type)
+                         (register-route :get-object
+                           (GET "/:id" [id]
+                             (str [type id])))))
+                     (GET "/succeed" [] (url-for :get-object "user" 10))
+                     (GET "/fail1" [] (url-for :get-object nil 10))
+                     (GET "/fail2" [] (url-for :get-object "person" 10)))
+                    wrap-reverse-routing)]
+    (is (= (make-request handler "/succeed"), "/user/10"))
+    (is (nil? (make-request handler "/fail1")))
+    (is (nil? (make-request handler "/fail2")))))