Improve consistency of async client arguments

Ensures arguments for request & method-functions (get/put/post etc.) have
similarly.

Fixes #520.

Author: rymndhng

README.org

@@ -428,12 +428,19 @@ start an async request is easy, for example:
 
 #+BEGIN_SRC clojure
 ;; :async? in options map need to be true
-(client/get "http://example.com"
-            {:async? true}
-            ;; respond callback
-            (fn [response] (println "response is:" response))
-            ;; raise callback
-            (fn [exception] (println "exception message is: " (.getMessage exception))))
+  (client/get "http://example.com"
+              {:async true }
+              ;; respond callback
+              (fn [response] (println "response is:" response))
+              ;; raise callback
+              (fn [exception] (println "exception message is: " (.getMessage exception))))
+
+  ;; equivalent using request
+  (client/request {:method :get
+                   :url "http://example.com"
+                   :async true
+                   :respond (fn [response] (println "response is:" response))
+                   :raise (fn [exception] println "exception message is: " (.getMessage exception))})
 #+END_SRC
 
 All exceptions thrown during the request will be passed to the raise callback.

src/clj_http/client.clj

@@ -1112,15 +1112,34 @@
   Automatically bound when `with-middleware` is used."
   default-middleware)
 
+(defn- async-transform
+  [client]
+  (fn
+    ([req]
+     (cond
+       (opt req :async)
+       (let [{:keys [respond raise]} req]
+         (when (some nil? [respond raise])
+           (throw (IllegalArgumentException. "If :async? is true, you must pass respond and raise")))
+         (client req respond raise))
+
+       :else
+       (client req)))
+
+    ;; In versions of clj-http older than 3.11, the 3-arity invocation implied the
+    ;; request should be handled as async
+    ([req respond raise]
+     (client (assoc req :async true) respond raise))))
+
 (defn wrap-request
   "Returns a batteries-included HTTP request function corresponding to the given
   core client. See default-middleware for the middleware wrappers that are used
   by default"
-  [request]
-  (reduce (fn wrap-request* [request middleware]
-            (middleware request))
-          request
-          default-middleware))
+  ([request]
+   (wrap-request request default-middleware))
+  ([request middleware]
+   (async-transform
+    (reduce #(%2 %1) request middleware))))
 
 (def ^:dynamic request
   "Executes the HTTP request corresponding to the given map and returns
@@ -1149,74 +1168,43 @@
   option."
   (wrap-request #'core/request))
 
+(alter-meta! #'request assoc :arglists '([req] [req respond raise]))
+
 ;; Inline function to throw a slightly more readable exception when
 ;; the URL is nil
 (definline check-url! [url]
   `(when (nil? ~url)
      (throw (IllegalArgumentException. "Host URL cannot be nil"))))
 
-(defn- request*
-  [req [respond raise]]
-  (if (opt req :async)
-    (if (some nil? [respond raise])
-      (throw (IllegalArgumentException.
-              "If :async? is true, you must pass respond and raise"))
-      (request (dissoc req :respond :raise) respond raise))
-    (request req)))
-
-(defn get
-  "Like #'request, but sets the :method and :url as appropriate."
-  [url & [req & r]]
-  (check-url! url)
-  (request* (merge req {:method :get :url url}) r))
-
-(defn head
-  "Like #'request, but sets the :method and :url as appropriate."
-  [url & [req & r]]
-  (check-url! url)
-  (request* (merge req {:method :head :url url}) r))
-
-(defn post
-  "Like #'request, but sets the :method and :url as appropriate."
-  [url & [req & r]]
-  (check-url! url)
-  (request* (merge req {:method :post :url url}) r))
-
-(defn put
-  "Like #'request, but sets the :method and :url as appropriate."
-  [url & [req & r]]
-  (check-url! url)
-  (request* (merge req {:method :put :url url}) r))
-
-(defn delete
-  "Like #'request, but sets the :method and :url as appropriate."
-  [url & [req & r]]
-  (check-url! url)
-  (request* (merge req {:method :delete :url url}) r))
-
-(defn options
-  "Like #'request, but sets the :method and :url as appropriate."
-  [url & [req & r]]
-  (check-url! url)
-  (request* (merge req {:method :options :url url}) r))
-
-(defn copy
-  "Like #'request, but sets the :method and :url as appropriate."
-  [url & [req & r]]
-  (check-url! url)
-  (request* (merge req {:method :copy :url url}) r))
-
-(defn move
-  "Like #'request, but sets the :method and :url as appropriate."
-  [url & [req & r]]
-  (check-url! url)
-  (request* (merge req {:method :move :url url}) r))
-
-(defn patch
-  "Like #'request, but sets the :method and :url as appropriate."
-  [url & [req & r]]
-  (check-url! url)
-  (request* (merge req {:method :patch :url url}) r))
+(defn- request-method
+  ([method url]
+   (check-url! url)
+   (request {:method method :url url}))
+  ([method url req]
+   (check-url! url)
+   (request (merge req {:method method :url url})))
+  ([method url req respond raise]
+   (check-url! url)
+   (request (merge req {:method method :url url :async true :respond respond :raise raise}))))
+
+(defmacro ^:private def-http-method [method]
+  `(do
+     (def ~method (partial request-method ~(keyword method)))
+     (alter-meta! (resolve '~method) assoc
+                  :doc ~(str "Like #'request, but sets the :method and :url as appropriate.")
+                  :arglists '([~(symbol "url")]
+                              [~(symbol "url") ~(symbol "req")]
+                              [~(symbol "url") ~(symbol "req") ~(symbol "respond") ~(symbol "raise")]))))
+
+(def-http-method get)
+(def-http-method head)
+(def-http-method post)
+(def-http-method put)
+(def-http-method delete)
+(def-http-method options)
+(def-http-method copy)
+(def-http-method move)
+(def-http-method patch)
 
 (defmacro with-middleware
   "Perform the body of the macro with a custom middleware list.
@@ -1229,9 +1217,7 @@
   [middleware & body]
   `(let [m# ~middleware]
      (binding [*current-middleware* m#
-               clj-http.client/request (reduce #(%2 %1)
-                                               clj-http.core/request
-                                               m#)]
+               clj-http.client/request (wrap-request clj-http.core/request m#)]
        ~@body)))
 
 (defmacro with-additional-middleware

test/clj_http/test/client_test.clj

@@ -40,76 +40,132 @@
 
 (deftest ^:integration roundtrip
   (run-server)
-  ;; roundtrip with scheme as a keyword
-  (let [resp (request {:uri "/get" :method :get})]
-    (is (= 200 (:status resp)))
-    (is (= "close" (get-in resp [:headers "connection"])))
-    (is (= "get" (:body resp))))
-  ;; roundtrip with scheme as a string
-  (let [resp (request {:uri "/get" :method :get
-                       :scheme "http"})]
-    (is (= 200 (:status resp)))
-    (is (= "close" (get-in resp [:headers "connection"])))
-    (is (= "get" (:body resp))))
-  (let [params {:a "1" :b "2"}]
-    (doseq [[content-type read-fn]
-            [[nil (comp parse-form-params slurp)]
-             [:x-www-form-urlencoded (comp parse-form-params slurp)]
-             [:edn (comp read-string slurp)]
-             [:transit+json #(client/parse-transit % :json)]
-             [:transit+msgpack #(client/parse-transit % :msgpack)]]]
-      (let [resp (request {:uri "/post"
-                           :as :stream
-                           :method :post
-                           :content-type content-type
-                           :form-params params})]
-        (is (= 200 (:status resp)))
-        (is (= "close" (get-in resp [:headers "connection"])))
-        (is (= params (read-fn (:body resp)))
-            (str "failed with content-type [" content-type "]"))))))
+  (testing "roundtrip with scheme as a keyword"
+    (let [resp (request {:uri "/get" :method :get})]
+      (is (= 200 (:status resp)))
+      (is (= "close" (get-in resp [:headers "connection"])))
+      (is (= "get" (:body resp)))))
+  (testing "roundtrip with scheme as string"
+          (let [resp (request {:uri "/get" :method :get
+                               :scheme "http"})]
+            (is (= 200 (:status resp)))
+            (is (= "close" (get-in resp [:headers "connection"])))
+            (is (= "get" (:body resp)))))
+  (testing "roundtrip with response parsing"
+    (let [params {:a "1" :b "2"}]
+      (doseq [[content-type read-fn]
+              [[nil (comp parse-form-params slurp)]
+               [:x-www-form-urlencoded (comp parse-form-params slurp)]
+               [:edn (comp read-string slurp)]
+               [:transit+json #(client/parse-transit % :json)]
+               [:transit+msgpack #(client/parse-transit % :msgpack)]]]
+        (let [resp (request {:uri "/post"
+                             :as :stream
+                             :method :post
+                             :content-type content-type
+                             :form-params params})]
+          (is (= 200 (:status resp)))
+          (is (= "close" (get-in resp [:headers "connection"])))
+          (is (= params (read-fn (:body resp)))
+              (str "failed with content-type [" content-type "]")))))))
 
 (deftest ^:integration roundtrip-async
   (run-server)
-  ;; roundtrip with scheme as a keyword
-  (let [resp (promise)
-        exception (promise)
-        _ (request {:uri "/get" :method :get
-                    :async? true} resp exception)]
-    (is (= 200 (:status @resp)))
-    (is (= "close" (get-in @resp [:headers "connection"])))
-    (is (= "get" (:body @resp)))
-    (is (not (realized? exception))))
-  ;; roundtrip with scheme as a string
-  (let [resp (promise)
-        exception (promise)
-        _ (request {:uri "/get" :method :get
-                    :scheme "http"
-                    :async? true} resp exception)]
-    (is (= 200 (:status @resp)))
-    (is (= "close" (get-in @resp [:headers "connection"])))
-    (is (= "get" (:body @resp)))
-    (is (not (realized? exception))))
-
-  (let [params {:a "1" :b "2"}]
-    (doseq [[content-type read-fn]
-            [[nil (comp parse-form-params slurp)]
-             [:x-www-form-urlencoded (comp parse-form-params slurp)]
-             [:edn (comp read-string slurp)]
-             [:transit+json #(client/parse-transit % :json)]
-             [:transit+msgpack #(client/parse-transit % :msgpack)]]]
+  (testing "roundtrip with scheme as keyword"
+    (testing "with async callback arguments"
       (let [resp (promise)
             exception (promise)
-            _ (request {:uri "/post"
-                        :as :stream
-                        :method :post
-                        :content-type content-type
-                        :flatten-nested-keys []
-                        :form-params params
+            _ (request {:uri "/get" :method :get
                         :async? true} resp exception)]
         (is (= 200 (:status @resp)))
         (is (= "close" (get-in @resp [:headers "connection"])))
-        (is (= params (read-fn (:body @resp))))
-        (is (not (realized? exception)))))))
+        (is (= "get" (:body @resp)))
+        (is (not (realized? exception)))))
+    (testing "with respond and raise attributes"
+      (let [resp (promise)
+            exception (promise)
+            _ (request {:uri "/get" :method :get
+                        :async? true
+                        :respond resp
+                        :raise exception
+                        })]
+        (is (= 200 (:status @resp)))
+        (is (= "close" (get-in @resp [:headers "connection"])))
+        (is (= "get" (:body @resp)))
+        (is (not (realized? exception))))))
+  (testing "round trip with scheme as string"
+    (let [resp (promise)
+          exception (promise)
+          _ (request {:uri "/get" :method :get
+                      :scheme "http"
+                      :async? true} resp exception)]
+      (is (= 200 (:status @resp)))
+      (is (= "close" (get-in @resp [:headers "connection"])))
+      (is (= "get" (:body @resp)))
+      (is (not (realized? exception)))))
+  (testing "roundtrip with error handling"
+    (testing "with async callback arguments"
+      (let [resp (promise)
+            exception (promise)
+            _ (request {:uri "/error" :method :get
+                        :scheme "http"
+                        :async? true} resp exception)]
+        (is (instance? Exception @exception))))
+    (testing "with respond and raise attributes"
+      (let [resp (promise)
+            exception (promise)
+            _ (request {:uri "/error" :method :get
+                        :scheme "http"
+                        :async? true
+                        :respond resp
+                        :raise exception})]
+        (is (instance? Exception @exception)))))
+  (testing "roundtrip with response parsing"
+    (testing "with async callback arguments"
+      (let [params {:a "1" :b "2"}]
+        (doseq [[content-type read-fn]
+                [[nil (comp parse-form-params slurp)]
+                 [:x-www-form-urlencoded (comp parse-form-params slurp)]
+                 [:edn (comp read-string slurp)]
+                 [:transit+json #(client/parse-transit % :json)]
+                 [:transit+msgpack #(client/parse-transit % :msgpack)]]]
+          (let [resp (promise)
+                exception (promise)
+                _ (request {:uri "/post"
+                            :as :stream
+                            :method :post
+                            :content-type content-type
+                            :flatten-nested-keys []
+                            :form-params params
+                            :async? true} resp exception)]
+            (is (= 200 (:status @resp)))
+            (is (= "close" (get-in @resp [:headers "connection"])))
+            (is (= params (read-fn (:body @resp))))
+            (is (not (realized? exception)))))))
+
+    (testing "with respond and raise attributes"
+      (let [params {:a "1" :b "2"}]
+        (doseq [[content-type read-fn]
+                [[nil (comp parse-form-params slurp)]
+                 [:x-www-form-urlencoded (comp parse-form-params slurp)]
+                 [:edn (comp read-string slurp)]
+                 [:transit+json #(client/parse-transit % :json)]
+                 [:transit+msgpack #(client/parse-transit % :msgpack)]]]
+          (let [resp (promise)
+                exception (promise)
+                _ (request {:uri "/post"
+                            :as :stream
+                            :method :post
+                            :content-type content-type
+                            :flatten-nested-keys []
+                            :form-params params
+                            :async? true
+                            :respond resp
+                            :raise exception})]
+            (is (= 200 (:status @resp)))
+            (is (= "close" (get-in @resp [:headers "connection"])))
+            (is (= params (read-fn (:body @resp))))
+            (is (not (realized? exception)))))))))
 
 (def ^:dynamic *test-dynamic-var* nil)