You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ofbiz.apache.org by Jacques Le Roux <ja...@les7arts.com> on 2016/09/02 10:48:02 UTC

Re: svn commit: r1758927 - in /ofbiz/trunk: applications/accounting/src/main/java/org/apache/ofbiz/accounting/thirdpart y/gosoftware/ applications/content/src/main/java/org/apache/ofbiz/content/survey/ applications/marketing/src/main/java/org/apache/ofbiz/s...

Thanks Jacopo,

Reverted at revision: 1758935, I'll check that!

Jacques


Le 02/09/2016 � 12:30, Jacopo Cappellato a �crit :
> Jacques,
>
> after a cursory review it seems to me that in your commit there are a few
> issues; for example:
> 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
>
> I suggest to revert this commit, spend more time in reviewing and testing
> this contribution before submitting it again.
>
> Thanks,
>
> Jacopo
>
> On Fri, Sep 2, 2016 at 12:07 PM, <jl...@apache.org> wrote:
>
>> Author: jleroux
>> Date: Fri Sep  2 10:07:31 2016
>> New Revision: 1758927
>>
>> URL: http://svn.apache.org/viewvc?rev=1758927&view=rev
>> Log:
>> Fixes a bunch of small leaks (closes missing, only warnings in Eclipse)
>> By-product: automatically sort few imports
>>
>> 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/framework/base/src/main/java/org/apache/
>> ofbiz/base/util/Debug.java
>>      ofbiz/trunk/framework/entity/src/main/java/org/apache/
>> ofbiz/entity/jdbc/DatabaseUtil.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=1758927&
>> r1=1758926&r2=1758927&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
>> 10:07:31 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=1758927&r1=1758926&r2=1758927&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 10:07:31
>> 2016
>> @@ -585,6 +585,7 @@ public class PdfSurveyServices {
>>                       ByteArrayOutputStream baos = new
>> ByteArrayOutputStream();
>>                       while ((c = fis.read()) != -1) baos.write(c);
>>                       inputByteBuffer = ByteBuffer.wrap(baos.
>> toByteArray());
>> +                    fis.close();
>>                   } catch (FileNotFoundException e) {
>>                       throw(new GeneralException(e.getMessage()));
>>                   } catch (IOException e) {
>>
>> 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=1758927&r1=1758926&r2=1758927&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 10:07:31 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,10 @@ 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
>> +        VCardReader vCardReader = new VCardReader(in);
>>
>>           try {
>> -            VCardReader vCardReader = new VCardReader(in);
>>               ezvcard.VCard vcard = null;
>>               while ((vcard = vCardReader.readNext()) != null) {
>>
>> @@ -162,6 +164,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,12 +218,13 @@ public class VCard {
>>                       resp = dispatcher.runSync("createPartyIdentification",
>> createPartyIdentificationMap);
>>                   }
>>               }
>> -            vCardReader.close();
>>           } catch (IOException | GenericEntityException |
>> GenericServiceException e) {
>>               Debug.logError(e, module);
>> +            vCardReader.close();
>>               return ServiceUtil.returnError(UtilProperties.getMessage(
>> resourceError,
>>                       "SfaImportVCardError", UtilMisc.toMap("errorString",
>> e.getMessage()), locale));
>>           }
>> +        vCardReader.close();
>>           result.put("partiesCreated", partiesCreated);
>>           result.put("partiesExist", partiesExist);
>>           return result;
>>
>> 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=1758927&r1=1758926&r2=1758927&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 10:07:31 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/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=1758927&
>> r1=1758926&r2=1758927&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 10:07:31 2016
>> @@ -1883,6 +1883,7 @@ public class DatabaseUtil {
>>               try {
>>                   stmt = connection.createStatement();
>>                   stmt.executeUpdate(sql2);
>> +                stmt.close();
>>               } catch (SQLException e2) {
>>                   // if this also fails report original error, not this
>> error...
>>                   return "SQL Exception while executing the following:\n" +
>> sql + "\nError was: " + e.toString();
>>
>>
>>