svn commit: r1852847 - in /ofbiz/ofbiz-framework/trunk/framework: base/src/main/java/org/apache/ofbiz/base/container/ start/src/main/java/org/apache/ofbiz/base/start/

[Taher Alkhateeb]

Author: taher
Date: Sun Feb  3 19:47:27 2019
New Revision: 1852847

URL: http://svn.apache.org/viewvc?rev=1852847&view=rev
Log:
Improved: Refactor the startup API to completely remove the startup loaders 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

Removed:
    ofbiz/ofbiz-framework/trunk/framework/start/src/main/java/org/apache/ofbiz/base/start/StartupLoader.java
Modified:
    ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/container/ContainerLoader.java
    ofbiz/ofbiz-framework/trunk/framework/start/src/main/java/org/apache/ofbiz/base/start/AdminServer.java
    ofbiz/ofbiz-framework/trunk/framework/start/src/main/java/org/apache/ofbiz/base/start/StartupControlPanel.java

Modified: ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/container/ContainerLoader.java
URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/container/ContainerLoader.java?rev=1852847&r1=1852846&r2=1852847&view=diff
==============================================================================
--- ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/container/ContainerLoader.java (original)
+++ ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/container/ContainerLoader.java Sun Feb  3 19:47:27 2019
@@ -28,7 +28,6 @@ import org.apache.ofbiz.base.component.C
 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 @@ import edu.emory.mathcs.backport.java.ut
 /**
  * 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 @@ public class ContainerLoader implements
     }
 
     /**
-     * @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);

Modified: ofbiz/ofbiz-framework/trunk/framework/start/src/main/java/org/apache/ofbiz/base/start/AdminServer.java
URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/framework/start/src/main/java/org/apache/ofbiz/base/start/AdminServer.java?rev=1852847&r1=1852846&r2=1852847&view=diff
==============================================================================
--- ofbiz/ofbiz-framework/trunk/framework/start/src/main/java/org/apache/ofbiz/base/start/AdminServer.java (original)
+++ ofbiz/ofbiz-framework/trunk/framework/start/src/main/java/org/apache/ofbiz/base/start/AdminServer.java Sun Feb  3 19:47:27 2019
@@ -26,9 +26,9 @@ import java.io.PrintWriter;
 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,11 +47,11 @@ final class AdminServer extends Thread {
     }
 
     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);
@@ -59,7 +59,7 @@ final class AdminServer extends Thread {
             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,7 +74,7 @@ final class AdminServer extends Thread {
                         + clientSocket.getInetAddress() + " : "
                         + clientSocket.getPort());
 
-                processClientRequest(clientSocket, loaders, serverState);
+                processClientRequest(clientSocket, loader, serverState);
 
             } catch (IOException e) {
                 e.printStackTrace();
@@ -82,8 +82,8 @@ final class AdminServer extends Thread {
         }
     }
 
-    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 @@ final class AdminServer extends Thread {
             // 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);
             }
         }
     }

Modified: ofbiz/ofbiz-framework/trunk/framework/start/src/main/java/org/apache/ofbiz/base/start/StartupControlPanel.java
URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/framework/start/src/main/java/org/apache/ofbiz/base/start/StartupControlPanel.java?rev=1852847&r1=1852846&r2=1852847&view=diff
==============================================================================
--- ofbiz/ofbiz-framework/trunk/framework/start/src/main/java/org/apache/ofbiz/base/start/StartupControlPanel.java (original)
+++ ofbiz/ofbiz-framework/trunk/framework/start/src/main/java/org/apache/ofbiz/base/start/StartupControlPanel.java Sun Feb  3 19:47:27 2019
@@ -21,10 +21,10 @@ package org.apache.ofbiz.base.start;
 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 @@ final class StartupControlPanel {
             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 @@ final class StartupControlPanel {
      * - 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,7 +120,7 @@ final class StartupControlPanel {
         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();
@@ -129,18 +128,10 @@ final class StartupControlPanel {
                 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 @@ final class StartupControlPanel {
     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,14 +175,14 @@ final class StartupControlPanel {
 
     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 {
@@ -199,38 +190,27 @@ final class StartupControlPanel {
         }
     }
 
-    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);
         }
     }
 }