Big refactor of connection pooling stuff

Author: dakrone

Readme.md

@@ -461,7 +461,21 @@ This is MUCH faster than sequentially performing all requests, because
 a persistent connection can be used instead creating a new connection
 for each request.
 
-This feature is fairly new, please let me know if you have any feedback!
+If you would prefer to handle managing the connection manager
+yourself, you can create a connection manager yourself and specify it
+for each request:
+
+```clojure
+(def cm (clj-http.conn-mgr/make-reusable-conn-manager {:timeout 2 :threads 3}))
+(def cm2 (clj-http.conn-mgr/make-reusable-conn-manager {:timeout 10 :threads 1}))
+
+(get "http://aoeu.com/1" {:connection-manager cm2})
+(post "http://aoeu.com/2" {:connection-manager cm})
+(get "http://aoeu.com/3" {:connection-manager cm2})
+```
+
+See the docstring on `make-reusable-conn-manager` for options and
+default values.
 
 ### Redirects handling
 clj-http conforms its behaviour regarding automatic redirects to the

changelog.org

@@ -232,6 +232,11 @@
 * Work log
   Log of merges/issues/work that's gone in so I know what to put in
   the changelog for the next release
+** 2012-01-30
+   - merged https://github.com/dakrone/clj-http/pull/113 to update
+     the connection pooling code
+   - refactor pooled connection managers to allow specifying
+     the :connection-manager option
 ** 2012-01-19
    - merged https://github.com/dakrone/clj-http/pull/112 to allow
      json coercion on error responses when :as :auto is used

src/clj_http/client.clj

@@ -691,11 +691,9 @@
        (filter #(.startsWith (str (:name (meta %))) "wrap-"))
        vec))
 
-(def dmcpr ConnPerRouteBean/DEFAULT_MAX_CONNECTIONS_PER_ROUTE)
-
 (defmacro with-connection-pool
   "Macro to execute the body using a connection manager. Creates a
-  ThreadSafeClientConnectionManager to use for all requests within the body of
+  PoolingClientConnectionManager to use for all requests within the body of
   the expression. An option map is allowed to set options for the connection
   manager.
 
@@ -720,32 +718,14 @@
   If the value 'nil' is specified or the value is not set, the default value
   will be used."
   [opts & body]
-  `(let [timeout# (or (:timeout ~opts) 5)
-         threads# (or (:threads ~opts) 4)
-         default-per-route# (or (:default-per-route ~opts)
-                                clj-http.client/dmcpr)
-         insecure?# (:insecure? ~opts)
-         leftovers# (dissoc ~opts :timeout :threads :insecure?)]
-     ;; I'm leaving the connection bindable for now because in the
-     ;; future I'm toying with the idea of managing the connection
-     ;; manager yourself and passing it into the request
-     (binding [conn/*connection-manager*
-               (doto (conn/make-reusable-conn-manager
-                      (merge {:timeout timeout#
-                              :insecure? insecure?#}
-                             leftovers#))
-                 (.setMaxTotal threads#)
-                 (.setDefaultMaxPerRoute default-per-route#))]
+  ;; I'm leaving the connection bindable for now because in the
+  ;; future I'm toying with the idea of managing the connection
+  ;; manager yourself and passing it into the request
+  `(let [cm# (conn/make-reusable-conn-manager ~opts)]
+     (binding [conn/*connection-manager* cm#]
        (try
          ~@body
          (finally
            (.shutdown
-             ^org.apache.http.impl.conn.PoolingClientConnectionManager
+            ^org.apache.http.impl.conn.PoolingClientConnectionManager
             conn/*connection-manager*))))))
-
-(defmacro with-named-connection-pool
-  "Implementation of the above comment for binding."
-  [connection & body]
-     `(binding [conn/*connection-manager*
-               ~connection]
-       ~@body))

src/clj_http/conn_mgr.clj

@@ -84,9 +84,10 @@
 ;; need the fully qualified class name because this fn is later used in a
 ;; macro from a different ns
 (defn ^org.apache.http.impl.conn.PoolingClientConnectionManager
-  make-reusable-conn-manager
+  make-reusable-conn-manager*
   "Given an timeout and optional insecure? flag, create a
-  PoolingClientConnectionManager with <timeout> seconds set as the timeout value."
+  PoolingClientConnectionManager with <timeout> seconds set as the
+  timeout value."
   [{:keys [timeout insecure? keystore trust-store] :as config}]
   (let [registry (cond
                   insecure? insecure-scheme-registry
@@ -95,33 +96,51 @@
                   (get-keystore-scheme-registry config)
 
                   :else regular-scheme-registry)]
-     (PoolingClientConnectionManager.
+    (PoolingClientConnectionManager.
      registry timeout java.util.concurrent.TimeUnit/SECONDS)))
 
-;; needed to import/define this here since it being called outside the client ns
 (def dmcpr ConnPerRouteBean/DEFAULT_MAX_CONNECTIONS_PER_ROUTE)
 
-(defn make-manager
-  "returns reusable PoolingClientConnectionManager"
-  [opts]
-  (let [timeout# (or (:timeout opts) 5)
-        threads# (or (:threads opts) 4)
-        default-per-route# (or (:default-per-route opts) dmcpr)
-        insecure?# (:insecure? opts)
-        leftovers# (dissoc opts :timeout :threads :insecure?)]
-  (doto (make-reusable-conn-manager 
-     (merge {:timeout timeout#
-             :insecure? insecure?#}
-             leftovers#))
-         (.setMaxTotal threads#)
-         (.setDefaultMaxPerRoute default-per-route#))))
+(defn make-reusable-conn-manager
+  "Creates a default pooling connection manager with the specified options.
+
+  The following options are supported:
 
+  :timeout - Time that connections are left open before automatically closing
+    default: 5
+  :threads - Maximum number of threads that will be used for connecting
+    default: 4
+  :default-per-route - Maximum number of simultaneous connections per host
+    default: 2
+  :insecure? - Boolean flag to specify allowing insecure HTTPS connections
+    default: false
 
+  :keystore - keystore file to be used for connection manager
+  :keystore-pass - keystore password
+  :trust-store - trust store file to be used for connection manager
+  :trust-store-pass - trust store password
 
-(defn shutdown-manager 
+  Note that :insecure? and :keystore/:trust-store options are mutually exclusive
+
+  If the value 'nil' is specified or the value is not set, the default value
+  will be used."
+  [opts]
+  (let [timeout (or (:timeout opts) 5)
+        threads (or (:threads opts) 4)
+        default-per-route (or (:default-per-route opts) dmcpr)
+        insecure? (:insecure? opts)
+        leftovers (dissoc opts :timeout :threads :insecure?)]
+    (doto (make-reusable-conn-manager* (merge {:timeout timeout
+                                               :insecure? insecure?}
+                                              leftovers))
+      (.setMaxTotal threads)
+      (.setDefaultMaxPerRoute default-per-route))))
+
+(defn shutdown-manager
   "Define function to shutdown manager"
   [manager]
-  (doto manager (.shutdown)))
+  (and manager (.shutdown manager)))
 
-;; connection manager to be rebound during request execution
-(def ^{:dynamic true} *connection-manager* nil)
+(def ^{:dynamic true
+       :doc "connection manager to be rebound during request execution"}
+  *connection-manager* nil)

src/clj_http/core.clj

@@ -173,10 +173,12 @@
   [{:keys [request-method scheme server-name server-port uri query-string
            headers body socket-timeout conn-timeout multipart debug debug-body
            insecure? save-request? proxy-host proxy-port as cookie-store
-           retry-handler response-interceptor digest-auth
+           retry-handler response-interceptor digest-auth connection-manager
            client-params] :as req}]
   (let [^ClientConnectionManager conn-mgr
-        (or conn/*connection-manager* (conn/make-regular-conn-manager req))
+        (or connection-manager
+            conn/*connection-manager*
+            (conn/make-regular-conn-manager req))
         ^DefaultHttpClient http-client (set-routing
                                         (DefaultHttpClient. conn-mgr))
         scheme (name scheme)]

test/clj_http/test/client.clj

@@ -3,6 +3,7 @@
         [clojure.java.io :only [resource]]
         [clj-http.test.core :only [run-server]])
   (:require [clj-http.client :as client]
+            [clj-http.conn-mgr :as conn]
             [clj-http.util :as util]
             [cheshire.core :as json])
   (:import (java.net UnknownHostException)
@@ -497,3 +498,19 @@
     (is (= 200 (:status resp)))
     (is (= "close" (get-in resp [:headers "connection"])))
     (is (= "get" (:body resp)))))
+
+(deftest ^{:integration true} t-reusable-conn-mgrs
+  (run-server)
+  (Thread/sleep 1000)
+  (let [cm (conn/make-reusable-conn-manager {:timeout 10 :insecure? false})
+        resp1 (client/request (merge base-req {:uri "/redirect-to-get"
+                                               :method :get
+                                               :connection-manager cm}))
+        resp2 (client/request (merge base-req {:uri "/redirect-to-get"
+                                               :method :get}))]
+    (is (= 200 (:status resp1) (:status resp2)))
+    (is (nil? (get-in resp1 [:headers "connection"]))
+        "connection should remain open")
+    (is (= "close" (get-in resp2 [:headers "connection"]))
+        "connection should be closed")
+    (.shutdown cm)))