From 8705c4fe35dc8d7eead0ac4778fda528c697a0ec Mon Sep 17 00:00:00 2001 From: Greg Kubisa Date: Wed, 18 Apr 2018 16:27:13 +0100 Subject: [PATCH 1/6] Fix Doc.prototype.destroy The problem was that unsubscribe re-added the doc to the connection. Now the doc is removed from the connection after unsubscribe. Additionally, we're no longer waiting for the unsubscribe response before executing the callback. It is consistent with Query, unsubscribe can't fail anyway and the subscribed state is updated synchronously on the client side. --- lib/client/doc.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/lib/client/doc.js b/lib/client/doc.js index 05e17976d..bf128eb51 100644 --- a/lib/client/doc.js +++ b/lib/client/doc.js @@ -104,10 +104,8 @@ emitter.mixin(Doc); Doc.prototype.destroy = function(callback) { var doc = this; doc.whenNothingPending(function() { + if (doc.wantSubscribe) doc.unsubscribe(); doc.connection._destroyDoc(doc); - if (doc.wantSubscribe) { - return doc.unsubscribe(callback); - } if (callback) callback(); }); }; From af84be65f208856e0933d627464b79ab053a6dc0 Mon Sep 17 00:00:00 2001 From: Greg Kubisa Date: Wed, 18 Apr 2018 16:36:58 +0100 Subject: [PATCH 2/6] Update tested nodejs versions in .travis.yml See See https://github.com/nodejs/Release --- .travis.yml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/.travis.yml b/.travis.yml index 1b9165051..66e0be28c 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,9 +1,9 @@ language: node_js node_js: - - 6 - - 5 - - 4 - - 0.10 + - "9" + - "8" + - "6" + - "4" script: "npm run jshint && npm run test-cover" # Send coverage data to Coveralls after_script: "cat ./coverage/lcov.info | ./node_modules/coveralls/bin/coveralls.js" From 09edf920eedf6d5116efb2271d693a9a59da1517 Mon Sep 17 00:00:00 2001 From: Greg Kubisa Date: Thu, 19 Apr 2018 12:37:24 +0100 Subject: [PATCH 3/6] Add a test --- lib/client/doc.js | 4 +++- test/client/subscribe.js | 7 +++++-- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/lib/client/doc.js b/lib/client/doc.js index bf128eb51..33621cb9c 100644 --- a/lib/client/doc.js +++ b/lib/client/doc.js @@ -104,7 +104,9 @@ emitter.mixin(Doc); Doc.prototype.destroy = function(callback) { var doc = this; doc.whenNothingPending(function() { - if (doc.wantSubscribe) doc.unsubscribe(); + if (doc.wantSubscribe) { + doc.unsubscribe(); + } doc.connection._destroyDoc(doc); if (callback) callback(); }); diff --git a/test/client/subscribe.js b/test/client/subscribe.js index 567031d0a..db2bea1b2 100644 --- a/test/client/subscribe.js +++ b/test/client/subscribe.js @@ -405,8 +405,10 @@ describe('client subscribe', function() { }); it('doc destroy stops op updates', function(done) { - var doc = this.backend.connect().get('dogs', 'fido'); - var doc2 = this.backend.connect().get('dogs', 'fido'); + var connection1 = this.backend.connect(); + var connection2 = this.backend.connect(); + var doc = connection1.get('dogs', 'fido'); + var doc2 = connection2.get('dogs', 'fido'); doc.create({age: 3}, function(err) { if (err) return done(err); doc2.subscribe(function(err) { @@ -416,6 +418,7 @@ describe('client subscribe', function() { }); doc2.destroy(function(err) { if (err) return done(err); + expect(connection2.getExisting('dogs', 'fido')).equal(undefined); doc.submitOp({p: ['age'], na: 1}, done); }); }); From 15cdd1d3d451a2b5dcbdadd9fb551c912975738d Mon Sep 17 00:00:00 2001 From: Greg Kubisa Date: Thu, 12 Jul 2018 12:24:25 +0200 Subject: [PATCH 4/6] Update tested nodejs versions --- .travis.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 5c87e1e6d..21efafe46 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,7 +1,6 @@ language: node_js node_js: - "10" - - "9" - "8" - "6" script: "npm run jshint && npm run test-cover" From cfca37ff92ecfe0cfba0de985e3651d4f3a51ec9 Mon Sep 17 00:00:00 2001 From: Greg Kubisa Date: Thu, 12 Jul 2018 12:38:21 +0200 Subject: [PATCH 5/6] Make destroy wait for unsubscribe --- lib/client/doc.js | 11 ++++++++--- test/client/subscribe.js | 13 ++++++++++++- 2 files changed, 20 insertions(+), 4 deletions(-) diff --git a/lib/client/doc.js b/lib/client/doc.js index 33621cb9c..796df6bd7 100644 --- a/lib/client/doc.js +++ b/lib/client/doc.js @@ -105,10 +105,15 @@ Doc.prototype.destroy = function(callback) { var doc = this; doc.whenNothingPending(function() { if (doc.wantSubscribe) { - doc.unsubscribe(); + doc.unsubscribe(function(err) { + if (!err) doc.connection._destroyDoc(doc); + if (callback) return callback(err); + if (err) this.emit('error', err); + }); + } else { + doc.connection._destroyDoc(doc); + if (callback) callback(); } - doc.connection._destroyDoc(doc); - if (callback) callback(); }); }; diff --git a/test/client/subscribe.js b/test/client/subscribe.js index db2bea1b2..b24a94749 100644 --- a/test/client/subscribe.js +++ b/test/client/subscribe.js @@ -414,7 +414,7 @@ describe('client subscribe', function() { doc2.subscribe(function(err) { if (err) return done(err); doc2.on('op', function(op, context) { - done(); + done(new Error('Should not get op event')); }); doc2.destroy(function(err) { if (err) return done(err); @@ -425,6 +425,17 @@ describe('client subscribe', function() { }); }); + it('doc destroy removes doc from connection when doc is not subscribed', function(done) { + var connection = this.backend.connect(); + var doc = connection.get('dogs', 'fido'); + expect(connection.getExisting('dogs', 'fido')).equal(doc); + doc.destroy(function(err) { + if (err) return done(err); + expect(connection.getExisting('dogs', 'fido')).equal(undefined); + done(); + }); + }); + it('bulk unsubscribe stops op updates', function(done) { var connection = this.backend.connect(); var connection2 = this.backend.connect(); From 5e009d17d0b6b665eb25abc0d538d1bc67e5dc6b Mon Sep 17 00:00:00 2001 From: Greg Kubisa Date: Thu, 12 Jul 2018 12:52:59 +0200 Subject: [PATCH 6/6] Simplify the code --- lib/client/doc.js | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/lib/client/doc.js b/lib/client/doc.js index 796df6bd7..d75e83085 100644 --- a/lib/client/doc.js +++ b/lib/client/doc.js @@ -106,9 +106,13 @@ Doc.prototype.destroy = function(callback) { doc.whenNothingPending(function() { if (doc.wantSubscribe) { doc.unsubscribe(function(err) { - if (!err) doc.connection._destroyDoc(doc); - if (callback) return callback(err); - if (err) this.emit('error', err); + if (err) { + if (callback) callback(err); + else this.emit('error', err); + return; + } + doc.connection._destroyDoc(doc); + if (callback) callback(); }); } else { doc.connection._destroyDoc(doc);