From 4e177a1422e81081873dc607a1ac051b01a9efe4 Mon Sep 17 00:00:00 2001
From: lutovich <konstantin.lutovich@neo4j.com>
Date: Fri, 14 Sep 2018 18:44:29 +0200
Subject: [PATCH] Allow rollback transaction after failure

Previously, it was not possible to rollback transaction after failure.
Doing this resulted in an error. This commit turns it into a no-op.
Transaction is automatically rolled back after failure in the database
because failure is acknowledged by sending a RESET message. So this is
just a driver API level change.
---
 src/v1/internal/stream-observer.js    |  8 ++++++++
 src/v1/transaction.js                 |  4 +---
 test/internal/stream-observer.test.js | 12 ++++++++++++
 test/v1/session.test.js               |  2 +-
 test/v1/transaction.test.js           | 13 +++++++++++++
 5 files changed, 35 insertions(+), 4 deletions(-)

diff --git a/src/v1/internal/stream-observer.js b/src/v1/internal/stream-observer.js
index c6794b7e2..a4f7bcdff 100644
--- a/src/v1/internal/stream-observer.js
+++ b/src/v1/internal/stream-observer.js
@@ -113,6 +113,14 @@ class StreamObserver {
     this._fieldKeys = [];
   }
 
+  /**
+   * Mark this observer as if it has completed with no metadata.
+   */
+  markCompleted() {
+    this._fieldKeys = [];
+    this._tail = {};
+  }
+
   /**
    * Will be called on errors.
    * If user-provided observer is present, pass the error
diff --git a/src/v1/transaction.js b/src/v1/transaction.js
index a94bd271a..a0b5d54b0 100644
--- a/src/v1/transaction.js
+++ b/src/v1/transaction.js
@@ -177,9 +177,7 @@ let _states = {
       return {result: _newDummyResult(observer, "COMMIT", {}), state: _states.FAILED};
     },
     rollback: (connectionHolder, observer) => {
-      observer.onError({error:
-      "Cannot rollback transaction, because previous statements in the " +
-      "transaction has failed and the transaction has already been rolled back."});
+      observer.markCompleted();
       return {result: _newDummyResult(observer, "ROLLBACK", {}), state: _states.FAILED};
     },
     run: (connectionHolder, observer, statement, parameters) => {
diff --git a/test/internal/stream-observer.test.js b/test/internal/stream-observer.test.js
index be322cdbc..584b07f5b 100644
--- a/test/internal/stream-observer.test.js
+++ b/test/internal/stream-observer.test.js
@@ -172,6 +172,18 @@ describe('StreamObserver', () => {
     streamObserver.onCompleted({key: 42});
   });
 
+  it('should mark as completed', done => {
+    const streamObserver = new StreamObserver();
+    streamObserver.markCompleted();
+
+    streamObserver.subscribe({
+      onCompleted: metadata => {
+        expect(metadata).toEqual({});
+        done();
+      }
+    });
+  });
+
 });
 
 function newStreamObserver() {
diff --git a/test/v1/session.test.js b/test/v1/session.test.js
index ace637b55..484d51345 100644
--- a/test/v1/session.test.js
+++ b/test/v1/session.test.js
@@ -114,7 +114,7 @@ describe('session', () => {
     const session = driver.session();
     const tx = session.beginTransaction();
     tx.run('INVALID QUERY').catch(() => {
-      tx.rollback().catch(() => {
+      tx.rollback().then(() => {
         session.close(() => {
           driver.close();
           done();
diff --git a/test/v1/transaction.test.js b/test/v1/transaction.test.js
index 8085f1ee6..8f93cfbf9 100644
--- a/test/v1/transaction.test.js
+++ b/test/v1/transaction.test.js
@@ -527,6 +527,19 @@ describe('transaction', () => {
     tx.rollback().then(() => done());
   });
 
+  it('should allow rollback after failure', done => {
+    const tx = session.beginTransaction();
+    tx.run('WRONG QUERY')
+      .then(() => done.fail('Expected to fail'))
+      .catch(error => {
+        expectSyntaxError(error);
+
+        tx.rollback()
+          .catch(error => done.fail(error))
+          .then(() => done());
+      });
+  });
+
   function expectSyntaxError(error) {
     expect(error.code).toBe('Neo.ClientError.Statement.SyntaxError');
   }