You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ofbiz.apache.org by mb...@apache.org on 2017/12/11 07:54:13 UTC

svn commit: r1817750 - in /ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/component: ComponentConfig.java ComponentLoaderConfig.java ComponentResourceHandler.java

Author: mbrohl
Date: Mon Dec 11 07:54:13 2017
New Revision: 1817750

URL: http://svn.apache.org/viewvc?rev=1817750&view=rev
Log:
Improved: General refactoring and code improvements, package 
org.apache.ofbiz.base.component.
(OFBIZ-9872)

The patches were modified to remove unnecessary Debug.is[Loglevel]On()
conditions, see OFBIZ-10049.

Thanks Dennis Balkir for reporting and providing the patches.

Modified:
    ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/component/ComponentConfig.java
    ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/component/ComponentLoaderConfig.java
    ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/component/ComponentResourceHandler.java

Modified: ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/component/ComponentConfig.java
URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/component/ComponentConfig.java?rev=1817750&r1=1817749&r2=1817750&view=diff
==============================================================================
--- ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/component/ComponentConfig.java (original)
+++ ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/component/ComponentConfig.java Mon Dec 11 07:54:13 2017
@@ -47,7 +47,7 @@ import org.w3c.dom.Element;
 
 /**
  * An object that models the <code>&lt;ofbiz-component&gt;</code> element.
- * 
+ *
  * @see <code>ofbiz-component.xsd</code>
  *
  */
@@ -458,8 +458,7 @@ public final class ComponentConfig {
         } catch (ContainerException ce) {
             throw new ComponentException("Error reading container configurations for component: " + this.globalName, ce);
         }
-        if (Debug.verboseOn())
-            Debug.logVerbose("Read component config : [" + rootLocation + "]", module);
+        Debug.logVerbose("Read component config : [" + rootLocation + "]", module);
     }
 
     public boolean enabled() {
@@ -596,7 +595,7 @@ public final class ComponentConfig {
 
     /**
      * An object that models the <code>&lt;classpath&gt;</code> element.
-     * 
+     *
      * @see <code>ofbiz-component.xsd</code>
      *
      */
@@ -619,7 +618,7 @@ public final class ComponentConfig {
         private final Map<String, ComponentConfig> componentConfigs = new LinkedHashMap<>();
         // Root location mapped to global name.
         private final Map<String, String> componentLocations = new HashMap<>();
-        
+
         private synchronized ComponentConfig fromGlobalName(String globalName) {
             return componentConfigs.get(globalName);
         }
@@ -646,7 +645,7 @@ public final class ComponentConfig {
 
     /**
      * An object that models the <code>&lt;entity-resource&gt;</code> element.
-     * 
+     *
      * @see <code>ofbiz-component.xsd</code>
      *
      */
@@ -663,7 +662,7 @@ public final class ComponentConfig {
 
     /**
      * An object that models the <code>&lt;keystore&gt;</code> element.
-     * 
+     *
      * @see <code>ofbiz-component.xsd</code>
      *
      */
@@ -740,7 +739,7 @@ public final class ComponentConfig {
 
     /**
      * An object that models the <code>&lt;resource-loader&gt;</code> element.
-     * 
+     *
      * @see <code>ofbiz-component.xsd</code>
      *
      */
@@ -760,7 +759,7 @@ public final class ComponentConfig {
 
     /**
      * An object that models the <code>&lt;service-resource&gt;</code> element.
-     * 
+     *
      * @see <code>ofbiz-component.xsd</code>
      *
      */
@@ -775,7 +774,7 @@ public final class ComponentConfig {
 
     /**
      * An object that models the <code>&lt;test-suite&gt;</code> element.
-     * 
+     *
      * @see <code>ofbiz-component.xsd</code>
      *
      */
@@ -787,7 +786,7 @@ public final class ComponentConfig {
 
     /**
      * An object that models the <code>&lt;webapp&gt;</code> element.
-     * 
+     *
      * @see <code>ofbiz-component.xsd</code>
      *
      */

Modified: ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/component/ComponentLoaderConfig.java
URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/component/ComponentLoaderConfig.java?rev=1817750&r1=1817749&r2=1817750&view=diff
==============================================================================
--- ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/component/ComponentLoaderConfig.java (original)
+++ ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/component/ComponentLoaderConfig.java Mon Dec 11 07:54:13 2017
@@ -55,7 +55,7 @@ public final class ComponentLoaderConfig
     public static List<ComponentDef> getComponentsFromConfig(URL configUrl) throws ComponentException {
         Document document = parseDocumentFromUrl(configUrl);
         List<? extends Element> toLoad = UtilXml.childElementList(document.getDocumentElement());
-        List<ComponentDef> componentsFromConfig = new ArrayList<ComponentDef>();
+        List<ComponentDef> componentsFromConfig = new ArrayList<>();
 
         for (Element element : toLoad) {
             componentsFromConfig.add(retrieveComponentDefFromElement(element, configUrl));

Modified: ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/component/ComponentResourceHandler.java
URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/component/ComponentResourceHandler.java?rev=1817750&r1=1817749&r2=1817750&view=diff
==============================================================================
--- ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/component/ComponentResourceHandler.java (original)
+++ ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/component/ComponentResourceHandler.java Mon Dec 11 07:54:13 2017
@@ -18,15 +18,19 @@
  *******************************************************************************/
 package org.apache.ofbiz.base.component;
 
+import java.io.IOException;
 import java.io.InputStream;
 import java.net.URL;
 
+import javax.xml.parsers.ParserConfigurationException;
+
 import org.apache.ofbiz.base.config.GenericConfigException;
 import org.apache.ofbiz.base.config.ResourceHandler;
 import org.apache.ofbiz.base.util.Debug;
 import org.apache.ofbiz.base.util.UtilXml;
 import org.w3c.dom.Document;
 import org.w3c.dom.Element;
+import org.xml.sax.SAXException;
 
 /**
  * Contains resource information and provides for loading data
@@ -50,7 +54,7 @@ public class ComponentResourceHandler im
         this.componentName = componentName;
         this.loaderName = loaderName;
         this.location = location;
-        if (Debug.verboseOn()) Debug.logVerbose("Created " + this.toString(), module);
+        Debug.logVerbose("Created " + this.toString(), module);
     }
 
     public String getLoaderName() {
@@ -64,11 +68,7 @@ public class ComponentResourceHandler im
     public Document getDocument() throws GenericConfigException {
         try {
             return UtilXml.readXmlDocument(this.getStream(), this.getFullLocation(), true);
-        } catch (org.xml.sax.SAXException e) {
-            throw new GenericConfigException("Error reading " + this.toString(), e);
-        } catch (javax.xml.parsers.ParserConfigurationException e) {
-            throw new GenericConfigException("Error reading " + this.toString(), e);
-        } catch (java.io.IOException e) {
+        } catch (SAXException | ParserConfigurationException | IOException  e) {
             throw new GenericConfigException("Error reading " + this.toString(), e);
         }
     }



Re: svn commit: r1817750 - in /ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/ base/component: ComponentConfig.java ComponentLoaderConfig.java ComponentResourceHandler.java

Posted by Jacques Le Roux <ja...@les7arts.com>.
Le 11/12/2017 à 09:31, Jacques Le Roux a écrit :
> Why all these changes in JavaDoc? Is that related with EOLs? If so then something is wrong because I set the SVN repo to handle that automatically 
> at http://markmail.org/message/gzghk44kyz646e4d. Maybe you and/or Dennis are not using a svn >= 1.8 client? 
OK I checked by downloading the patch (it else opens in FF and I can't see the differences). Those are removed ending white spaces. We long ago agreed 
about not committing such changes because it makes things harder to review. I was doing that too, so I know it well ;)
So please change your IDE config to not remove ending white spaces.

Thanks

Jacques


Re: svn commit: r1817750 - in /ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/ base/component: ComponentConfig.java ComponentLoaderConfig.java ComponentResourceHandler.java

Posted by Jacques Le Roux <ja...@les7arts.com>.
Yes better to keep it indeed

Jacques


Le 11/12/2017 à 10:05, Taher Alkhateeb a écrit :
> Unless maybe something changed with the log4j API that I am unaware of. But
> I suspect that's hard to implement
>
> On Dec 11, 2017 12:01 PM, "Taher Alkhateeb" <sl...@gmail.com>
> wrote:
>
>> I agree with Jacopo. String concatenation is a relatively expensive
>> operation because strings are immutable and this is exactly why the log4j
>> library provides the checking methods. I would also note that you need
>> these checking functions the most when the call is most utilized (verbose
>> or debug).
>>
>> On Dec 11, 2017 11:55 AM, "Jacopo Cappellato" <jacopo.cappellato@
>> hotwaxsystems.com> wrote:
>>
>> On Mon, Dec 11, 2017 at 9:31 AM, Jacques Le Roux <
>> jacques.le.roux@les7arts.com> wrote:
>>
>>> [...]
>>> Also for OFBIZ-9872 the Jira lacks a description to possibly understand
>>> why theses changes (notably why you rightly removed the "if
>>> (Debug.verboseOn())" test because it's done at the bottom level with  "if
>>> (isOn(level)) {"
>>>
>> There is actually a valid reason for maintaining the pattern:
>>
>> if (Debug.verboseOn()) {
>>      Debug.logVerbose(string + concatenation + here);
>> }
>>
>> In fact string concatenation (in any of its forms) adds some overhead
>> (computational, memory, garbage collection); since the string concatenation
>> is done before calling Debug.logVerbose (or similar methods), but it is
>> only useful if that log level is on (otherwise the call to logVerbose will
>> not produce any output) then it is wise to perform the verboseOn() check
>> before: if the call returns a false then at runtime the sting concatenation
>> will not be executed at all.
>>
>> Jacopo
>>
>>
>>


Re: svn commit: r1817750 - in /ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/ base/component: ComponentConfig.java ComponentLoaderConfig.java ComponentResourceHandler.java

Posted by Taher Alkhateeb <sl...@gmail.com>.
Unless maybe something changed with the log4j API that I am unaware of. But
I suspect that's hard to implement

On Dec 11, 2017 12:01 PM, "Taher Alkhateeb" <sl...@gmail.com>
wrote:

> I agree with Jacopo. String concatenation is a relatively expensive
> operation because strings are immutable and this is exactly why the log4j
> library provides the checking methods. I would also note that you need
> these checking functions the most when the call is most utilized (verbose
> or debug).
>
> On Dec 11, 2017 11:55 AM, "Jacopo Cappellato" <jacopo.cappellato@
> hotwaxsystems.com> wrote:
>
> On Mon, Dec 11, 2017 at 9:31 AM, Jacques Le Roux <
> jacques.le.roux@les7arts.com> wrote:
>
> > [...]
> > Also for OFBIZ-9872 the Jira lacks a description to possibly understand
> > why theses changes (notably why you rightly removed the "if
> > (Debug.verboseOn())" test because it's done at the bottom level with  "if
> > (isOn(level)) {"
> >
>
> There is actually a valid reason for maintaining the pattern:
>
> if (Debug.verboseOn()) {
>     Debug.logVerbose(string + concatenation + here);
> }
>
> In fact string concatenation (in any of its forms) adds some overhead
> (computational, memory, garbage collection); since the string concatenation
> is done before calling Debug.logVerbose (or similar methods), but it is
> only useful if that log level is on (otherwise the call to logVerbose will
> not produce any output) then it is wise to perform the verboseOn() check
> before: if the call returns a false then at runtime the sting concatenation
> will not be executed at all.
>
> Jacopo
>
>
>

Re: svn commit: r1817750 - in /ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/ base/component: ComponentConfig.java ComponentLoaderConfig.java ComponentResourceHandler.java

Posted by Taher Alkhateeb <sl...@gmail.com>.
I agree with Jacopo. String concatenation is a relatively expensive
operation because strings are immutable and this is exactly why the log4j
library provides the checking methods. I would also note that you need
these checking functions the most when the call is most utilized (verbose
or debug).

On Dec 11, 2017 11:55 AM, "Jacopo Cappellato" <
jacopo.cappellato@hotwaxsystems.com> wrote:

On Mon, Dec 11, 2017 at 9:31 AM, Jacques Le Roux <
jacques.le.roux@les7arts.com> wrote:

> [...]
> Also for OFBIZ-9872 the Jira lacks a description to possibly understand
> why theses changes (notably why you rightly removed the "if
> (Debug.verboseOn())" test because it's done at the bottom level with  "if
> (isOn(level)) {"
>

There is actually a valid reason for maintaining the pattern:

if (Debug.verboseOn()) {
    Debug.logVerbose(string + concatenation + here);
}

In fact string concatenation (in any of its forms) adds some overhead
(computational, memory, garbage collection); since the string concatenation
is done before calling Debug.logVerbose (or similar methods), but it is
only useful if that log level is on (otherwise the call to logVerbose will
not produce any output) then it is wise to perform the verboseOn() check
before: if the call returns a false then at runtime the sting concatenation
will not be executed at all.

Jacopo

Re: svn commit: r1817750 - in /ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/ base/component: ComponentConfig.java ComponentLoaderConfig.java ComponentResourceHandler.java

Posted by Nicolas Malin <ni...@nereide.fr>.
Same feel from France center :)

Nicolas


Le 11/12/2017 à 11:50, Taher Alkhateeb a écrit :
> On a side note to Michael kudos on the massive bug hunting effort lately.
>
> On Dec 11, 2017 1:33 PM, "Michael Brohl" <mi...@ecomify.de> wrote:
>
>> I will see how I can handle this effectively without doing all my review
>> work twice.
>>
>> There should be not too much commits which are affected.
>>
>> I'll see if I can repair it this evening.
>>
>>
>> Am 11.12.17 um 11:29 schrieb Jacques Le Roux:
>>
>>> Michael,
>>>
>>> It would be easier for reviewers if you could revert your commits (not
>>> those of the weekend, I have not reviewed them all yet but most of them,
>>> and so far did not find any important issues, just few improvements I did).
>>> This would prevent us from double reviewing. For me at least r1817748 and
>>> 1817742 1817743 1817744 (the bigger ones ;))
>>>
>>> Thanks
>>>
>>> Jacques
>>>
>>>
>>> Le 11/12/2017 à 10:21, Michael Brohl a écrit :
>>>
>>>> These are good points, I wasn't aware of this and just saw the (in my
>>>> view) unnecessary extra lines of code.
>>>>
>>>> I will check my latest commits and change this back where it is
>>>> reasonable (i.e. where we have concatenations).
>>>>
>>>> Luckily, I just started this this morning and not during the heavier
>>>> commits over the weekend.
>>>>
>>>> This reminds me that it is NOT a good idea to do such changes along with
>>>> the commits of patches, it's too confusing.
>>>>
>>>> Thanks,
>>>>
>>>> Michael
>>>>
>>>>
>>>> Am 11.12.17 um 09:55 schrieb Jacopo Cappellato:
>>>>
>>>>> On Mon, Dec 11, 2017 at 9:31 AM, Jacques Le Roux <
>>>>> jacques.le.roux@les7arts.com> wrote:
>>>>>
>>>>> [...]
>>>>>> Also for OFBIZ-9872 the Jira lacks a description to possibly understand
>>>>>> why theses changes (notably why you rightly removed the "if
>>>>>> (Debug.verboseOn())" test because it's done at the bottom level with
>>>>>> "if
>>>>>> (isOn(level)) {"
>>>>>>
>>>>>> There is actually a valid reason for maintaining the pattern:
>>>>> if (Debug.verboseOn()) {
>>>>>       Debug.logVerbose(string + concatenation + here);
>>>>> }
>>>>>
>>>>> In fact string concatenation (in any of its forms) adds some overhead
>>>>> (computational, memory, garbage collection); since the string
>>>>> concatenation
>>>>> is done before calling Debug.logVerbose (or similar methods), but it is
>>>>> only useful if that log level is on (otherwise the call to logVerbose
>>>>> will
>>>>> not produce any output) then it is wise to perform the verboseOn() check
>>>>> before: if the call returns a false then at runtime the sting
>>>>> concatenation
>>>>> will not be executed at all.
>>>>>
>>>>> Jacopo
>>>>>
>>>>>
>>>>
>>


Re: svn commit: r1817750 - in /ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/ base/component: ComponentConfig.java ComponentLoaderConfig.java ComponentResourceHandler.java

Posted by Michael Brohl <mi...@ecomify.de>.
I've reverted these commits and reapplied the original patches.

Thanks all for taking care,

Michael


Am 11.12.17 um 15:21 schrieb Jacques Le Roux:
> I reviewed r1817748 1817742 1817743 1817744 there are OK with me
>
> So you just have to revert the removing of the tests on Debug in these 
> commits  (IIRW only in r1817748 and r1817750)
>
> Notably beware of
>
> "if (debug) {" in r1817748.
>
> Jacques
>
>
> Le 11/12/2017 à 12:39, Michael Brohl a écrit :
>> Thanks, Taher! :-)
>>
>> Am 11.12.17 um 11:50 schrieb Taher Alkhateeb:
>>> On a side note to Michael kudos on the massive bug hunting effort 
>>> lately.
>>>
>>> On Dec 11, 2017 1:33 PM, "Michael Brohl" <mi...@ecomify.de> 
>>> wrote:
>>>
>>>> I will see how I can handle this effectively without doing all my 
>>>> review
>>>> work twice.
>>>>
>>>> There should be not too much commits which are affected.
>>>>
>>>> I'll see if I can repair it this evening.
>>>>
>>>>
>>>> Am 11.12.17 um 11:29 schrieb Jacques Le Roux:
>>>>
>>>>> Michael,
>>>>>
>>>>> It would be easier for reviewers if you could revert your commits 
>>>>> (not
>>>>> those of the weekend, I have not reviewed them all yet but most of 
>>>>> them,
>>>>> and so far did not find any important issues, just few 
>>>>> improvements I did).
>>>>> This would prevent us from double reviewing. For me at least 
>>>>> r1817748 and
>>>>> 1817742 1817743 1817744 (the bigger ones ;))
>>>>>
>>>>> Thanks
>>>>>
>>>>> Jacques
>>>>>
>>>>>
>>>>> Le 11/12/2017 à 10:21, Michael Brohl a écrit :
>>>>>
>>>>>> These are good points, I wasn't aware of this and just saw the 
>>>>>> (in my
>>>>>> view) unnecessary extra lines of code.
>>>>>>
>>>>>> I will check my latest commits and change this back where it is
>>>>>> reasonable (i.e. where we have concatenations).
>>>>>>
>>>>>> Luckily, I just started this this morning and not during the heavier
>>>>>> commits over the weekend.
>>>>>>
>>>>>> This reminds me that it is NOT a good idea to do such changes 
>>>>>> along with
>>>>>> the commits of patches, it's too confusing.
>>>>>>
>>>>>> Thanks,
>>>>>>
>>>>>> Michael
>>>>>>
>>>>>>
>>>>>> Am 11.12.17 um 09:55 schrieb Jacopo Cappellato:
>>>>>>
>>>>>>> On Mon, Dec 11, 2017 at 9:31 AM, Jacques Le Roux <
>>>>>>> jacques.le.roux@les7arts.com> wrote:
>>>>>>>
>>>>>>> [...]
>>>>>>>> Also for OFBIZ-9872 the Jira lacks a description to possibly 
>>>>>>>> understand
>>>>>>>> why theses changes (notably why you rightly removed the "if
>>>>>>>> (Debug.verboseOn())" test because it's done at the bottom level 
>>>>>>>> with
>>>>>>>> "if
>>>>>>>> (isOn(level)) {"
>>>>>>>>
>>>>>>>> There is actually a valid reason for maintaining the pattern:
>>>>>>> if (Debug.verboseOn()) {
>>>>>>>       Debug.logVerbose(string + concatenation + here);
>>>>>>> }
>>>>>>>
>>>>>>> In fact string concatenation (in any of its forms) adds some 
>>>>>>> overhead
>>>>>>> (computational, memory, garbage collection); since the string
>>>>>>> concatenation
>>>>>>> is done before calling Debug.logVerbose (or similar methods), 
>>>>>>> but it is
>>>>>>> only useful if that log level is on (otherwise the call to 
>>>>>>> logVerbose
>>>>>>> will
>>>>>>> not produce any output) then it is wise to perform the 
>>>>>>> verboseOn() check
>>>>>>> before: if the call returns a false then at runtime the sting
>>>>>>> concatenation
>>>>>>> will not be executed at all.
>>>>>>>
>>>>>>> Jacopo
>>>>>>>
>>>>>>>
>>>>>>
>>>>
>>
>>
>



Re: svn commit: r1817750 - in /ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/ base/component: ComponentConfig.java ComponentLoaderConfig.java ComponentResourceHandler.java

Posted by Jacques Le Roux <ja...@les7arts.com>.
I reviewed r1817748 1817742 1817743 1817744 there are OK with me

So you just have to revert the removing of the tests on Debug in these commits  (IIRW only in r1817748 and r1817750)

Notably beware of

"if (debug) {" in r1817748.

Jacques


Le 11/12/2017 à 12:39, Michael Brohl a écrit :
> Thanks, Taher! :-)
>
> Am 11.12.17 um 11:50 schrieb Taher Alkhateeb:
>> On a side note to Michael kudos on the massive bug hunting effort lately.
>>
>> On Dec 11, 2017 1:33 PM, "Michael Brohl" <mi...@ecomify.de> wrote:
>>
>>> I will see how I can handle this effectively without doing all my review
>>> work twice.
>>>
>>> There should be not too much commits which are affected.
>>>
>>> I'll see if I can repair it this evening.
>>>
>>>
>>> Am 11.12.17 um 11:29 schrieb Jacques Le Roux:
>>>
>>>> Michael,
>>>>
>>>> It would be easier for reviewers if you could revert your commits (not
>>>> those of the weekend, I have not reviewed them all yet but most of them,
>>>> and so far did not find any important issues, just few improvements I did).
>>>> This would prevent us from double reviewing. For me at least r1817748 and
>>>> 1817742 1817743 1817744 (the bigger ones ;))
>>>>
>>>> Thanks
>>>>
>>>> Jacques
>>>>
>>>>
>>>> Le 11/12/2017 à 10:21, Michael Brohl a écrit :
>>>>
>>>>> These are good points, I wasn't aware of this and just saw the (in my
>>>>> view) unnecessary extra lines of code.
>>>>>
>>>>> I will check my latest commits and change this back where it is
>>>>> reasonable (i.e. where we have concatenations).
>>>>>
>>>>> Luckily, I just started this this morning and not during the heavier
>>>>> commits over the weekend.
>>>>>
>>>>> This reminds me that it is NOT a good idea to do such changes along with
>>>>> the commits of patches, it's too confusing.
>>>>>
>>>>> Thanks,
>>>>>
>>>>> Michael
>>>>>
>>>>>
>>>>> Am 11.12.17 um 09:55 schrieb Jacopo Cappellato:
>>>>>
>>>>>> On Mon, Dec 11, 2017 at 9:31 AM, Jacques Le Roux <
>>>>>> jacques.le.roux@les7arts.com> wrote:
>>>>>>
>>>>>> [...]
>>>>>>> Also for OFBIZ-9872 the Jira lacks a description to possibly understand
>>>>>>> why theses changes (notably why you rightly removed the "if
>>>>>>> (Debug.verboseOn())" test because it's done at the bottom level with
>>>>>>> "if
>>>>>>> (isOn(level)) {"
>>>>>>>
>>>>>>> There is actually a valid reason for maintaining the pattern:
>>>>>> if (Debug.verboseOn()) {
>>>>>>       Debug.logVerbose(string + concatenation + here);
>>>>>> }
>>>>>>
>>>>>> In fact string concatenation (in any of its forms) adds some overhead
>>>>>> (computational, memory, garbage collection); since the string
>>>>>> concatenation
>>>>>> is done before calling Debug.logVerbose (or similar methods), but it is
>>>>>> only useful if that log level is on (otherwise the call to logVerbose
>>>>>> will
>>>>>> not produce any output) then it is wise to perform the verboseOn() check
>>>>>> before: if the call returns a false then at runtime the sting
>>>>>> concatenation
>>>>>> will not be executed at all.
>>>>>>
>>>>>> Jacopo
>>>>>>
>>>>>>
>>>>>
>>>
>
>


Re: svn commit: r1817750 - in /ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/ base/component: ComponentConfig.java ComponentLoaderConfig.java ComponentResourceHandler.java

Posted by Michael Brohl <mi...@ecomify.de>.
Thanks, Taher! :-)

Am 11.12.17 um 11:50 schrieb Taher Alkhateeb:
> On a side note to Michael kudos on the massive bug hunting effort lately.
>
> On Dec 11, 2017 1:33 PM, "Michael Brohl" <mi...@ecomify.de> wrote:
>
>> I will see how I can handle this effectively without doing all my review
>> work twice.
>>
>> There should be not too much commits which are affected.
>>
>> I'll see if I can repair it this evening.
>>
>>
>> Am 11.12.17 um 11:29 schrieb Jacques Le Roux:
>>
>>> Michael,
>>>
>>> It would be easier for reviewers if you could revert your commits (not
>>> those of the weekend, I have not reviewed them all yet but most of them,
>>> and so far did not find any important issues, just few improvements I did).
>>> This would prevent us from double reviewing. For me at least r1817748 and
>>> 1817742 1817743 1817744 (the bigger ones ;))
>>>
>>> Thanks
>>>
>>> Jacques
>>>
>>>
>>> Le 11/12/2017 à 10:21, Michael Brohl a écrit :
>>>
>>>> These are good points, I wasn't aware of this and just saw the (in my
>>>> view) unnecessary extra lines of code.
>>>>
>>>> I will check my latest commits and change this back where it is
>>>> reasonable (i.e. where we have concatenations).
>>>>
>>>> Luckily, I just started this this morning and not during the heavier
>>>> commits over the weekend.
>>>>
>>>> This reminds me that it is NOT a good idea to do such changes along with
>>>> the commits of patches, it's too confusing.
>>>>
>>>> Thanks,
>>>>
>>>> Michael
>>>>
>>>>
>>>> Am 11.12.17 um 09:55 schrieb Jacopo Cappellato:
>>>>
>>>>> On Mon, Dec 11, 2017 at 9:31 AM, Jacques Le Roux <
>>>>> jacques.le.roux@les7arts.com> wrote:
>>>>>
>>>>> [...]
>>>>>> Also for OFBIZ-9872 the Jira lacks a description to possibly understand
>>>>>> why theses changes (notably why you rightly removed the "if
>>>>>> (Debug.verboseOn())" test because it's done at the bottom level with
>>>>>> "if
>>>>>> (isOn(level)) {"
>>>>>>
>>>>>> There is actually a valid reason for maintaining the pattern:
>>>>> if (Debug.verboseOn()) {
>>>>>       Debug.logVerbose(string + concatenation + here);
>>>>> }
>>>>>
>>>>> In fact string concatenation (in any of its forms) adds some overhead
>>>>> (computational, memory, garbage collection); since the string
>>>>> concatenation
>>>>> is done before calling Debug.logVerbose (or similar methods), but it is
>>>>> only useful if that log level is on (otherwise the call to logVerbose
>>>>> will
>>>>> not produce any output) then it is wise to perform the verboseOn() check
>>>>> before: if the call returns a false then at runtime the sting
>>>>> concatenation
>>>>> will not be executed at all.
>>>>>
>>>>> Jacopo
>>>>>
>>>>>
>>>>
>>



Re: svn commit: r1817750 - in /ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/ base/component: ComponentConfig.java ComponentLoaderConfig.java ComponentResourceHandler.java

Posted by Taher Alkhateeb <sl...@gmail.com>.
On a side note to Michael kudos on the massive bug hunting effort lately.

On Dec 11, 2017 1:33 PM, "Michael Brohl" <mi...@ecomify.de> wrote:

> I will see how I can handle this effectively without doing all my review
> work twice.
>
> There should be not too much commits which are affected.
>
> I'll see if I can repair it this evening.
>
>
> Am 11.12.17 um 11:29 schrieb Jacques Le Roux:
>
>> Michael,
>>
>> It would be easier for reviewers if you could revert your commits (not
>> those of the weekend, I have not reviewed them all yet but most of them,
>> and so far did not find any important issues, just few improvements I did).
>> This would prevent us from double reviewing. For me at least r1817748 and
>> 1817742 1817743 1817744 (the bigger ones ;))
>>
>> Thanks
>>
>> Jacques
>>
>>
>> Le 11/12/2017 à 10:21, Michael Brohl a écrit :
>>
>>> These are good points, I wasn't aware of this and just saw the (in my
>>> view) unnecessary extra lines of code.
>>>
>>> I will check my latest commits and change this back where it is
>>> reasonable (i.e. where we have concatenations).
>>>
>>> Luckily, I just started this this morning and not during the heavier
>>> commits over the weekend.
>>>
>>> This reminds me that it is NOT a good idea to do such changes along with
>>> the commits of patches, it's too confusing.
>>>
>>> Thanks,
>>>
>>> Michael
>>>
>>>
>>> Am 11.12.17 um 09:55 schrieb Jacopo Cappellato:
>>>
>>>> On Mon, Dec 11, 2017 at 9:31 AM, Jacques Le Roux <
>>>> jacques.le.roux@les7arts.com> wrote:
>>>>
>>>> [...]
>>>>> Also for OFBIZ-9872 the Jira lacks a description to possibly understand
>>>>> why theses changes (notably why you rightly removed the "if
>>>>> (Debug.verboseOn())" test because it's done at the bottom level with
>>>>> "if
>>>>> (isOn(level)) {"
>>>>>
>>>>> There is actually a valid reason for maintaining the pattern:
>>>>
>>>> if (Debug.verboseOn()) {
>>>>      Debug.logVerbose(string + concatenation + here);
>>>> }
>>>>
>>>> In fact string concatenation (in any of its forms) adds some overhead
>>>> (computational, memory, garbage collection); since the string
>>>> concatenation
>>>> is done before calling Debug.logVerbose (or similar methods), but it is
>>>> only useful if that log level is on (otherwise the call to logVerbose
>>>> will
>>>> not produce any output) then it is wise to perform the verboseOn() check
>>>> before: if the call returns a false then at runtime the sting
>>>> concatenation
>>>> will not be executed at all.
>>>>
>>>> Jacopo
>>>>
>>>>
>>>
>>>
>>
>
>

Re: svn commit: r1817750 - in /ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/ base/component: ComponentConfig.java ComponentLoaderConfig.java ComponentResourceHandler.java

Posted by Michael Brohl <mi...@ecomify.de>.
I will see how I can handle this effectively without doing all my review 
work twice.

There should be not too much commits which are affected.

I'll see if I can repair it this evening.


Am 11.12.17 um 11:29 schrieb Jacques Le Roux:
> Michael,
>
> It would be easier for reviewers if you could revert your commits (not 
> those of the weekend, I have not reviewed them all yet but most of 
> them, and so far did not find any important issues, just few 
> improvements I did).
> This would prevent us from double reviewing. For me at least r1817748 
> and 1817742 1817743 1817744 (the bigger ones ;))
>
> Thanks
>
> Jacques
>
>
> Le 11/12/2017 à 10:21, Michael Brohl a écrit :
>> These are good points, I wasn't aware of this and just saw the (in my 
>> view) unnecessary extra lines of code.
>>
>> I will check my latest commits and change this back where it is 
>> reasonable (i.e. where we have concatenations).
>>
>> Luckily, I just started this this morning and not during the heavier 
>> commits over the weekend.
>>
>> This reminds me that it is NOT a good idea to do such changes along 
>> with the commits of patches, it's too confusing.
>>
>> Thanks,
>>
>> Michael
>>
>>
>> Am 11.12.17 um 09:55 schrieb Jacopo Cappellato:
>>> On Mon, Dec 11, 2017 at 9:31 AM, Jacques Le Roux <
>>> jacques.le.roux@les7arts.com> wrote:
>>>
>>>> [...]
>>>> Also for OFBIZ-9872 the Jira lacks a description to possibly 
>>>> understand
>>>> why theses changes (notably why you rightly removed the "if
>>>> (Debug.verboseOn())" test because it's done at the bottom level 
>>>> with  "if
>>>> (isOn(level)) {"
>>>>
>>> There is actually a valid reason for maintaining the pattern:
>>>
>>> if (Debug.verboseOn()) {
>>>      Debug.logVerbose(string + concatenation + here);
>>> }
>>>
>>> In fact string concatenation (in any of its forms) adds some overhead
>>> (computational, memory, garbage collection); since the string 
>>> concatenation
>>> is done before calling Debug.logVerbose (or similar methods), but it is
>>> only useful if that log level is on (otherwise the call to 
>>> logVerbose will
>>> not produce any output) then it is wise to perform the verboseOn() 
>>> check
>>> before: if the call returns a false then at runtime the sting 
>>> concatenation
>>> will not be executed at all.
>>>
>>> Jacopo
>>>
>>
>>
>



Re: svn commit: r1817750 - in /ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/ base/component: ComponentConfig.java ComponentLoaderConfig.java ComponentResourceHandler.java

Posted by Jacques Le Roux <ja...@les7arts.com>.
Michael,

It would be easier for reviewers if you could revert your commits (not those of the weekend, I have not reviewed them all yet but most of them, and so 
far did not find any important issues, just few improvements I did).
This would prevent us from double reviewing. For me at least r1817748 and 1817742 1817743 1817744 (the bigger ones ;))

Thanks

Jacques


Le 11/12/2017 à 10:21, Michael Brohl a écrit :
> These are good points, I wasn't aware of this and just saw the (in my view) unnecessary extra lines of code.
>
> I will check my latest commits and change this back where it is reasonable (i.e. where we have concatenations).
>
> Luckily, I just started this this morning and not during the heavier commits over the weekend.
>
> This reminds me that it is NOT a good idea to do such changes along with the commits of patches, it's too confusing.
>
> Thanks,
>
> Michael
>
>
> Am 11.12.17 um 09:55 schrieb Jacopo Cappellato:
>> On Mon, Dec 11, 2017 at 9:31 AM, Jacques Le Roux <
>> jacques.le.roux@les7arts.com> wrote:
>>
>>> [...]
>>> Also for OFBIZ-9872 the Jira lacks a description to possibly understand
>>> why theses changes (notably why you rightly removed the "if
>>> (Debug.verboseOn())" test because it's done at the bottom level with  "if
>>> (isOn(level)) {"
>>>
>> There is actually a valid reason for maintaining the pattern:
>>
>> if (Debug.verboseOn()) {
>>      Debug.logVerbose(string + concatenation + here);
>> }
>>
>> In fact string concatenation (in any of its forms) adds some overhead
>> (computational, memory, garbage collection); since the string concatenation
>> is done before calling Debug.logVerbose (or similar methods), but it is
>> only useful if that log level is on (otherwise the call to logVerbose will
>> not produce any output) then it is wise to perform the verboseOn() check
>> before: if the call returns a false then at runtime the sting concatenation
>> will not be executed at all.
>>
>> Jacopo
>>
>
>


Re: svn commit: r1817750 - in /ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/ base/component: ComponentConfig.java ComponentLoaderConfig.java ComponentResourceHandler.java

Posted by Jacopo Cappellato <ja...@hotwaxsystems.com>.
Thank you Michael!
Please see a minor comment inline:

On Mon, Dec 11, 2017 at 10:21 AM, Michael Brohl <mi...@ecomify.de>
wrote:
>
> I will check my latest commits and change this back where it is reasonable
> (i.e. where we have concatenations).
>

Actually this is not only relevant for concatenations, but also when a
plain string constant is defined (i.e. "some string here").

Jacopo

Re: svn commit: r1817750 - in /ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/ base/component: ComponentConfig.java ComponentLoaderConfig.java ComponentResourceHandler.java

Posted by Michael Brohl <mi...@ecomify.de>.
These are good points, I wasn't aware of this and just saw the (in my 
view) unnecessary extra lines of code.

I will check my latest commits and change this back where it is 
reasonable (i.e. where we have concatenations).

Luckily, I just started this this morning and not during the heavier 
commits over the weekend.

This reminds me that it is NOT a good idea to do such changes along with 
the commits of patches, it's too confusing.

Thanks,

Michael


Am 11.12.17 um 09:55 schrieb Jacopo Cappellato:
> On Mon, Dec 11, 2017 at 9:31 AM, Jacques Le Roux <
> jacques.le.roux@les7arts.com> wrote:
>
>> [...]
>> Also for OFBIZ-9872 the Jira lacks a description to possibly understand
>> why theses changes (notably why you rightly removed the "if
>> (Debug.verboseOn())" test because it's done at the bottom level with  "if
>> (isOn(level)) {"
>>
> There is actually a valid reason for maintaining the pattern:
>
> if (Debug.verboseOn()) {
>      Debug.logVerbose(string + concatenation + here);
> }
>
> In fact string concatenation (in any of its forms) adds some overhead
> (computational, memory, garbage collection); since the string concatenation
> is done before calling Debug.logVerbose (or similar methods), but it is
> only useful if that log level is on (otherwise the call to logVerbose will
> not produce any output) then it is wise to perform the verboseOn() check
> before: if the call returns a false then at runtime the sting concatenation
> will not be executed at all.
>
> Jacopo
>



Re: svn commit: r1817750 - in /ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/ base/component: ComponentConfig.java ComponentLoaderConfig.java ComponentResourceHandler.java

Posted by Jacopo Cappellato <ja...@hotwaxsystems.com>.
On Mon, Dec 11, 2017 at 9:31 AM, Jacques Le Roux <
jacques.le.roux@les7arts.com> wrote:

> [...]
> Also for OFBIZ-9872 the Jira lacks a description to possibly understand
> why theses changes (notably why you rightly removed the "if
> (Debug.verboseOn())" test because it's done at the bottom level with  "if
> (isOn(level)) {"
>

There is actually a valid reason for maintaining the pattern:

if (Debug.verboseOn()) {
    Debug.logVerbose(string + concatenation + here);
}

In fact string concatenation (in any of its forms) adds some overhead
(computational, memory, garbage collection); since the string concatenation
is done before calling Debug.logVerbose (or similar methods), but it is
only useful if that log level is on (otherwise the call to logVerbose will
not produce any output) then it is wise to perform the verboseOn() check
before: if the call returns a false then at runtime the sting concatenation
will not be executed at all.

Jacopo

Re: svn commit: r1817750 - in /ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/ base/component: ComponentConfig.java ComponentLoaderConfig.java ComponentResourceHandler.java

Posted by Jacques Le Roux <ja...@les7arts.com>.
Hi Michael,

I'm not sure about all the other commits for "General refactoring and code improvements" (subtasks of OFBIZ-9836)

But here I found a lot of unnecessary changes and this makes things a bit harder to review.

Why all these changes in JavaDoc? Is that related with EOLs? If so then something is wrong because I set the SVN repo to handle that automatically at 
http://markmail.org/message/gzghk44kyz646e4d. Maybe you and/or Dennis are not using a svn >= 1.8 client?

Also for OFBIZ-9872 the Jira lacks a description to possibly understand why theses changes (notably why you rightly removed the "if 
(Debug.verboseOn())" test because it's done at the bottom level with  "if (isOn(level)) {"

Maybe some could find this last change disputable for performance reason (use of String concatenation when logging message).
But if that was true with older Java versions it's no longer a problem (apart in loops) and it's easier to read. We can (should) trust the compiler ;)
https://stackoverflow.com/questions/21805557/string-format-vs-string-concatenation-performance
http://www.java67.com/2015/05/4-ways-to-concatenate-strings-in-java.html (see 1st comment)

Thanks for your work!

Jacques


Le 11/12/2017 à 08:54, mbrohl@apache.org a écrit :
> Author: mbrohl
> Date: Mon Dec 11 07:54:13 2017
> New Revision: 1817750
>
> URL: http://svn.apache.org/viewvc?rev=1817750&view=rev
> Log:
> Improved: General refactoring and code improvements, package
> org.apache.ofbiz.base.component.
> (OFBIZ-9872)
>
> The patches were modified to remove unnecessary Debug.is[Loglevel]On()
> conditions, see OFBIZ-10049.
>
> Thanks Dennis Balkir for reporting and providing the patches.
>
> Modified:
>      ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/component/ComponentConfig.java
>      ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/component/ComponentLoaderConfig.java
>      ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/component/ComponentResourceHandler.java
>
> Modified: ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/component/ComponentConfig.java
> URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/component/ComponentConfig.java?rev=1817750&r1=1817749&r2=1817750&view=diff
> ==============================================================================
> --- ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/component/ComponentConfig.java (original)
> +++ ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/component/ComponentConfig.java Mon Dec 11 07:54:13 2017
> @@ -47,7 +47,7 @@ import org.w3c.dom.Element;
>   
>   /**
>    * An object that models the <code>&lt;ofbiz-component&gt;</code> element.
> - *
> + *
>    * @see <code>ofbiz-component.xsd</code>
>    *
>    */
> @@ -458,8 +458,7 @@ public final class ComponentConfig {
>           } catch (ContainerException ce) {
>               throw new ComponentException("Error reading container configurations for component: " + this.globalName, ce);
>           }
> -        if (Debug.verboseOn())
> -            Debug.logVerbose("Read component config : [" + rootLocation + "]", module);
> +        Debug.logVerbose("Read component config : [" + rootLocation + "]", module);
>       }
>   
>       public boolean enabled() {
> @@ -596,7 +595,7 @@ public final class ComponentConfig {
>   
>       /**
>        * An object that models the <code>&lt;classpath&gt;</code> element.
> -     *
> +     *
>        * @see <code>ofbiz-component.xsd</code>
>        *
>        */
> @@ -619,7 +618,7 @@ public final class ComponentConfig {
>           private final Map<String, ComponentConfig> componentConfigs = new LinkedHashMap<>();
>           // Root location mapped to global name.
>           private final Map<String, String> componentLocations = new HashMap<>();
> -
> +
>           private synchronized ComponentConfig fromGlobalName(String globalName) {
>               return componentConfigs.get(globalName);
>           }
> @@ -646,7 +645,7 @@ public final class ComponentConfig {
>   
>       /**
>        * An object that models the <code>&lt;entity-resource&gt;</code> element.
> -     *
> +     *
>        * @see <code>ofbiz-component.xsd</code>
>        *
>        */
> @@ -663,7 +662,7 @@ public final class ComponentConfig {
>   
>       /**
>        * An object that models the <code>&lt;keystore&gt;</code> element.
> -     *
> +     *
>        * @see <code>ofbiz-component.xsd</code>
>        *
>        */
> @@ -740,7 +739,7 @@ public final class ComponentConfig {
>   
>       /**
>        * An object that models the <code>&lt;resource-loader&gt;</code> element.
> -     *
> +     *
>        * @see <code>ofbiz-component.xsd</code>
>        *
>        */
> @@ -760,7 +759,7 @@ public final class ComponentConfig {
>   
>       /**
>        * An object that models the <code>&lt;service-resource&gt;</code> element.
> -     *
> +     *
>        * @see <code>ofbiz-component.xsd</code>
>        *
>        */
> @@ -775,7 +774,7 @@ public final class ComponentConfig {
>   
>       /**
>        * An object that models the <code>&lt;test-suite&gt;</code> element.
> -     *
> +     *
>        * @see <code>ofbiz-component.xsd</code>
>        *
>        */
> @@ -787,7 +786,7 @@ public final class ComponentConfig {
>   
>       /**
>        * An object that models the <code>&lt;webapp&gt;</code> element.
> -     *
> +     *
>        * @see <code>ofbiz-component.xsd</code>
>        *
>        */
>
> Modified: ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/component/ComponentLoaderConfig.java
> URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/component/ComponentLoaderConfig.java?rev=1817750&r1=1817749&r2=1817750&view=diff
> ==============================================================================
> --- ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/component/ComponentLoaderConfig.java (original)
> +++ ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/component/ComponentLoaderConfig.java Mon Dec 11 07:54:13 2017
> @@ -55,7 +55,7 @@ public final class ComponentLoaderConfig
>       public static List<ComponentDef> getComponentsFromConfig(URL configUrl) throws ComponentException {
>           Document document = parseDocumentFromUrl(configUrl);
>           List<? extends Element> toLoad = UtilXml.childElementList(document.getDocumentElement());
> -        List<ComponentDef> componentsFromConfig = new ArrayList<ComponentDef>();
> +        List<ComponentDef> componentsFromConfig = new ArrayList<>();
>   
>           for (Element element : toLoad) {
>               componentsFromConfig.add(retrieveComponentDefFromElement(element, configUrl));
>
> Modified: ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/component/ComponentResourceHandler.java
> URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/component/ComponentResourceHandler.java?rev=1817750&r1=1817749&r2=1817750&view=diff
> ==============================================================================
> --- ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/component/ComponentResourceHandler.java (original)
> +++ ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/component/ComponentResourceHandler.java Mon Dec 11 07:54:13 2017
> @@ -18,15 +18,19 @@
>    *******************************************************************************/
>   package org.apache.ofbiz.base.component;
>   
> +import java.io.IOException;
>   import java.io.InputStream;
>   import java.net.URL;
>   
> +import javax.xml.parsers.ParserConfigurationException;
> +
>   import org.apache.ofbiz.base.config.GenericConfigException;
>   import org.apache.ofbiz.base.config.ResourceHandler;
>   import org.apache.ofbiz.base.util.Debug;
>   import org.apache.ofbiz.base.util.UtilXml;
>   import org.w3c.dom.Document;
>   import org.w3c.dom.Element;
> +import org.xml.sax.SAXException;
>   
>   /**
>    * Contains resource information and provides for loading data
> @@ -50,7 +54,7 @@ public class ComponentResourceHandler im
>           this.componentName = componentName;
>           this.loaderName = loaderName;
>           this.location = location;
> -        if (Debug.verboseOn()) Debug.logVerbose("Created " + this.toString(), module);
> +        Debug.logVerbose("Created " + this.toString(), module);
>       }
>   
>       public String getLoaderName() {
> @@ -64,11 +68,7 @@ public class ComponentResourceHandler im
>       public Document getDocument() throws GenericConfigException {
>           try {
>               return UtilXml.readXmlDocument(this.getStream(), this.getFullLocation(), true);
> -        } catch (org.xml.sax.SAXException e) {
> -            throw new GenericConfigException("Error reading " + this.toString(), e);
> -        } catch (javax.xml.parsers.ParserConfigurationException e) {
> -            throw new GenericConfigException("Error reading " + this.toString(), e);
> -        } catch (java.io.IOException e) {
> +        } catch (SAXException | ParserConfigurationException | IOException  e) {
>               throw new GenericConfigException("Error reading " + this.toString(), e);
>           }
>       }
>
>
>