svn commit: r1758951 - in /ofbiz/trunk: applications/accounting/src/main/java/org/apache/ofbiz/accounting/thirdparty/gosoftware/ applications/content/src/main/java/org/apache/ofbiz/content/survey/ applications/marketing/src/main/java/org/apache/ofbiz/s...

Author: jleroux
Date: Fri Sep  2 13:18:35 2016
New Revision: 1758951

URL: http://svn.apache.org/viewvc?rev=1758951&view=rev
Log:
Fixes "a bunch of small leaks (closes missing, reported as warnings in Eclipse)" - https://issues.apache.org/jira/browse/OFBIZ-8115

At r1758927 I tried to fixe a bunch of closes missing, reported as warnings in Eclipse

Jacopo noticed and reported on dev ML http://markmail.org/message/b4hvzi7foftfq3j5 2 possible issues:
1) you are adding a close statement to code that already had the close statement in the "finally" block; your modification actually introduces a code pattern that is not correct (if an exception is thrown your close statement are not executed)
2) I suspect that you are closing the socket connection too early in PcChargeApi

1)The best option when closing resources (which implement AutoCloseable) is actually to use the try-with-resources statement. At least when there are no performance issues.
I have fixed the issue (only in DatabaseUtil.java) this way because
* there are not performance issues, we anyway create a Statement in both cases, we don't reuse one.
* The finally part is cleaner

I have also used the try-with-resources statement in other places where it fitted.

There is only 1 place where I was undecided: ScriptUtil.parseScript() which uses GroovyUtil.parseClass().
I put only a logError() there and returned null.
I did not throw an IOException because else it spreads everywhere in FlexibleStringExpander.
Here introducing Optional would be better, but I have no time or that today.

2) was not a problem

Modified:
    ofbiz/trunk/applications/accounting/src/main/java/org/apache/ofbiz/accounting/thirdparty/gosoftware/PcChargeApi.java
    ofbiz/trunk/applications/content/src/main/java/org/apache/ofbiz/content/survey/PdfSurveyServices.java
    ofbiz/trunk/applications/marketing/src/main/java/org/apache/ofbiz/sfa/vcard/VCard.java
    ofbiz/trunk/applications/product/src/main/java/org/apache/ofbiz/product/spreadsheetimport/ImportProductServices.java
    ofbiz/trunk/framework/base/src/main/java/org/apache/ofbiz/base/util/Debug.java
    ofbiz/trunk/framework/base/src/main/java/org/apache/ofbiz/base/util/GroovyUtil.java
    ofbiz/trunk/framework/base/src/main/java/org/apache/ofbiz/base/util/ScriptUtil.java
    ofbiz/trunk/framework/entity/src/main/java/org/apache/ofbiz/entity/jdbc/DatabaseUtil.java
    ofbiz/trunk/specialpurpose/ebaystore/src/main/java/org/apache/ofbiz/ebaystore/EbayStore.java

Modified: ofbiz/trunk/applications/accounting/src/main/java/org/apache/ofbiz/accounting/thirdparty/gosoftware/PcChargeApi.java
URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/accounting/src/main/java/org/apache/ofbiz/accounting/thirdparty/gosoftware/PcChargeApi.java?rev=1758951&r1=1758950&r2=1758951&view=diff
==============================================================================
--- ofbiz/trunk/applications/accounting/src/main/java/org/apache/ofbiz/accounting/thirdparty/gosoftware/PcChargeApi.java (original)
+++ ofbiz/trunk/applications/accounting/src/main/java/org/apache/ofbiz/accounting/thirdparty/gosoftware/PcChargeApi.java Fri Sep  2 13:18:35 2016
@@ -18,18 +18,17 @@
  *******************************************************************************/
 package org.apache.ofbiz.accounting.thirdparty.gosoftware;
 
+import java.io.DataInputStream;
 import java.io.IOException;
 import java.io.PrintStream;
-import java.io.DataInputStream;
 import java.net.Socket;
 
 import javax.xml.parsers.ParserConfigurationException;
 
-import org.apache.ofbiz.base.util.UtilXml;
-import org.apache.ofbiz.base.util.ObjectType;
-import org.apache.ofbiz.base.util.GeneralException;
 import org.apache.ofbiz.base.util.Debug;
-
+import org.apache.ofbiz.base.util.GeneralException;
+import org.apache.ofbiz.base.util.ObjectType;
+import org.apache.ofbiz.base.util.UtilXml;
 import org.w3c.dom.Document;
 import org.w3c.dom.Element;
 import org.xml.sax.SAXException;
@@ -189,6 +188,7 @@ public class PcChargeApi {
             Socket sock = new Socket(host, port);
             PrintStream ps = new PrintStream(sock.getOutputStream());
             DataInputStream dis = new DataInputStream(sock.getInputStream());
+            sock.close();
             ps.print(this.toString());
             ps.flush();
 

Modified: ofbiz/trunk/applications/content/src/main/java/org/apache/ofbiz/content/survey/PdfSurveyServices.java
URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/content/src/main/java/org/apache/ofbiz/content/survey/PdfSurveyServices.java?rev=1758951&r1=1758950&r2=1758951&view=diff
==============================================================================
--- ofbiz/trunk/applications/content/src/main/java/org/apache/ofbiz/content/survey/PdfSurveyServices.java (original)
+++ ofbiz/trunk/applications/content/src/main/java/org/apache/ofbiz/content/survey/PdfSurveyServices.java Fri Sep  2 13:18:35 2016
@@ -579,8 +579,7 @@ public class PdfSurveyServices {
             String pdfFileNameIn = (String)context.get("pdfFileNameIn");
             String contentId = (String)context.get("contentId");
             if (UtilValidate.isNotEmpty(pdfFileNameIn)) {
-                try {
-                    FileInputStream fis = new FileInputStream(pdfFileNameIn);
+                try (FileInputStream fis = new FileInputStream(pdfFileNameIn)) {
                     int c;
                     ByteArrayOutputStream baos = new ByteArrayOutputStream();
                     while ((c = fis.read()) != -1) baos.write(c);

Modified: ofbiz/trunk/applications/marketing/src/main/java/org/apache/ofbiz/sfa/vcard/VCard.java
URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/marketing/src/main/java/org/apache/ofbiz/sfa/vcard/VCard.java?rev=1758951&r1=1758950&r2=1758951&view=diff
==============================================================================
--- ofbiz/trunk/applications/marketing/src/main/java/org/apache/ofbiz/sfa/vcard/VCard.java (original)
+++ ofbiz/trunk/applications/marketing/src/main/java/org/apache/ofbiz/sfa/vcard/VCard.java Fri Sep  2 13:18:35 2016
@@ -31,16 +31,6 @@ import java.util.List;
 import java.util.Locale;
 import java.util.Map;
 
-import ezvcard.Ezvcard;
-import ezvcard.io.text.VCardReader;
-import ezvcard.parameter.AddressType;
-import ezvcard.parameter.TelephoneType;
-import ezvcard.parameter.EmailType;
-import ezvcard.property.Address;
-import ezvcard.property.Email;
-import ezvcard.property.FormattedName;
-import ezvcard.property.StructuredName;
-import ezvcard.property.Telephone;
 import org.apache.ofbiz.base.util.Debug;
 import org.apache.ofbiz.base.util.FileUtil;
 import org.apache.ofbiz.base.util.StringUtil;
@@ -62,6 +52,17 @@ import org.apache.ofbiz.service.GenericS
 import org.apache.ofbiz.service.LocalDispatcher;
 import org.apache.ofbiz.service.ServiceUtil;
 
+import ezvcard.Ezvcard;
+import ezvcard.io.text.VCardReader;
+import ezvcard.parameter.AddressType;
+import ezvcard.parameter.EmailType;
+import ezvcard.parameter.TelephoneType;
+import ezvcard.property.Address;
+import ezvcard.property.Email;
+import ezvcard.property.FormattedName;
+import ezvcard.property.StructuredName;
+import ezvcard.property.Telephone;
+
 public class VCard {
     public static final String module = VCard.class.getName();
     public static final String resourceError = "MarketingUiLabels";
@@ -71,8 +72,9 @@ public class VCard {
      * @param dctx
      * @param context
      * @return
+     * @throws IOException
      */
-    public static Map<String, Object> importVCard(DispatchContext dctx, Map<String, ? extends Object> context) {
+    public static Map<String, Object> importVCard(DispatchContext dctx, Map<String, ? extends Object> context) throws IOException {
         LocalDispatcher dispatcher = dctx.getDispatcher();
         Delegator delegator = dctx.getDelegator();
         Locale locale = (Locale) context.get("locale");
@@ -84,10 +86,9 @@ public class VCard {
         boolean isGroup = false;
         List<Map<String, String>> partiesCreated = new ArrayList<Map<String,String>>();
         List<Map<String, String>> partiesExist = new ArrayList<Map<String,String>>();
-        String partyName = "";
+        String partyName = ""; // TODO this is not used yet
 
-        try {
-            VCardReader vCardReader = new VCardReader(in);
+        try (VCardReader vCardReader = new VCardReader(in)) {
             ezvcard.VCard vcard = null;
             while ((vcard = vCardReader.readNext()) != null) {
 
@@ -162,6 +163,7 @@ public class VCard {
                     } else {
                         //TODO change uncorrect labellisation
                         String emailFormatErrMsg = UtilProperties.getMessage(resourceError, "SfaImportVCardEmailFormatError", locale);
+                        vCardReader.close();
                         return ServiceUtil.returnError(structuredName.getGiven() + " " + structuredName.getFamily() + " has " + emailFormatErrMsg);
                     }
                 }
@@ -215,7 +217,6 @@ public class VCard {
                     resp = dispatcher.runSync("createPartyIdentification", createPartyIdentificationMap);
                 }
             }
-            vCardReader.close();
         } catch (IOException | GenericEntityException | GenericServiceException e) {
             Debug.logError(e, module);
             return ServiceUtil.returnError(UtilProperties.getMessage(resourceError,

Modified: ofbiz/trunk/applications/product/src/main/java/org/apache/ofbiz/product/spreadsheetimport/ImportProductServices.java
URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/product/src/main/java/org/apache/ofbiz/product/spreadsheetimport/ImportProductServices.java?rev=1758951&r1=1758950&r2=1758951&view=diff
==============================================================================
--- ofbiz/trunk/applications/product/src/main/java/org/apache/ofbiz/product/spreadsheetimport/ImportProductServices.java (original)
+++ ofbiz/trunk/applications/product/src/main/java/org/apache/ofbiz/product/spreadsheetimport/ImportProductServices.java Fri Sep  2 13:18:35 2016
@@ -28,11 +28,6 @@ import java.util.List;
 import java.util.Locale;
 import java.util.Map;
 
-import org.apache.poi.hssf.usermodel.HSSFCell;
-import org.apache.poi.hssf.usermodel.HSSFRow;
-import org.apache.poi.hssf.usermodel.HSSFSheet;
-import org.apache.poi.hssf.usermodel.HSSFWorkbook;
-import org.apache.poi.poifs.filesystem.POIFSFileSystem;
 import org.apache.ofbiz.base.util.Debug;
 import org.apache.ofbiz.base.util.UtilProperties;
 import org.apache.ofbiz.base.util.UtilValidate;
@@ -41,6 +36,11 @@ import org.apache.ofbiz.entity.GenericEn
 import org.apache.ofbiz.entity.GenericValue;
 import org.apache.ofbiz.service.DispatchContext;
 import org.apache.ofbiz.service.ServiceUtil;
+import org.apache.poi.hssf.usermodel.HSSFCell;
+import org.apache.poi.hssf.usermodel.HSSFRow;
+import org.apache.poi.hssf.usermodel.HSSFSheet;
+import org.apache.poi.hssf.usermodel.HSSFWorkbook;
+import org.apache.poi.poifs.filesystem.POIFSFileSystem;
 
 public class ImportProductServices {
 
@@ -59,8 +59,9 @@ public class ImportProductServices {
      * @param dctx the dispatch context
      * @param context the context
      * @return the result of the service execution
+     * @throws IOException
      */
-    public static Map<String, Object> productImportFromSpreadsheet(DispatchContext dctx, Map<String, ? extends Object> context) {
+    public static Map<String, Object> productImportFromSpreadsheet(DispatchContext dctx, Map<String, ? extends Object> context) throws IOException {
         Delegator delegator = dctx.getDelegator();
         Locale locale = (Locale) context.get("locale");
         // System.getProperty("user.dir") returns the path upto ofbiz home
@@ -110,6 +111,7 @@ public class ImportProductServices {
 
             // get first sheet
             HSSFSheet sheet = wb.getSheetAt(0);
+            wb.close();
             int sheetLastRowNumber = sheet.getLastRowNum();
             for (int j = 1; j <= sheetLastRowNumber; j++) {
                 HSSFRow row = sheet.getRow(j);

Modified: ofbiz/trunk/framework/base/src/main/java/org/apache/ofbiz/base/util/Debug.java
URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/base/src/main/java/org/apache/ofbiz/base/util/Debug.java?rev=1758951&r1=1758950&r2=1758951&view=diff
==============================================================================
--- ofbiz/trunk/framework/base/src/main/java/org/apache/ofbiz/base/util/Debug.java (original)
+++ ofbiz/trunk/framework/base/src/main/java/org/apache/ofbiz/base/util/Debug.java Fri Sep  2 13:18:35 2016
@@ -108,6 +108,7 @@ public final class Debug {
                 Formatter formatter = new Formatter(sb);
                 formatter.format(msg, params);
                 msg = sb.toString();
+                formatter.close();
             }
 
             // log

Modified: ofbiz/trunk/framework/base/src/main/java/org/apache/ofbiz/base/util/GroovyUtil.java
URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/base/src/main/java/org/apache/ofbiz/base/util/GroovyUtil.java?rev=1758951&r1=1758950&r2=1758951&view=diff
==============================================================================
--- ofbiz/trunk/framework/base/src/main/java/org/apache/ofbiz/base/util/GroovyUtil.java (original)
+++ ofbiz/trunk/framework/base/src/main/java/org/apache/ofbiz/base/util/GroovyUtil.java Fri Sep  2 13:18:35 2016
@@ -18,11 +18,6 @@
  */
 package org.apache.ofbiz.base.util;
 
-import groovy.lang.Binding;
-import groovy.lang.GroovyClassLoader;
-import groovy.lang.GroovyShell;
-import groovy.lang.Script;
-
 import java.io.IOException;
 import java.io.InputStream;
 import java.net.URL;
@@ -31,11 +26,16 @@ import java.util.Map;
 
 import javax.script.ScriptContext;
 
+import org.apache.ofbiz.base.location.FlexibleLocation;
+import org.apache.ofbiz.base.util.cache.UtilCache;
 import org.codehaus.groovy.control.CompilationFailedException;
 import org.codehaus.groovy.control.CompilerConfiguration;
 import org.codehaus.groovy.runtime.InvokerHelper;
-import org.apache.ofbiz.base.location.FlexibleLocation;
-import org.apache.ofbiz.base.util.cache.UtilCache;
+
+import groovy.lang.Binding;
+import groovy.lang.GroovyClassLoader;
+import groovy.lang.GroovyShell;
+import groovy.lang.Script;
 
 /**
  * Groovy Utilities.
@@ -173,19 +173,28 @@ public class GroovyUtil {
         }
     }
 
-    public static Class<?> loadClass(String path) throws ClassNotFoundException {
-        return new GroovyClassLoader().loadClass(path);
+    public static Class<?> loadClass(String path) throws ClassNotFoundException, IOException {
+        GroovyClassLoader groovyClassLoader = new GroovyClassLoader();
+        Class<?> classLoader = groovyClassLoader.loadClass(path);
+        groovyClassLoader.close();
+        return classLoader;
     }
 
     public static Class<?> parseClass(InputStream in, String location) throws IOException {
-        return new GroovyClassLoader().parseClass(UtilIO.readString(in), location);
+        GroovyClassLoader groovyClassLoader = new GroovyClassLoader();
+        Class<?> classLoader = groovyClassLoader.parseClass(UtilIO.readString(in), location);
+        groovyClassLoader.close();
+        return classLoader;
     }
     public static Class<?> parseClass(InputStream in, String location, GroovyClassLoader groovyClassLoader) throws IOException {
         return groovyClassLoader.parseClass(UtilIO.readString(in), location);
     }
 
-    public static Class<?> parseClass(String text) {
-        return new GroovyClassLoader().parseClass(text);
+    public static Class<?> parseClass(String text) throws IOException {
+        GroovyClassLoader groovyClassLoader = new GroovyClassLoader();
+        Class<?> classLoader = groovyClassLoader.parseClass(text);
+        groovyClassLoader.close();
+        return classLoader;
     }
 
     public static Object runScriptAtLocation(String location, String methodName, Map<String, Object> context) throws GeneralException {

Modified: ofbiz/trunk/framework/base/src/main/java/org/apache/ofbiz/base/util/ScriptUtil.java
URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/base/src/main/java/org/apache/ofbiz/base/util/ScriptUtil.java?rev=1758951&r1=1758950&r2=1758951&view=diff
==============================================================================
--- ofbiz/trunk/framework/base/src/main/java/org/apache/ofbiz/base/util/ScriptUtil.java (original)
+++ ofbiz/trunk/framework/base/src/main/java/org/apache/ofbiz/base/util/ScriptUtil.java Fri Sep  2 13:18:35 2016
@@ -46,9 +46,9 @@ import javax.script.ScriptException;
 import javax.script.SimpleBindings;
 import javax.script.SimpleScriptContext;
 
-import org.codehaus.groovy.runtime.InvokerHelper;
 import org.apache.ofbiz.base.location.FlexibleLocation;
 import org.apache.ofbiz.base.util.cache.UtilCache;
+import org.codehaus.groovy.runtime.InvokerHelper;
 
 /**
  * Scripting utility methods. This is a facade class that is used to connect OFBiz to JSR-223 scripting engines.
@@ -408,7 +408,12 @@ public final class ScriptUtil {
     public static Class<?> parseScript(String language, String script) {
         Class<?> scriptClass = null;
         if ("groovy".equals(language)) {
-            scriptClass = GroovyUtil.parseClass(script);
+            try {
+                scriptClass = GroovyUtil.parseClass(script);
+            } catch (IOException e) {
+                Debug.logError(e, module);
+                return null;
+            }
         }
         return scriptClass;
     }

Modified: ofbiz/trunk/framework/entity/src/main/java/org/apache/ofbiz/entity/jdbc/DatabaseUtil.java
URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/entity/src/main/java/org/apache/ofbiz/entity/jdbc/DatabaseUtil.java?rev=1758951&r1=1758950&r2=1758951&view=diff
==============================================================================
--- ofbiz/trunk/framework/entity/src/main/java/org/apache/ofbiz/entity/jdbc/DatabaseUtil.java (original)
+++ ofbiz/trunk/framework/entity/src/main/java/org/apache/ofbiz/entity/jdbc/DatabaseUtil.java Fri Sep  2 13:18:35 2016
@@ -1809,7 +1809,6 @@ public class DatabaseUtil {
         }
 
         Connection connection = null;
-        Statement stmt = null;
 
         try {
             connection = getConnection();
@@ -1852,8 +1851,7 @@ public class DatabaseUtil {
 
         String sql = sqlBuf.toString();
         if (Debug.infoOn()) Debug.logInfo("[addColumn] sql=" + sql, module);
-        try {
-            stmt = connection.createStatement();
+        try (Statement  stmt = connection.createStatement()) {
             stmt.executeUpdate(sql);
         } catch (SQLException e) {
             // if that failed try the alternate syntax real quick
@@ -1880,8 +1878,7 @@ public class DatabaseUtil {
 
             String sql2 = sql2Buf.toString();
             if (Debug.infoOn()) Debug.logInfo("[addColumn] sql failed, trying sql2=" + sql2, module);
-            try {
-                stmt = connection.createStatement();
+            try (Statement  stmt = connection.createStatement()) {
                 stmt.executeUpdate(sql2);
             } catch (SQLException e2) {
                 // if this also fails report original error, not this error...
@@ -1889,13 +1886,6 @@ public class DatabaseUtil {
             }
         } finally {
             try {
-                if (stmt != null) {
-                    stmt.close();
-                }
-            } catch (SQLException e) {
-                Debug.logError(e, module);
-            }
-            try {
                 if (connection != null) {
                     connection.close();
                 }

Modified: ofbiz/trunk/specialpurpose/ebaystore/src/main/java/org/apache/ofbiz/ebaystore/EbayStore.java
URL: http://svn.apache.org/viewvc/ofbiz/trunk/specialpurpose/ebaystore/src/main/java/org/apache/ofbiz/ebaystore/EbayStore.java?rev=1758951&r1=1758950&r2=1758951&view=diff
==============================================================================
--- ofbiz/trunk/specialpurpose/ebaystore/src/main/java/org/apache/ofbiz/ebaystore/EbayStore.java (original)
+++ ofbiz/trunk/specialpurpose/ebaystore/src/main/java/org/apache/ofbiz/ebaystore/EbayStore.java Fri Sep  2 13:18:35 2016
@@ -2263,9 +2263,11 @@ public class EbayStore {
                 // Upload image to ofbiz path /runtime/tmp .
                 ByteBuffer byteWrap = (ByteBuffer) context.get("imageData");
                 File file = new File(System.getProperty("ofbiz.home"), "runtime" + File.separator + "tmp" + File.separator + imageFileName);
-                FileChannel wChannel = new FileOutputStream(file, false).getChannel();
+                FileOutputStream fileOutputStream = new FileOutputStream(file, false);
+                FileChannel wChannel = fileOutputStream.getChannel();
                 wChannel.write(byteWrap);
                 wChannel.close();
+                fileOutputStream.close();
 
                 // Set path file picture to api and set picture details.
                 String [] pictureFiles = {System.getProperty("ofbiz.home") + File.separator + "runtime" + File.separator + "tmp" + File.separator + imageFileName};