Merge pull request #232 from clojure-liberator/issue-76-npe-with-post…

…-options

Fix handler invocation: status, messages etc.

CHANGES.markdown

@@ -6,7 +6,13 @@
 
 * Values can be added to the context at the beginning of the execution
   flow using the :initialize-context action.
+* If no handler is specified, the key :message is looked up from the
+  context to create a default response.
 
+## Bugs fixed
+
+* #76 Nullpointer with post options
+* Allow decisions to override status in context
 * JSON body can be parsed into :request-entity by setting representation/parse-request-entity for :processable?
 
 ## Bugs fixed

src/liberator/core.clj

@@ -136,7 +136,7 @@
          combine
 
          ;; Status
-         {:status status}
+         {:status (:status context)}
 
          ;; ETags
          (when-let [etag (gen-etag context)]
@@ -147,7 +147,7 @@
            {:headers {"Last-Modified" (http-date last-modified)}})
 
          ;; 201 created required a location header to be send
-         (when (#{201 301 303 307} status)
+         (when (#{201 301 303 307} (:status context))
            (if-let [f (or (get context :location)
                           (get resource :location))]
              {:headers {"Location" (str ((make-function f) context))}}))
@@ -171,17 +171,14 @@
                  (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)))))]
+            (when-let [result (if-let [handler (get resource (keyword name))]
+                                (handler context)
+                                (get context :message))]
+              (let [as-response (:as-response resource)]
+                (as-response result context))))))]
     (cond
      (or (= :options (:request-method request)) (= 405 (:status response)))
-     (merge-with merge
+     (merge-with combine
                  {:headers (build-options-headers resource)}
                  response)
      (= :head (:request-method request))
@@ -563,7 +560,8 @@
    :patch-content-types []
 
    ;; The default function used extract a ring response from a handler's response
-   :as-response               as-response
+   :as-response               (fn [data ctx]
+                                (when data (as-response data ctx)))
 
    ;; Directives
    :available-media-types     []

test/test_errors.clj

@@ -55,7 +55,7 @@
     (fact resp => (content-type #"text/plain;charset=.*"))))
 
 (facts "custom exception handler not invoked if handler throws exception"
-  (let [res (resource :service-available? (fn [_ _] (throw (RuntimeException. "foo")))
-                      :handle-exception (fn [_ _] (throw (RuntimeException. "bar"))))]
+  (let [res (resource :service-available? (fn [_] (throw (RuntimeException. "error in service-available")))
+                      :handle-exception (fn [_] (throw (RuntimeException. "error in handle-exception"))))]
     (fact (res (-> (request :get "/")
-                   (header "Accept" "text/plain"))) => (throws RuntimeException))))
+                   (header "Accept" "text/plain"))) => (throws #"handle-exception"))))

test/test_execution_model.clj

@@ -50,11 +50,24 @@
     => (contains {:a 1 :status 404})))
 
 (facts "handler functions"
-  (fact "keyword as handler"
+  (fact "handler is a function"
     (-> (request :get "/")
-        ((resource :exists? {:some-key "foo"}
-                   :handle-ok :some-key)))
-    => (contains {:status 200 :body "foo"})))
+        ((resource :exists? false
+                   :handle-not-found (fn [ctx] "not found"))))
+    => (contains {:status 404 :body "not found"}))
+  (fact "keyword as handler"
+        (-> (request :get "/")
+            ((resource :exists? {:some-key "foo"}
+                       :handle-ok :some-key)))
+        => (contains {:status 200 :body "foo"}))
+  (fact "default handler uses message key"
+        (-> (request :get "/")
+            ((resource :exists? [false {:message "absent"}])))
+        => (contains {:status 404 :body "absent"}))
+  (fact "decisions can override status"
+        (-> (request :get "/")
+            ((resource :exists? [false {:status 444 :message "user defined status code"}])))
+        => (contains {:status 444 :body "user defined status code"})))
 
 (facts "context merge leaves nested objects intact (see #206)"
   (fact "using etag and if-match"

test/test_response.clj

@@ -83,3 +83,26 @@
         ((resource :handle-ok "ok")))
     => (fn [c] (not (contains? (:headers c) "Allow"))))
 )
+
+
+(facts "Options can return a body"
+      (fact "return a simple response"
+             (-> (request :options "/")
+                 ((resource :allowed-methods [:get :options]
+                            :handle-ok "ok"
+                            :handle-options "options")))
+             => (body "options"))
+      (fact "return a ring response"
+            (let [resp (-> (request :options "/")
+                           ((resource :allowed-methods [:get :options]
+                                      :available-media-types ["text/plain" "text/html"]
+                                      :handle-ok "ok"
+                                      :handle-options (fn [ctx]
+                                                        ;; workaround until issue #152 is fixed
+                                                        (-> "options"
+                                                            (as-response (assoc-in ctx [:representation :media-type]
+                                                                                   "text/plain"))
+                                                            (assoc-in [:headers "X-Foo"] "bar")
+                                                            (ring-response))))))]
+              resp => (body "options")
+              resp) => (header-value "X-Foo" "bar")))