Merge pull request #63 from metosin/NotFound

Support not-found with ring

.travis.yml

@@ -1,4 +1,4 @@
-sudo: false
+sudo: required
 language: clojure
 lein: 2.7.1
 install:
@@ -20,3 +20,5 @@ cache:
   directories:
   - "$HOME/.m2"
   - "node_modules"
+addons:
+  chrome: stable

doc/ring/ring.md

@@ -112,22 +112,85 @@ Middleware is applied correctly:
 ; {:status 200, :body [:api :admin :db :delete :handler]}
 ```
 
-# Not found
+# Default handler
 
-If no routes match, `nil` is returned, which is not understood by Ring.
+By default, if no routes match, `nil` is returned, which is not valid response in Ring:
 
-Enabling custom error messages:
+```clj
+(defn handler [_]
+  {:status 200, :body ""})
+
+(def app
+  (ring/ring-handler
+    (ring/router
+      ["/ping" handler])))
+
+(app {:uri "/invalid"})
+; nil
+```
+
+Setting the default-handler as a second argument to `ring-handler`:
 
 ```clj
 (def app
-  (some-fn
+  (ring/ring-handler
-    (ring/ring-handler
+    (ring/router
-      (ring/router
+      ["/ping" handler])
-        ["/ping" handler]))
+    (constantly {:status 404, :body ""})))
-    (constantly {:status 404})))
 
 (app {:uri "/invalid"})
-; {:status 404}
+; {:status 404, :body ""}
+```
+
+To get more correct http error responses, `ring/create-default-handler` can be used. It differentiates `:not-found` (no route matched), `:method-not-accepted` (no method matched) and `:not-acceptable` (handler returned `nil`).
+
+With defaults:
+
+```clj
+(def app
+  (ring/ring-handler
+    (ring/router
+      [["/ping" {:get handler}]
+       ["/pong" (constantly nil)]])
+    (ring/create-default-handler)))
+
+(app {:request-method :get, :uri "/ping"})
+; {:status 200, :body ""}
+
+(app {:request-method :get, :uri "/"})
+; {:status 404, :body ""}
+
+(app {:request-method :post, :uri "/ping"})
+; {:status 405, :body ""}
+
+(app {:request-method :get, :uri "/pong"})
+; {:status 406, :body ""}
+```
+
+With custom responses:
+
+```clj
+(def app
+  (ring/ring-handler
+    (ring/router
+      [["/ping" {:get handler}]
+       ["/pong" (constantly nil)]])
+    (ring/create-default-handler
+      {:not-found (constantly {:status 404, :body "kosh"})
+       :method-not-allowed (constantly {:status 405, :body "kosh"})
+       :not-acceptable (constantly {:status 406, :body "kosh"})})))
+
+(app {:request-method :get, :uri "/ping"})
+; {:status 200, :body ""}
+
+(app {:request-method :get, :uri "/"})
+; {:status 404, :body "kosh"}
+
+(app {:request-method :post, :uri "/ping"})
+; {:status 405, :body "kosh"}
+
+(app {:request-method :get, :uri "/pong"})
+; {:status 406, :body "kosh"}
 ```
 
 # Async 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