4.2 tx handle error

cypher-shell/src/integration-test/java/org/neo4j/shell/commands/CypherShellTransactionIntegrationTest.java

@@ -0,0 +1,200 @@
+package org.neo4j.shell.commands;
+
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.ExpectedException;
+
+import org.neo4j.shell.CypherShell;
+import org.neo4j.shell.ShellParameterMap;
+import org.neo4j.shell.StringLinePrinter;
+import org.neo4j.shell.cli.Format;
+import org.neo4j.shell.exception.CommandException;
+import org.neo4j.shell.prettyprint.PrettyConfig;
+import org.neo4j.shell.state.ErrorWhileInTransactionException;
+
+import static org.hamcrest.CoreMatchers.containsString;
+import static org.hamcrest.CoreMatchers.not;
+import static org.hamcrest.MatcherAssert.assertThat;
+
+public class CypherShellTransactionIntegrationTest extends CypherShellIntegrationTest
+{
+    @Rule
+    public final ExpectedException thrown = ExpectedException.none();
+
+    private final StringLinePrinter linePrinter = new StringLinePrinter();
+    private Command rollbackCommand;
+    private Command commitCommand;
+    private Command beginCommand;
+
+    @Before
+    public void setUp() throws Exception
+    {
+        linePrinter.clear();
+        shell = new CypherShell( linePrinter, new PrettyConfig( Format.VERBOSE, true, 1000 ), false, new ShellParameterMap() );
+        rollbackCommand = new Rollback( shell );
+        commitCommand = new Commit( shell );
+        beginCommand = new Begin( shell );
+
+        connect( "neo" );
+        shell.execute( "MATCH (n) DETACH DELETE (n)" );
+    }
+
+    @Test
+    public void rollbackScenario() throws CommandException
+    {
+        //given
+        shell.execute( "CREATE (:TestPerson {name: \"Jane Smith\"})" );
+
+        //when
+        beginCommand.execute( "" );
+        shell.execute( "CREATE (:NotCreated)" );
+        rollbackCommand.execute( "" );
+
+        //then
+        shell.execute( "MATCH (n) RETURN n" );
+
+        String output = linePrinter.output();
+        assertThat( output, containsString( "| n " ) );
+        assertThat( output, containsString( "| (:TestPerson {name: \"Jane Smith\"}) |" ) );
+        assertThat( output, not( containsString( ":NotCreated" ) ) );
+    }
+
+    @Test
+    public void failureInTxScenario() throws CommandException
+    {
+        // given
+        beginCommand.execute( "" );
+
+        // then
+        thrown.expect( ErrorWhileInTransactionException.class );
+        thrown.expectMessage( "/ by zero" );
+        thrown.expectMessage( "An error occurred while in an open transaction. The transaction will be rolled back and terminated." );
+
+        shell.execute( "RETURN 1/0" );
+    }
+
+    @Test
+    public void failureInTxScenarioWithCypherFollowing() throws CommandException
+    {
+        // given
+        beginCommand.execute( "" );
+        try
+        {
+            shell.execute( "RETURN 1/0" );
+        }
+        catch ( ErrorWhileInTransactionException ignored )
+        {
+            // This is OK
+        }
+
+        // when
+        shell.execute( "RETURN 42" );
+
+        // then
+        assertThat(linePrinter.output(), containsString("42"));
+    }
+
+    @Test
+    public void failureInTxScenarioWithCommitFollowing() throws CommandException
+    {
+        // given
+        beginCommand.execute( "" );
+        try
+        {
+            shell.execute( "RETURN 1/0" );
+        }
+        catch ( ErrorWhileInTransactionException ignored )
+        {
+            // This is OK
+        }
+
+        // then
+        thrown.expect( CommandException.class );
+        thrown.expectMessage( "There is no open transaction to commit" );
+
+        // when
+        commitCommand.execute( "" );
+    }
+
+    @Test
+    public void failureInTxScenarioWithRollbackFollowing() throws CommandException
+    {
+        // given
+        beginCommand.execute( "" );
+        try
+        {
+            shell.execute( "RETURN 1/0" );
+        }
+        catch ( ErrorWhileInTransactionException ignored )
+        {
+            // This is OK
+        }
+
+        // then
+        thrown.expect( CommandException.class );
+        thrown.expectMessage( "There is no open transaction to rollback" );
+
+        // when
+        rollbackCommand.execute( "" );
+    }
+
+    @Test
+    public void resetInFailedTxScenario() throws CommandException
+    {
+        //when
+        beginCommand.execute( "" );
+        try
+        {
+            shell.execute( "RETURN 1/0" );
+        }
+        catch ( ErrorWhileInTransactionException ignored )
+        {
+            // This is OK
+        }
+        shell.reset();
+
+        //then
+        shell.execute( "CREATE (:TestPerson {name: \"Jane Smith\"})" );
+        shell.execute( "MATCH (n) RETURN n" );
+
+        String result = linePrinter.output();
+        assertThat( result, containsString( "| (:TestPerson {name: \"Jane Smith\"}) |" ) );
+        assertThat( result, not( containsString( ":NotCreated" ) ) );
+    }
+
+    @Test
+    public void resetInTxScenario() throws CommandException
+    {
+        //when
+        beginCommand.execute( "" );
+        shell.execute( "CREATE (:NotCreated)" );
+        shell.reset();
+
+        //then
+        shell.execute( "CREATE (:TestPerson {name: \"Jane Smith\"})" );
+        shell.execute( "MATCH (n) RETURN n" );
+
+        String result = linePrinter.output();
+        assertThat( result, containsString( "| (:TestPerson {name: \"Jane Smith\"}) |" ) );
+        assertThat( result, not( containsString( ":NotCreated" ) ) );
+    }
+
+    @Test
+    public void commitScenario() throws CommandException
+    {
+        beginCommand.execute( "" );
+        shell.execute( "CREATE (:TestPerson {name: \"Joe Smith\"})" );
+        assertThat( linePrinter.output(), containsString( "0 rows available after" ) );
+
+        linePrinter.clear();
+        shell.execute( "CREATE (:TestPerson {name: \"Jane Smith\"})" );
+        assertThat( linePrinter.output(), containsString( "0 rows available after" ) );
+
+        linePrinter.clear();
+        shell.execute( "MATCH (n:TestPerson) RETURN n ORDER BY n.name" );
+        assertThat( linePrinter.output(), containsString( "\n| (:TestPerson {name: \"Jane Smith\"}) |\n| (:TestPerson {name: \"Joe Smith\"})  |\n" ) );
+
+        commitCommand.execute( "" );
+    }
+}

cypher-shell/src/integration-test/java/org/neo4j/shell/commands/CypherShellVerboseIntegrationTest.java

@@ -31,17 +31,11 @@ public class CypherShellVerboseIntegrationTest extends CypherShellIntegrationTes
     public final ExpectedException thrown = ExpectedException.none();
 
     private StringLinePrinter linePrinter = new StringLinePrinter();
-    private Command rollbackCommand;
-    private Command commitCommand;
-    private Command beginCommand;
 
     @Before
     public void setUp() throws Exception {
         linePrinter.clear();
         shell = new CypherShell(linePrinter, new PrettyConfig(Format.VERBOSE, true, 1000), false, new ShellParameterMap());
-        rollbackCommand = new Rollback(shell);
-        commitCommand = new Commit(shell);
-        beginCommand = new Begin(shell);
 
         connect( "neo" );
     }
@@ -81,25 +75,6 @@ public void connectTwiceThrows() throws CommandException {
         connect( "neo" );
     }
 
-    @Test
-    public void rollbackScenario() throws CommandException {
-        //given
-        shell.execute("CREATE (:TestPerson {name: \"Jane Smith\"})");
-
-        //when
-        beginCommand.execute("");
-        shell.execute("CREATE (:NotCreated)");
-        rollbackCommand.execute("");
-
-        //then
-        shell.execute("MATCH (n) RETURN n");
-
-        String output = linePrinter.output();
-        assertThat(output, containsString("| n "));
-        assertThat(output, containsString("| (:TestPerson {name: \"Jane Smith\"}) |"));
-        assertThat(output, not(containsString(":NotCreated")));
-    }
-
     @Test
     public void resetOutOfTxScenario() throws CommandException {
         //when
@@ -116,39 +91,6 @@ public void resetOutOfTxScenario() throws CommandException {
                 "| (:TestPerson {name: \"Jane Smith\"}) |"));
     }
 
-    @Test
-    public void resetInTxScenario() throws CommandException {
-        //when
-        beginCommand.execute("");
-        shell.execute("CREATE (:NotCreated)");
-        shell.reset();
-
-        //then
-        shell.execute("CREATE (:TestPerson {name: \"Jane Smith\"})");
-        shell.execute("MATCH (n) RETURN n");
-
-        String result = linePrinter.output();
-        assertThat(result, containsString("| (:TestPerson {name: \"Jane Smith\"}) |"));
-        assertThat(result, not(containsString(":NotCreated")));
-    }
-
-    @Test
-    public void commitScenario() throws CommandException {
-        beginCommand.execute("");
-        shell.execute("CREATE (:TestPerson {name: \"Joe Smith\"})");
-        assertThat(linePrinter.output(), containsString("0 rows available after"));
-
-        linePrinter.clear();
-        shell.execute("CREATE (:TestPerson {name: \"Jane Smith\"})");
-        assertThat(linePrinter.output(), containsString("0 rows available after"));
-
-        linePrinter.clear();
-        shell.execute("MATCH (n:TestPerson) RETURN n ORDER BY n.name");
-        assertThat(linePrinter.output(), containsString("\n| (:TestPerson {name: \"Jane Smith\"}) |\n| (:TestPerson {name: \"Joe Smith\"})  |\n"));
-
-        commitCommand.execute("");
-    }
-
     @Test
     public void paramsAndListVariables() throws EvaluationException, CommandException {
         assertTrue(shell.getParameterMap().allParameterValues().isEmpty());

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

@@ -1,6 +1,5 @@
 package org.neo4j.shell;
 
-import java.util.List;
 import java.util.Optional;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
@@ -102,7 +101,7 @@ private void executeCypher(@Nonnull final String cypher) throws CommandException
             lastNeo4jErrorCode = null;
         } catch (Neo4jException e) {
             lastNeo4jErrorCode = getErrorCode(e);
-            throw e;
+            throw boltStateHandler.handleException( e );
         }
     }
 
@@ -158,12 +157,10 @@ public void beginTransaction() throws CommandException {
     }
 
     @Override
-    public Optional<List<BoltResult>> commitTransaction() throws CommandException {
+    public void commitTransaction() throws CommandException {
         try {
-            Optional<List<BoltResult>> results = boltStateHandler.commitTransaction();
-            results.ifPresent(boltResult -> boltResult.forEach(result -> prettyPrinter.format(result, linePrinter)));
+            boltStateHandler.commitTransaction();
             lastNeo4jErrorCode = null;
-            return results;
         } catch (Neo4jException e) {
             lastNeo4jErrorCode = getErrorCode(e);
             throw e;

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

@@ -1,10 +1,6 @@
 package org.neo4j.shell;
 
 import org.neo4j.shell.exception.CommandException;
-import org.neo4j.shell.state.BoltResult;
-
-import java.util.List;
-import java.util.Optional;
 
 /**
  * An object capable of starting, committing, and rolling back transactions.
@@ -21,7 +17,7 @@ public interface TransactionHandler {
      *
      * @throws CommandException if current transaction could not be committed
      */
-    Optional<List<BoltResult>> commitTransaction() throws CommandException;
+    void commitTransaction() throws CommandException;
 
     /**
      *

cypher-shell/src/main/java/org/neo4j/shell/state/BoltStateHandler.java

@@ -118,7 +118,7 @@ public void beginTransaction() throws CommandException {
     }
 
     @Override
-    public Optional<List<BoltResult>> commitTransaction() throws CommandException {
+    public void commitTransaction() throws CommandException {
         if (!isConnected()) {
             throw new CommandException("Not connected to Neo4j");
         }
@@ -128,8 +128,6 @@ public Optional<List<BoltResult>> commitTransaction() throws CommandException {
         tx.commit();
         tx.close();
         tx = null;
-
-        return Optional.empty();
     }
 
     @Override
@@ -145,6 +143,29 @@ public void rollbackTransaction() throws CommandException {
         tx = null;
     }
 
+    /**
+     * Handle an exception while getting or consuming the result.
+     * If not in TX, return the given exception.
+     * If in a TX, terminate the TX and return a more verbose error message.
+     *
+     * @param e the thrown exception.
+     * @return a suitable exception to rethrow.
+     */
+    public Neo4jException handleException( Neo4jException e )
+    {
+        if ( isTransactionOpen() )
+        {
+            tx.close();
+            tx = null;
+            return new ErrorWhileInTransactionException(
+                    "An error occurred while in an open transaction. The transaction will be rolled back and terminated. Error: " + e.getMessage(), e );
+        }
+        else
+        {
+            return e;
+        }
+    }
+
     @Override
     public boolean isTransactionOpen() {
         return tx != null;