svn commit: r1747288 - in /ofbiz/trunk/framework/start/src/org/ofbiz/base/start: AdminClient.java AdminPortThread.java AdminServer.java Config.java Start.java StartupCommand.java StartupCommandUtil.java StartupControlPanel.java StartupException.java

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
1 message Options
Reply | Threaded
Open this post in threaded view
|

svn commit: r1747288 - in /ofbiz/trunk/framework/start/src/org/ofbiz/base/start: AdminClient.java AdminPortThread.java AdminServer.java Config.java Start.java StartupCommand.java StartupCommandUtil.java StartupControlPanel.java StartupException.java

Taher Alkhateeb
Author: taher
Date: Tue Jun  7 18:12:46 2016
New Revision: 1747288

URL: http://svn.apache.org/viewvc?rev=1747288&view=rev
Log:
Major commit for refactoring the start component - Ref OFBIZ-6783

A thank you is due to Jacques Le Roux, whom substantially improved
the refactoring of the start component with his QA work, and also
credit is due to Jacopo Capellato and Gil Portenseigne for their help

This commit is the last commit on OFBIZ-6783 and applies the following:

- convert the startup loaders from ArrayList to List. Given that
  only two loaders existed in the system since the beginning of
  OFBiz means that it is really unnecessary to use the
  loaders.trimToSize() method. It's simply too much specificity for
  no apparent performance value. this leads to changing many method
  signatures from ArrayList to list across different classes
- More simplification of Config.java by simplifying timezone setting
  logic
- remove the throws StartupException from main. This was confusing
  confused me a lot and lead to a regression from my refactoring
  in OFBIZ-7167 because some exceptions bubble up all the way to
  main but then the system does not terminate because other
  threads are running. It took me and Jacques a long while to
  dig it out and figure the root cause.
- Introduce a function called fullyTerminateSystem(StartupException e).
  This function replaces the exception bubble issue that is
  described above. Consequently, the methods init, status and
  shutdown no longer throw an exception but instead terminate
  the system (which is the right thing to do).
- finetune adaptStartupCommandsToLoaderArgs(...) and other
  places in StartupControlPanel
- separate the Classpath creation from NativeLibClassLoader
  creation. This makes it cleaner and easier to refactor
  in the future
- break the StartupControlPanel.start(...) method into
  smaller more readable methods. It is easier now to understand
  the startup sequence for ofbiz in english words (the method names).
  It also allows for cleaner refactoring in the future.
- add JavaDoc to a lot of methods all over the start component
- rename AdminPortThread to AdminServer and create a new class called
  AdminClient. Now everything relating to controlling OFBiz directly
  happens through StartupControlPanel such as start, stop or init
  for the server, and everything which happens indirectly through
  the network (by an admin client) is done through the AdminClient
  which talks to AdminServer
- more refactoring of AdminServer to simplify the code and break it
  down into multiple methods and refactor some messy old code

Assuming this commit is stable, and after waiting for a sufficient time
I will close the JIRA and move on with the refactoring exercise.


Added:
    ofbiz/trunk/framework/start/src/org/ofbiz/base/start/AdminClient.java   (with props)
    ofbiz/trunk/framework/start/src/org/ofbiz/base/start/AdminServer.java   (with props)
Removed:
    ofbiz/trunk/framework/start/src/org/ofbiz/base/start/AdminPortThread.java
Modified:
    ofbiz/trunk/framework/start/src/org/ofbiz/base/start/Config.java
    ofbiz/trunk/framework/start/src/org/ofbiz/base/start/Start.java
    ofbiz/trunk/framework/start/src/org/ofbiz/base/start/StartupCommand.java
    ofbiz/trunk/framework/start/src/org/ofbiz/base/start/StartupCommandUtil.java
    ofbiz/trunk/framework/start/src/org/ofbiz/base/start/StartupControlPanel.java
    ofbiz/trunk/framework/start/src/org/ofbiz/base/start/StartupException.java

Added: ofbiz/trunk/framework/start/src/org/ofbiz/base/start/AdminClient.java
URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/start/src/org/ofbiz/base/start/AdminClient.java?rev=1747288&view=auto
==============================================================================
--- ofbiz/trunk/framework/start/src/org/ofbiz/base/start/AdminClient.java (added)
+++ ofbiz/trunk/framework/start/src/org/ofbiz/base/start/AdminClient.java Tue Jun  7 18:12:46 2016
@@ -0,0 +1,95 @@
+/*******************************************************************************
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ *******************************************************************************/
+package org.ofbiz.base.start;
+
+import java.io.BufferedReader;
+import java.io.IOException;
+import java.io.InputStreamReader;
+import java.io.PrintWriter;
+import java.net.ConnectException;
+import java.net.Socket;
+
+import org.ofbiz.base.start.AdminServer.OfbizSocketCommand;
+
+/**
+ * The AdminClient communicates with a running OFBiz server instance
+ * through the AdminServer by sending commands like requesting server
+ * status or requesting system shutdown
+ */
+class AdminClient {
+
+    /**
+     * Send a command through network to OFBiz server
+     * to show its status (running, stopping, ...)
+     *
+     * @param config: OFBiz configuration
+     * @return status: OFBiz server status
+     */
+    static String requestStatus(Config config) {
+        String status = null;
+        try {
+            status = sendSocketCommand(OfbizSocketCommand.STATUS, config);
+        } catch (ConnectException e) {
+            status = "Not Running";
+        } catch (IOException e) {
+            status = "IO Error when trying to connect to OFBiz: " + e.getMessage();
+        }
+        return status;
+    }
+
+    /**
+     * Send a command through network to OFBiz server
+     * to shut itself down.
+     *
+     * @param config: OFBiz configuration
+     * @return shutdownMessage: message from server
+     *   on receiving shutdown request
+     */
+    static String requestShutdown(Config config) {
+        String shutdownMessage = null;
+        try {
+            shutdownMessage = sendSocketCommand(OfbizSocketCommand.SHUTDOWN, config);
+        } catch (IOException e) {
+            shutdownMessage = "IO Error when trying to connect to OFBiz: " + e.getMessage();
+        }
+        return shutdownMessage;
+    }
+
+    private static String sendSocketCommand(OfbizSocketCommand socketCommand, Config config) throws IOException {
+        String response = "OFBiz is Down";
+        try {
+            Socket socket = new Socket(config.adminAddress, config.adminPort);
+            // send the command
+            PrintWriter writer = new PrintWriter(socket.getOutputStream(), true);
+            writer.println(config.adminKey + ":" + socketCommand);
+            writer.flush();
+            // read the reply
+            BufferedReader reader = new BufferedReader(new InputStreamReader(socket.getInputStream()));
+            response = reader.readLine();
+            reader.close();
+            // close the socket
+            writer.close();
+            socket.close();
+
+        } catch (ConnectException e) {
+            System.out.println("Could not connect to " + config.adminAddress + ":" + config.adminPort);
+        }
+        return response;
+    }
+}

Propchange: ofbiz/trunk/framework/start/src/org/ofbiz/base/start/AdminClient.java
------------------------------------------------------------------------------
    svn:eol-style = native

Propchange: ofbiz/trunk/framework/start/src/org/ofbiz/base/start/AdminClient.java
------------------------------------------------------------------------------
    svn:keywords = Date Rev Author URL Id

Propchange: ofbiz/trunk/framework/start/src/org/ofbiz/base/start/AdminClient.java
------------------------------------------------------------------------------
    svn:mime-type = text/plain

Added: ofbiz/trunk/framework/start/src/org/ofbiz/base/start/AdminServer.java
URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/start/src/org/ofbiz/base/start/AdminServer.java?rev=1747288&view=auto
==============================================================================
--- ofbiz/trunk/framework/start/src/org/ofbiz/base/start/AdminServer.java (added)
+++ ofbiz/trunk/framework/start/src/org/ofbiz/base/start/AdminServer.java Tue Jun  7 18:12:46 2016
@@ -0,0 +1,129 @@
+package org.ofbiz.base.start;
+
+import java.io.BufferedReader;
+import java.io.IOException;
+import java.io.InputStreamReader;
+import java.io.PrintWriter;
+import java.net.ServerSocket;
+import java.net.Socket;
+import java.util.List;
+import java.util.concurrent.atomic.AtomicReference;
+
+import org.ofbiz.base.start.Start.ServerState;
+
+/**
+ * The AdminServer provides a way to communicate with a running
+ * OFBiz instance after it has started and send commands to that instance
+ * such as inquiring on server status or requesting system shutdown
+ */
+final class AdminServer extends Thread {
+
+    /**
+     * Commands communicated between AdminClient and AdminServer
+     */
+    enum OfbizSocketCommand {
+        SHUTDOWN, STATUS, FAIL
+    }
+
+    private ServerSocket serverSocket = null;
+    private List<StartupLoader> loaders = null;
+    private AtomicReference<ServerState> serverState = null;
+    private Config config = null;
+
+    AdminServer(List<StartupLoader> loaders, 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.serverState = serverState;
+        this.config = config;
+    }
+
+    @Override
+    public void run() {
+        System.out.println("Admin socket configured on - " + config.adminAddress + ":" + config.adminPort);
+        while (!Thread.interrupted()) {
+            try {
+                Socket clientSocket = serverSocket.accept();
+                System.out.println("Received connection from - " + clientSocket.getInetAddress() + " : "
+                        + clientSocket.getPort());
+                processClientRequest(clientSocket, loaders, serverState);
+                clientSocket.close();
+            } catch (IOException e) {
+                e.printStackTrace();
+            }
+        }
+    }
+
+    private void processClientRequest(Socket client, List<StartupLoader> loaders, AtomicReference<ServerState> serverState) throws IOException {
+        BufferedReader reader = null;
+        PrintWriter writer = null;
+        try {
+            reader = new BufferedReader(new InputStreamReader(client.getInputStream()));
+            writer = new PrintWriter(client.getOutputStream(), true);
+
+            executeClientRequest(reader, writer, loaders, serverState);
+        } finally {
+            if (reader != null) {
+                reader.close();
+            }
+            if (writer != null) {
+                writer.flush();
+                writer.close();
+            }
+        }
+    }
+
+    private void executeClientRequest(BufferedReader reader, PrintWriter writer,
+            List<StartupLoader> loaders, AtomicReference<ServerState> serverState) throws IOException {
+
+        String clientRequest = reader.readLine();
+        OfbizSocketCommand clientCommand = determineClientCommand(clientRequest);
+        String serverResponse = prepareResponseToClient(clientCommand, serverState);
+        
+        writer.println(serverResponse);
+
+        if(clientCommand.equals(OfbizSocketCommand.SHUTDOWN)) {
+            writer.flush();
+            StartupControlPanel.stop(loaders, serverState, this);
+        }
+    }
+
+    private OfbizSocketCommand determineClientCommand(String request) {
+        OfbizSocketCommand clientCommand;
+        if(request == null
+                || request.isEmpty()
+                || !request.contains(":")
+                || !request.substring(0, request.indexOf(':')).equals(config.adminKey)
+                || request.substring(request.indexOf(':') + 1) == null) {
+            clientCommand = OfbizSocketCommand.FAIL;
+        } else {
+            clientCommand = OfbizSocketCommand.valueOf(request.substring(request.indexOf(':') + 1));
+        }
+        return clientCommand;
+    }
+
+    private String prepareResponseToClient(OfbizSocketCommand control, AtomicReference<ServerState> serverState) {
+        String response = null;
+        switch(control) {
+            case SHUTDOWN:
+                if (serverState.get() == ServerState.STOPPING) {
+                    response = "IN-PROGRESS";
+                } else {
+                    response = "OK";
+                }
+                break;
+            case STATUS:
+                response = serverState.get().toString();
+                break;
+            case FAIL:
+                response = "FAIL";
+                break;
+        }
+        return response;
+    }
+}

Propchange: ofbiz/trunk/framework/start/src/org/ofbiz/base/start/AdminServer.java
------------------------------------------------------------------------------
    svn:eol-style = native

Propchange: ofbiz/trunk/framework/start/src/org/ofbiz/base/start/AdminServer.java
------------------------------------------------------------------------------
    svn:keywords = Date Rev Author URL Id

Propchange: ofbiz/trunk/framework/start/src/org/ofbiz/base/start/AdminServer.java
------------------------------------------------------------------------------
    svn:mime-type = text/plain

Modified: ofbiz/trunk/framework/start/src/org/ofbiz/base/start/Config.java
URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/start/src/org/ofbiz/base/start/Config.java?rev=1747288&r1=1747287&r2=1747288&view=diff
==============================================================================
--- ofbiz/trunk/framework/start/src/org/ofbiz/base/start/Config.java (original)
+++ ofbiz/trunk/framework/start/src/org/ofbiz/base/start/Config.java Tue Jun  7 18:12:46 2016
@@ -35,6 +35,10 @@ import java.util.Optional;
 import java.util.Properties;
 import java.util.TimeZone;
 
+/**
+ * OFBiz server parameters needed on system startup and retrieved from
+ * one of the properties files in the start component
+ */
 public final class Config {
 
     public final String ofbizHome;
@@ -89,10 +93,8 @@ public final class Config {
         setDefaultLocale(props);
 
         // set the default timezone
-        String tzString = props.getProperty("ofbiz.timeZone.default");
-        if (tzString != null && tzString.length() > 0) {
-            TimeZone.setDefault(TimeZone.getTimeZone(tzString));
-        }
+        String tzString = props.getProperty("ofbiz.timeZone.default", TimeZone.getDefault().getID());
+        TimeZone.setDefault(TimeZone.getTimeZone(tzString));
     }
 
     private String getAbsolutePath(Properties props, String key, String def, String ofbizHome) {

Modified: ofbiz/trunk/framework/start/src/org/ofbiz/base/start/Start.java
URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/start/src/org/ofbiz/base/start/Start.java?rev=1747288&r1=1747287&r2=1747288&view=diff
==============================================================================
--- ofbiz/trunk/framework/start/src/org/ofbiz/base/start/Start.java (original)
+++ ofbiz/trunk/framework/start/src/org/ofbiz/base/start/Start.java Tue Jun  7 18:12:46 2016
@@ -49,13 +49,12 @@ public final class Start {
     }
 
     /**
-     * main is the entry point to execute high level ofbiz commands
+     * main is the entry point to execute high level OFBiz commands
      * such as starting, stopping or checking the status of the server.
      *
-     * @param args: The commands for ofbiz
-     * @throws StartupException: terminates ofbiz or propagates to caller
+     * @param args: The commands for OFBiz
      */
-    public static void main(String[] args) throws StartupException {
+    public static void main(String[] args) {
         List<StartupCommand> ofbizCommands = null;
         try {
             ofbizCommands = StartupCommandUtil.parseOfbizCommands(args);
@@ -66,7 +65,7 @@ public final class Start {
             System.exit(1);
         }
 
-        CommandType commandType = evaluateOfbizCommands(ofbizCommands);
+        CommandType commandType = determineCommandType(ofbizCommands);
         if(!commandType.equals(CommandType.HELP)) {
             instance.config = StartupControlPanel.init(ofbizCommands);
         }
@@ -75,18 +74,16 @@ public final class Start {
                 StartupCommandUtil.printOfbizStartupHelp(System.out);
                 break;
             case STATUS:
-                System.out.println("Current Status : " + StartupControlPanel.status(instance.config));
+                System.out.println("Current Status : " + AdminClient.requestStatus(instance.config));
                 break;
             case SHUTDOWN:
-                System.out.println("Shutting down server : " + StartupControlPanel.shutdown(instance.config));
+                System.out.println("Shutting down server : " + AdminClient.requestShutdown(instance.config));
                 break;
             case START:
                 try {
                     StartupControlPanel.start(instance.config, instance.serverState, ofbizCommands);
                 } catch (StartupException e) {
-                    // if startup logic fails, execute shutdown hooks
-                    e.printStackTrace();
-                    System.exit(99);
+                    StartupControlPanel.fullyTerminateSystem(e);
                 }
                 break;
         }
@@ -125,7 +122,7 @@ public final class Start {
         }
     }
 
-    private static CommandType evaluateOfbizCommands(List<StartupCommand> ofbizCommands) {
+    private static CommandType determineCommandType(List<StartupCommand> ofbizCommands) {
         if (ofbizCommands.stream().anyMatch(
                 command -> command.getName().equals(StartupCommandUtil.StartupOption.HELP.getName()))) {
             return CommandType.HELP;

Modified: ofbiz/trunk/framework/start/src/org/ofbiz/base/start/StartupCommand.java
URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/start/src/org/ofbiz/base/start/StartupCommand.java?rev=1747288&r1=1747287&r2=1747288&view=diff
==============================================================================
--- ofbiz/trunk/framework/start/src/org/ofbiz/base/start/StartupCommand.java (original)
+++ ofbiz/trunk/framework/start/src/org/ofbiz/base/start/StartupCommand.java Tue Jun  7 18:12:46 2016
@@ -21,11 +21,11 @@ package org.ofbiz.base.start;
 import java.util.Map;
 
 /**
- * A command line argument passed to ofbiz
+ * A command line argument passed to OFBiz
  *
  * <p>
  * A <tt>StartupCommand</tt> represents a processed command line argument passed
- * to ofbiz such that it is no longer a raw string but an instance of this class.
+ * to OFBiz such that it is no longer a raw string but an instance of this class.
  * For example: <code>java -jar ofbiz.jar --status</code> where status is a command.
  * </p>
  */

Modified: ofbiz/trunk/framework/start/src/org/ofbiz/base/start/StartupCommandUtil.java
URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/start/src/org/ofbiz/base/start/StartupCommandUtil.java?rev=1747288&r1=1747287&r2=1747288&view=diff
==============================================================================
--- ofbiz/trunk/framework/start/src/org/ofbiz/base/start/StartupCommandUtil.java (original)
+++ ofbiz/trunk/framework/start/src/org/ofbiz/base/start/StartupCommandUtil.java Tue Jun  7 18:12:46 2016
@@ -216,8 +216,8 @@ final class StartupCommandUtil {
      *
      * TODO A better solution is to change the signature of all
      * containers to receive a <tt>List</tt> of <tt>StartupCommand</tt>s
-     * instead and delete the methods populateLoaderArgs, commandExistsInList
-     * and retrieveCommandArgumentEntries along with the loaderArgs list.
+     * instead and delete the methods adaptStartupCommandsToLoaderArgs
+     * and retrieveCommandArguments along with the loaderArgs list.
      */
     static List<String> adaptStartupCommandsToLoaderArgs(List<StartupCommand> ofbizCommands) {
         List<String> loaderArgs = new ArrayList<String>();
@@ -225,13 +225,13 @@ final class StartupCommandUtil {
         final String TEST = StartupCommandUtil.StartupOption.TEST.getName();
         final String TEST_LIST = StartupCommandUtil.StartupOption.TEST_LIST.getName();
         
-        if(commandExistsInList(ofbizCommands, LOAD_DATA)) {
+        if(ofbizCommands.stream().anyMatch(command -> command.getName().equals(LOAD_DATA))) {
             retrieveCommandArguments(ofbizCommands, LOAD_DATA).entrySet().stream().forEach(entry ->
             loaderArgs.add("-" + entry.getKey() + "=" + entry.getValue()));
-        } else if(commandExistsInList(ofbizCommands, TEST)) {
+        } else if(ofbizCommands.stream().anyMatch(command -> command.getName().equals(TEST))) {
             retrieveCommandArguments(ofbizCommands, TEST).entrySet().stream().forEach(entry ->
             loaderArgs.add("-" + entry.getKey() + "=" + entry.getValue()));
-        } else if(commandExistsInList(ofbizCommands, TEST_LIST)) {
+        } else if(ofbizCommands.stream().anyMatch(command -> command.getName().equals(TEST_LIST))) {
             Map<String,String> testListArgs = retrieveCommandArguments(ofbizCommands, TEST_LIST);
             loaderArgs.add(testListArgs.get("file"));
             loaderArgs.add("-" + testListArgs.get("mode"));
@@ -239,16 +239,30 @@ final class StartupCommandUtil {
         return loaderArgs;
     }
 
-    private static boolean commandExistsInList(List<StartupCommand> ofbizCommands, String commandName) {
-        return ofbizCommands.stream().anyMatch(command -> command.getName().equals(commandName));
-    }
-
     private static Map<String,String> retrieveCommandArguments(List<StartupCommand> ofbizCommands, String commandName) {
         return ofbizCommands.stream()
                 .filter(option-> option.getName().equals(commandName))
                 .collect(Collectors.toList()).get(0).getProperties();
     }
 
+    private static final Options getOfbizStartupOptions() {
+        OptionGroup ofbizCommandOptions = new OptionGroup();
+        ofbizCommandOptions.addOption(BOTH);
+        ofbizCommandOptions.addOption(HELP);
+        ofbizCommandOptions.addOption(LOAD_DATA);
+        ofbizCommandOptions.addOption(POS);
+        ofbizCommandOptions.addOption(SHUTDOWN);
+        ofbizCommandOptions.addOption(START);
+        ofbizCommandOptions.addOption(STATUS);
+        ofbizCommandOptions.addOption(TEST);
+        ofbizCommandOptions.addOption(TEST_LIST);
+
+        Options options = new Options();
+        options.addOptionGroup(ofbizCommandOptions);
+        options.addOption(PORTOFFSET);
+        return options;
+    }
+
     private static final List<StartupCommand> mapCommonsCliOptionsToStartupCommands(final CommandLine commandLine) {
         List<Option> optionList = Arrays.asList(commandLine.getOptions());
         return optionList.stream()
@@ -293,22 +307,4 @@ final class StartupCommandUtil {
         }
         //TODO add more validations
     }
-
-    private static final Options getOfbizStartupOptions() {
-        OptionGroup ofbizCommandOptions = new OptionGroup();
-        ofbizCommandOptions.addOption(BOTH);
-        ofbizCommandOptions.addOption(HELP);
-        ofbizCommandOptions.addOption(LOAD_DATA);
-        ofbizCommandOptions.addOption(POS);
-        ofbizCommandOptions.addOption(SHUTDOWN);
-        ofbizCommandOptions.addOption(START);
-        ofbizCommandOptions.addOption(STATUS);
-        ofbizCommandOptions.addOption(TEST);
-        ofbizCommandOptions.addOption(TEST_LIST);
-
-        Options options = new Options();
-        options.addOptionGroup(ofbizCommandOptions);
-        options.addOption(PORTOFFSET);
-        return options;
-    }
 }

Modified: ofbiz/trunk/framework/start/src/org/ofbiz/base/start/StartupControlPanel.java
URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/start/src/org/ofbiz/base/start/StartupControlPanel.java?rev=1747288&r1=1747287&r2=1747288&view=diff
==============================================================================
--- ofbiz/trunk/framework/start/src/org/ofbiz/base/start/StartupControlPanel.java (original)
+++ ofbiz/trunk/framework/start/src/org/ofbiz/base/start/StartupControlPanel.java Tue Jun  7 18:12:46 2016
@@ -18,14 +18,9 @@
  *******************************************************************************/
 package org.ofbiz.base.start;
 
-import java.io.BufferedReader;
 import java.io.File;
 import java.io.FileInputStream;
 import java.io.IOException;
-import java.io.InputStreamReader;
-import java.io.PrintWriter;
-import java.net.ConnectException;
-import java.net.Socket;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.Map;
@@ -33,91 +28,85 @@ import java.util.concurrent.atomic.Atomi
 
 import org.ofbiz.base.start.Start.ServerState;
 
+/**
+ * The StartupControlPanel controls OFBiz by executing high level
+ * functions such as init, start and stop on the server. The bulk
+ * of the startup sequence logic resides in this class.
+ */
 final class StartupControlPanel {
 
-    enum OfbizSocketCommand {
-        SHUTDOWN, STATUS, FAIL
-    }
-
-    static Config init(List<StartupCommand> ofbizCommands) throws StartupException {
-        loadGlobalOfbizSystemProperties("ofbiz.system.props");
+    /**
+     * Initialize OFBiz by:
+     * - setting high level JVM and OFBiz system properties
+     * - creating a Config object holding startup configuration parameters
+     *
+     * @param ofbizCommands: commands passed by the user to OFBiz on start
+     * @return config: OFBiz configuration
+     */
+    static Config init(List<StartupCommand> ofbizCommands) {
+        Config config = null;
         try {
-            return new Config(ofbizCommands);
+            loadGlobalOfbizSystemProperties("ofbiz.system.props");
+            config =  new Config(ofbizCommands);
         } catch (StartupException e) {
-            throw new StartupException("Could not fetch config instance", e);
-        }
-    }
-
-    static String status(Config config) throws StartupException {
-        try {
-            return sendSocketCommand(OfbizSocketCommand.STATUS, config);
-        } catch (ConnectException e) {
-            return "Not Running";
-        } catch (IOException e) {
-            throw new StartupException(e);
-        }
-    }
-
-    static String shutdown(Config config) throws StartupException {
-        try {
-            return sendSocketCommand(OfbizSocketCommand.SHUTDOWN, config);
-        } catch (Exception e) {
-            throw new StartupException(e);
+            fullyTerminateSystem(e);
         }
+        return config;
     }
 
+    /**
+     * Execute the startup sequence for OFBiz
+     */
     static void start(Config config,
             AtomicReference<ServerState> serverState,
             List<StartupCommand> ofbizCommands) throws StartupException {
-        
-        List<String> loaderArgs = StartupCommandUtil.adaptStartupCommandsToLoaderArgs(ofbizCommands);
-        ArrayList<StartupLoader> loaders = new ArrayList<StartupLoader>();
-        Thread adminPortThread = null;
-
-        //create log dir
-        File logDir = new File(config.logDir);
-        if (!logDir.exists()) {
-            if (logDir.mkdir()) {
-                System.out.println("Created OFBiz log dir [" + logDir.getAbsolutePath() + "]");
-            }
-        }
-
-        // create the listener thread
-        if (config.adminPort > 0) {
-            adminPortThread = new AdminPortThread(loaders, serverState, config);
-            adminPortThread.start();
-        } else {
-            System.out.println("Admin socket not configured; set to port 0");
-        }
 
-        // set the shutdown hook
-        if (config.useShutdownHook) {
-            Runtime.getRuntime().addShutdownHook(new Thread() {
-                @Override
-                public void run() {
-                    StartupControlPanel.shutdownServer(loaders, serverState, this);
-                }
-            });
-        } else {
-            System.out.println("Shutdown hook disabled");
-        }
-
-        // load and start the startup loaders
-        loadStartupLoaders(config, loaderArgs, loaders, serverState);
+        List<StartupLoader> loaders = new ArrayList<StartupLoader>();
+        List<String> loaderArgs = StartupCommandUtil.adaptStartupCommandsToLoaderArgs(ofbizCommands);
+        Thread adminServer = createAdminServer(config, serverState, loaders);
+        Classpath classPath = createClassPath(config);
+        NativeLibClassLoader classLoader = createAndSetContextClassLoader(config, classPath);
+
+        createLogDirectoryIfMissing(config);
+        createRuntimeShutdownHook(config, loaders, serverState);
+        loadStartupLoaders(config, loaders, loaderArgs, serverState, classLoader);
         startStartupLoaders(loaders, serverState);
-
-        // execute shutdown if applicable
-        if (config.shutdownAfterLoad) {
-            StartupControlPanel.stopServer(loaders, serverState, adminPortThread);
-        }
+        executeShutdownAfterLoadIfConfigured(config, loaders, serverState, adminServer);
     }
 
-    static void stopServer(ArrayList<StartupLoader> loaders, AtomicReference<ServerState> serverState, Thread adminPortThread) {
-        shutdownServer(loaders, serverState, adminPortThread);
+    /**
+     * 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(List<StartupLoader> loaders, AtomicReference<ServerState> serverState, Thread adminServer) {
+        shutdownServer(loaders, serverState, adminServer);
         System.exit(0);
     }
 
-    private static void shutdownServer(ArrayList<StartupLoader> loaders, AtomicReference<ServerState> serverState, Thread adminPortThread) {
+    /**
+     * Properly exit from the system when a StartupException cannot or
+     * should not be handled except by exiting the system.
+     *
+     * A proper system exit is achieved by:
+     *
+     * - Printing the stack trace for users to see what happened
+     * - Executing the shutdown hooks (if existing) through System.exit
+     * - Terminating any lingering threads (if existing) through System.exit
+     * - Providing an exit code that is not 0 to signal to the build system
+     *   or user of failure to execute.
+     *
+     * @param e: The startup exception that cannot / should not be handled
+     *   except by terminating the system
+     */
+    static void fullyTerminateSystem(StartupException e) {
+        e.printStackTrace();
+        System.exit(1);
+    }
+
+    private static void shutdownServer(List<StartupLoader> loaders, AtomicReference<ServerState> serverState, Thread adminServer) {
         ServerState currentState;
         do {
             currentState = serverState.get();
@@ -138,8 +127,8 @@ final class StartupControlPanel {
                 }
             }
         }
-        if (adminPortThread != null && adminPortThread.isAlive()) {
-            adminPortThread.interrupt();
+        if (adminServer != null && adminServer.isAlive()) {
+            adminServer.interrupt();
         }
     }
 
@@ -157,79 +146,95 @@ final class StartupControlPanel {
         }
     }
 
-    private static String sendSocketCommand(OfbizSocketCommand socketCommand, Config config) throws IOException {
-        String response = "OFBiz is Down";
-        try {
-            Socket socket = new Socket(config.adminAddress, config.adminPort);
-            // send the command
-            PrintWriter writer = new PrintWriter(socket.getOutputStream(), true);
-            writer.println(config.adminKey + ":" + socketCommand);
-            writer.flush();
-            // read the reply
-            BufferedReader reader = new BufferedReader(new InputStreamReader(socket.getInputStream()));
-            response = reader.readLine();
-            reader.close();
-            // close the socket
-            writer.close();
-            socket.close();
+    private static Thread createAdminServer(
+            Config config,
+            AtomicReference<ServerState> serverState,
+            List<StartupLoader> loaders) throws StartupException {
 
-        } catch (ConnectException e) {
-            System.out.println("Could not connect to " + config.adminAddress + ":" + config.adminPort);
+        Thread adminServer = null;
+        if (config.adminPort > 0) {
+            adminServer = new AdminServer(loaders, serverState, config);
+            adminServer.start();
+        } else {
+            System.out.println("Admin socket not configured; set to port 0");
         }
-        return response;
+        return adminServer;
     }
 
-    private static NativeLibClassLoader createClassLoader(Config config) throws IOException {
-        ClassLoader parent = Thread.currentThread().getContextClassLoader();
-        if (parent instanceof NativeLibClassLoader) {
-            parent = parent.getParent();
-        }
-        if (parent == null) {
-            parent = Start.class.getClassLoader();
-            if (parent == null) {
-                parent = ClassLoader.getSystemClassLoader();
-            }
-        }
+    private static Classpath createClassPath(Config config) throws StartupException {
         Classpath classPath = new Classpath();
-        /*
-         * Class paths needed to get StartupLoaders to work.
-         */
-        classPath.addComponent(config.ofbizHome);
-        String ofbizHomeTmp = config.ofbizHome;
-        if (!ofbizHomeTmp.isEmpty() && !ofbizHomeTmp.endsWith("/")) {
-            ofbizHomeTmp = ofbizHomeTmp.concat("/");
-        }
-        if (config.classpathAddComponent != null) {
-            String[] components = config.classpathAddComponent.split(",");
-            for (String component : components) {
-                classPath.addComponent(ofbizHomeTmp.concat(component.trim()));
-            }
-        }
-        if (config.classpathAddFilesFromPath != null) {
-            String[] paths = config.classpathAddFilesFromPath.split(",");
-            for (String path : paths) {
-                classPath.addFilesFromPath(new File(ofbizHomeTmp.concat(path.trim())));
-            }
-        }
-        NativeLibClassLoader classloader = new NativeLibClassLoader(classPath.getUrls(), parent);
-        classloader.addNativeClassPath(System.getProperty("java.library.path"));
-        for (File folder : classPath.getNativeFolders()) {
-            classloader.addNativeClassPath(folder);
+        try {
+            classPath.addComponent(config.ofbizHome);
+            String ofbizHomeTmp = config.ofbizHome;
+            if (!ofbizHomeTmp.isEmpty() && !ofbizHomeTmp.endsWith("/")) {
+                ofbizHomeTmp = ofbizHomeTmp.concat("/");
+            }
+            if (config.classpathAddComponent != null) {
+                String[] components = config.classpathAddComponent.split(",");
+                for (String component : components) {
+                    classPath.addComponent(ofbizHomeTmp.concat(component.trim()));
+                }
+            }
+            if (config.classpathAddFilesFromPath != null) {
+                String[] paths = config.classpathAddFilesFromPath.split(",");
+                for (String path : paths) {
+                    classPath.addFilesFromPath(new File(ofbizHomeTmp.concat(path.trim())));
+                }
+            }
+        } catch (IOException e) {
+            throw new StartupException("Cannot create classpath", e);
         }
-        return classloader;
+        return classPath;
     }
 
-    private static void loadStartupLoaders(Config config,
-            List<String> loaderArgs, ArrayList<StartupLoader> loaders,
-            AtomicReference<ServerState> serverState) throws StartupException {
-
+    private static NativeLibClassLoader createAndSetContextClassLoader(Config config, Classpath classPath) throws StartupException {
+        ClassLoader parent = Thread.currentThread().getContextClassLoader();
         NativeLibClassLoader classloader = null;
         try {
-            classloader = createClassLoader(config);
+            classloader = new NativeLibClassLoader(classPath.getUrls(), parent);
+            classloader.addNativeClassPath(System.getProperty("java.library.path"));
+            for (File folder : classPath.getNativeFolders()) {
+                classloader.addNativeClassPath(folder);
+            }
         } catch (IOException e) {
             throw new StartupException("Couldn't create NativeLibClassLoader", e);
         }
         Thread.currentThread().setContextClassLoader(classloader);
+        return classloader;
+    }
+
+    private static void createLogDirectoryIfMissing(Config config) {
+        File logDir = new File(config.logDir);
+        if (!logDir.exists()) {
+            if (logDir.mkdir()) {
+                System.out.println("Created OFBiz log dir [" + logDir.getAbsolutePath() + "]");
+            }
+        }
+    }
+
+    private static void createRuntimeShutdownHook(
+            Config config,
+            List<StartupLoader> loaders,
+            AtomicReference<ServerState> serverState) {
+
+        if (config.useShutdownHook) {
+            Runtime.getRuntime().addShutdownHook(new Thread() {
+                @Override
+                public void run() {
+                    shutdownServer(loaders, serverState, this);
+                }
+            });
+        } else {
+            System.out.println("Shutdown hook disabled");
+        }
+    }
+
+    private static void loadStartupLoaders(Config config,
+            List<StartupLoader> loaders,
+            List<String> loaderArgs,
+            AtomicReference<ServerState> serverState,
+            NativeLibClassLoader classloader) throws StartupException {
+
         String[] argsArray = loaderArgs.toArray(new String[loaderArgs.size()]);
         synchronized (loaders) {
             for (Map<String, String> loaderMap : config.loaders) {
@@ -246,7 +251,6 @@ final class StartupControlPanel {
                     throw new StartupException(e.getMessage(), e);
                 }
             }
-            loaders.trimToSize();
         }
         StringBuilder sb = new StringBuilder();
         for (String path : classloader.getNativeLibPaths()) {
@@ -275,4 +279,15 @@ final class StartupControlPanel {
             throw new StartupException("Error during start");
         }
     }
+
+    private static void executeShutdownAfterLoadIfConfigured(
+            Config config,
+            List<StartupLoader> loaders,
+            AtomicReference<ServerState> serverState,
+            Thread adminServer) {
+
+        if (config.shutdownAfterLoad) {
+            stop(loaders, serverState, adminServer);
+        }
+    }
 }

Modified: ofbiz/trunk/framework/start/src/org/ofbiz/base/start/StartupException.java
URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/start/src/org/ofbiz/base/start/StartupException.java?rev=1747288&r1=1747287&r2=1747288&view=diff
==============================================================================
--- ofbiz/trunk/framework/start/src/org/ofbiz/base/start/StartupException.java (original)
+++ ofbiz/trunk/framework/start/src/org/ofbiz/base/start/StartupException.java Tue Jun  7 18:12:46 2016
@@ -19,8 +19,11 @@
 package org.ofbiz.base.start;
 
 /**
- * StartupException
- *
+ * StartupException is an exception that is thrown when something wrong happens
+ * during executing any OFBiz high level commands.
+ *
+ * If StartupException is not handled then it will bubble up to main
+ * and lead to system termination.
  */
 @SuppressWarnings("serial")
 public final class StartupException extends Exception {