Fix the non delivery of Bye message

Signed-off-by: Emmanuel Bigeon <emmanuel@bigeon.fr>

gclc-socket/pom.xml

@@ -87,7 +87,7 @@ of Emmanuel Bigeon.                                                   -->
     <dependency>
     	<groupId>fr.bigeon</groupId>
     	<artifactId>gclc</artifactId>
-    	<version>1.3.2</version>
+    	<version>1.3.3-SNAPSHOT</version>
     </dependency>
     <dependency>
     	<groupId>fr.bigeon</groupId>

gclc-socket/src/main/java/fr/bigeon/gclc/socket/SocketConsoleApplicationShell.java

@@ -259,8 +259,8 @@ public class SocketConsoleApplicationShell implements Runnable, AutoCloseable {
      * @throws IOException if the communication failed */
     private void communicate(final Socket socket, final PrintWriter writer,
                              BufferedReader in) throws IOException {
-        Thread th = new Thread(new OutputForwardRunnable(writer, socket),
+        OutputForwardRunnable cc = new OutputForwardRunnable(writer, socket);
-                "ClientComm"); //$NON-NLS-1$
+        Thread th = new Thread(cc, "ClientComm"); //$NON-NLS-1$
         th.start();
         if (autoClose) {
             communicateOnce(socket, in);
@@ -322,7 +322,14 @@ public class SocketConsoleApplicationShell implements Runnable, AutoCloseable {
         }
         String ln = reading.getMessage();
         if (ln.equals(close)) {
+            Thread wait = consoleManager.getWaitForDelivery("Bye."); //$NON-NLS-1$
             consoleManager.println("Bye."); //$NON-NLS-1$
+            try {
+                wait.join();
+            } catch (InterruptedException e) {
+                LOGGER.warning("The Bye wait was interrupted."); //$NON-NLS-1$
+                LOGGER.log(Level.FINE, "An interruption occured", e); //$NON-NLS-1$
+            }
             return false;
         }
         // Pass command to application

gclc-socket/src/test/java/fr/bigeon/gclc/socket/SocketConsoleApplicationTest.java

@@ -85,9 +85,10 @@ public class SocketConsoleApplicationTest {
                      new InputStreamReader(kkSocket.getInputStream()));) {
 
             String fromServer;
-            int i = 0;
+            int i = -1;
             String[] cmds = {"help", "toto", "test", "bye"};
             while ((fromServer = in.readLine()) != null) {
+                i++;
                 LOGGER.fine("Server: \n" + fromServer);
                 if (fromServer.equals("Bye.")) {
                     break;
@@ -105,7 +106,6 @@ public class SocketConsoleApplicationTest {
                     LOGGER.fine("Client: " + fromUser);
                     out.println(fromUser);
                 }
-                i++;
             }
             assertEquals(4, i);
         } catch (final IOException e) {

gclc/src/main/java/fr/bigeon/gclc/ConsoleApplication.java

@@ -187,8 +187,10 @@ public class ConsoleApplication implements ICommandProvider {
         } catch (IOException e) {
             // The manager was closed
             running = false;
-            LOGGER.log(Level.WARNING,
+            LOGGER.warning(
-                    "The console manager was closed. Closing the application as no one can reach it.", //$NON-NLS-1$
+                    "The console manager was closed. Closing the application as no one can reach it."); //$NON-NLS-1$
+            LOGGER.log(Level.FINE,
+                    "An exception caused the closing of the application", //$NON-NLS-1$
                     e);
             return;
         }
@@ -201,8 +203,9 @@ public class ConsoleApplication implements ICommandProvider {
             } catch (IOException e) {
                 // The manager was closed
                 running = false;
-                LOGGER.log(Level.WARNING,
+                LOGGER.warning("The console manager was closed."); //$NON-NLS-1$
-                        "The console manager was closed.", //$NON-NLS-1$
+                LOGGER.log(Level.FINE,
+                        "An exception occured when trying to print the good by e message... The application will still close.", //$NON-NLS-1$
                         e);
             }
         }
@@ -225,14 +228,17 @@ public class ConsoleApplication implements ICommandProvider {
             }
             interpretCommand(cmd);
         } catch (InterruptedIOException e) {
-            LOGGER.log(Level.INFO,
+            LOGGER.info(
-                    "Prompt interrupted. It is likely the application is closing.", //$NON-NLS-1$
+                    "Prompt interrupted. It is likely the application is closing."); //$NON-NLS-1$
+            LOGGER.log(Level.FINER, "Interruption of the prompt.", //$NON-NLS-1$
                     e);
         } catch (IOException e) {
             // The manager was closed
             running = false;
-            LOGGER.log(Level.WARNING,
+            LOGGER.warning(
-                    "The console manager was closed. Closing the application as no one can reach it.", //$NON-NLS-1$
+                    "The console manager was closed. Closing the application as no one can reach it."); //$NON-NLS-1$
+            LOGGER.log(Level.FINE,
+                    "An exception caused the closing of the application", //$NON-NLS-1$
                     e);
         }
     }

gclc/src/main/java/fr/bigeon/gclc/command/CommandParameters.java

@@ -111,7 +111,7 @@ public class CommandParameters {
             int p = parseArg(args[i], next);
             if (p == 0) {
                 throw new CommandParsingException(
-                        "Invalid parameter " + args[i]);
+                        "Invalid parameter " + args[i]); //$NON-NLS-1$
             }
             i += p;
         }

gclc/src/main/java/fr/bigeon/gclc/command/ParametrizedCommand.java

@@ -61,8 +61,7 @@ import fr.bigeon.gclc.manager.ConsoleManager;
 public abstract class ParametrizedCommand extends Command {
 
     /** If the command may use interactive prompting for required parameters
-     * that
+     * that were not provided on execution */
-     * were not provided on execution */
     private boolean interactive = true;
     /** The manager */
     protected final ConsoleManager manager;
@@ -73,8 +72,7 @@ public abstract class ParametrizedCommand extends Command {
     /** The parameters mandatory status */
     private final Map<String, Boolean> params = new HashMap<>();
     /** The restriction of provided parameters on execution to declared
-     * paramters
+     * paramters in the status maps. */
-     * in the status maps. */
     private final boolean strict;
 
     /** @param manager the manager
@@ -111,7 +109,11 @@ public abstract class ParametrizedCommand extends Command {
      * @param param the parameter identification
      * @param stringParameter if the parameter is a parameter with an argument
      * @param needed if the parameter is required
-     * @throws InvalidParameterException if the parameter was invalid */
+     * @throws InvalidParameterException if the parameter was invalid
+     * @deprecated since gclc-1.3.3, use the
+     *             {@link #addStringParameter(String, boolean)} and
+     *             {@link #addBooleanParameter(String)} */
+    @Deprecated
     protected void addParameter(String param, boolean stringParameter,
                                 boolean needed) throws InvalidParameterException {
         if (params.containsKey(param)) {
@@ -132,10 +134,41 @@ public abstract class ParametrizedCommand extends Command {
         }
     }
 
+    /** Add a boolean parameter to defined parmaters.
+     * 
+     * @param flag the boolean flag
+     * @throws InvalidParameterException if the parameter is already defined as
+     *             a string parameter */
+    protected void addBooleanParameter(String flag) throws InvalidParameterException {
+        if (params.containsKey(flag) && stringParams.containsKey(flag)) {
+            throw new InvalidParameterException(
+                    "Parameter is already defined as string"); //$NON-NLS-1$
+        }
+        boolParams.add(flag);
+        params.put(flag, Boolean.valueOf(false));
+    }
+
+    /** Add a string parameter to defined parmaters.
+     * 
+     * @param flag the parameter flag
+     * @param needed if the parameter's absence should cause an exception
+     * @throws InvalidParameterException if the parameter is already defined as
+     *             a boolean parameter */
+    protected void addStringParameter(String flag,
+                                      boolean needed) throws InvalidParameterException {
+        if (params.containsKey(flag)) {
+            checkParam(flag, needed);
+            return;
+        }
+        stringParams.put(flag, Boolean.valueOf(needed));
+        params.put(flag, Boolean.valueOf(needed));
+    }
+
     /** @param param the parameter
      * @param stringParameter the string parameter type
      * @param needed if the parameter is needed
      * @throws InvalidParameterException if the new definition is invalid */
+    @Deprecated
     private void checkParam(String param, boolean stringParameter,
                             boolean needed) throws InvalidParameterException {
         if (stringParameter) {
@@ -155,6 +188,22 @@ public abstract class ParametrizedCommand extends Command {
         }
     }
 
+    /** @param param the string parameter
+     * @param needed if the parameter is needed
+     * @throws InvalidParameterException if the new definition is invalid */
+    private void checkParam(String param,
+                            boolean needed) throws InvalidParameterException {
+        if (stringParams.containsKey(param)) {
+            Boolean need = Boolean
+                    .valueOf(needed || stringParams.get(param).booleanValue());
+            stringParams.put(param, need);
+            params.put(param, need);
+            return;
+        }
+        throw new InvalidParameterException(
+                "Parameter is already defined as boolean"); //$NON-NLS-1$
+    }
+
     /** @param parameters the command parameters
      * @throws CommandRunException if the command failed */
     protected abstract void doExecute(CommandParameters parameters) throws CommandRunException;
@@ -164,8 +213,8 @@ public abstract class ParametrizedCommand extends Command {
     @SuppressWarnings("boxing")
     @Override
     public final void execute(String... args) throws CommandRunException {
-        final CommandParameters parameters = new CommandParameters(
+        final CommandParameters parameters = new CommandParameters(boolParams,
-                boolParams, stringParams.keySet(), strict);
+                stringParams.keySet(), strict);
         try {
             parameters.parseArgs(args);
         } catch (CommandParsingException e) {

gclc/src/main/java/fr/bigeon/gclc/manager/PipedConsoleManager.java

@@ -172,4 +172,12 @@ public final class PipedConsoleManager
     public void interruptPrompt() {
         innerManager.interruptPrompt();
     }
+
+    /** @param message the message
+     * @return the thread to join to wait for message delivery
+     * @see fr.bigeon.gclc.manager.ReadingRunnable#getWaitForDelivery(java.lang.String) */
+    public Thread getWaitForDelivery(String message) {
+        return reading.getWaitForDelivery(message);
+    }
+
 }

gclc/src/main/java/fr/bigeon/gclc/manager/ReadingRunnable.java

@@ -43,6 +43,8 @@ import java.io.IOException;
 import java.io.InterruptedIOException;
 import java.util.ArrayDeque;
 import java.util.Deque;
+import java.util.HashMap;
+import java.util.Map;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 
@@ -69,6 +71,15 @@ public class ReadingRunnable implements Runnable {
     private final Object lock = new Object();
     /** The waiting status for a message */
     private boolean waiting;
+    /**
+     * The blocker for a given message
+     */
+    private final Map<String, Object> messageBlocker = new HashMap<>();
+    /**
+     * The lock
+     */
+    private final Object messageBlockerLock = new Object();
+    private String delivering;
 
     /** @param reader the input to read from */
     public ReadingRunnable(BufferedReader reader) {
@@ -96,10 +107,13 @@ public class ReadingRunnable implements Runnable {
                     lock.notify();
                 }
             } catch (InterruptedIOException e) {
-                LOGGER.log(Level.INFO, "Reading interrupted", e); //$NON-NLS-1$
+                LOGGER.info("Reading interrupted"); //$NON-NLS-1$
-
+                LOGGER.log(Level.FINER,
+                        "Read interruption was caused by an exception", e); //$NON-NLS-1$
             } catch (IOException e) {
-                LOGGER.log(Level.SEVERE, "Unable to read from stream", e); //$NON-NLS-1$
+                LOGGER.severe("Unable to read from stream"); //$NON-NLS-1$
+                LOGGER.log(Level.FINE, "The stream reading threw an exception", //$NON-NLS-1$
+                        e);
                 running = false;
                 return;
             }
@@ -141,6 +155,7 @@ public class ReadingRunnable implements Runnable {
             }
             LOGGER.finest("Polled: " + messages.peek()); //$NON-NLS-1$
             waiting = false;
+            notifyMessage(messages.peek());
             return messages.poll();
         }
     }
@@ -179,4 +194,63 @@ public class ReadingRunnable implements Runnable {
             }
         }
     }
+
+    /** @param message the message */
+    private void notifyMessage(String message) {
+        synchronized (messageBlockerLock) {
+            delivering = message;
+            if (messageBlocker.containsKey(message)) {
+                Object mLock = messageBlocker.get(message);
+                synchronized (mLock) {
+                    mLock.notify();
+                }
+                messageBlocker.remove(message);
+            }
+        }
+    }
+
+    /** @param message the message
+     * @return the thread to join to wait for message delivery */
+    public Thread getWaitForDelivery(final String message) {
+        synchronized (messageBlockerLock) {
+            if (!messageBlocker.containsKey(message)) {
+                messageBlocker.put(message, new Object());
+            }
+            final Object obj = messageBlocker.get(message);
+            final Object start = new Object();
+            Thread th = new Thread(new Runnable() {
+    
+                @SuppressWarnings("synthetic-access")
+                @Override
+                public void run() {
+                    synchronized (obj) {
+                        synchronized (start) {
+                            start.notify();
+                        }
+                        while (isRunning()) {
+                            try {
+                                obj.wait();
+                                if (delivering.equals(message)) {
+                                    return;
+                                }
+                            } catch (InterruptedException e) {
+                                LOGGER.log(Level.SEVERE,
+                                        "Thread interruption exception.", e); //$NON-NLS-1$
+                            }
+                        }
+                    }
+                }
+            });
+            synchronized (start) {
+                th.start();
+                try {
+                    start.wait();
+                } catch (InterruptedException e) {
+                    LOGGER.log(Level.SEVERE, "Thread interruption exception.", //$NON-NLS-1$
+                            e);
+                }
+            }
+            return th;
+        }
+    }
 }

gclc/src/main/java/fr/bigeon/gclc/prompt/CLIPrompter.java

@@ -245,8 +245,8 @@ public class CLIPrompter {
                 r = Integer.parseInt(result);
                 still = false;
             } catch (final NumberFormatException e) {
-                LOGGER.log(Level.INFO,
+                LOGGER.info("User input a non parsable integer: " + result); //$NON-NLS-1$
-                        "User input a non parsable integer: " + result, e); //$NON-NLS-1$
+                LOGGER.log(Level.FINEST, "Unrecognized integer", e); //$NON-NLS-1$
                 still = true;
             }
         }

gclc/src/main/java/fr/bigeon/gclc/prompt/CLIPrompterMessages.java

@@ -74,7 +74,8 @@ public class CLIPrompterMessages {
         try {
             return MessageFormat.format(RESOURCE_BUNDLE.getString(key), args);
         } catch (final MissingResourceException e) {
-            LOGGER.log(Level.WARNING, "Unrecognized key: " + key, e); //$NON-NLS-1$
+            LOGGER.warning("Unrecognized key: " + key); //$NON-NLS-1$
+            LOGGER.log(Level.FINE, "Missing key in " + BUNDLE_NAME, e); //$NON-NLS-1$
             return '!' + key + '!';
         }
     }

gclc/src/main/resources/fr/bigeon/gclc/messages.properties

@@ -16,7 +16,8 @@ promptbool.choices.no1=N
 promptbool.choices.no2=no
 
 promptchoice.outofbounds=Please choose something between {0} and {1}. The choices were:
-promptchoice.formaterr=The input seems to be something that is not an integer.\nPlease choose something between {0} and {1}. The choices were:
+promptchoice.formaterr=The input seems to be something that is not an integer.\
+Please choose something between {0} and {1}. The choices were:
 
 promptlongtext.exit.defaultkey=\\q
 promptlongtext.exit.dispkey=\ (exit with a new line made of "{0}")

gclc/src/test/java/fr/bigeon/gclc/command/ParametrizedCommandTest.java

@@ -392,21 +392,20 @@ public class ParametrizedCommandTest {
                 assertEquals(2, parameters.getStringArgumentKeys().size());
                 switch (call) {
                 case 0:
-                case 1:
                     assertNull(parameters.get(str1));
                     assertNull(parameters.get(str2));
                     assertFalse(parameters.getBool(bool1));
                     assertFalse(parameters.getBool(bool2));
                     call++;
                     break;
-                case 2:
+                case 1:
                     assertEquals(str2, parameters.get(str1));
                     assertNull(parameters.get(str2));
                     assertFalse(parameters.getBool(bool1));
                     assertFalse(parameters.getBool(bool2));
                     call++;
                     break;
-                case 3:
+                case 2:
                     assertEquals(str2, parameters.get(str1));
                     assertNull(parameters.get(str2));
                     assertTrue(parameters.getBool(bool1));
@@ -420,12 +419,17 @@ public class ParametrizedCommandTest {
         };
         try {
             cmd.execute();
-            cmd.execute(addParam);
             cmd.execute("-" + str1, str2);
             cmd.execute("-" + str1, str2, "-" + bool1);
         } catch (CommandRunException e) {
             assertNull(e);
-            fail("unepected error");
+            fail("unexpected error");
+        }
+        try {
+            cmd.execute(addParam);
+            fail("Strict should fail with unexpected argument");
+        } catch (CommandRunException e) {
+            assertNotNull(e);
         }
         try {
             cmd.execute("-" + addParam);
@@ -504,11 +508,16 @@ public class ParametrizedCommandTest {
         try {
             cmd.execute("-" + str1, str2);
             cmd.execute("-" + str1, str2, "-" + bool1);
-            cmd.execute("-" + str1, str2, addParam);
         } catch (CommandRunException e) {
             assertNull(e);
             fail("unepected error");
         }
+        try {
+            cmd.execute("-" + str1, str2, addParam);
+            fail("Additional parameter should cause failure");
+        } catch (CommandRunException e) {
+            assertNotNull(e);
+        }
         try {
             cmd.execute();
             fail("needed " + str1 + " not provided shall fail");