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

[mbrohl]

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);
         }
     }

[Jacques Le Roux]

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, [hidden email] 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);
>           }
>       }
>
>
>

[Jacques Le Roux]

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