Make the not-found-handler fix backwards compatible

PR #471 aimed to fix issue #464. However, the change was slightly
backwards-incompatible, since it made the file and resource handlers use
the default 404 handler when mounted outside of the router. The previous
behavior was to return nil in that case.

This patch restores the previous behavior and clarifies that `:path`
option can be used only when the file/resource handler is mounted
outside of a router.
This commit is contained in:
Miikka Koskinen 2021-02-26 08:14:01 +02:00
parent 88897a2264
commit 5824d9eeef
2 changed files with 45 additions and 3 deletions

View file

@ -195,7 +195,9 @@
root "public"
index-files ["index.html"]
paths (constantly nil)
not-found-handler (constantly {:status 404, :body "", :headers {}})}}]
not-found-handler (if path
(constantly nil)
(constantly {:status 404, :body "", :headers {}}))}}]
(let [options {:root root
:loader loader
:index-files? false
@ -239,7 +241,7 @@
| -------------------|-------------|
| :parameter | optional name of the wildcard parameter, defaults to unnamed keyword `:`
| :root | optional resource root, defaults to `\"public\"`
| :path | optional path to mount the handler to. Works only if mounted outside of a router.
| :path | path to mount the handler to. Required when mounted outside of a router, does not work inside a router.
| :loader | optional class loader to resolve the resources
| :index-files | optional vector of index-files to look in a resource directory, defaults to `[\"index.html\"]`
| :not-found-handler | optional handler function to use if the requested resource is missing (404 Not Found)"
@ -256,7 +258,7 @@
| -------------------|-------------|
| :parameter | optional name of the wildcard parameter, defaults to unnamed keyword `:`
| :root | optional resource root, defaults to `\"public\"`
| :path | optional path to mount the handler to. Works only if mounted outside of a router.
| :path | path to mount the handler to. Required when mounted outside of a router, does not work inside a router.
| :loader | optional class loader to resolve the resources
| :index-files | optional vector of index-files to look in a resource directory, defaults to `[\"index.html\"]`
| :not-found-handler | optional handler function to use if the requested resource is missing (404 Not Found)"

View file

@ -618,6 +618,46 @@
(is (get-in @result [:headers "Last-Modified"]))
(is (= "<xml><hello>file</hello></xml>\n" (slurp (:body @result))))))))))))))
#?(:clj
(deftest file-resource-handler-not-found-test
(let [redirect (fn [uri] {:status 302, :body "", :headers {"Location" uri}})
request (fn [uri] {:uri uri, :request-method :get})
not-found-handler (fn [_] {:status 404, :body "not-found-handler"})]
(doseq [[name create] [["resource-handler" ring/create-resource-handler]
["file-handler" #(ring/create-file-handler (assoc % :root "dev-resources/public"))]]]
(testing (str "for " name)
(testing "inside a router"
(let [create-app (fn [handler]
(ring/ring-handler
(ring/router
["/files/*" handler])))]
(testing "not-found-handler not set"
(let [app (create-app (create nil))]
(is (nil? (app (request "/not-found"))))
(is (= "" (:body (app (request "/files/not-found")))))))
(testing "not-found-handler set"
(let [app (create-app (create {:not-found-handler not-found-handler}))]
(is (nil? (app (request "/not-found"))))
(is (= "not-found-handler" (:body (app (request "/files/not-found")))))))))
(testing "outside a router"
(let [create-app (fn [handler]
(ring/ring-handler
(ring/router [])
handler))]
(testing "not-found-handler not set"
(let [app (create-app (create {:path "/files"}))]
(is (nil? (app (request "/not-found"))))
(is (nil? (app (request "/files/not-found"))))))
(testing "not-found-handler set"
(let [app (create-app (create {:path "/files" :not-found-handler not-found-handler}))]
(is (nil? (app (request "/not-found"))))
(is (= "not-found-handler" (:body (app (request "/files/not-found"))))))))))))))
(deftest router-available-in-default-branch
(testing "1-arity"
((ring/ring-handler