svn commit: r1812895 - in /ofbiz/ofbiz-framework/trunk/applications/accounting/src/main/java/org/apache/ofbiz/accounting/thirdparty/sagepay: SagePayPaymentServices.java SagePayServices.java SagePayUtil.java

[mbrohl]

Author: mbrohl
Date: Sun Oct 22 11:02:11 2017
New Revision: 1812895

URL: http://svn.apache.org/viewvc?rev=1812895&view=rev
Log:
Improved: Fixing defects reported by FindBugs, package
org.apache.ofbiz.accounting.thirdparty.sagepay.
(OFBIZ-9545)

Thanks Karsten Tymann for reporting and providing the patch.

Modified:
    ofbiz/ofbiz-framework/trunk/applications/accounting/src/main/java/org/apache/ofbiz/accounting/thirdparty/sagepay/SagePayPaymentServices.java
    ofbiz/ofbiz-framework/trunk/applications/accounting/src/main/java/org/apache/ofbiz/accounting/thirdparty/sagepay/SagePayServices.java
    ofbiz/ofbiz-framework/trunk/applications/accounting/src/main/java/org/apache/ofbiz/accounting/thirdparty/sagepay/SagePayUtil.java

Modified: ofbiz/ofbiz-framework/trunk/applications/accounting/src/main/java/org/apache/ofbiz/accounting/thirdparty/sagepay/SagePayPaymentServices.java
URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/applications/accounting/src/main/java/org/apache/ofbiz/accounting/thirdparty/sagepay/SagePayPaymentServices.java?rev=1812895&r1=1812894&r2=1812895&view=diff
==============================================================================
--- ofbiz/ofbiz-framework/trunk/applications/accounting/src/main/java/org/apache/ofbiz/accounting/thirdparty/sagepay/SagePayPaymentServices.java (original)
+++ ofbiz/ofbiz-framework/trunk/applications/accounting/src/main/java/org/apache/ofbiz/accounting/thirdparty/sagepay/SagePayPaymentServices.java Sun Oct 22 11:02:11 2017
@@ -130,7 +130,11 @@ public class SagePayPaymentServices {
         }
 
         billingInfo.put("orderId", orderId);
-        billingInfo.put("amount", processAmount.toString());
+        if(processAmount != null){
+            billingInfo.put("amount", processAmount.toString());
+        } else {
+            billingInfo.put("amount", "");
+        }
         billingInfo.put("currency", currency);
         billingInfo.put("description", orderId);
         billingInfo.put("cardNumber", cardNumber);
@@ -156,7 +160,7 @@ public class SagePayPaymentServices {
         GenericValue orderPaymentPreference = (GenericValue) context.get("orderPaymentPreference");
         
         if (orderPaymentPreference == null) {
-            response = ServiceUtil.returnError(UtilProperties.getMessage(resource, "AccountingSagePayOrderPaymenPreferenceIsNull", UtilMisc.toMap("orderId", orderId, "orderPaymentPreference", orderPaymentPreference), locale));
+            response = ServiceUtil.returnError(UtilProperties.getMessage(resource, "AccountingSagePayOrderPaymenPreferenceIsNull", UtilMisc.toMap("orderId", orderId, "orderPaymentPreference", null), locale));
         } else {
             response = processCardAuthorisationPayment(dctx, context);
         }

Modified: ofbiz/ofbiz-framework/trunk/applications/accounting/src/main/java/org/apache/ofbiz/accounting/thirdparty/sagepay/SagePayServices.java
URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/applications/accounting/src/main/java/org/apache/ofbiz/accounting/thirdparty/sagepay/SagePayServices.java?rev=1812895&r1=1812894&r2=1812895&view=diff
==============================================================================
--- ofbiz/ofbiz-framework/trunk/applications/accounting/src/main/java/org/apache/ofbiz/accounting/thirdparty/sagepay/SagePayServices.java (original)
+++ ofbiz/ofbiz-framework/trunk/applications/accounting/src/main/java/org/apache/ofbiz/accounting/thirdparty/sagepay/SagePayServices.java Sun Oct 22 11:02:11 2017
@@ -24,8 +24,7 @@ import java.io.UnsupportedEncodingExcept
 import java.util.HashMap;
 import java.util.Locale;
 import java.util.Map;
-import java.util.Set;
-
+import java.util.Map.Entry;
 import org.apache.http.HttpHost;
 import org.apache.http.HttpResponse;
 import org.apache.http.client.ClientProtocolException;
@@ -58,11 +57,12 @@ public class SagePayServices
             try {
                 GenericValue sagePay = EntityQuery.use(delegator).from("PaymentGatewaySagePay").where("paymentGatewayConfigId", paymentGatewayConfigId).queryOne();
                 if (sagePay != null) {
-                    Map<String, Object> tmp = sagePay.getAllFields();
-                    Set<String> keys = tmp.keySet();
-                    for (String key : keys) {
-                        String value = tmp.get(key).toString();
-                        sagePayConfig.put(key, value);
+                    for (Entry<String, Object> set : sagePay.entrySet()) {
+                        if(set.getValue() == null){
+                            sagePayConfig.put(set.getKey(), null);
+                        } else {
+                            sagePayConfig.put(set.getKey(), set.getValue().toString());
+                        }
                     }
                 }
             } catch (GenericEntityException e) {
@@ -129,8 +129,22 @@ public class SagePayServices
         String vpsProtocol = props.get("protocolVersion");
         String vendor = props.get("vendor");
         String txType = props.get("authenticationTransType");
-
+        
         //start - required parameters
+        StringBuilder errorRequiredParameters = new StringBuilder();
+        if(vpsProtocol == null){
+            errorRequiredParameters.append("Required transaction parameter 'protocolVersion' is missing. ");
+        }
+        if(vendor == null){
+            errorRequiredParameters.append("Required transaction parameter 'vendor' is missing. ");
+        }
+        if(txType == null){
+            errorRequiredParameters.append("Required transaction parameter 'authenticationsTransType' is missing. ");
+        }
+        if(errorRequiredParameters.length() > 0){
+            return ServiceUtil.returnError(UtilProperties.getMessage(resource, "AccountingSagePayPaymentAuthorisationException", UtilMisc.toMap("errorString", errorRequiredParameters), locale));
+        }
+        
         parameters.put("VPSProtocol", vpsProtocol);
         parameters.put("TxType", txType);
         parameters.put("Vendor", vendor);

Modified: ofbiz/ofbiz-framework/trunk/applications/accounting/src/main/java/org/apache/ofbiz/accounting/thirdparty/sagepay/SagePayUtil.java
URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/applications/accounting/src/main/java/org/apache/ofbiz/accounting/thirdparty/sagepay/SagePayUtil.java?rev=1812895&r1=1812894&r2=1812895&view=diff
==============================================================================
--- ofbiz/ofbiz-framework/trunk/applications/accounting/src/main/java/org/apache/ofbiz/accounting/thirdparty/sagepay/SagePayUtil.java (original)
+++ ofbiz/ofbiz-framework/trunk/applications/accounting/src/main/java/org/apache/ofbiz/accounting/thirdparty/sagepay/SagePayUtil.java Sun Oct 22 11:02:11 2017
@@ -25,12 +25,12 @@ import java.io.InputStream;
 import java.io.InputStreamReader;
 import java.io.UnsupportedEncodingException;
 import java.math.BigDecimal;
+import java.nio.charset.StandardCharsets;
 import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
-import java.util.Set;
-
+import java.util.Map.Entry;
 import org.apache.http.HttpEntity;
 import org.apache.http.HttpHost;
 import org.apache.http.HttpResponse;
@@ -114,6 +114,12 @@ public final class SagePayUtil {
         return result;
     }
 
+    /**
+     * Builds HttpHost with the given SagePayProperties.
+     * @param
+     * @throws IllegalArgumentException if neither productionHost nor testingHost found in properties.
+     * @return
+     */
     public static HttpHost getHost(Map<String, String> props) {
         String hostUrl = null;
         if("PRODUCTION".equals(props.get("sagePayMode"))) {
@@ -121,6 +127,9 @@ public final class SagePayUtil {
         } else if("TEST".equals(props.get("sagePayMode"))) {
             hostUrl = props.get("testingHost");
         }
+        if(hostUrl == null){
+            throw new IllegalArgumentException("Could not find host-url via SagePay Properties");
+        }
         String scheme = hostUrl.substring(0, 5);
         String host = hostUrl.substring(8, hostUrl.lastIndexOf(":"));
         String port = hostUrl.substring(hostUrl.lastIndexOf(":")+1);
@@ -138,16 +147,20 @@ public final class SagePayUtil {
         HttpEntity httpEntity = response.getEntity();
         if (httpEntity != null) {
             InputStream inputStream = httpEntity.getContent();
-            BufferedReader reader = new BufferedReader(new InputStreamReader(inputStream));
-
-            String data = null;
-            while( (data = reader.readLine()) != null ) {
-                if(data.indexOf("=") != -1) {
-                    String name = data.substring(0, data.indexOf("="));
-                    String value = data.substring(data.indexOf("=")+1);
-                    responseData.put(name, value);
+            BufferedReader reader = new BufferedReader(new InputStreamReader(inputStream, StandardCharsets.UTF_8));
+            try{
+                String data = null;
+                while ((data = reader.readLine()) != null) {
+                    if (data.indexOf("=") != -1) {
+                        String name = data.substring(0, data.indexOf("="));
+                        String value = data.substring(data.indexOf("=") + 1);
+                        responseData.put(name, value);
+                    }
                 }
             }
+            finally {
+                reader.close();
+            }
         }
         Debug.logInfo("SagePay Response Data : " + responseData, module);
         return responseData;
@@ -159,10 +172,8 @@ public final class SagePayUtil {
         httpPost.addHeader("User-Agent", "HTTP Client");
         httpPost.addHeader("Content-type", "application/x-www-form-urlencoded");
         List<NameValuePair> postParameters = new ArrayList<NameValuePair>();
-        Set<String> keys = parameters.keySet();
-        for (String key : keys) {
-            String value = parameters.get(key);
-            postParameters.add(new BasicNameValuePair(key, value));
+        for (Entry<String,String> entry : parameters.entrySet()) {
+            postParameters.add(new BasicNameValuePair(entry.getKey(), entry.getValue()));
         }
 
         Debug.logInfo("SagePay PostParameters - " + postParameters, module);