svn commit: r1737688 - in /ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/thirdparty/sagepay: SagePayServices.java SagePayUtil.java

Author: jleroux
Date: Mon Apr  4 13:30:27 2016
New Revision: 1737688

URL: http://svn.apache.org/viewvc?rev=1737688&view=rev
Log:
"SagePay classes make use of deprecated httpclient features" - https://issues.apache.org/jira/browse/OFBIZ-6286

No functional change. Replaces deprecated code. I stumbled upon this by chance. I wanted to entertain myself and learn something, this was much fun (not really :/)
The new CloseableHttpClient is great but I was very surprise to have to put a catch block into a finally. I double-checked I see no other ways (but to throw a IOException and I wanted to log)

In SagePayUtil.java, I wondered if deprecated BasicHttpParams was useless or not. I checked the source, which extends AbstractHttpParams and HttpParams, the default constructor does not set anything. We can get rid of it, I guess it was forgotten there.

Modified:
    ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/thirdparty/sagepay/SagePayServices.java
    ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/thirdparty/sagepay/SagePayUtil.java

Modified: ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/thirdparty/sagepay/SagePayServices.java
URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/thirdparty/sagepay/SagePayServices.java?rev=1737688&r1=1737687&r2=1737688&view=diff
==============================================================================
--- ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/thirdparty/sagepay/SagePayServices.java (original)
+++ ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/thirdparty/sagepay/SagePayServices.java Mon Apr  4 13:30:27 2016
@@ -29,8 +29,8 @@ import java.util.Set;
 import org.apache.http.HttpHost;
 import org.apache.http.HttpResponse;
 import org.apache.http.client.ClientProtocolException;
-import org.apache.http.client.HttpClient;
 import org.apache.http.client.methods.HttpPost;
+import org.apache.http.impl.client.CloseableHttpClient;
 import org.ofbiz.base.util.Debug;
 import org.ofbiz.base.util.UtilMisc;
 import org.ofbiz.base.util.UtilProperties;
@@ -122,7 +122,7 @@ public class SagePayServices
         String clientIPAddress = (String) context.get("clientIPAddress");
         Locale locale = (Locale) context.get("locale");
 
-        HttpClient httpClient = SagePayUtil.getHttpClient();
+        CloseableHttpClient httpClient = null;
         HttpHost host = SagePayUtil.getHost(props);
 
         //start - authentication parameters
@@ -195,6 +195,7 @@ public class SagePayServices
         try {
 
             String successMessage = null;
+            httpClient = SagePayUtil.getHttpClient();
             HttpPost httpPost = SagePayUtil.getHttpPost(props.get("authenticationUrl"), parameters);
             HttpResponse response = httpClient.execute(host, httpPost);
             Map<String, String> responseData = SagePayUtil.getResponseData(response);
@@ -279,7 +280,13 @@ public class SagePayServices
             Debug.logError(ioe, "Error occurred in HttpClient execute or getting response (" + ioe.getMessage() + ")", module);
             resultMap = ServiceUtil.returnError(UtilProperties.getMessage(resource, "AccountingSagePayErrorHttpClientExecuteOrGettingResponse", UtilMisc.toMap("errorString", ioe.getMessage()), locale));
         } finally {
-            httpClient.getConnectionManager().shutdown();
+            // Incredible, you need to put a try catch block into a finally, how Java can be verbose :/
+            try {                
+                httpClient.close();
+            } catch (IOException ioe) {
+                Debug.logError(ioe, "Error occurred in HttpClient execute or getting response (" + ioe.getMessage() + ")", module);
+                resultMap = ServiceUtil.returnError(UtilProperties.getMessage(resource, "AccountingSagePayErrorHttpClientExecuteOrGettingResponse", UtilMisc.toMap("errorString", ioe.getMessage()), locale));
+            }
         }
         return resultMap;
     }
@@ -301,7 +308,7 @@ public class SagePayServices
         String amount = (String) context.get("amount");
         Locale locale = (Locale) context.get("locale");
 
-        HttpClient httpClient = SagePayUtil.getHttpClient();
+        CloseableHttpClient httpClient = null;
         HttpHost host = SagePayUtil.getHost(props);
 
         //start - authorization parameters
@@ -325,6 +332,7 @@ public class SagePayServices
 
         try {
             String successMessage = null;
+            httpClient = SagePayUtil.getHttpClient();
             HttpPost httpPost = SagePayUtil.getHttpPost(props.get("authoriseUrl"), parameters);
             HttpResponse response = httpClient.execute(host, httpPost);
 
@@ -375,7 +383,13 @@ public class SagePayServices
             Debug.logError(ioe, "Error occurred in HttpClient execute or getting response (" + ioe.getMessage() + ")", module);
             resultMap = ServiceUtil.returnError(UtilProperties.getMessage(resource, "AccountingSagePayErrorHttpClientExecuteOrGettingResponse", UtilMisc.toMap("errorString", ioe.getMessage()), locale));
         } finally {
-            httpClient.getConnectionManager().shutdown();
+            // Incredible, you need to put a try catch block into a finally, how Java can be verbose :/
+            try {                
+                httpClient.close();
+            } catch (IOException ioe) {
+                Debug.logError(ioe, "Error occurred in HttpClient execute or getting response (" + ioe.getMessage() + ")", module);
+                resultMap = ServiceUtil.returnError(UtilProperties.getMessage(resource, "AccountingSagePayErrorHttpClientExecuteOrGettingResponse", UtilMisc.toMap("errorString", ioe.getMessage()), locale));
+            }
         }
         return resultMap;
     }
@@ -396,7 +410,7 @@ public class SagePayServices
         String txAuthNo = (String) context.get("txAuthNo");
         Locale locale = (Locale) context.get("locale");
 
-        HttpClient httpClient = SagePayUtil.getHttpClient();
+        CloseableHttpClient httpClient = null;
         HttpHost host = SagePayUtil.getHost(props);
 
         //start - release parameters
@@ -418,6 +432,7 @@ public class SagePayServices
         try {
 
             String successMessage = null;
+            httpClient = SagePayUtil.getHttpClient();
             HttpPost httpPost = SagePayUtil.getHttpPost(props.get("releaseUrl"), parameters);
             HttpResponse response = httpClient.execute(host, httpPost);
 
@@ -469,7 +484,13 @@ public class SagePayServices
             Debug.logError(ioe, "Error occurred in HttpClient execute or getting response (" + ioe.getMessage() + ")", module);
             resultMap = ServiceUtil.returnError(UtilProperties.getMessage(resource, "AccountingSagePayErrorHttpClientExecuteOrGettingResponse", UtilMisc.toMap("errorString", ioe.getMessage()), locale));
         } finally {
-            httpClient.getConnectionManager().shutdown();
+            // Incredible, you need to put a try catch block into a finally, how Java can be verbose :/
+            try {                
+                httpClient.close();
+            } catch (IOException ioe) {
+                Debug.logError(ioe, "Error occurred in HttpClient execute or getting response (" + ioe.getMessage() + ")", module);
+                resultMap = ServiceUtil.returnError(UtilProperties.getMessage(resource, "AccountingSagePayErrorHttpClientExecuteOrGettingResponse", UtilMisc.toMap("errorString", ioe.getMessage()), locale));
+            }
         }
         return resultMap;
     }
@@ -490,7 +511,7 @@ public class SagePayServices
         String txAuthNo = (String) context.get("txAuthNo");
         Locale locale = (Locale) context.get("locale");
 
-        HttpClient httpClient = SagePayUtil.getHttpClient();
+        CloseableHttpClient httpClient = null;
         HttpHost host = SagePayUtil.getHost(props);
 
         //start - void parameters
@@ -510,7 +531,7 @@ public class SagePayServices
 
         try {
             String successMessage = null;
-
+            httpClient = SagePayUtil.getHttpClient();
             HttpPost httpPost = SagePayUtil.getHttpPost(props.get("voidUrl"), parameters);
             HttpResponse response = httpClient.execute(host, httpPost);
             Map<String, String> responseData = SagePayUtil.getResponseData(response);
@@ -561,7 +582,13 @@ public class SagePayServices
             Debug.logError(ioe, "Error occurred in HttpClient execute or getting response (" + ioe.getMessage() + ")", module);
             resultMap = ServiceUtil.returnError(UtilProperties.getMessage(resource, "AccountingSagePayErrorHttpClientExecuteOrGettingResponse", UtilMisc.toMap("errorString", ioe.getMessage()), locale));
         } finally {
-            httpClient.getConnectionManager().shutdown();
+            // Incredible, you need to put a try catch block into a finally, how Java can be verbose :/
+            try {                
+                httpClient.close();
+            } catch (IOException ioe) {
+                Debug.logError(ioe, "Error occurred in HttpClient execute or getting response (" + ioe.getMessage() + ")", module);
+                resultMap = ServiceUtil.returnError(UtilProperties.getMessage(resource, "AccountingSagePayErrorHttpClientExecuteOrGettingResponse", UtilMisc.toMap("errorString", ioe.getMessage()), locale));
+            }
         }
         return resultMap;
     }
@@ -587,7 +614,7 @@ public class SagePayServices
         String relatedTxAuthNo = (String) context.get("relatedTxAuthNo");
         Locale locale = (Locale) context.get("locale");
 
-        HttpClient httpClient = SagePayUtil.getHttpClient();
+        CloseableHttpClient httpClient = null;
         HttpHost host = SagePayUtil.getHost(props);
 
         //start - refund parameters
@@ -611,7 +638,7 @@ public class SagePayServices
 
         try {
             String successMessage = null;
-
+            httpClient = SagePayUtil.getHttpClient();
             HttpPost httpPost = SagePayUtil.getHttpPost(props.get("refundUrl"), parameters);
             HttpResponse response = httpClient.execute(host, httpPost);
             Map<String, String> responseData = SagePayUtil.getResponseData(response);
@@ -672,7 +699,13 @@ public class SagePayServices
             Debug.logError(ioe, "Error occurred in HttpClient execute or getting response (" + ioe.getMessage() + ")", module);
             resultMap = ServiceUtil.returnError(UtilProperties.getMessage(resource, "AccountingSagePayErrorHttpClientExecuteOrGettingResponse", UtilMisc.toMap("errorString", ioe.getMessage()), locale));
         } finally {
-            httpClient.getConnectionManager().shutdown();
+            // Incredible, you need to put a try catch block into a finally, how Java can be verbose :/
+            try {                
+                httpClient.close();
+            } catch (IOException ioe) {
+                Debug.logError(ioe, "Error occurred in HttpClient execute or getting response (" + ioe.getMessage() + ")", module);
+                resultMap = ServiceUtil.returnError(UtilProperties.getMessage(resource, "AccountingSagePayErrorHttpClientExecuteOrGettingResponse", UtilMisc.toMap("errorString", ioe.getMessage()), locale));
+            }
         }
 
         return resultMap;

Modified: ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/thirdparty/sagepay/SagePayUtil.java
URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/thirdparty/sagepay/SagePayUtil.java?rev=1737688&r1=1737687&r2=1737688&view=diff
==============================================================================
--- ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/thirdparty/sagepay/SagePayUtil.java (original)
+++ ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/thirdparty/sagepay/SagePayUtil.java Mon Apr  4 13:30:27 2016
@@ -35,13 +35,11 @@ import org.apache.http.HttpEntity;
 import org.apache.http.HttpHost;
 import org.apache.http.HttpResponse;
 import org.apache.http.NameValuePair;
-import org.apache.http.client.HttpClient;
 import org.apache.http.client.entity.UrlEncodedFormEntity;
 import org.apache.http.client.methods.HttpPost;
-import org.apache.http.impl.client.DefaultHttpClient;
+import org.apache.http.impl.client.CloseableHttpClient;
+import org.apache.http.impl.client.HttpClients;
 import org.apache.http.message.BasicNameValuePair;
-import org.apache.http.params.BasicHttpParams;
-import org.apache.http.params.HttpParams;
 import org.ofbiz.base.util.Debug;
 
 
@@ -161,9 +159,6 @@ public class SagePayUtil
         httpPost.addHeader("Content-type", "application/x-www-form-urlencoded");
         //postMethod.addHeader("Content-Length", "0");
 
-        HttpParams params = new BasicHttpParams();
-        httpPost.setParams(params);
-
         List<NameValuePair> postParameters = new ArrayList<NameValuePair>();
         Set<String> keys = parameters.keySet();
         for (String key : keys) {
@@ -178,8 +173,8 @@ public class SagePayUtil
         return httpPost;
     }
 
-    public static HttpClient getHttpClient() {
-        HttpClient  httpClient = new DefaultHttpClient();
+    public static CloseableHttpClient getHttpClient() {
+        CloseableHttpClient httpClient = HttpClients.createDefault();
         return httpClient;
     }
 }