Merge pull request #184 from lutovich/1.1-no-bookmark-after-failure

Make sure session does not have bookmark after unsuccessful tx

Author: lutovich

src/v1/transaction.js

@@ -31,6 +31,7 @@ class Transaction {
    * @param {function()} onClose - Function to be called when transaction is committed or rolled back.
    * @param errorTransformer callback use to transform error
    * @param bookmark optional bookmark
+   * @param onBookmark callback invoked when new bookmark is produced
    */
   constructor(connectionPromise, onClose, errorTransformer, bookmark, onBookmark) {
     this._connectionPromise = connectionPromise;
@@ -129,10 +130,8 @@ class _TransactionStreamObserver extends StreamObserver {
 
   onCompleted(meta) {
     super.onCompleted(meta);
-    let bookmark = meta.bookmark;
-    if (bookmark) {
-      this._tx._onBookmark(bookmark);
-    }
+    const bookmark = meta.bookmark;
+    this._tx._onBookmark(bookmark);
   }
 
   serverMeta() {

test/v1/transaction.test.js

@@ -219,9 +219,7 @@ describe('transaction', function() {
   });
 
   it('should provide bookmark on commit', function (done) {
-    //bookmarking is not in 3.0
-    if (!server) {
-      done();
+    if (neo4jVersionOlderThan31(done)) {
       return;
     }
 
@@ -232,11 +230,71 @@ describe('transaction', function() {
     tx.run("CREATE (:TXNode2)");
     tx.commit()
       .then(function () {
-        expect(session.lastBookmark()).toBeDefined();
+        expectValidLastBookmark(session);
         done();
       });
   });
 
+  it('should have no bookmark when tx is rolled back', function (done) {
+    if (neo4jVersionOlderThan31(done)) {
+      return;
+    }
+
+    expect(session.lastBookmark()).not.toBeDefined();
+    const tx1 = session.beginTransaction();
+
+    tx1.run('CREATE ()').then(() => {
+      tx1.commit().then(() => {
+        expectValidLastBookmark(session);
+
+        const tx2 = session.beginTransaction();
+        tx2.run('CREATE ()').then(() => {
+          tx2.rollback().then(() => {
+            expect(session.lastBookmark()).not.toBeDefined();
+
+            const tx3 = session.beginTransaction();
+            tx3.run('CREATE ()').then(() => {
+              tx3.commit().then(() => {
+                expectValidLastBookmark(session);
+                done();
+              });
+            });
+          });
+        });
+      });
+    });
+  });
+
+  it('should have no bookmark when tx fails', function (done) {
+    if (neo4jVersionOlderThan31(done)) {
+      return;
+    }
+
+    expect(session.lastBookmark()).not.toBeDefined();
+    const tx1 = session.beginTransaction();
+
+    tx1.run('CREATE ()').then(() => {
+      tx1.commit().then(() => {
+        expectValidLastBookmark(session);
+
+        const tx2 = session.beginTransaction();
+
+        tx2.run('RETURN').catch(error => {
+          expectSyntaxError(error);
+          expect(session.lastBookmark()).not.toBeDefined();
+
+          const tx3 = session.beginTransaction();
+          tx3.run('CREATE ()').then(() => {
+            tx3.commit().then(() => {
+              expectValidLastBookmark(session);
+              done();
+            });
+          });
+        });
+      });
+    });
+  });
+
   it('should rollback when very first run fails', done => {
       const tx1 = session.beginTransaction();
       tx1.run('RETURN foo')
@@ -349,6 +407,11 @@ describe('transaction', function() {
     expect(code).toBe('Neo.ClientError.Statement.SyntaxError');
   }
 
+  function expectValidLastBookmark(session) {
+    expect(session.lastBookmark()).toBeDefined();
+    expect(session.lastBookmark()).not.toBeNull();
+  }
+
   function neo4jVersionOlderThan31(done) {
     //lazy way of checking the version number
     //if server has been set we know it is at least