Skip to content

Improve consistency of async client arguments #526

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: 3.x
Choose a base branch
from
Open

Improve consistency of async client arguments #526

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 13 additions & 6 deletions README.org
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
126 changes: 56 additions & 70 deletions src/clj_http/client.clj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand All @@ -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
Expand Down
180 changes: 118 additions & 62 deletions test/clj_http/test/client_test.clj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down