From 0fb981dc28813873dccf76e0edc9fb90a2f0409b Mon Sep 17 00:00:00 2001
From: moritz <moritz.siuts@scoyo.com>
Date: Wed, 14 May 2014 18:06:56 +0200
Subject: [PATCH] Configure the used HttpClient to be thread-safe

This fixes #16 (https://github.com/Silverpop/engage-api-client/issues/16).
Using the ApiClient in a multi-threaded environment can lead to an exception and this warning:

WARN org.apache.commons.httpclient.SimpleHttpConnectionManager  - SimpleHttpConnectionManager being used incorrectly.  Be sure that HttpMethod.releaseConnection() is always called and that only one thread and/or method is using this connection manager at a time.

To fix this issue the HttpClient is now constructed with a MultiThreadedHttpConnectionManager. Also it is taken care, that releaseConnection() is called after excuting a HttpMethod. To ensure this the scope of the httpClient and the executeMethod() is narrowed to private.

For details on  how to use HttpClient safely from within a multi-threaded environment see:
http://hc.apache.org/httpclient-3.x/threading.html
---
 .../com/silverpop/api/client/ApiClient.java   | 20 ++++++++++++++-----
 1 file changed, 15 insertions(+), 5 deletions(-)

diff --git a/src/main/java/com/silverpop/api/client/ApiClient.java b/src/main/java/com/silverpop/api/client/ApiClient.java
index 099757e..42f4aad 100644
--- a/src/main/java/com/silverpop/api/client/ApiClient.java
+++ b/src/main/java/com/silverpop/api/client/ApiClient.java
@@ -6,6 +6,7 @@
 import org.apache.commons.httpclient.Header;
 import org.apache.commons.httpclient.HttpClient;
 import org.apache.commons.httpclient.HttpMethodBase;
+import org.apache.commons.httpclient.MultiThreadedHttpConnectionManager;
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 
@@ -16,9 +17,10 @@ public abstract class ApiClient<REQUEST extends ApiRequest> {
     private Log log = LogFactory.getLog(this.getClass());
 
 	private ApiCommandProcessor<REQUEST> commandProcessor;
-	protected HttpClient httpClient;
     protected ApiSession session;
 
+    private final HttpClient httpClient;
+
 
     public ApiSession getSession() {
    		return session;
@@ -26,9 +28,15 @@ public ApiSession getSession() {
 
 
 	protected ApiClient(ApiCommandProcessor<REQUEST> commandProcessor, ApiSession session) {
-		this(commandProcessor, new HttpClient(), session);
+		this(commandProcessor, new HttpClient(new MultiThreadedHttpConnectionManager()), session);
 	}
 
+    /**
+     * Constructor.
+     * @param commandProcessor can not be {@code null}.
+     * @param httpClient this should be configured with a {@code MultiThreadedHttpConnectionManager}, otherwise the ApiClient constructed will not be thread-safe.
+     * @param session can not be {@code null}.
+     */
 	protected ApiClient(ApiCommandProcessor<REQUEST> commandProcessor, HttpClient httpClient, ApiSession session) {
 		this.commandProcessor = commandProcessor;
 		this.httpClient = httpClient;
@@ -84,7 +92,7 @@ private void ensureSessionIsOpen() {
 		}
 	}
 
-	protected String executeMethod(HttpMethodBase method) throws ApiResultException {
+	private String executeMethod(HttpMethodBase method) throws ApiResultException {
 		try {
             log.info("executing method:" + method);
 			int responseCode = httpClient.executeMethod(method);
@@ -106,8 +114,10 @@ protected String executeMethod(HttpMethodBase method) throws ApiResultException
 			}
 		} catch(IOException e) {
 			throw new ApiException("Error executing API: ", e);
-		}
-	}
+		} finally {
+            method.releaseConnection();
+        }
+    }
 
 	private ApiResult extractResult(String requestName, ApiResponse response) throws ApiResultException {
 		if(response.isSuccessful()) {