svn commit: r1076469 - in /ofbiz/trunk/framework/base/src/org/ofbiz/base/container: ContainerConfig.java ContainerLoader.java

[adrianc]

Author: adrianc
Date: Thu Mar  3 00:34:49 2011
New Revision: 1076469

URL: http://svn.apache.org/viewvc?rev=1076469&view=rev
Log:
Small code cleanup in container loader code.

Small functional change: ContainerConfig.getContainers(String configFile) used to return ALL loaded containers, not just the ones in configFile. This commit fixes that - now the method returns only the containers found in the specified file.

Modified:
    ofbiz/trunk/framework/base/src/org/ofbiz/base/container/ContainerConfig.java
    ofbiz/trunk/framework/base/src/org/ofbiz/base/container/ContainerLoader.java

Modified: ofbiz/trunk/framework/base/src/org/ofbiz/base/container/ContainerConfig.java
URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/base/src/org/ofbiz/base/container/ContainerConfig.java?rev=1076469&r1=1076468&r2=1076469&view=diff
==============================================================================
--- ofbiz/trunk/framework/base/src/org/ofbiz/base/container/ContainerConfig.java (original)
+++ ofbiz/trunk/framework/base/src/org/ofbiz/base/container/ContainerConfig.java Thu Mar  3 00:34:49 2011
@@ -20,6 +20,7 @@ package org.ofbiz.base.container;
 
 import java.io.IOException;
 import java.net.URL;
+import java.util.ArrayList;
 import java.util.Collection;
 import java.util.LinkedHashMap;
 import java.util.LinkedList;
@@ -28,6 +29,7 @@ import java.util.Map;
 
 import javax.xml.parsers.ParserConfigurationException;
 
+import org.ofbiz.base.lang.LockedBy;
 import org.ofbiz.base.util.UtilURL;
 import org.ofbiz.base.util.UtilValidate;
 import org.ofbiz.base.util.UtilXml;
@@ -43,43 +45,43 @@ public class ContainerConfig {
 
     public static final String module = ContainerConfig.class.getName();
 
-    protected static Map<String, Container> containers = new LinkedHashMap<String, Container>();
+    @LockedBy("ContainerConfig.class")
+    private static Map<String, Container> containers = new LinkedHashMap<String, Container>();
 
     public static Container getContainer(String containerName, String configFile) throws ContainerException {
         Container container = containers.get(containerName);
         if (container == null) {
-            synchronized (ContainerConfig.class) {
-                container = containers.get(containerName);
-                if (container == null) {
-                    if (configFile == null) {
-                        throw new ContainerException("Container config file cannot be null");
-                    }
-                    new ContainerConfig(configFile);
-                    container = containers.get(containerName);
-                }
-            }
-            if (container == null) {
-                throw new ContainerException("No container found with the name : " + containerName);
-            }
+            getContainers(configFile);
+            container = containers.get(containerName);
+        }
+        if (container == null) {
+            throw new ContainerException("No container found with the name : " + containerName);
         }
         return container;
     }
 
     public static Collection<Container> getContainers(String configFile) throws ContainerException {
-        if (containers.size() == 0) {
-            synchronized (ContainerConfig.class) {
-                if (containers.size() == 0) {
-                    if (configFile == null) {
-                        throw new ContainerException("Container config file cannot be null");
-                    }
-                    new ContainerConfig(configFile);
-                }
-            }
-            if (containers.size() == 0) {
-                throw new ContainerException("No containers loaded; problem with configuration");
+        if (UtilValidate.isEmpty(configFile)) {
+            throw new ContainerException("configFile argument cannot be null or empty");
+        }
+        URL xmlUrl = UtilURL.fromResource(configFile);
+        if (xmlUrl == null) {
+            throw new ContainerException("Could not find container config file " + configFile);
+        }
+        return getContainers(xmlUrl);
+    }
+
+    public static Collection<Container> getContainers(URL xmlUrl) throws ContainerException {
+        if (xmlUrl == null) {
+            throw new ContainerException("xmlUrl argument cannot be null");
+        }
+        Collection<Container> result = getContainerPropsFromXml(xmlUrl);
+        synchronized (ContainerConfig.class) {
+            for (Container container : result) {
+                containers.put(container.name, container);
             }
         }
-        return containers.values();
+        return result;
     }
 
     public static String getPropertyValue(ContainerConfig.Container parentProp, String name, String defaultValue) {
@@ -148,16 +150,9 @@ public class ContainerConfig {
         }
     }
 
-    protected ContainerConfig() {}
+    private ContainerConfig() {}
 
-    protected ContainerConfig(String configFileLocation) throws ContainerException {
-        // load the config file
-        URL xmlUrl = UtilURL.fromResource(configFileLocation);
-        if (xmlUrl == null) {
-            throw new ContainerException("Could not find " + configFileLocation + " master OFBiz container configuration");
-        }
-
-        // read the document
+    private static Collection<Container> getContainerPropsFromXml(URL xmlUrl) throws ContainerException {
         Document containerDocument = null;
         try {
             containerDocument = UtilXml.readXmlDocument(xmlUrl, true);
@@ -168,15 +163,12 @@ public class ContainerConfig {
         } catch (IOException e) {
             throw new ContainerException("Error reading the container config file: " + xmlUrl, e);
         }
-
-        // root element
         Element root = containerDocument.getDocumentElement();
-
-        // containers
+        List<Container> result = new ArrayList<Container>();
         for (Element curElement: UtilXml.childElementList(root, "container")) {
-            Container container = new Container(curElement);
-            containers.put(container.name, container);
+            result.add(new Container(curElement));
         }
+        return result;
     }
 
     public static class Container {

Modified: ofbiz/trunk/framework/base/src/org/ofbiz/base/container/ContainerLoader.java
URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/base/src/org/ofbiz/base/container/ContainerLoader.java?rev=1076469&r1=1076468&r2=1076469&view=diff
==============================================================================
--- ofbiz/trunk/framework/base/src/org/ofbiz/base/container/ContainerLoader.java (original)
+++ ofbiz/trunk/framework/base/src/org/ofbiz/base/container/ContainerLoader.java Thu Mar  3 00:34:49 2011
@@ -28,6 +28,7 @@ import org.ofbiz.base.start.Start;
 import org.ofbiz.base.start.StartupException;
 import org.ofbiz.base.start.StartupLoader;
 import org.ofbiz.base.util.Debug;
+import org.ofbiz.base.util.UtilValidate;
 
 /**
  * ContainerLoader - StartupLoader for the container
@@ -52,37 +53,36 @@ public class ContainerLoader implements
 
         // get the master container configuration file
         this.configFile = config.containerConfig;
-
         Collection<ContainerConfig.Container> containers = null;
         try {
             containers = ContainerConfig.getContainers(configFile);
+            if (UtilValidate.isEmpty(containers)) {
+                throw new StartupException("No containers loaded; problem with configuration");
+            }
         } catch (ContainerException e) {
             throw new StartupException(e);
         }
-
-        if (containers != null) {
-            for (ContainerConfig.Container containerCfg: containers) {
-                Container tmpContainer = loadContainer(containerCfg, args);
-                loadedContainers.add(tmpContainer);
-
-                // This is only used in case of OFBiz running in Geronimo or WASCE. It allows to use the RMIDispatcher
-                if (containerCfg.name.equals("rmi-dispatcher") && configFile.equals("limited-containers.xml")) {
-                    try {
-                        ContainerConfig.Container.Property initialCtxProp = containerCfg.getProperty("use-initial-context");
-                        String useCtx = initialCtxProp == null || initialCtxProp.value == null ? "false" : initialCtxProp.value;
-                        if (!useCtx.equalsIgnoreCase("true")) {
-                            //system.setProperty("java.security.policy", "client.policy"); maybe used if needed...
-                            if (System.getSecurityManager() == null) { // needed by WASCE with a client.policy file.
-                                System.setSecurityManager(new java.rmi.RMISecurityManager());
-                            }
-                            tmpContainer.start();
-                            rmiLoadedContainer = tmpContainer; // used in Geronimo/WASCE to allow to deregister
+        for (ContainerConfig.Container containerCfg : containers) {
+            Container tmpContainer = loadContainer(containerCfg, args);
+            loadedContainers.add(tmpContainer);
+
+            // This is only used in case of OFBiz running in Geronimo or WASCE. It allows to use the RMIDispatcher
+            if (containerCfg.name.equals("rmi-dispatcher") && configFile.equals("limited-containers.xml")) {
+                try {
+                    ContainerConfig.Container.Property initialCtxProp = containerCfg.getProperty("use-initial-context");
+                    String useCtx = initialCtxProp == null || initialCtxProp.value == null ? "false" : initialCtxProp.value;
+                    if (!useCtx.equalsIgnoreCase("true")) {
+                        //system.setProperty("java.security.policy", "client.policy"); maybe used if needed...
+                        if (System.getSecurityManager() == null) { // needed by WASCE with a client.policy file.
+                            System.setSecurityManager(new java.rmi.RMISecurityManager());
                         }
-                    } catch (ContainerException e) {
-                        throw new StartupException("Cannot start() " + tmpContainer.getClass().getName(), e);
-                    } catch (java.lang.AbstractMethodError e) {
-                        throw new StartupException("Cannot start() " + tmpContainer.getClass().getName(), e);
+                        tmpContainer.start();
+                        rmiLoadedContainer = tmpContainer; // used in Geronimo/WASCE to allow to deregister
                     }
+                } catch (ContainerException e) {
+                    throw new StartupException("Cannot start() " + tmpContainer.getClass().getName(), e);
+                } catch (java.lang.AbstractMethodError e) {
+                    throw new StartupException("Cannot start() " + tmpContainer.getClass().getName(), e);
                 }
             }
         }