Merge pull request #57 from spacecowboy/fix-windows-interactive

Fixed lack of interactive detection on Windows

Author: spacecowboy

cypher-shell/src/main/java/org/neo4j/shell/ShellRunner.java

@@ -1,11 +1,17 @@
 package org.neo4j.shell;
 
-import org.neo4j.shell.cli.*;
+import org.neo4j.shell.cli.CliArgs;
+import org.neo4j.shell.cli.FileHistorian;
+import org.neo4j.shell.cli.InteractiveShellRunner;
+import org.neo4j.shell.cli.NonInteractiveShellRunner;
+import org.neo4j.shell.cli.StringShellRunner;
 import org.neo4j.shell.log.Logger;
 import org.neo4j.shell.parser.ShellStatementParser;
 
 import javax.annotation.Nonnull;
+import javax.annotation.Nullable;
 import java.io.IOException;
+import java.util.Optional;
 
 import static org.fusesource.jansi.internal.CLibrary.STDIN_FILENO;
 import static org.fusesource.jansi.internal.CLibrary.isatty;
@@ -19,7 +25,6 @@ public interface ShellRunner {
     int runUntilEnd();
 
     /**
-     *
      * @return an object which can provide the history of commands executed
      */
     @Nonnull
@@ -40,7 +45,7 @@ static ShellRunner getShellRunner(@Nonnull CliArgs cliArgs,
                                       @Nonnull Logger logger) throws IOException {
         if (cliArgs.getCypher().isPresent()) {
             return new StringShellRunner(cliArgs, cypherShell, logger);
-        } else if (isInputInteractive()) {
+        } else if (shouldBeInteractive(cliArgs)) {
             return new InteractiveShellRunner(cypherShell, cypherShell, logger, new ShellStatementParser(),
                     System.in, FileHistorian.getDefaultHistoryFile());
         } else {
@@ -49,19 +54,36 @@ static ShellRunner getShellRunner(@Nonnull CliArgs cliArgs,
         }
     }
 
+    /**
+     * @param cliArgs
+     * @return true if an interactive shellrunner should be used, false otherwise
+     */
+    static boolean shouldBeInteractive(@Nonnull CliArgs cliArgs) {
+        if (cliArgs.getNonInteractive()) {
+            return false;
+        }
+
+        return isInputInteractive(System.getProperty("os.name")).orElse(true);
+    }
+
     /**
      * Checks if STDIN is a TTY. In case TTY checking is not possible (lack of libc), then the check falls back to
      * the built in Java {@link System#console()} which checks if EITHER STDIN or STDOUT has been redirected.
      *
-     * @return true if the shell reading from a TTY, false otherwise (e.g., we are reading from a file)
+     * @return true if the shell reading from a TTY, false otherwise (e.g., we are reading from a file). If on windows,
+     * no result is returned.
      */
-    static boolean isInputInteractive() {
+    static Optional<Boolean> isInputInteractive(@Nullable final String osName) {
+        if (osName != null && osName.toLowerCase().contains("windows")) {
+            // System.console is always null on windows
+            return Optional.empty();
+        }
         try {
-            return 1 == isatty(STDIN_FILENO);
+            return Optional.of(1 == isatty(STDIN_FILENO));
         } catch (NoClassDefFoundError e) {
             // system is not using libc (like Alpine Linux)
             // Fallback to checking stdin OR stdout
-            return System.console() != null;
+            return Optional.of(System.console() != null);
         }
     }
 }

cypher-shell/src/main/java/org/neo4j/shell/cli/CliArgHelper.java

@@ -84,6 +84,8 @@ public static CliArgs parse(@Nonnull String... args) {
 
         cliArgs.setDebugMode(ns.getBoolean("debug"));
 
+        cliArgs.setNonInteractive(ns.getBoolean("force-non-interactive"));
+
         return cliArgs;
     }
 
@@ -146,6 +148,11 @@ private static ArgumentParser setupParser() {
                 .help("print additional debug information")
                 .action(new StoreTrueArgumentAction());
 
+        parser.addArgument("--non-interactive")
+                .help("force non-interactive mode, only useful if auto-detection fails (like on Windows)")
+                .dest("force-non-interactive")
+              .action(new StoreTrueArgumentAction());
+
         parser.addArgument("cypher")
                 .nargs("?")
                 .help("an optional string of cypher to execute and then exit");

cypher-shell/src/main/java/org/neo4j/shell/cli/CliArgs.java

@@ -14,6 +14,7 @@ public class CliArgs {
     private Optional<String> cypher = Optional.empty();
     private boolean encryption;
     private boolean debugMode;
+    private boolean nonInteractive = false;
 
     /**
      * Set the host to the primary value, or if null, the fallback value.
@@ -71,6 +72,13 @@ public void setEncryption(boolean encryption) {
         this.encryption = encryption;
     }
 
+    /**
+     * Force the shell to use non-interactive mode. Only useful on systems where auto-detection fails, such as Windows.
+     */
+    public void setNonInteractive(boolean nonInteractive) {
+        this.nonInteractive = nonInteractive;
+    }
+
     /**
      * Enable/disable debug mode
      */
@@ -120,4 +128,8 @@ public boolean getEncryption() {
     public boolean getDebugMode() {
         return debugMode;
     }
+
+    public boolean getNonInteractive() {
+        return nonInteractive;
+    }
 }

cypher-shell/src/test/java/org/neo4j/shell/ShellRunnerTest.java

@@ -0,0 +1,52 @@
+package org.neo4j.shell;
+
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.ExpectedException;
+import org.neo4j.shell.cli.CliArgs;
+import org.neo4j.shell.cli.InteractiveShellRunner;
+import org.neo4j.shell.cli.NonInteractiveShellRunner;
+import org.neo4j.shell.log.Logger;
+
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assume.assumeTrue;
+import static org.mockito.Mockito.mock;
+import static org.neo4j.shell.ShellRunner.getShellRunner;
+import static org.neo4j.shell.ShellRunner.isInputInteractive;
+
+public class ShellRunnerTest {
+    @Rule
+    public final ExpectedException thrown = ExpectedException.none();
+
+    @Test
+    public void isInputInteractiveThrowsOnWindows() throws Exception {
+        assertFalse("No result should be returned on Windows",
+                isInputInteractive("Windows 7").isPresent());
+    }
+
+    @Test
+    public void isInputInteractiveDoesNotThrowOnNonWindows() throws Exception {
+        assertTrue(isInputInteractive(null).isPresent());
+        assertTrue(isInputInteractive("").isPresent());
+        assertTrue(isInputInteractive("Linux").isPresent());
+        assertTrue(isInputInteractive("BeOS").isPresent());
+    }
+
+    @Test
+    public void inputIsInteractiveByDefaultOnWindows() throws Exception {
+        assumeTrue(System.getProperty("os.name", "").toLowerCase().contains("windows"));
+        ShellRunner runner = getShellRunner(new CliArgs(), mock(CypherShell.class), mock(Logger.class));
+        assertTrue("Should be interactive shell runner by default on windows",
+                runner instanceof InteractiveShellRunner);
+    }
+
+    @Test
+    public void inputIsNonInteractiveIfForced() throws Exception {
+        CliArgs args = new CliArgs();
+        args.setNonInteractive(true);
+        ShellRunner runner = getShellRunner(args, mock(CypherShell.class), mock(Logger.class));
+        assertTrue("Should be non-interactive shell runner when forced",
+                runner instanceof NonInteractiveShellRunner);
+    }
+}

cypher-shell/src/test/java/org/neo4j/shell/cli/CliArgHelperTest.java

@@ -26,6 +26,18 @@ public void setup() {
         mockedStdErr = mock(PrintStream.class);
     }
 
+    @Test
+    public void testForceNonInteractiveIsNotDefault() {
+        assertFalse("Force non-interactive should not be the default mode",
+                CliArgHelper.parse(asArray()).getNonInteractive());
+    }
+
+    @Test
+    public void testForceNonInteractiveIsParsed() {
+        assertTrue("Force non-interactive should have been parsed to true",
+                CliArgHelper.parse(asArray("--non-interactive")).getNonInteractive());
+    }
+
     @Test
     public void testDebugIsNotDefault() {
         assertFalse("Debug should not be the default mode",

cypher-shell/src/test/java/org/neo4j/shell/cli/FileHistorianTest.java

@@ -17,6 +17,7 @@
 import static java.lang.System.getProperty;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotNull;
+import static org.mockito.Matchers.contains;
 import static org.mockito.Mockito.doReturn;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.verify;
@@ -46,11 +47,11 @@ public void defaultHistoryFile() throws Exception {
     @Test
     public void noHistoryFileGivesMemoryHistory() throws Exception {
         Historian historian = FileHistorian.setupHistory(reader, logger,
-                new File("/temp/aasbzs/asfaz/asdfasvzx/asfdasdf/asdfasd"));
+                Paths.get("temp", "aasbzs", "asfaz").toFile());
 
         assertNotNull(historian);
 
-        verify(logger).printError("Could not load history file. Falling back to session-based history.\n" +
-                "Failed to create directory for history: /temp/aasbzs/asfaz/asdfasvzx/asfdasdf");
+        verify(logger).printError(contains("Could not load history file. Falling back to session-based history.\n" +
+                "Failed to create directory for history"));
     }
 }