Skip to content

Commit 7cd65ef

Browse files
committed
Better middleware testing
Mimic clj-http tests' use of middleware Translate clj-http's middleware to Aleph's when possible Split default middleware into client and request parts - most middleware changes req maps, but wrap-exceptions and wrap-request-timing change the client fn. Add missing :reason-phrase resp key Add missing :url wrap-url key Directly compare response bodies as bytes when not InputStreams Ignore DSN resolution tests
1 parent d1de95c commit 7cd65ef

File tree

7 files changed

+190
-72
lines changed

7 files changed

+190
-72
lines changed

project.clj

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@
5656
:java-source-paths ["src/aleph/utils"]
5757
:cljfmt {:indents {#".*" [[:inner 0]]}}
5858
:test-selectors {:default #(not
59-
(some #{:benchmark :stress :integration}
59+
(some #{:benchmark :stress :integration :ignore}
6060
(cons (:tag %) (keys %))))
6161
:benchmark :benchmark
6262
:integration :integration

src/aleph/http.clj

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,7 @@
102102
| `max-queue-size` | the maximum number of pending acquires from the pool that are allowed before `acquire` will start to throw a `java.util.concurrent.RejectedExecutionException`, defaults to `65536`
103103
| `control-period` | the interval, in milliseconds, between use of the controller to adjust the size of the pool, defaults to `60000`
104104
| `dns-options` | an optional map with async DNS resolver settings, for more information check `aleph.netty/dns-resolver-group`. When set, ignores `name-resolver` setting from `connection-options` in favor of shared DNS resolver instance
105-
| `middleware` | a function to modify request before sending, defaults to `aleph.http.client-middleware/wrap-request`
105+
| `middleware` | a function to modify clients/requests before sending, defaults to `aleph.http.client-middleware/wrap-request`
106106
| `pool-builder-fn` | an optional one arity function which returns a `io.aleph.dirigiste.IPool` from a map containing the following keys: `generate`, `destroy`, `control-period`, `max-queue-length` and `stats-callback`.
107107
| `pool-controller-builder-fn` | an optional zero arity function which returns a `io.aleph.dirigiste.IPool$Controller`.
108108

src/aleph/http/client_middleware.clj

Lines changed: 33 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,7 @@
155155
{:scheme (keyword (.getProtocol url-parsed))
156156
:server-name (.getHost url-parsed)
157157
:server-port (when-pos (.getPort url-parsed))
158+
:url url
158159
:uri (url-encode-illegal-characters (.getPath url-parsed))
159160
:user-info (when-let [user-info (.getUserInfo url-parsed)]
160161
(URLDecoder/decode user-info))
@@ -243,12 +244,13 @@
243244
(if (unexceptional-status? status)
244245
rsp
245246
(cond
246-
247247
(false? (opt req :throw-exceptions))
248248
rsp
249249

250250
(instance? InputStream body)
251-
(d/chain' (d/future (bs/to-byte-array body))
251+
(d/chain'
252+
(d/future
253+
(bs/to-byte-array body))
252254
(fn [body]
253255
(d/error-deferred
254256
(ex-info
@@ -563,9 +565,8 @@
563565
[req]
564566
(if-let [url (:url req)]
565567
(-> req
566-
(dissoc :url)
567-
(assoc :request-url url)
568-
(merge (parse-url url)))
568+
(assoc :request-url url)
569+
(merge (parse-url url)))
569570
req))
570571

571572
(defn wrap-request-timing
@@ -918,7 +919,13 @@
918919
(opt req :save-request)
919920
(assoc :aleph/request req'))))
920921

922+
(def default-client-middleware
923+
"Default middleware that takes a client fn"
924+
[wrap-exceptions
925+
wrap-request-timing])
926+
921927
(def default-middleware
928+
"Default middleware that takes a request map"
922929
[wrap-method
923930
wrap-url
924931
wrap-nested-params
@@ -937,22 +944,24 @@
937944
"Returns a batteries-included HTTP request function corresponding to the given
938945
core client. See default-middleware for the middleware wrappers that are used
939946
by default"
940-
[client]
941-
(let [client' (-> client
942-
wrap-exceptions
943-
wrap-request-timing)]
944-
(fn [req]
945-
(let [executor (ex/executor)]
946-
(if (:aleph.http.client/close req)
947-
(client req)
948-
949-
(let [req' (reduce #(%2 %1) req default-middleware)]
950-
(d/chain' (client' req')
951-
952-
;; coerce the response body
953-
(fn [{:keys [body] :as rsp}]
954-
(let [rsp' (handle-response-debug req' rsp)]
955-
(if (and (some? body) (some? (:as req')))
956-
(d/future-with (or executor (ex/wait-pool))
957-
(coerce-response-body req' rsp'))
958-
rsp'))))))))))
947+
([client]
948+
(wrap-request client default-client-middleware default-middleware))
949+
([client client-middleware middleware]
950+
(let [client' (reduce #(%2 %1)
951+
client
952+
client-middleware)]
953+
(fn [req]
954+
(let [executor (ex/executor)]
955+
(if (:aleph.http.client/close req)
956+
(client req)
957+
958+
(let [req' (reduce #(%2 %1) req middleware)]
959+
(d/chain' (client' req')
960+
961+
;; coerce the response body
962+
(fn [{:keys [body] :as rsp}]
963+
(let [rsp' (handle-response-debug req' rsp)]
964+
(if (and (some? body) (some? (:as req')))
965+
(d/future-with (or executor (ex/wait-pool))
966+
(coerce-response-body req' rsp'))
967+
rsp')))))))))))

src/aleph/http/core.clj

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -224,9 +224,10 @@
224224
:remote-addr (netty/channel-remote-address ch))
225225

226226
(p/def-derived-map NettyResponse [^HttpResponse rsp complete body]
227-
:status (-> rsp .status .code)
227+
:status (-> rsp (.status) (.code))
228+
:reason-phrase (-> rsp (.status) (.reasonPhrase))
228229
:aleph/keep-alive? (HttpUtil/isKeepAlive rsp)
229-
:headers (-> rsp .headers headers->map)
230+
:headers (-> rsp (.headers) headers->map)
230231
:aleph/complete complete
231232
:body body)
232233

test/aleph/http/clj_http/client_test.clj

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
(ns aleph.http.clj-http.client-test
22
(:require [aleph.http.clj-http.core-test :refer [run-server]]
3-
[aleph.http.clj-http.util :refer [request]]
3+
[aleph.http.clj-http.util :refer [make-request]]
44
[cheshire.core :as json]
55
[clj-http.client :as client]
66
[clj-http.conn-mgr :as conn]
@@ -26,6 +26,8 @@
2626
:server-name "localhost"
2727
:server-port 18080})
2828

29+
(def request (make-request #'client/request {:using-middleware? true}))
30+
2931
(defn parse-form-params [s]
3032
(->> (str/split (form-decode-str s) #"&")
3133
(map #(str/split % #"="))

test/aleph/http/clj_http/core_test.clj

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
(ns aleph.http.clj-http.core-test
2-
(:require [aleph.http.clj-http.util :refer [request]]
2+
(:require [aleph.http.clj-http.util :refer [make-request]]
33
[cheshire.core :as json]
44
[clj-http.client :as client]
55
[clj-http.conn-mgr :as conn]
@@ -156,6 +156,8 @@
156156
:server-name "localhost"
157157
:server-port 18080})
158158

159+
(def request (make-request #'core/request {:using-middleware? false}))
160+
159161
(defn slurp-body [req]
160162
(slurp (:body req)))
161163

@@ -165,7 +167,7 @@
165167
(is (= 200 (:status resp)))
166168
(is (= "get" (slurp-body resp)))))
167169

168-
(deftest ^:integration dns-resolver
170+
(deftest ^:ignore dns-resolver
169171
(run-server)
170172
(let [custom-dns-resolver (doto (InMemoryDnsResolver.)
171173
(.add "foo.bar.com" (into-array[(InetAddress/getByAddress (byte-array [127 0 0 1]))])))
@@ -175,15 +177,15 @@
175177
(is (= 200 (:status resp)))
176178
(is (= "get" (slurp-body resp)))))
177179

178-
(deftest ^:integration dns-resolver-unknown-host
180+
(deftest ^:ignore dns-resolver-unknown-host
179181
(run-server)
180182
(let [custom-dns-resolver (doto (InMemoryDnsResolver.)
181183
(.add "foo.bar.com" (into-array[(InetAddress/getByAddress (byte-array [127 0 0 1]))])))]
182184
(is (thrown? java.net.UnknownHostException (request {:request-method :get :uri "/get"
183185
:server-name "www.google.com"
184186
:dns-resolver custom-dns-resolver})))))
185187

186-
(deftest ^:integration dns-resolver-reusable-connection-manager
188+
(deftest ^:ignore dns-resolver-reusable-connection-manager
187189
(run-server)
188190
(let [custom-dns-resolver (doto (InMemoryDnsResolver.)
189191
(.add "totallynonexistant.google.com"

test/aleph/http/clj_http/util.clj

Lines changed: 143 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,14 @@
11
(ns aleph.http.clj-http.util
22
(:require
33
[aleph.http :as http]
4+
[aleph.http.client-middleware :as aleph.mid]
45
[clj-commons.byte-streams :as bs]
56
[clj-http.core :as clj-http]
7+
[clj-http.client]
68
[clojure.set :as set]
7-
[clojure.test :refer :all])
9+
[clojure.string :as str]
10+
[clojure.test :refer :all]
11+
[clojure.tools.logging :as log])
812
(:import
913
(java.io ByteArrayInputStream
1014
ByteArrayOutputStream
@@ -41,47 +45,147 @@
4145
aleph-common-headers (select-keys aleph-headers ks-intersection)]
4246
(is (= clj-http-common-headers aleph-common-headers)))))
4347

44-
(defn is-input-stream=
45-
"Are the two body InputStreams equal?
48+
(defn bodies=
49+
"Are the two bodies equal? clj-http's client/request fn coerces to strings by default,
50+
while the core/request leaves the body an InputStream.
51+
Aleph, in keeping with it's stream-based nature, leaves as an InputStream by default.
4652
47-
Returns a new ByteArrayInputStream based on the consumed original"
48-
[^InputStream clj-http-body ^InputStream aleph-body]
53+
If an InputStream, returns a new ByteArrayInputStream based on the consumed original"
54+
[clj-http-body ^InputStream aleph-body]
4955
(if clj-http-body
50-
(do
51-
(is (some? aleph-body) "Why is aleph body nil? It should be an empty InputStream for now...")
52-
(let [baos (ByteArrayOutputStream.)]
53-
(.transferTo clj-http-body baos) ; not avail until JDK 9
54-
55-
(let [clj-http-body-bytes (.toByteArray baos)]
56-
(is (= (count clj-http-body-bytes)
57-
(.available aleph-body)))
58-
(is (bs/bytes= clj-http-body-bytes aleph-body))
59-
60-
(proxy [FilterInputStream]
61-
[^InputStream (ByteArrayInputStream. clj-http-body-bytes)]
62-
(close []
63-
(.close clj-http-body)
64-
(proxy-super close))))))
56+
(condp instance? clj-http-body
57+
InputStream
58+
(do
59+
(is (some? aleph-body) "Why is aleph body nil? It should be an empty InputStream for now...")
60+
(let [baos (ByteArrayOutputStream.)]
61+
(.transferTo clj-http-body baos) ; not avail until JDK 9
62+
63+
(let [clj-http-body-bytes (.toByteArray baos)]
64+
(is (= (count clj-http-body-bytes)
65+
(.available aleph-body)))
66+
(is (bs/bytes= clj-http-body-bytes aleph-body))
67+
68+
(proxy [FilterInputStream]
69+
[^InputStream (ByteArrayInputStream. clj-http-body-bytes)]
70+
(close []
71+
(.close clj-http-body)
72+
(proxy-super close))))))
73+
74+
(do
75+
(is (bs/bytes= clj-http-body aleph-body))
76+
clj-http-body))
6577
(do
6678
(is (= clj-http-body aleph-body))
6779
clj-http-body)))
6880

69-
(defn request
70-
"Modified version of original, that also sends request via Aleph, and
71-
tests the responses for equality."
72-
([req]
73-
(request req nil nil))
74-
([req respond raise]
75-
(if (or respond raise)
76-
;; do not attempt to compare when using async clj-http...for now
77-
(let [ring-map (merge base-req req)]
78-
(clj-http/request ring-map respond raise))
79-
80-
(let [clj-http-ring-map (merge base-req req)
81-
aleph-ring-map (merge base-req req {:pool no-middleware-pool})
82-
clj-http-resp (clj-http/request clj-http-ring-map)
83-
aleph-resp @(http/request aleph-ring-map)]
84-
(is (= (:status clj-http-resp) (:status aleph-resp)))
85-
(is-headers= (:headers clj-http-resp) (:headers aleph-resp))
86-
(let [new-clj-http-body (is-input-stream= (:body clj-http-resp) (:body aleph-resp))]
87-
(assoc clj-http-resp :body new-clj-http-body))))))
81+
82+
(defn- defined-middleware
83+
"Returns a set of symbols beginning with `wrap-` in the ns"
84+
[ns]
85+
(->> (ns-publics ns)
86+
keys
87+
(map str)
88+
(filter #(str/starts-with? % "wrap-"))
89+
(map symbol)
90+
set))
91+
92+
(defn- aleph-test-conn-pool
93+
"clj-http middleware is traditional fn-based middleware, using a 3-arity version to handle async.
94+
95+
Aleph usually uses a more async-friendly interceptor-style, where the middleware transforms the request maps,
96+
but does nothing about calling the next fn in the chain.
97+
98+
Unfortunately, a couple middleware cannot be converted to interceptor-style, complicating things."
99+
[middleware-list]
100+
(let [missing-midw (set/difference
101+
(defined-middleware 'clj-http.client)
102+
(defined-middleware 'aleph.http.client-middleware))]
103+
(when-not (seq missing-midw)
104+
(println "clj-http is using middleware that aleph lacks:")
105+
(prn missing-midw)
106+
(log/warn "clj-http is using middleware that aleph lacks"
107+
:missing-middleware missing-midw)))
108+
(let [non-interceptor-middleware (set aleph.mid/default-client-middleware)
109+
client-middleware (cond-> []
110+
(some #{clj-http.client/wrap-exceptions} middleware-list)
111+
(conj aleph.mid/wrap-exceptions)
112+
113+
(some #{clj-http.client/wrap-request-timing} middleware-list)
114+
(conj aleph.mid/wrap-request-timing))
115+
middleware-list' (->> middleware-list
116+
(map (fn [midw]
117+
(-> midw
118+
class
119+
str
120+
(str/split #"\$")
121+
peek
122+
(str/replace "_" "-")
123+
(->> (symbol "aleph.http.client-middleware"))
124+
requiring-resolve)))
125+
(filter some?)
126+
(map var-get)
127+
(remove non-interceptor-middleware)
128+
vec)]
129+
;;(println "Client-based middleware:")
130+
;;(prn client-middleware)
131+
;;(println "Regular middleware:")
132+
;;(prn middleware-list')
133+
(http/connection-pool {:middleware #(aleph.mid/wrap-request % client-middleware middleware-list')})))
134+
135+
136+
(defn- print-middleware-list
137+
[middleware-list]
138+
(prn (mapv (fn [midw]
139+
(-> midw
140+
class
141+
str
142+
(str/split #"\$")
143+
peek
144+
(str/replace "_" "-")
145+
symbol))
146+
middleware-list)))
147+
148+
(defn make-request
149+
"Need to switch between clj-http's core/request and client/request.
150+
151+
Modified version of original request fns, that also sends requests
152+
via Aleph, and tests the responses for equality."
153+
[clj-http-request {:keys [using-middleware?]}]
154+
(fn compare-request
155+
([req]
156+
(compare-request req nil nil))
157+
([req respond raise]
158+
(if (or respond raise)
159+
;; do not attempt to compare when using async clj-http...for now
160+
(let [ring-map (merge base-req req)]
161+
(clj-http-request ring-map respond raise))
162+
163+
(let [clj-http-ring-map (merge base-req req)
164+
;;_ (prn clj-http-ring-map)
165+
clj-http-middleware (if using-middleware? clj-http.client/*current-middleware* [])
166+
;;_ (print-middleware-list clj-http.client/*current-middleware*)
167+
aleph-ring-map (merge base-req req {:pool (aleph-test-conn-pool clj-http-middleware)})
168+
;;_ (prn aleph-ring-map)
169+
clj-http-resp (clj-http-request clj-http-ring-map)
170+
aleph-resp @(http/request aleph-ring-map)]
171+
(is (= (:status clj-http-resp) (:status aleph-resp)))
172+
173+
174+
#_(when (not= (:status clj-http-resp) (:status aleph-resp))
175+
(println "clj-http req:")
176+
(prn clj-http-ring-map)
177+
(println)
178+
(println "clj-http resp:")
179+
(prn clj-http-resp)
180+
(println)
181+
(println)
182+
(println "aleph req:")
183+
(prn aleph-ring-map)
184+
(println)
185+
(println "aleph resp:")
186+
(prn aleph-resp))
187+
188+
(is-headers= (:headers clj-http-resp) (:headers aleph-resp))
189+
(is (instance? InputStream (:body aleph-resp)))
190+
(let [new-clj-http-body (bodies= (:body clj-http-resp) (:body aleph-resp))]
191+
(assoc clj-http-resp :body new-clj-http-body)))))))

0 commit comments

Comments
 (0)