Make sure terminated transactions throw correct exceptions

Behaviour of KernelTransactionImplementation#close() is:
 * terminated transaction that was neither marked for success nor for failure
   simply rolls back without any exception
 * terminated transaction marked for success throws TransactionTerminatedException
 * terminated transaction marked for failure simply rolls back without any exception
 * terminated transaction marked for both success and failure throws
   TransactionTerminatedException

Added unit tests for all these cases.

community/kernel/src/main/java/org/neo4j/kernel/impl/api/KernelTransactionImplementation.java

@@ -30,6 +30,7 @@
 import org.neo4j.collection.primitive.PrimitiveIntCollections;
 import org.neo4j.collection.primitive.PrimitiveIntIterator;
 import org.neo4j.cursor.Cursor;
+import org.neo4j.graphdb.TransactionTerminatedException;
 import org.neo4j.helpers.Clock;
 import org.neo4j.helpers.ThisShouldNotHappenError;
 import org.neo4j.kernel.api.KernelTransaction;
@@ -491,15 +492,7 @@ public void close() throws TransactionFailureException
             if ( failure || !success )
             {
                 rollback();
-                if ( success )
-                {
-                    // Success was called, but also failure which means that the client code using this
-                    // transaction passed through a happy path, but the transaction was still marked as
-                    // failed for one or more reasons. Tell the user that although it looked happy it
-                    // wasn't committed, but was instead rolled back.
-                    throw new TransactionFailureException( Status.Transaction.MarkedAsFailed,
-                            "Transaction rolled back even if marked as successful" );
-                }
+                failOnNonExplicitRollbackIfNeeded();
             }
             else
             {
@@ -532,6 +525,37 @@ public void close() throws TransactionFailureException
         }
     }
 
+    /**
+     * Throws exception if this transaction was marked as successful but failure flag has also been set to true.
+     * <p>
+     * This could happen when:
+     * <ul>
+     * <li>caller explicitly calls both {@link #success()} and {@link #failure()}</li>
+     * <li>caller explicitly calls {@link #success()} but transaction execution fails</li>
+     * <li>caller explicitly calls {@link #success()} but transaction is terminated</li>
+     * </ul>
+     * <p>
+     *
+     * @throws TransactionFailureException when execution failed
+     * @throws TransactionTerminatedException when transaction was terminated
+     */
+    private void failOnNonExplicitRollbackIfNeeded() throws TransactionFailureException
+    {
+        if ( success && terminated )
+        {
+            throw new TransactionTerminatedException();
+        }
+        if ( success )
+        {
+            // Success was called, but also failure which means that the client code using this
+            // transaction passed through a happy path, but the transaction was still marked as
+            // failed for one or more reasons. Tell the user that although it looked happy it
+            // wasn't committed, but was instead rolled back.
+            throw new TransactionFailureException( Status.Transaction.MarkedAsFailed,
+                    "Transaction rolled back even if marked as successful" );
+        }
+    }
+
     protected void dispose()
     {
         if ( locks != null )
@@ -729,6 +753,7 @@ private void release()
         {
             locks.close();
             locks = null;
+            terminated = false;
             pool.release( this );
             if ( storeStatement != null )
             {

community/kernel/src/test/java/org/neo4j/kernel/TopLevelTransactionTest.java

@@ -21,6 +21,7 @@
 
 import org.junit.Test;
 
+import org.neo4j.graphdb.TransactionTerminatedException;
 import org.neo4j.graphdb.TransientDatabaseFailureException;
 import org.neo4j.graphdb.TransientFailureException;
 import org.neo4j.graphdb.TransientTransactionFailureException;
@@ -29,7 +30,11 @@
 import org.neo4j.kernel.api.exceptions.TransactionFailureException;
 import org.neo4j.kernel.impl.core.ThreadToStatementContextBridge;
 
+import static org.hamcrest.Matchers.instanceOf;
+import static org.junit.Assert.assertSame;
+import static org.junit.Assert.assertThat;
 import static org.junit.Assert.fail;
+import static org.mockito.Mockito.doReturn;
 import static org.mockito.Mockito.doThrow;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
@@ -82,7 +87,7 @@ public void shouldThrowTransactionExceptionOnTransientKernelException() throws E
     }
 
     @Test
-    public void shouldLetThroughTransactionFailureException() throws Exception
+    public void shouldLetThroughTransientFailureException() throws Exception
     {
         // GIVEN
         KernelTransaction kernelTransaction = mock( KernelTransaction.class );
@@ -102,4 +107,27 @@ public void shouldLetThroughTransactionFailureException() throws Exception
         {   // THEN Good
         }
     }
+
+    @Test
+    public void shouldLetThroughTransactionTerminatedException() throws Exception
+    {
+        KernelTransaction kernelTransaction = mock( KernelTransaction.class );
+        doReturn( true ).when( kernelTransaction ).isOpen();
+        RuntimeException error = new TransactionTerminatedException();
+        doThrow( error ).when( kernelTransaction ).close();
+        ThreadToStatementContextBridge bridge = new ThreadToStatementContextBridge();
+        TopLevelTransaction transaction = new TopLevelTransaction( kernelTransaction, bridge );
+
+        transaction.success();
+        try
+        {
+            transaction.close();
+            fail( "Should have failed" );
+        }
+        catch ( Exception e )
+        {
+            assertThat( e, instanceOf( org.neo4j.graphdb.TransactionFailureException.class ) );
+            assertSame( error, e.getCause() );
+        }
+    }
 }