svn commit: r1862665 - in /ofbiz/ofbiz-framework/trunk/framework/start/src/main/java/org/apache/ofbiz/base/start: AdminServer.java StartupControlPanel.java

[mthl]

Author: mthl
Date: Sat Jul  6 14:39:25 2019
New Revision: 1862665

URL: http://svn.apache.org/viewvc?rev=1862665&view=rev
Log:
Improved: Avoid confusing indirections in ‘StartupControlPanel#start’
(OFBIZ-11137)

The ‘StartupControlPanel#start’ method is delegating its job to
smaller private methods to improve readability.  However the
conditionnals were previously hidden inside those methods with the
unfortunate consequence of making case analysis hard.

To avoid this undesirable effect, the conditionals has been moved
inside the ‘StartupControlPanel#start’ method and some delegate
methods has been inlined when their implementation was trivial.  The
parameters of the remaining delegate methods has be refined to avoid
passing around unneeded things.

Modified:
    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/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=1862665&r1=1862664&r2=1862665&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 Sat Jul  6 14:39:25 2019
@@ -98,7 +98,8 @@ final class AdminServer extends Thread {
             // if the client request is shutdown, execute shutdown sequence
             if(clientCommand.equals(OfbizSocketCommand.SHUTDOWN)) {
                 writer.flush();
-                StartupControlPanel.stop(loader, serverState, this);
+                StartupControlPanel.shutdownServer(loader, serverState, this);
+                System.exit(0);
             }
         }
     }

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=1862665&r1=1862664&r2=1862665&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 Sat Jul  6 14:39:25 2019
@@ -65,39 +65,29 @@ final class StartupControlPanel {
         ContainerLoader loader = new ContainerLoader();
         Thread adminServer = createAdminServer(config, serverState, loader);
 
-        createLogDirectoryIfMissing(config);
-        createRuntimeShutdownHook(config, loader, serverState);
-        loadContainers(config, loader, ofbizCommands, serverState);
-        printStartupMessage(config);
-        executeShutdownAfterLoadIfConfigured(config, loader, serverState, adminServer);
-    }
+        createLogDirectoryIfMissing(config.logDir);
 
-    /**
-     * Print OFBiz startup message only if the OFBiz server is not scheduled for shutdown.
-     * @param config contains parameters for system startup
-     */
-    private static void printStartupMessage(Config config) {
-        if (!config.shutdownAfterLoad) {
-            String lineSeparator = System.lineSeparator();
-            System.out.println(lineSeparator + "   ____  __________  _" +
-                               lineSeparator + "  / __ \\/ ____/ __ )(_)___" +
-                               lineSeparator + " / / / / /_  / __  / /_  /" +
-                               lineSeparator + "/ /_/ / __/ / /_/ / / / /_" +
-                               lineSeparator + "\\____/_/   /_____/_/ /___/  is started and ready." +
-                               lineSeparator);
+        if (config.useShutdownHook) {
+            createRuntimeShutdownHook(loader, serverState);
+        } else {
+            System.out.println("Shutdown hook disabled");
         }
-    }
 
-    /**
-     * Shutdown the OFBiz server. This method is invoked in one of the
-     * following ways:
-     *
-     * - Manually if requested by the client AdminClient
-     * - Automatically if Config.shutdownAfterLoad is set to true
-     */
-    static void stop(ContainerLoader loader, AtomicReference<ServerState> serverState, Thread adminServer) {
-        shutdownServer(loader, serverState, adminServer);
-        System.exit(0);
+        loadContainers(config, loader, ofbizCommands, serverState);
+
+        if (config.shutdownAfterLoad) {
+            shutdownServer(loader, serverState, adminServer);
+            System.exit(0);
+        } else {
+            // Print startup message.
+            String ls = System.lineSeparator();
+            System.out.println(ls + "   ____  __________  _" +
+                               ls + "  / __ \\/ ____/ __ )(_)___" +
+                               ls + " / / / / /_  / __  / /_  /" +
+                               ls + "/ /_/ / __/ / /_/ / / / /_" +
+                               ls + "\\____/_/   /_____/_/ /___/  is started and ready." +
+                               ls);
+        }
     }
 
     /**
@@ -120,7 +110,7 @@ final class StartupControlPanel {
         System.exit(1);
     }
 
-    private static void shutdownServer(ContainerLoader loader, AtomicReference<ServerState> serverState, Thread adminServer) {
+    static void shutdownServer(ContainerLoader loader, AtomicReference<ServerState> serverState, Thread adminServer) {
         ServerState currentState;
         do {
             currentState = serverState.get();
@@ -164,8 +154,8 @@ final class StartupControlPanel {
         return adminServer;
     }
 
-    private static void createLogDirectoryIfMissing(Config config) {
-        File logDir = new File(config.logDir);
+    private static void createLogDirectoryIfMissing(String logDirName) {
+        File logDir = new File(logDirName);
         if (!logDir.exists()) {
             if (logDir.mkdir()) {
                 System.out.println("Created OFBiz log dir [" + logDir.getAbsolutePath() + "]");
@@ -173,21 +163,13 @@ final class StartupControlPanel {
         }
     }
 
-    private static void createRuntimeShutdownHook(
-            Config config,
-            ContainerLoader loader,
-            AtomicReference<ServerState> serverState) {
-
-        if (config.useShutdownHook) {
-            Runtime.getRuntime().addShutdownHook(new Thread() {
-                @Override
-                public void run() {
-                    shutdownServer(loader, serverState, this);
-                }
-            });
-        } else {
-            System.out.println("Shutdown hook disabled");
-        }
+    private static void createRuntimeShutdownHook(ContainerLoader loader, AtomicReference<ServerState> serverState) {
+        Runtime.getRuntime().addShutdownHook(new Thread() {
+            @Override
+            public void run() {
+                shutdownServer(loader, serverState, this);
+            }
+        });
     }
 
     private static void loadContainers(Config config,
@@ -202,15 +184,4 @@ final class StartupControlPanel {
         }
         serverState.compareAndSet(ServerState.STARTING, ServerState.RUNNING);
     }
-
-    private static void executeShutdownAfterLoadIfConfigured(
-            Config config,
-            ContainerLoader loader,
-            AtomicReference<ServerState> serverState,
-            Thread adminServer) {
-
-        if (config.shutdownAfterLoad) {
-            stop(loader, serverState, adminServer);
-        }
-    }
 }