Support not-found with ring

modules/reitit-ring/src/reitit/ring.cljc

@@ -15,36 +15,76 @@
         [top (assoc childs k v)]
         [(assoc top k v) childs])) [{} {}] data))
 
+(defn create-default-handler
+  "A default ring handler that can handle the following cases,
+  configured via options:
+
+  | key                    | description |
+  | -----------------------|-------------|
+  | `:not-found`           | 404, no routes matches
+  | `:method-not-accepted` | 405, no method matches
+  | `:not-acceptable`      | 406, handler returned `nil`"
+  ([]
+   (create-default-handler
+     {:not-found (constantly {:status 404, :body ""})
+      :method-not-allowed (constantly {:status 405, :body ""})
+      :not-acceptable (constantly {:status 406, :body ""})}))
+  ([{:keys [not-found method-not-allowed not-acceptable]}]
+   (fn
+     ([request]
+      (if-let [match (::match request)]
+        (let [method (:request-method request :any)
+              result (:result match)
+              handler? (or (-> result method :handler) (-> result :any :handler))
+              error-handler (if handler? not-acceptable method-not-allowed)]
+          (error-handler request))
+        (not-found request)))
+     ([request respond _]
+      (if-let [match (::match request)]
+        (let [method (:request-method request :any)
+              result (:result match)
+              handler? (or (-> result method :handler) (-> result :any :handler))
+              error-handler (if handler? not-acceptable method-not-allowed)]
+          (respond (error-handler request)))
+        (respond (not-found request)))))))
+
 (defn ring-handler
   "Creates a ring-handler out of a ring-router.
-  Supports both 1 (sync) and 3 (async) arities."
+  Supports both 1 (sync) and 3 (async) arities.
+  Optionally takes a ring-handler which is called
+  in no route matches."
   ([router]
-    (ring-handler router (constantly nil)))
+   (ring-handler router nil))
   ([router default-handler]
-    (with-meta
+   (let [default-handler (or default-handler (fn ([_]) ([_ respond _] (respond nil))))]
-      (fn
+     (with-meta
-        ([request]
+       (fn
-         (if-let [match (r/match-by-path router (:uri request))]
+         ([request]
-           (let [method (:request-method request :any)
+          (if-let [match (r/match-by-path router (:uri request))]
-                 params (:params match)
+            (let [method (:request-method request :any)
-                 result (:result match)
+                  params (:params match)
-                 handler (or (-> result method :handler)
+                  result (:result match)
-                             (-> result :any (:handler default-handler)))
+                  handler (or (-> result method :handler)
-                 request (cond-> (impl/fast-assoc request ::match match)
+                              (-> result :any (:handler default-handler)))
-                                 (seq params) (impl/fast-assoc :path-params params))]
+                  request (cond-> (impl/fast-assoc request ::match match)
-             (handler request))))
+                                  (seq params) (impl/fast-assoc :path-params params))
-        ([request respond raise]
+                  response (handler request)]
-         (if-let [match (r/match-by-path router (:uri request))]
+              (if (nil? response)
-           (let [method (:request-method request :any)
+                (default-handler request)
-                 params (:params match)
+                response))
-                 result (:result match)
+            (default-handler request)))
-                 handler (or (-> result method :handler)
+         ([request respond raise]
-                             (-> result :any (:handler default-handler)))
+          (if-let [match (r/match-by-path router (:uri request))]
-                 request (cond-> (impl/fast-assoc request ::match match)
+            (let [method (:request-method request :any)
-                                 (seq params) (impl/fast-assoc :path-params params))]
+                  params (:params match)
-             (handler request respond raise))
+                  result (:result match)
-           (respond nil))))
+                  handler (or (-> result method :handler)
-      {::router router})))
+                              (-> result :any (:handler default-handler)))
+                  request (cond-> (impl/fast-assoc request ::match match)
+                                  (seq params) (impl/fast-assoc :path-params params))]
+              (handler request respond raise))
+            (default-handler request respond raise))))
+       {::router router}))))
 
 (defn get-router [handler]
   (some-> handler meta ::router))

test/cljc/reitit/ring_test.cljc

@@ -130,29 +130,109 @@
                      :request-method :get
                      ::roles #{:admin}})))))))
 
+(deftest default-handler-test
+  (let [response {:status 200, :body "ok"}
+        router (ring/router
+                 [["/ping" {:get (constantly response)}]
+                  ["/pong" (constantly nil)]])
+        app (ring/ring-handler router)]
+
+    (testing "match"
+      (is (= response (app {:request-method :get, :uri "/ping"}))))
+
+    (testing "no match"
+      (testing "with defaults"
+        (testing "route doesn't match yields nil"
+          (is (= nil (app {:request-method :get, :uri "/"}))))
+        (testing "method doesn't match yields nil"
+          (is (= nil (app {:request-method :post, :uri "/ping"}))))
+        (testing "handler rejects yields nil"
+          (is (= nil (app {:request-method :get, :uri "/pong"})))))
+
+      (testing "with default http responses"
+        (let [app (ring/ring-handler router (ring/create-default-handler))]
+          (testing "route doesn't match yields 404"
+            (is (= 404 (:status (app {:request-method :get, :uri "/"})))))
+          (testing "method doesn't match yields 405"
+            (is (= 405 (:status (app {:request-method :post, :uri "/ping"})))))
+          (testing "handler rejects yields nil"
+            (is (= 406 (:status (app {:request-method :get, :uri "/pong"})))))))
+
+      (testing "with custom http responses"
+        (let [app (ring/ring-handler router (ring/create-default-handler
+                                              {:not-found (constantly {:status -404})
+                                               :method-not-allowed (constantly {:status -405})
+                                               :not-acceptable (constantly {:status -406})}))]
+          (testing "route doesn't match"
+            (is (= -404 (:status (app {:request-method :get, :uri "/"})))))
+          (testing "method doesn't match"
+            (is (= -405 (:status (app {:request-method :post, :uri "/ping"})))))
+          (testing "handler rejects"
+            (is (= -406 (:status (app {:request-method :get, :uri "/pong"}))))))))))
+
 (deftest async-ring-test
   (let [promise #(let [value (atom ::nil)]
                    (fn
                      ([] @value)
                      ([x] (reset! value x))))
         response {:status 200, :body "ok"}
-        handler (fn [_ respond raise]
+        router (ring/router
-                  (respond response))
+                 [["/ping" {:get (fn [_ respond _]
-        app (ring/ring-handler
+                                   (respond response))}]
-              (ring/router
+                  ["/pong" (fn [_ respond _]
-                ["/ping" handler]))]
+                             (respond nil))]])
+        app (ring/ring-handler router)]
+
     (testing "match"
       (let [respond (promise)
             raise (promise)]
         (app {:request-method :get, :uri "/ping"} respond raise)
         (is (= response (respond)))
         (is (= ::nil (raise)))))
+
     (testing "no match"
-      (let [respond (promise)
+
-            raise (promise)]
+      (testing "with defaults"
-        (app {:request-method :get, :uri "/pong"} respond raise)
+        (testing "route doesn't match"
-        (is (= nil (respond)))
+          (let [respond (promise)
-        (is (= ::nil (raise)))))))
+                raise (promise)]
+            (app {:request-method :get, :uri "/"} respond raise)
+            (is (= nil (respond)))
+            (is (= ::nil (raise)))))
+        (testing "method doesn't match"
+          (let [respond (promise)
+                raise (promise)]
+            (app {:request-method :post, :uri "/ping"} respond raise)
+            (is (= nil (respond)))
+            (is (= ::nil (raise)))))
+        (testing "handler rejects"
+          (let [respond (promise)
+                raise (promise)]
+            (app {:request-method :get, :uri "/pong"} respond raise)
+            (is (= nil (respond)))
+            (is (= ::nil (raise))))))
+
+      (testing "with default http responses"
+
+        (let [app (ring/ring-handler router (ring/create-default-handler))]
+          (testing "route doesn't match"
+            (let [respond (promise)
+                  raise (promise)]
+              (app {:request-method :get, :uri "/"} respond raise)
+              (is (= 404 (:status (respond))))
+              (is (= ::nil (raise)))))
+          (testing "method doesn't match"
+            (let [respond (promise)
+                  raise (promise)]
+              (app {:request-method :post, :uri "/ping"} respond raise)
+              (is (= 405 (:status (respond))))
+              (is (= ::nil (raise)))))
+          (testing "if handler rejects, nil in still returned."
+            (let [respond (promise)
+                  raise (promise)]
+              (app {:request-method :get, :uri "/pong"} respond raise)
+              (is (= nil (respond)))
+              (is (= ::nil (raise))))))))))
 
 (deftest middleware-transform-test
   (let [middleware (fn [name] {:name name