Merge pull request #173 from clojure-liberator/169-always-call-as-res…

…ponse Call as-resource even for default handlers. fixes #169
clojure-liberator · Nov 24, 2014 · 32addcd · 32addcd
2 parents 1001b18 + 775ade8
commit 32addcd
Show file tree

Hide file tree

Showing 5 changed files with 54 additions and 44 deletions.
diff --git a/CHANGES.markdown b/CHANGES.markdown
@@ -1,10 +1,16 @@
 # Changelog
 
+# New in 0.12.3
+
+## Bugs fixed
+
+* #169 Always call as-response, even for default handlers
+
 # New in 0.12.2
 
 ## Bugs fixed
 
-* This release actually contains the changes announced for 0.12.1
+* #162 This release actually contains the changes announced for 0.12.1
   Due to whatever reason the revision in clojars did not match 
   what was tagged as 0.12.1 in the git repository.
 

diff --git a/src/liberator/core.clj b/src/liberator/core.clj
@@ -140,7 +140,7 @@
                    {:keys [resource request representation] :as context}]
   (let [context
         (merge {:status status :message message} context)
-        response 
+        response
         (merge-with
          combine
 
@@ -150,7 +150,7 @@
          ;; ETags
          (when-let [etag (gen-etag context)]
            {:headers {"ETag" etag}})
-         
+
          ;; Last modified
          (when-let [last-modified (gen-last-modified context)]
            {:headers {"Last-Modified" (http-date last-modified)}})
@@ -160,45 +160,42 @@
            (if-let [f (or (get context :location)
                           (get resource :location))]
              {:headers {"Location" (str ((make-function f) context))}}))
-
-         (if-let [handler (get resource (keyword name))]
-           (do
-             (log! :handler (keyword name))
-             ;; Content negotiations         
-             (merge-with
-              merge
-              {:headers
-               (-> {} 
-                   (set-header-maybe "Content-Type"
-                                     (str (:media-type representation)
-                                          (when-let [charset (:charset representation)] (str ";charset=" charset))))
-                   (set-header-maybe "Content-Language" (:language representation))
-                   (set-header-maybe "Content-Encoding"
-                                     (let [e (:encoding representation)]
-                                       (if-not (= "identity" e) e)))
-                   (set-header-maybe "Vary" (build-vary-header representation)))}
-              ;; Finally the result of the handler.  We allow the handler to
-              ;; override the status and headers.
-              (let [handler-response (handler context)
-                    ring-response ((:as-response resource) handler-response context)]
-                ring-response)))
-
-           ;; If there is no handler we just return the information we
-           ;; have so far.
-           (let [message (get context :message)]
-             (do (log! :handler (keyword name) "(default implementation)")
-                 {:status status 
-                  :headers {"Content-Type" "text/plain"} 
-                  :body (if (fn? message) (message context) message)}))))]
+
+
+         (do
+           (log! :handler (keyword name))
+           ;; Content negotiations
+           (merge-with
+            merge
+            {:headers
+             (-> {}
+                 (set-header-maybe "Content-Type"
+                                   (str (:media-type representation)
+                                        (when-let [charset (:charset representation)]
+                                          (str ";charset=" charset))))
+                 (set-header-maybe "Content-Language" (:language representation))
+                 (set-header-maybe "Content-Encoding"
+                                   (let [e (:encoding representation)]
+                                     (if-not (= "identity" e) e)))
+                 (set-header-maybe "Vary" (build-vary-header representation)))}
+            ;; Finally the result of the handler.  We allow the handler to
+            ;; override the status and headers.
+
+
+            (let [as-response (:as-response resource)]
+              (as-response
+               (if-let [handler (get resource (keyword name))]
+                 (handler context)
+                 (get context :message))
+               context)))))]
     (cond
      (or (= :options (:request-method request)) (= 405 (:status response)))
      (merge-with merge
                  {:headers (build-options-headers resource)}
                  response)
-
-      (= :head (:request-method request))
-        (dissoc response :body)
-      :else response)))
+     (= :head (:request-method request))
+     (dissoc response :body)
+     :else response)))
 
 (defmacro ^:private defhandler [name status message]
   `(defn ~name [context#]

diff --git a/test/test_conditionals.clj b/test/test_conditionals.clj
@@ -38,7 +38,7 @@
                 (-> (request :get "/")
                     (if-modified-since (http-date (as-date 1000)))))]
       (fact resp => NOT-MODIFIED)
-      (fact resp => (body nil?))
+      (fact resp => (no-body))
       (fact resp => (header-value "Last-Modified" (http-date (as-date 1000))))))
 
   (facts "if-unmodified-since true"

diff --git a/test/test_examples.clj b/test/test_examples.clj
@@ -64,8 +64,8 @@
          response => (contains {:status ?status}))
        (fact "has expected content-type"
          response => (content-type ?content-type))))
-   
+
    ?accept              ?available         ?status    ?content-type
    "text/html"          ["text/html"]      200        "text/html;charset=UTF-8"
-   "text/plain"         ["text/html"]      406         "text/plain"))
+   "text/plain"         ["text/html"]      406        "text/plain;charset=UTF-8"))
 
diff --git a/test/test_override_as_response.clj b/test/test_override_as_response.clj
@@ -11,12 +11,12 @@
         ((resource :available-media-types ["application/json"]
                    :handle-ok (fn [_] "some string")
                    :as-response (fn [d ctx] {:status 666 :body d}))
-         
+
          (request :get "/"))
         => (contains {:body "some string"
                       :headers (contains  {"Content-Type" "application/json"})
                       :status 666}))
-  
+
   (fact "necessary headers are added"
         ((resource :available-media-types ["application/json"]
                    :handle-ok (fn [_] "some string")
@@ -26,7 +26,7 @@
                                           "Vary" "Accept"})
                       :status 200
                       :body "some string"}))
-  
+
   (fact "custom as-reponse can call default as-response"
         ((resource :available-media-types ["text/plain"]
                    :handle-ok (fn [_] "some text")
@@ -35,4 +35,11 @@
          (request :get "/"))
         => (contains {:body "some text"
                       :headers (contains {"X-FOO" "BAR"})
-                      :status 200})))
+                      :status 200}))
+
+  (fact "custom as-response works with default handlers"
+        ((resource :available-media-types ["text/plain"]
+                   :as-response (fn [d ctx] {:foo :bar}))
+         (-> (request :get "/")
+             (header "Accept" "foo/bar")))
+        => (contains {:foo :bar })))