From a11ea507007970ae517f397616c416c214a94731 Mon Sep 17 00:00:00 2001 From: Isan-Rivkin Date: Thu, 21 Feb 2019 14:44:22 +0200 Subject: [PATCH 1/7] docs: unsubscribe with no parameters --- SPEC/PUBSUB.md | 21 +++++++++++++++++++++ src/pubsub/unsubscribe.js | 16 ++++++++++++++++ 2 files changed, 37 insertions(+) diff --git a/SPEC/PUBSUB.md b/SPEC/PUBSUB.md index ef5c14ca..87d38cf0 100644 --- a/SPEC/PUBSUB.md +++ b/SPEC/PUBSUB.md @@ -51,6 +51,8 @@ A great source of [examples][] can be found in the tests for this API. If no `callback` is passed, a [promise][] is returned. This works like `EventEmitter.removeListener`, as that only the `handler` passed to a `subscribe` call before is removed from listening. The underlying subscription will only be canceled once all listeners for a topic have been removed. +The other options is to use this method with only `topic` as input, without the need to keep a reference to the `handler` function. +This works like `EventEmitter.remoteAllListeners`, removes all listeners from the given topic. **Example:** @@ -78,6 +80,25 @@ ipfs.pubsub.subscribe(topic, receiveMsg, (err) => { }) ``` +Or removing all listeners: +```JavaScript +const topic = 'fruit-of-the-day' +const receiveMsg = (msg) => console.log(msg.toString()) + +ipfs.pubsub.subscribe(topic, receiveMsg, (err) => { + if (err) { + return console.error(`failed to subscribe to ${topic}`, err) + } + + console.log(`subscribed to ${topic}`) + + setTimeout(() => { + // Will unsubscribe ALL handlers for the given topic + ipfs.pubsub.unsubscribe(topic); + }, 1000) +}) +``` + A great source of [examples][] can be found in the tests for this API. #### `pubsub.publish` diff --git a/src/pubsub/unsubscribe.js b/src/pubsub/unsubscribe.js index 98e6ee11..92fa8842 100644 --- a/src/pubsub/unsubscribe.js +++ b/src/pubsub/unsubscribe.js @@ -57,5 +57,21 @@ module.exports = (createCommon, options) => { ) }) }) + it('should subscribe and unsubscribe with no handler', (done) => { + const someTopic = getTopic() + const handler = (msg) => {} + ipfs.pubsub.subscribe(someTopic, handler, (err) => { + expect(err).to.not.exist() + ipfs.pubsub.unsubscribe(someTopic) + setTimeout(() => { + // Assert unsubscribe worked + ipfs.pubsub.ls((err, topics) => { + expect(err).to.not.exist() + expect(topics).to.eql([]) + done() + }) + }, 500) + }) + }) }) } From 8b8dfeafc37743ff6ab98a7ea317d66dd2ea531e Mon Sep 17 00:00:00 2001 From: Isan-Rivkin Date: Thu, 21 Feb 2019 15:54:35 +0200 Subject: [PATCH 2/7] test: add test with many handlers for unsubscribe --- src/pubsub/unsubscribe.js | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/pubsub/unsubscribe.js b/src/pubsub/unsubscribe.js index 92fa8842..7dd5c69b 100644 --- a/src/pubsub/unsubscribe.js +++ b/src/pubsub/unsubscribe.js @@ -57,10 +57,15 @@ module.exports = (createCommon, options) => { ) }) }) - it('should subscribe and unsubscribe with no handler', (done) => { + + it('should subscribe 10 handlers and unsunscribe once with no reference to the handlers', (done) => { + const count = 10 const someTopic = getTopic() - const handler = (msg) => {} - ipfs.pubsub.subscribe(someTopic, handler, (err) => { + + timesSeries(count, (_, cb) => { + const handler = (msg) => {} + ipfs.pubsub.subscribe(someTopic, handler, (err) => cb(err)) + }, (err) => { expect(err).to.not.exist() ipfs.pubsub.unsubscribe(someTopic) setTimeout(() => { From f907f8dcd025894e982e3f253d6105c6f7b52298 Mon Sep 17 00:00:00 2001 From: Isan-Rivkin Date: Thu, 21 Feb 2019 16:00:26 +0200 Subject: [PATCH 3/7] docs: edit unsubscribe explanation --- SPEC/PUBSUB.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/SPEC/PUBSUB.md b/SPEC/PUBSUB.md index 87d38cf0..1575323c 100644 --- a/SPEC/PUBSUB.md +++ b/SPEC/PUBSUB.md @@ -50,9 +50,9 @@ A great source of [examples][] can be found in the tests for this API. If no `callback` is passed, a [promise][] is returned. -This works like `EventEmitter.removeListener`, as that only the `handler` passed to a `subscribe` call before is removed from listening. The underlying subscription will only be canceled once all listeners for a topic have been removed. -The other options is to use this method with only `topic` as input, without the need to keep a reference to the `handler` function. -This works like `EventEmitter.remoteAllListeners`, removes all listeners from the given topic. +If the `topic` and `handler` are provided, the `handler` will no longer receive updates for the `topic`. This behaves like `EventEmitter.removeListener`. If the `handler` is not equivalent to the `handler` provided on `subscribe`, no action will be taken. + +If **only** the `topic` param is provided, unsubscribe will remove **all** handlers for the `topic`. This behaves like `EventEmitter.remoteAllListeners`. Use this if you would like to no longer receive any updates for the `topic`. **Example:** From b06b5ee80820ddca815b0154318da80223aef6c0 Mon Sep 17 00:00:00 2001 From: Isan-Rivkin Date: Mon, 25 Feb 2019 14:45:11 +0200 Subject: [PATCH 4/7] test: remove setTimeout --- src/pubsub/unsubscribe.js | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/src/pubsub/unsubscribe.js b/src/pubsub/unsubscribe.js index 7dd5c69b..80b24f93 100644 --- a/src/pubsub/unsubscribe.js +++ b/src/pubsub/unsubscribe.js @@ -68,14 +68,12 @@ module.exports = (createCommon, options) => { }, (err) => { expect(err).to.not.exist() ipfs.pubsub.unsubscribe(someTopic) - setTimeout(() => { - // Assert unsubscribe worked - ipfs.pubsub.ls((err, topics) => { - expect(err).to.not.exist() - expect(topics).to.eql([]) - done() - }) - }, 500) + // Assert unsubscribe worked + ipfs.pubsub.ls((err, topics) => { + expect(err).to.not.exist() + expect(topics).to.eql([]) + done() + }) }) }) }) From 3b36489432fbcd2eb8e241235107c24c67356eff Mon Sep 17 00:00:00 2001 From: Isan-Rivkin Date: Mon, 25 Feb 2019 15:00:09 +0200 Subject: [PATCH 5/7] docs: use async/await in unsubscribe --- SPEC/PUBSUB.md | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) diff --git a/SPEC/PUBSUB.md b/SPEC/PUBSUB.md index 1575323c..78115486 100644 --- a/SPEC/PUBSUB.md +++ b/SPEC/PUBSUB.md @@ -85,17 +85,9 @@ Or removing all listeners: const topic = 'fruit-of-the-day' const receiveMsg = (msg) => console.log(msg.toString()) -ipfs.pubsub.subscribe(topic, receiveMsg, (err) => { - if (err) { - return console.error(`failed to subscribe to ${topic}`, err) - } - - console.log(`subscribed to ${topic}`) - - setTimeout(() => { - // Will unsubscribe ALL handlers for the given topic - ipfs.pubsub.unsubscribe(topic); - }, 1000) +ipfs.pubsub.subscribe(topic, receiveMsg).then(() => { + // Will unsubscribe ALL handlers for the given topic + ipfs.pubsub.unsubscribe(topic); }) ``` From 125579fdfbb3f4ed7950bcade99d03f5dd72b775 Mon Sep 17 00:00:00 2001 From: Isan-Rivkin Date: Mon, 25 Feb 2019 15:09:00 +0200 Subject: [PATCH 6/7] docs: added extra note on promise --- SPEC/PUBSUB.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/SPEC/PUBSUB.md b/SPEC/PUBSUB.md index 78115486..7eb3b1c1 100644 --- a/SPEC/PUBSUB.md +++ b/SPEC/PUBSUB.md @@ -48,7 +48,7 @@ A great source of [examples][] can be found in the tests for this API. - `handler: (msg) => {}` - The handler to remove. - `callback: (Error) => {}` (Optional) Called once the unsubscribe is done. -If no `callback` is passed, a [promise][] is returned. +If a **handler is passed** and no `callback` is passed, a [promise][] is returned. If the `topic` and `handler` are provided, the `handler` will no longer receive updates for the `topic`. This behaves like `EventEmitter.removeListener`. If the `handler` is not equivalent to the `handler` provided on `subscribe`, no action will be taken. From 38939a378426177483a7f3fed00cbcc2124abfb1 Mon Sep 17 00:00:00 2001 From: Isan-Rivkin Date: Mon, 25 Feb 2019 18:26:31 +0200 Subject: [PATCH 7/7] test: convert unsubscribe use to async api --- SPEC/PUBSUB.md | 20 +++++++++++++------- src/pubsub/unsubscribe.js | 22 +++++++--------------- 2 files changed, 20 insertions(+), 22 deletions(-) diff --git a/SPEC/PUBSUB.md b/SPEC/PUBSUB.md index 7eb3b1c1..b8977ff7 100644 --- a/SPEC/PUBSUB.md +++ b/SPEC/PUBSUB.md @@ -48,11 +48,17 @@ A great source of [examples][] can be found in the tests for this API. - `handler: (msg) => {}` - The handler to remove. - `callback: (Error) => {}` (Optional) Called once the unsubscribe is done. -If a **handler is passed** and no `callback` is passed, a [promise][] is returned. +If no `callback` is passed, a [promise][] is returned. + +If the `topic` and `handler` are provided, the `handler` will no longer receive updates for the `topic`. This behaves like [EventEmitter.removeListener](https://nodejs.org/dist/latest/docs/api/events.html#events_emitter_removelistener_eventname_listener). If the `handler` is not equivalent to the `handler` provided on `subscribe`, no action will be taken. -If the `topic` and `handler` are provided, the `handler` will no longer receive updates for the `topic`. This behaves like `EventEmitter.removeListener`. If the `handler` is not equivalent to the `handler` provided on `subscribe`, no action will be taken. +If **only** the `topic` param is provided, unsubscribe will remove **all** handlers for the `topic`. This behaves like [EventEmitter.removeAllListeners](https://nodejs.org/dist/latest/docs/api/events.html#events_emitter_removealllisteners_eventname). Use this if you would like to no longer receive any updates for the `topic`. -If **only** the `topic` param is provided, unsubscribe will remove **all** handlers for the `topic`. This behaves like `EventEmitter.remoteAllListeners`. Use this if you would like to no longer receive any updates for the `topic`. +**WARNING:** Unsubscribe is an async operation, but removing **all** handlers for a topic can only be done using the Promises API (due to the difficulty in distinguishing between a "handler" and a "callback" - they are both functions). If you _need_ to know when unsubscribe has completed you must use `await` or `.then` on the return value from + +```JavaScript +ipfs.pubsub.unsubscribe('topic') +``` **Example:** @@ -85,10 +91,10 @@ Or removing all listeners: const topic = 'fruit-of-the-day' const receiveMsg = (msg) => console.log(msg.toString()) -ipfs.pubsub.subscribe(topic, receiveMsg).then(() => { - // Will unsubscribe ALL handlers for the given topic - ipfs.pubsub.unsubscribe(topic); -}) +await ipfs.pubsub.subscribe(topic, receiveMsg); + +// Will unsubscribe ALL handlers for the given topic +await ipfs.pubsub.unsubscribe(topic); ``` A great source of [examples][] can be found in the tests for this API. diff --git a/src/pubsub/unsubscribe.js b/src/pubsub/unsubscribe.js index 80b24f93..a5325cef 100644 --- a/src/pubsub/unsubscribe.js +++ b/src/pubsub/unsubscribe.js @@ -58,23 +58,15 @@ module.exports = (createCommon, options) => { }) }) - it('should subscribe 10 handlers and unsunscribe once with no reference to the handlers', (done) => { + it('should subscribe 10 handlers and unsunscribe once with no reference to the handlers', async () => { const count = 10 const someTopic = getTopic() - - timesSeries(count, (_, cb) => { - const handler = (msg) => {} - ipfs.pubsub.subscribe(someTopic, handler, (err) => cb(err)) - }, (err) => { - expect(err).to.not.exist() - ipfs.pubsub.unsubscribe(someTopic) - // Assert unsubscribe worked - ipfs.pubsub.ls((err, topics) => { - expect(err).to.not.exist() - expect(topics).to.eql([]) - done() - }) - }) + for (let i = 0; i < count; i++) { + await ipfs.pubsub.subscribe(someTopic, (msg) => {}) + } + await ipfs.pubsub.unsubscribe(someTopic) + const topics = await ipfs.pubsub.ls() + expect(topics).to.eql([]) }) }) }