svn commit: r1854671 - /ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/component/ComponentConfig.java

[mthl]

Author: mthl
Date: Sat Mar  2 20:28:36 2019
New Revision: 1854671

URL: http://svn.apache.org/viewvc?rev=1854671&view=rev
Log:
Improved: Rewrite ‘ComponentConfig’ constructor (OFBIZ-10829)

Previouly this constructor was containing a lot of duplicated code
that is now factorized by using the newly added ‘collectElements’
private method.

Modified:
    ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/component/ComponentConfig.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=1854671&r1=1854670&r2=1854671&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 Sat Mar  2 20:28:36 2019
@@ -29,6 +29,9 @@ import java.util.HashMap;
 import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.TreeMap;
+import java.util.function.BiFunction;
+import java.util.stream.Collectors;
 
 import org.apache.ofbiz.base.container.ContainerConfig;
 import org.apache.ofbiz.base.container.ContainerConfig.Configuration;
@@ -392,6 +395,15 @@ public final class ComponentConfig {
         }
     }
 
+    /**
+     * Instantiates a component config from a component name and a root location.
+     *
+     * @param globalName  the global name of the component which can be {@code null}
+     * @param rootLocation  the root location of the component
+     * @throws ComponentException when component directory does not exist or if the
+     *         {@code ofbiz-component.xml} file of that component is not properly defined
+     * @throws NullPointerException when {@code rootLocation} is {@code null}
+     */
     private ComponentConfig(String globalName, String rootLocation) throws ComponentException {
         if (!rootLocation.endsWith("/")) {
             rootLocation = rootLocation + "/";
@@ -400,134 +412,41 @@ public final class ComponentConfig {
         File rootLocationDir = new File(rootLocation);
         if (!rootLocationDir.exists()) {
             throw new ComponentException("The component root location does not exist: " + rootLocation);
-        }
-        if (!rootLocationDir.isDirectory()) {
+        } else if (!rootLocationDir.isDirectory()) {
             throw new ComponentException("The component root location is not a directory: " + rootLocation);
         }
-        String xmlFilename = rootLocation + "/" + OFBIZ_COMPONENT_XML_FILENAME;
+        String xmlFilename = this.rootLocation + "/" + OFBIZ_COMPONENT_XML_FILENAME;
         URL xmlUrl = UtilURL.fromFilename(xmlFilename);
         if (xmlUrl == null) {
-            throw new ComponentException("Could not find the " + OFBIZ_COMPONENT_XML_FILENAME + " configuration file in the component root location: " + rootLocation);
+            throw new ComponentException("Could not find the " + OFBIZ_COMPONENT_XML_FILENAME
+                    + " configuration file in the component root location: " + rootLocation);
         }
-        Document ofbizComponentDocument = null;
+        Element componentElement = null;
         try {
-            ofbizComponentDocument = UtilXml.readXmlDocument(xmlUrl, true);
+            Document ofbizComponentDocument = UtilXml.readXmlDocument(xmlUrl, true);
+            componentElement = ofbizComponentDocument.getDocumentElement();
         } catch (Exception e) {
             throw new ComponentException("Error reading the component config file: " + xmlUrl, e);
         }
-        Element ofbizComponentElement = ofbizComponentDocument.getDocumentElement();
-        this.componentName = ofbizComponentElement.getAttribute("name");
-        this.enabled = "true".equalsIgnoreCase(ofbizComponentElement.getAttribute("enabled"));
-        if (UtilValidate.isEmpty(globalName)) {
-            this.globalName = this.componentName;
-        } else {
-            this.globalName = globalName;
-        }
-        // resource-loader - resourceLoaderInfos
-        List<? extends Element> childElements = UtilXml.childElementList(ofbizComponentElement, "resource-loader");
-        if (!childElements.isEmpty()) {
-            Map<String, ResourceLoaderInfo> resourceLoaderInfos = new LinkedHashMap<>();
-            for (Element curElement : childElements) {
-                ResourceLoaderInfo resourceLoaderInfo = new ResourceLoaderInfo(curElement);
-                resourceLoaderInfos.put(resourceLoaderInfo.name, resourceLoaderInfo);
-            }
-            this.resourceLoaderInfos = Collections.unmodifiableMap(resourceLoaderInfos);
-        } else {
-            this.resourceLoaderInfos = Collections.emptyMap();
-        }
-
-        childElements = UtilXml.childElementList(ofbizComponentElement, "depends-on");
-        if (!childElements.isEmpty()) {
-            List<DependsOnInfo> dependsOnList = new ArrayList<>(childElements.size());
-            for (Element curElement : childElements) {
-                DependsOnInfo dependsOnInfo = new DependsOnInfo(this, curElement);
-                dependsOnList.add(dependsOnInfo);
-            }
-            this.dependsOnInfos = Collections.unmodifiableList(dependsOnList);
-        } else {
-            this.dependsOnInfos = Collections.emptyList();
-        }
-
-        // classpath - classpathInfos
-        childElements = UtilXml.childElementList(ofbizComponentElement, "classpath");
-        if (!childElements.isEmpty()) {
-            List<ClasspathInfo> classpathInfos = new ArrayList<>(childElements.size());
-            for (Element curElement : childElements) {
-                ClasspathInfo classpathInfo = new ClasspathInfo(this, curElement);
-                classpathInfos.add(classpathInfo);
-            }
-            this.classpathInfos = Collections.unmodifiableList(classpathInfos);
-        } else {
-            this.classpathInfos = Collections.emptyList();
-        }
-        // entity-resource - entityResourceInfos
-        childElements = UtilXml.childElementList(ofbizComponentElement, "entity-resource");
-        if (!childElements.isEmpty()) {
-            List<EntityResourceInfo> entityResourceInfos = new ArrayList<>(childElements.size());
-            for (Element curElement : childElements) {
-                EntityResourceInfo entityResourceInfo = new EntityResourceInfo(this, curElement);
-                entityResourceInfos.add(entityResourceInfo);
-            }
-            this.entityResourceInfos = Collections.unmodifiableList(entityResourceInfos);
-        } else {
-            this.entityResourceInfos = Collections.emptyList();
-        }
-        // service-resource - serviceResourceInfos
-        childElements = UtilXml.childElementList(ofbizComponentElement, "service-resource");
-        if (!childElements.isEmpty()) {
-            List<ServiceResourceInfo> serviceResourceInfos = new ArrayList<>(childElements.size());
-            for (Element curElement : childElements) {
-                ServiceResourceInfo serviceResourceInfo = new ServiceResourceInfo(this, curElement);
-                serviceResourceInfos.add(serviceResourceInfo);
-            }
-            this.serviceResourceInfos = Collections.unmodifiableList(serviceResourceInfos);
-        } else {
-            this.serviceResourceInfos = Collections.emptyList();
-        }
-        // test-suite - serviceResourceInfos
-        childElements = UtilXml.childElementList(ofbizComponentElement, "test-suite");
-        if (!childElements.isEmpty()) {
-            List<TestSuiteInfo> testSuiteInfos = new ArrayList<>(childElements.size());
-            for (Element curElement : childElements) {
-                TestSuiteInfo testSuiteInfo = new TestSuiteInfo(this, curElement);
-                testSuiteInfos.add(testSuiteInfo);
-            }
-            this.testSuiteInfos = Collections.unmodifiableList(testSuiteInfos);
-        } else {
-            this.testSuiteInfos = Collections.emptyList();
-        }
-        // keystore - (cert/trust store infos)
-        childElements = UtilXml.childElementList(ofbizComponentElement, "keystore");
-        if (!childElements.isEmpty()) {
-            List<KeystoreInfo> keystoreInfos = new ArrayList<>(childElements.size());
-            for (Element curElement : childElements) {
-                KeystoreInfo keystoreInfo = new KeystoreInfo(this, curElement);
-                keystoreInfos.add(keystoreInfo);
-            }
-            this.keystoreInfos = Collections.unmodifiableList(keystoreInfos);
-        } else {
-            this.keystoreInfos = Collections.emptyList();
-        }
-        // webapp - webappInfos
-        childElements = UtilXml.childElementList(ofbizComponentElement, "webapp");
-        if (!childElements.isEmpty()) {
-            List<WebappInfo> webappInfos = new ArrayList<>(childElements.size());
-            for (Element curElement : childElements) {
-                WebappInfo webappInfo = new WebappInfo(this, curElement);
-                webappInfos.add(webappInfo);
-            }
-            this.webappInfos = Collections.unmodifiableList(webappInfos);
-        } else {
-            this.webappInfos = Collections.emptyList();
-        }
-        // configurations
+
+        componentName = componentElement.getAttribute("name");
+        enabled = "true".equalsIgnoreCase(componentElement.getAttribute("enabled"));
+        this.globalName = UtilValidate.isEmpty(globalName) ? componentName : globalName;
+        dependsOnInfos = collectElements(componentElement, "depends-on", DependsOnInfo::new);
+        classpathInfos = collectElements(componentElement, "classpath", ClasspathInfo::new);
+        entityResourceInfos = collectElements(componentElement, "entity-resource", EntityResourceInfo::new);
+        serviceResourceInfos = collectElements(componentElement, "service-resource", ServiceResourceInfo::new);
+        testSuiteInfos = collectElements(componentElement, "test-suite", TestSuiteInfo::new);
+        keystoreInfos = collectElements(componentElement, "keystore", KeystoreInfo::new);
+        webappInfos = collectElements(componentElement, "webapp", WebappInfo::new);
+        resourceLoaderInfos = UtilXml.childElementList(componentElement, "resource-loader").stream()
+                .map(ResourceLoaderInfo::new)
+                .collect(Collectors.collectingAndThen(
+                        Collectors.toMap(rli -> rli.name, rli -> rli),
+                        Collections::unmodifiableMap));
         try {
             Collection<Configuration> configurations = ContainerConfig.getConfigurations(xmlUrl);
-            if (!configurations.isEmpty()) {
-                this.configurations = Collections.unmodifiableList(new ArrayList<>(configurations));
-            } else {
-                this.configurations = Collections.emptyList();
-            }
+            this.configurations = Collections.unmodifiableList(new ArrayList<>(configurations));
         } catch (ContainerException ce) {
             throw new ComponentException("Error reading container configurations for component: " + this.globalName, ce);
         }
@@ -536,6 +455,21 @@ public final class ComponentConfig {
         }
     }
 
+    /**
+     * Constructs an immutable list of objects from the the childs of an XML element.
+     *
+     * @param ofbizComponentElement  the XML element containing the childs
+     * @param elemName  the name of the child elements to collect
+     * @param mapper  the constructor use to map child elements to objects
+     * @return an immutable list of objects corresponding to {@code mapper}
+     */
+    private <T> List<T> collectElements(Element ofbizComponentElement, String elemName,
+            BiFunction<ComponentConfig, Element, T> mapper) {
+        return UtilXml.childElementList(ofbizComponentElement, elemName).stream()
+                .map(element -> mapper.apply(this, element))
+                .collect(Collectors.collectingAndThen(Collectors.toList(), Collections::unmodifiableList));
+    }
+
     public boolean enabled() {
         return this.enabled;
     }