diff --git a/cypher-shell/src/integration-test/java/org/neo4j/shell/commands/CypherShellTransactionIntegrationTest.java b/cypher-shell/src/integration-test/java/org/neo4j/shell/commands/CypherShellTransactionIntegrationTest.java new file mode 100644 index 00000000..a8702806 --- /dev/null +++ b/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( "" ); + } +} diff --git a/cypher-shell/src/integration-test/java/org/neo4j/shell/commands/CypherShellVerboseIntegrationTest.java b/cypher-shell/src/integration-test/java/org/neo4j/shell/commands/CypherShellVerboseIntegrationTest.java index 50f1d5ae..ec98c20e 100644 --- a/cypher-shell/src/integration-test/java/org/neo4j/shell/commands/CypherShellVerboseIntegrationTest.java +++ b/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()); diff --git a/cypher-shell/src/main/java/org/neo4j/shell/CypherShell.java b/cypher-shell/src/main/java/org/neo4j/shell/CypherShell.java index d1f69c82..9ddb646f 100644 --- a/cypher-shell/src/main/java/org/neo4j/shell/CypherShell.java +++ b/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> commitTransaction() throws CommandException { + public void commitTransaction() throws CommandException { try { - Optional> 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; diff --git a/cypher-shell/src/main/java/org/neo4j/shell/TransactionHandler.java b/cypher-shell/src/main/java/org/neo4j/shell/TransactionHandler.java index 69882bc7..33c55853 100644 --- a/cypher-shell/src/main/java/org/neo4j/shell/TransactionHandler.java +++ b/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> commitTransaction() throws CommandException; + void commitTransaction() throws CommandException; /** * diff --git a/cypher-shell/src/main/java/org/neo4j/shell/state/BoltStateHandler.java b/cypher-shell/src/main/java/org/neo4j/shell/state/BoltStateHandler.java index 0e03f825..e260a1fe 100644 --- a/cypher-shell/src/main/java/org/neo4j/shell/state/BoltStateHandler.java +++ b/cypher-shell/src/main/java/org/neo4j/shell/state/BoltStateHandler.java @@ -118,7 +118,7 @@ public void beginTransaction() throws CommandException { } @Override - public Optional> commitTransaction() throws CommandException { + public void commitTransaction() throws CommandException { if (!isConnected()) { throw new CommandException("Not connected to Neo4j"); } @@ -128,8 +128,6 @@ public Optional> 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; diff --git a/cypher-shell/src/main/java/org/neo4j/shell/state/ErrorWhileInTransactionException.java b/cypher-shell/src/main/java/org/neo4j/shell/state/ErrorWhileInTransactionException.java new file mode 100644 index 00000000..286228f7 --- /dev/null +++ b/cypher-shell/src/main/java/org/neo4j/shell/state/ErrorWhileInTransactionException.java @@ -0,0 +1,30 @@ +package org.neo4j.shell.state; + +import org.neo4j.driver.exceptions.Neo4jException; + +/** + * An exception throws if an error occurs while a transaction is open. + */ +public class ErrorWhileInTransactionException extends Neo4jException +{ + + public ErrorWhileInTransactionException( String message ) + { + super( message ); + } + + public ErrorWhileInTransactionException( String message, Throwable cause ) + { + super( message, cause ); + } + + public ErrorWhileInTransactionException( String code, String message ) + { + super( code, message ); + } + + public ErrorWhileInTransactionException( String code, String message, Throwable cause ) + { + super( code, message, cause ); + } +} diff --git a/cypher-shell/src/test/java/org/neo4j/shell/CypherShellTest.java b/cypher-shell/src/test/java/org/neo4j/shell/CypherShellTest.java index 19a30a9f..224cd9e6 100644 --- a/cypher-shell/src/test/java/org/neo4j/shell/CypherShellTest.java +++ b/cypher-shell/src/test/java/org/neo4j/shell/CypherShellTest.java @@ -100,7 +100,6 @@ public void verifyDelegationOfIsTransactionOpenMethod() throws CommandException @Test public void verifyDelegationOfTransactionMethods() throws CommandException { CypherShell shell = new CypherShell(logger, mockedBoltStateHandler, mockedPrettyPrinter, new ShellParameterMap()); - when(mockedBoltStateHandler.commitTransaction()).thenReturn(Optional.empty()); shell.beginTransaction(); verify(mockedBoltStateHandler).beginTransaction(); @@ -187,21 +186,6 @@ public void executeShouldPrintResult() throws CommandException { verify(logger).printOut(contains("999")); } - @Test - public void commitShouldPrintResult() throws CommandException { - BoltResult result = mock(ListBoltResult.class); - - BoltStateHandler boltStateHandler = mock(BoltStateHandler.class); - - doAnswer((a) -> { ((LinePrinter)a.getArguments()[1]).printOut("999"); return null;}).when(mockedPrettyPrinter).format(any(BoltResult.class), anyObject()); - when(boltStateHandler.commitTransaction()).thenReturn(Optional.of(asList(result))); - - OfflineTestShell shell = new OfflineTestShell(logger, boltStateHandler, mockedPrettyPrinter); - - shell.commitTransaction(); - verify(logger).printOut(contains("999")); - } - @Test public void shouldStripEndingSemicolonsFromCommand() throws Exception { // Should not throw