Improved: Refactor the startup API to completely remove the startup l…

…oaders collection

This commit refactors the startup bootstrapping logic to remove any references to the
StartupLoader API and any related files. This is due to the fact that any loaders
including the old POS is completely removed from the system. Thus this improvement
simplifies the startup logic and makes it easier to maintain.

Thank you: Mathieu Lirzin for the patch, careful review and patience


git-svn-id: https://svn.apache.org/repos/asf/ofbiz/ofbiz-framework/trunk@1852847 13f79535-47bb-0310-9956-ffa450edef68

framework/base/src/main/java/org/apache/ofbiz/base/container/ContainerLoader.java

@@ -28,7 +28,6 @@
 import org.apache.ofbiz.base.start.Config;
 import org.apache.ofbiz.base.start.StartupCommand;
 import org.apache.ofbiz.base.start.StartupException;
-import org.apache.ofbiz.base.start.StartupLoader;
 import org.apache.ofbiz.base.util.Debug;
 import org.apache.ofbiz.base.util.UtilValidate;
 
@@ -37,21 +36,31 @@
 /**
  * An object that loads containers (background processes).
  *
- * <p>Normally, instances of this class are created by OFBiz startup code, and
+ * Normally, instances of this class are created by OFBiz startup code, and
  * client code should not create instances of this class. Client code is
- * responsible for making sure containers are shut down properly. </p>
- *
+ * responsible for making sure containers are shut down properly.
+ * <p>
+ * When OFBiz starts, the main thread will create the {@code ContainerLoader} instance and
+ * then call the loader's {@code load} method.
+ * After this instance has been created and initialized, the main thread will call the
+ * {@code start} method. When OFBiz shuts down, a separate shutdown thread will call
+ * the {@code unload} method.
  */
-public class ContainerLoader implements StartupLoader {
+public class ContainerLoader {
 
     public static final String module = ContainerLoader.class.getName();
 
     private final List<Container> loadedContainers = new LinkedList<>();
 
     /**
-     * @see org.apache.ofbiz.base.start.StartupLoader#load(Config, List)
+     * Starts the containers.
+     *
+     * @param config Startup config
+     * @param ofbizCommands Command-line arguments
+     * @throws StartupException If an error was encountered. Throwing this exception
+     * will halt loader loading, so it should be thrown only when OFBiz can't
+     * operate without it.
      */
-    @Override
     public synchronized void load(Config config, List<StartupCommand> ofbizCommands) throws StartupException {
 
         // loaders defined in startup (e.g. main, test, load-data, etc ...)
@@ -154,10 +163,9 @@ private void startLoadedContainers() throws StartupException {
     }
 
     /**
-     * @see org.apache.ofbiz.base.start.StartupLoader#unload()
+     * Stops the containers.
      */
-    @Override
-    public synchronized void unload() throws StartupException {
+    public synchronized void unload() {
         Debug.logInfo("Shutting down containers", module);
 
         List<Container> reversedContainerList = new ArrayList<>(loadedContainers);

framework/start/src/main/java/org/apache/ofbiz/base/start/AdminServer.java

@@ -26,9 +26,9 @@
 import java.net.ServerSocket;
 import java.net.Socket;
 import java.nio.charset.StandardCharsets;
-import java.util.List;
 import java.util.concurrent.atomic.AtomicReference;
 
+import org.apache.ofbiz.base.container.ContainerLoader;
 import org.apache.ofbiz.base.start.Start.ServerState;
 import org.apache.ofbiz.base.util.UtilValidate;
 
@@ -47,19 +47,19 @@ enum OfbizSocketCommand {
     }
 
     private ServerSocket serverSocket = null;
-    private List<StartupLoader> loaders = null;
+    private ContainerLoader loader;
     private AtomicReference<ServerState> serverState = null;
     private Config config = null;
 
-    AdminServer(List<StartupLoader> loaders, AtomicReference<ServerState> serverState, Config config) throws StartupException {
+    AdminServer(ContainerLoader loader, AtomicReference<ServerState> serverState, Config config) throws StartupException {
         super("OFBiz-AdminServer");
         try {
             this.serverSocket = new ServerSocket(config.adminPort, 1, config.adminAddress);
         } catch (IOException e) {
             throw new StartupException("Couldn't create server socket(" + config.adminAddress + ":" + config.adminPort + ")", e);
         }
         setDaemon(false);
-        this.loaders = loaders;
+        this.loader = loader;
         this.serverState = serverState;
         this.config = config;
     }
@@ -74,16 +74,16 @@ public void run() {
                         + clientSocket.getInetAddress() + " : "
                         + clientSocket.getPort());
 
-                processClientRequest(clientSocket, loaders, serverState);
+                processClientRequest(clientSocket, loader, serverState);
 
             } catch (IOException e) {
                 e.printStackTrace();
             }
         }
     }
 
-    private void processClientRequest(Socket client, List<StartupLoader> loaders, AtomicReference<ServerState> serverState) throws IOException {
-
+    private void processClientRequest(Socket client, ContainerLoader loader, AtomicReference<ServerState> serverState)
+            throws IOException {
         try (BufferedReader reader = new BufferedReader(new InputStreamReader(client.getInputStream(), StandardCharsets.UTF_8));
                 PrintWriter writer = new PrintWriter(new OutputStreamWriter(client.getOutputStream(), StandardCharsets.UTF_8), true)) {
 
@@ -98,7 +98,7 @@ private void processClientRequest(Socket client, List<StartupLoader> loaders, At
             // if the client request is shutdown, execute shutdown sequence
             if(clientCommand.equals(OfbizSocketCommand.SHUTDOWN)) {
                 writer.flush();
-                StartupControlPanel.stop(loaders, serverState, this);
+                StartupControlPanel.stop(loader, serverState, this);
             }
         }
     }

framework/start/src/main/java/org/apache/ofbiz/base/start/StartupControlPanel.java

@@ -21,10 +21,10 @@
 import java.io.File;
 import java.io.FileInputStream;
 import java.io.IOException;
-import java.util.ArrayList;
 import java.util.List;
 import java.util.concurrent.atomic.AtomicReference;
 
+import org.apache.ofbiz.base.container.ContainerLoader;
 import org.apache.ofbiz.base.start.Start.ServerState;
 
 /**
@@ -62,15 +62,14 @@ static void start(Config config,
             AtomicReference<ServerState> serverState,
             List<StartupCommand> ofbizCommands) throws StartupException {
 
-        //TODO loaders should be converted to a single loader
-        List<StartupLoader> loaders = new ArrayList<>();
-        Thread adminServer = createAdminServer(config, serverState, loaders);
+        ContainerLoader loader = new ContainerLoader();
+        Thread adminServer = createAdminServer(config, serverState, loader);
 
         createLogDirectoryIfMissing(config);
-        createRuntimeShutdownHook(config, loaders, serverState);
-        loadStartupLoaders(config, loaders, ofbizCommands, serverState);
+        createRuntimeShutdownHook(config, loader, serverState);
+        loadContainers(config, loader, ofbizCommands, serverState);
         printStartupMessage(config);
-        executeShutdownAfterLoadIfConfigured(config, loaders, serverState, adminServer);
+        executeShutdownAfterLoadIfConfigured(config, loader, serverState, adminServer);
     }
 
     /**
@@ -96,8 +95,8 @@ private static void printStartupMessage(Config config) {
      * - Manually if requested by the client AdminClient
      * - Automatically if Config.shutdownAfterLoad is set to true
      */
-    static void stop(List<StartupLoader> loaders, AtomicReference<ServerState> serverState, Thread adminServer) {
-        shutdownServer(loaders, serverState, adminServer);
+    static void stop(ContainerLoader loader, AtomicReference<ServerState> serverState, Thread adminServer) {
+        shutdownServer(loader, serverState, adminServer);
         System.exit(0);
     }
 
@@ -121,26 +120,18 @@ static void fullyTerminateSystem(StartupException e) {
         System.exit(1);
     }
 
-    private static void shutdownServer(List<StartupLoader> loaders, AtomicReference<ServerState> serverState, Thread adminServer) {
+    private static void shutdownServer(ContainerLoader loader, AtomicReference<ServerState> serverState, Thread adminServer) {
         ServerState currentState;
         do {
             currentState = serverState.get();
             if (currentState == ServerState.STOPPING) {
                 return;
             }
         } while (!serverState.compareAndSet(currentState, ServerState.STOPPING));
-        // The current thread was the one that successfully changed the state;
-        // continue with further processing.
-        synchronized (loaders) {
-            // Unload in reverse order
-            for (int i = loaders.size(); i > 0; i--) {
-                StartupLoader loader = loaders.get(i - 1);
-                try {
-                    loader.unload();
-                } catch (Exception e) {
-                    e.printStackTrace();
-                }
-            }
+        try {
+            loader.unload();
+        } catch (Exception e) {
+            e.printStackTrace();
         }
         if (adminServer != null && adminServer.isAlive()) {
             adminServer.interrupt();
@@ -161,11 +152,11 @@ private static void loadGlobalOfbizSystemProperties(String globalOfbizProperties
     private static Thread createAdminServer(
             Config config,
             AtomicReference<ServerState> serverState,
-            List<StartupLoader> loaders) throws StartupException {
+            ContainerLoader loader) throws StartupException {
 
         Thread adminServer = null;
         if (config.adminPort > 0) {
-            adminServer = new AdminServer(loaders, serverState, config);
+            adminServer = new AdminServer(loader, serverState, config);
             adminServer.start();
         } else {
             System.out.println("Admin socket not configured; set to port 0");
@@ -184,53 +175,42 @@ private static void createLogDirectoryIfMissing(Config config) {
 
     private static void createRuntimeShutdownHook(
             Config config,
-            List<StartupLoader> loaders,
+            ContainerLoader loader,
             AtomicReference<ServerState> serverState) {
 
         if (config.useShutdownHook) {
             Runtime.getRuntime().addShutdownHook(new Thread() {
                 @Override
                 public void run() {
-                    shutdownServer(loaders, serverState, this);
+                    shutdownServer(loader, serverState, this);
                 }
             });
         } else {
             System.out.println("Shutdown hook disabled");
         }
     }
 
-    private static void loadStartupLoaders(Config config,
-            List<StartupLoader> loaders,
+    private static void loadContainers(Config config,
+            ContainerLoader loader,
             List<StartupCommand> ofbizCommands,
             AtomicReference<ServerState> serverState) throws StartupException {
-
-        String startupLoaderName = "org.apache.ofbiz.base.container.ContainerLoader";
-        ClassLoader classloader = Thread.currentThread().getContextClassLoader();
-
-        synchronized (loaders) {
+        synchronized (StartupControlPanel.class) {
             if (serverState.get() == ServerState.STOPPING) {
                 return;
             }
-            try {
-                Class<?> loaderClass = classloader.loadClass(startupLoaderName);
-                StartupLoader loader = (StartupLoader) loaderClass.newInstance();
-                loaders.add(loader); // add before loading, so unload can occur if error during loading
-                loader.load(config, ofbizCommands);
-            } catch (ReflectiveOperationException e) {
-                throw new StartupException(e);
-            }
+            loader.load(config, ofbizCommands);
         }
         serverState.compareAndSet(ServerState.STARTING, ServerState.RUNNING);
     }
 
     private static void executeShutdownAfterLoadIfConfigured(
             Config config,
-            List<StartupLoader> loaders,
+            ContainerLoader loader,
             AtomicReference<ServerState> serverState,
             Thread adminServer) {
 
         if (config.shutdownAfterLoad) {
-            stop(loaders, serverState, adminServer);
+            stop(loader, serverState, adminServer);
         }
     }
 }