From 4084d1538b1153291c8c4f10dcee0e38bdae8f52 Mon Sep 17 00:00:00 2001 From: Daniel Andrus Date: Tue, 14 May 2019 20:56:40 -0700 Subject: [PATCH 1/7] Move subscription to subscriptions subpackage --- src/{ => subscriptions}/subscription.lua | 0 tools/build.lua | 2 +- 2 files changed, 1 insertion(+), 1 deletion(-) rename src/{ => subscriptions}/subscription.lua (100%) diff --git a/src/subscription.lua b/src/subscriptions/subscription.lua similarity index 100% rename from src/subscription.lua rename to src/subscriptions/subscription.lua diff --git a/tools/build.lua b/tools/build.lua index 6cac333..a6f0fdf 100644 --- a/tools/build.lua +++ b/tools/build.lua @@ -4,7 +4,7 @@ local files = { 'src/util.lua', - 'src/subscription.lua', + 'src/subscriptions/subscription.lua', 'src/observer.lua', 'src/observable.lua', 'src/operators/all.lua', From 9cf456a57bbab14b04e1ccd3a77f0a2214ae3cf3 Mon Sep 17 00:00:00 2001 From: Daniel Andrus Date: Tue, 14 May 2019 21:26:07 -0700 Subject: [PATCH 2/7] Add CompositeSubscription --- doc/README.md | 30 +++++++++++++++++ rx.lua | 36 +++++++++++++++++++++ src/subscriptions/compositesubscription.lua | 36 +++++++++++++++++++++ tools/build.lua | 2 ++ 4 files changed, 104 insertions(+) create mode 100644 src/subscriptions/compositesubscription.lua diff --git a/doc/README.md b/doc/README.md index 1b631d3..011497a 100644 --- a/doc/README.md +++ b/doc/README.md @@ -3,6 +3,10 @@ RxLua - [Subscription](#subscription) - [create](#createaction) +c - [unsubscribe](#unsubscribe) +- [CompositeSubscription](#compositesubscription) + - [create](#createsubscriptions) + - [add](#add) - [unsubscribe](#unsubscribe) - [Observer](#observer) - [create](#createonnext-onerror-oncompleted) @@ -129,6 +133,32 @@ Creates a new Subscription. Unsubscribes the subscription, performing any necessary cleanup work. +# CompositeSubscription + +A composed set of subscriptions that enables convenient subscription management and easy unsubscribing. + +--- + +#### `.create(subscriptions)` + +Creates a new CompositeSubscription. + +| Name | Type | Default | Description | +|------|------|---------|-------------| +| `subscriptions` | Subscriptions... | | A set of subscriptions to initialize the object with. | + +--- + +#### `:add(...)` + +Adds the given Subscriptions to this object. + +--- + +#### `:unsubscribe()` + +Unsubscribes all registered subscriptions and removes them from this CompositeSubscription. + # Observer Observers are simple objects that receive values from Observables. diff --git a/rx.lua b/rx.lua index bef15ac..abac567 100644 --- a/rx.lua +++ b/rx.lua @@ -48,6 +48,41 @@ function Subscription:unsubscribe() self.unsubscribed = true end +--- @class CompositeSubscription +-- @description A composed set of subscriptions that enables convenient subscription management +-- and easy unsubscribing. +local CompositeSubscription = {} +CompositeSubscription.__index = CompositeSubscription +CompositeSubscription.__tostring = util.constant('CompositeSubscription') + +--- Creates a new CompositeSubscription. +-- @arg {Subscriptions...} subscriptions - A set of subscriptions to initialize the object with. +-- @returns {CompositeSubscription} +function CompositeSubscription.create(...) + local self = { + subscriptions = {...}, + } + + return setmetatable(self, CompositeSubscription) +end + +--- Adds the given Subscriptions to this object. +-- @returns {nil} +function CompositeSubscription:add(...) + for _,subscription in ipairs({...}) do + table.insert(self.subscriptions, subscription) + end +end + +--- Unsubscribes all registered subscriptions and removes them from this CompositeSubscription. +-- @returns {nil} +function CompositeSubscription:unsubscribe() + for _,subscription in ipairs(self.subscriptions) do + subscription:unsubscribe() + end + self.subscriptions = {} +end + --- @class Observer -- @description Observers are simple objects that receive values from Observables. local Observer = {} @@ -2301,6 +2336,7 @@ Observable['repeat'] = Observable.replicate return { util = util, Subscription = Subscription, + CompositeSubscription = CompositeSubscription, Observer = Observer, Observable = Observable, ImmediateScheduler = ImmediateScheduler, diff --git a/src/subscriptions/compositesubscription.lua b/src/subscriptions/compositesubscription.lua new file mode 100644 index 0000000..a1260c3 --- /dev/null +++ b/src/subscriptions/compositesubscription.lua @@ -0,0 +1,36 @@ +local util = require 'util' + +--- @class CompositeSubscription +-- @description A composed set of subscriptions that enables convenient subscription management +-- and easy unsubscribing. +local CompositeSubscription = {} +CompositeSubscription.__index = CompositeSubscription +CompositeSubscription.__tostring = util.constant('CompositeSubscription') + +--- Creates a new CompositeSubscription. +-- @arg {Subscriptions...} subscriptions - A set of subscriptions to initialize the object with. +-- @returns {CompositeSubscription} +function CompositeSubscription.create(...) + local self = { + subscriptions = {...}, + } + + return setmetatable(self, CompositeSubscription) +end + +--- Adds the given Subscriptions to this object. +-- @returns {nil} +function CompositeSubscription:add(...) + for _,subscription in ipairs({...}) do + table.insert(self.subscriptions, subscription) + end +end + +--- Unsubscribes all registered subscriptions and removes them from this CompositeSubscription. +-- @returns {nil} +function CompositeSubscription:unsubscribe() + for _,subscription in ipairs(self.subscriptions) do + subscription:unsubscribe() + end + self.subscriptions = {} +end \ No newline at end of file diff --git a/tools/build.lua b/tools/build.lua index a6f0fdf..836f370 100644 --- a/tools/build.lua +++ b/tools/build.lua @@ -5,6 +5,7 @@ local files = { 'src/util.lua', 'src/subscriptions/subscription.lua', + 'src/subscriptions/compositesubscription.lua', 'src/observer.lua', 'src/observable.lua', 'src/operators/all.lua', @@ -90,6 +91,7 @@ exports.homepage = 'https://github.com/bjornbytes/rxlua' local footer = [[return { util = util, Subscription = Subscription, + CompositeSubscription = CompositeSubscription, Observer = Observer, Observable = Observable, ImmediateScheduler = ImmediateScheduler, From f622592a5c2357e68ccb14a24d6c8bdc3652d2e7 Mon Sep 17 00:00:00 2001 From: Daniel Andrus Date: Tue, 14 May 2019 21:54:33 -0700 Subject: [PATCH 3/7] Add tests for compositesubscription --- tests/compositesubscription.lua | 70 +++++++++++++++++++++++++++++++++ tests/runner.lua | 1 + 2 files changed, 71 insertions(+) create mode 100644 tests/compositesubscription.lua diff --git a/tests/compositesubscription.lua b/tests/compositesubscription.lua new file mode 100644 index 0000000..fe81365 --- /dev/null +++ b/tests/compositesubscription.lua @@ -0,0 +1,70 @@ +subscriptionSpy = function() + local subscription = Rx.Subscription.create(function() end) + local unsubscribe = spy(subscription, 'unsubscribe') + return subscription, unsubscribe +end + +describe('CompositeSubscription', function() + describe('create', function() + it('returns a CompositeSubscription', function() + local compositeSubscription = Rx.CompositeSubscription.create() + expect(compositeSubscription).to.be.an(Rx.CompositeSubscription) + end) + end) + + describe('unsubscribe', function() + it('unsubscribes all subscriptions it was created with', function() + local subscription1, unsubscribe1 = subscriptionSpy() + local subscription2, unsubscribe2 = subscriptionSpy() + local compositeSubscription = Rx.CompositeSubscription.create(subscription1, subscription2) + compositeSubscription:unsubscribe() + expect(#unsubscribe1).to.equal(1) + expect(#unsubscribe2).to.equal(1) + end) + + it('removes all subscriptions it was created with', function() + local subscription1, unsubscribe1 = subscriptionSpy() + local subscription2, unsubscribe2 = subscriptionSpy() + local compositeSubscription = Rx.CompositeSubscription.create(subscription1, subscription2) + compositeSubscription:unsubscribe() + compositeSubscription:unsubscribe() + expect(#unsubscribe1).to.equal(1) + expect(#unsubscribe2).to.equal(1) + end) + + it('unsubscribes all subscriptions that were added to it', function() + local subscription1, unsubscribe1 = subscriptionSpy() + local subscription2, unsubscribe2 = subscriptionSpy() + local compositeSubscription = Rx.CompositeSubscription.create() + compositeSubscription:add(subscription1, subscription2) + compositeSubscription:unsubscribe() + expect(#unsubscribe1).to.equal(1) + expect(#unsubscribe2).to.equal(1) + end) + + it('removes all subscriptions that were added to it', function() + local subscription1, unsubscribe1 = subscriptionSpy() + local subscription2, unsubscribe2 = subscriptionSpy() + local compositeSubscription = Rx.CompositeSubscription.create() + compositeSubscription:add(subscription1, subscription2) + compositeSubscription:unsubscribe() + compositeSubscription:unsubscribe() + expect(#unsubscribe1).to.equal(1) + expect(#unsubscribe2).to.equal(1) + end) + end) + + it('can be reused after it has been unsubscribed', function() + local compositeSubscription = Rx.CompositeSubscription.create() + + local subscription1 = subscriptionSpy() + compositeSubscription:add(subscription1) + compositeSubscription:unsubscribe() + + local subscription2, unsubscribe2 = subscriptionSpy() + compositeSubscription:add(subscription2) + compositeSubscription:unsubscribe() + + expect(#unsubscribe2).to.equal(1) + end) +end) \ No newline at end of file diff --git a/tests/runner.lua b/tests/runner.lua index aac1d5c..1d4632a 100644 --- a/tests/runner.lua +++ b/tests/runner.lua @@ -68,6 +68,7 @@ else 'observer', 'observable', 'subscription', + 'compositesubscription', 'subject', 'asyncsubject', 'behaviorsubject', From a5139e7304969fb4432a40f0049a2fd0a1d9bebd Mon Sep 17 00:00:00 2001 From: Daniel Andrus Date: Tue, 14 May 2019 22:05:58 -0700 Subject: [PATCH 4/7] Update documentation --- doc/README.md | 20 ++++++++++++-------- rx.lua | 14 ++++++++------ src/subscriptions/compositesubscription.lua | 16 +++++++++------- 3 files changed, 29 insertions(+), 21 deletions(-) diff --git a/doc/README.md b/doc/README.md index 011497a..95e5e83 100644 --- a/doc/README.md +++ b/doc/README.md @@ -3,10 +3,10 @@ RxLua - [Subscription](#subscription) - [create](#createaction) -c - [unsubscribe](#unsubscribe) + - [unsubscribe](#unsubscribe) - [CompositeSubscription](#compositesubscription) - [create](#createsubscriptions) - - [add](#add) + - [add](#addsubscriptions) - [unsubscribe](#unsubscribe) - [Observer](#observer) - [create](#createonnext-onerror-oncompleted) @@ -135,29 +135,33 @@ Unsubscribes the subscription, performing any necessary cleanup work. # CompositeSubscription -A composed set of subscriptions that enables convenient subscription management and easy unsubscribing. +A Subscription that is composed of other subscriptions and can be used to unsubscribe multiple subscriptions at once. --- #### `.create(subscriptions)` -Creates a new CompositeSubscription. +Creates a new CompositeSubscription. It may be initialized empty or with a set of Subscriptions. | Name | Type | Default | Description | |------|------|---------|-------------| -| `subscriptions` | Subscriptions... | | A set of subscriptions to initialize the object with. | +| `subscriptions` | Subscription... | | A set of subscriptions to initialize the object with. | --- -#### `:add(...)` +#### `:add(subscriptions)` + +Adds one or more Subscriptions to this CompositeSubscription. -Adds the given Subscriptions to this object. +| Name | Type | Default | Description | +|------|------|---------|-------------| +| `subscriptions` | Subscription... | | The list of Subscriptions to add. | --- #### `:unsubscribe()` -Unsubscribes all registered subscriptions and removes them from this CompositeSubscription. +Unsubscribes all subscriptions that were added to this CompositeSubscription and removes them from this CompositeSubscription. # Observer diff --git a/rx.lua b/rx.lua index abac567..80e1a0d 100644 --- a/rx.lua +++ b/rx.lua @@ -49,14 +49,14 @@ function Subscription:unsubscribe() end --- @class CompositeSubscription --- @description A composed set of subscriptions that enables convenient subscription management --- and easy unsubscribing. +-- @description A Subscription that is composed of other subscriptions and can be used to +-- unsubscribe multiple subscriptions at once. local CompositeSubscription = {} CompositeSubscription.__index = CompositeSubscription CompositeSubscription.__tostring = util.constant('CompositeSubscription') ---- Creates a new CompositeSubscription. --- @arg {Subscriptions...} subscriptions - A set of subscriptions to initialize the object with. +--- Creates a new CompositeSubscription. It may be initialized empty or with a set of Subscriptions. +-- @arg {Subscription...} subscriptions - A set of subscriptions to initialize the object with. -- @returns {CompositeSubscription} function CompositeSubscription.create(...) local self = { @@ -66,7 +66,8 @@ function CompositeSubscription.create(...) return setmetatable(self, CompositeSubscription) end ---- Adds the given Subscriptions to this object. +--- Adds one or more Subscriptions to this CompositeSubscription. +-- @arg {Subscription...} subscriptions - The list of Subscriptions to add. -- @returns {nil} function CompositeSubscription:add(...) for _,subscription in ipairs({...}) do @@ -74,7 +75,8 @@ function CompositeSubscription:add(...) end end ---- Unsubscribes all registered subscriptions and removes them from this CompositeSubscription. +--- Unsubscribes all subscriptions that were added to this CompositeSubscription and removes them +-- from this CompositeSubscription. -- @returns {nil} function CompositeSubscription:unsubscribe() for _,subscription in ipairs(self.subscriptions) do diff --git a/src/subscriptions/compositesubscription.lua b/src/subscriptions/compositesubscription.lua index a1260c3..de70361 100644 --- a/src/subscriptions/compositesubscription.lua +++ b/src/subscriptions/compositesubscription.lua @@ -1,14 +1,14 @@ local util = require 'util' --- @class CompositeSubscription --- @description A composed set of subscriptions that enables convenient subscription management --- and easy unsubscribing. +-- @description A Subscription that is composed of other subscriptions and can be used to +-- unsubscribe multiple subscriptions at once. local CompositeSubscription = {} CompositeSubscription.__index = CompositeSubscription CompositeSubscription.__tostring = util.constant('CompositeSubscription') ---- Creates a new CompositeSubscription. --- @arg {Subscriptions...} subscriptions - A set of subscriptions to initialize the object with. +--- Creates a new CompositeSubscription. It may be initialized empty or with a set of Subscriptions. +-- @arg {Subscription...} subscriptions - A set of subscriptions to initialize the object with. -- @returns {CompositeSubscription} function CompositeSubscription.create(...) local self = { @@ -18,7 +18,8 @@ function CompositeSubscription.create(...) return setmetatable(self, CompositeSubscription) end ---- Adds the given Subscriptions to this object. +--- Adds one or more Subscriptions to this CompositeSubscription. +-- @arg {Subscription...} subscriptions - The list of Subscriptions to add. -- @returns {nil} function CompositeSubscription:add(...) for _,subscription in ipairs({...}) do @@ -26,11 +27,12 @@ function CompositeSubscription:add(...) end end ---- Unsubscribes all registered subscriptions and removes them from this CompositeSubscription. +--- Unsubscribes all subscriptions that were added to this CompositeSubscription and removes them +-- from this CompositeSubscription. -- @returns {nil} function CompositeSubscription:unsubscribe() for _,subscription in ipairs(self.subscriptions) do subscription:unsubscribe() end self.subscriptions = {} -end \ No newline at end of file +end From c7e73d7da50bf1aec7de1cf1d644a792ff2f53ff Mon Sep 17 00:00:00 2001 From: Dan Andrus Date: Sat, 4 Apr 2020 19:18:26 -0700 Subject: [PATCH 5/7] Make CompositeSubscription extend Subscription, add clear() method, enforce single-unsubscribe policy, and auto-unsubscribe new subscriptions when unsubscribed. --- src/subscriptions/compositesubscription.lua | 32 +++++++++++++++++---- 1 file changed, 26 insertions(+), 6 deletions(-) diff --git a/src/subscriptions/compositesubscription.lua b/src/subscriptions/compositesubscription.lua index de70361..4014908 100644 --- a/src/subscriptions/compositesubscription.lua +++ b/src/subscriptions/compositesubscription.lua @@ -1,9 +1,10 @@ +local Subscription = require 'subscription' local util = require 'util' --- @class CompositeSubscription -- @description A Subscription that is composed of other subscriptions and can be used to -- unsubscribe multiple subscriptions at once. -local CompositeSubscription = {} +local CompositeSubscription = setmetatable({}, Subscription) CompositeSubscription.__index = CompositeSubscription CompositeSubscription.__tostring = util.constant('CompositeSubscription') @@ -13,17 +14,33 @@ CompositeSubscription.__tostring = util.constant('CompositeSubscription') function CompositeSubscription.create(...) local self = { subscriptions = {...}, + unsubscribed = false, } return setmetatable(self, CompositeSubscription) end ---- Adds one or more Subscriptions to this CompositeSubscription. +--- Adds one or more Subscriptions to this CompositeSubscription. If this subscription has already +-- unsubscribed, then any added subscriptions will be immediately disposed. -- @arg {Subscription...} subscriptions - The list of Subscriptions to add. -- @returns {nil} function CompositeSubscription:add(...) for _,subscription in ipairs({...}) do - table.insert(self.subscriptions, subscription) + if not self.unsubscribed then + table.insert(self.subscriptions, subscription) + else + subscription:unsubscribe() + end + end +end + +--- Removes all subscriptions from this CompositeSubscription and calls `Subscription:unsubscribe()` +-- on each one. More subscriptions can be added to this CompositeSubscription in the future. +-- @returns {nil} +function CompositeSubscription:clear() + for _,subscription in ipairs(self.subscriptions) do + subscription:unsubscribe() + self.subscriptions = {} end end @@ -31,8 +48,11 @@ end -- from this CompositeSubscription. -- @returns {nil} function CompositeSubscription:unsubscribe() - for _,subscription in ipairs(self.subscriptions) do - subscription:unsubscribe() + if not self.unsubscribed then + self.unsubscribed = true + for _,subscription in ipairs(self.subscriptions) do + subscription:unsubscribe() + end + self.subscriptions = {} end - self.subscriptions = {} end From 1569cb9034871588ea64e9b15f11c28addc5ead1 Mon Sep 17 00:00:00 2001 From: Daniel Andrus Date: Sat, 4 Apr 2020 19:45:13 -0700 Subject: [PATCH 6/7] Add CompositeSubscription.clear() and better test coverage --- doc/README.md | 15 +- rx.lua | 33 +++- src/subscriptions/compositesubscription.lua | 26 +-- tests/compositesubscription.lua | 191 +++++++++++++++----- 4 files changed, 194 insertions(+), 71 deletions(-) diff --git a/doc/README.md b/doc/README.md index 95e5e83..0ea511f 100644 --- a/doc/README.md +++ b/doc/README.md @@ -6,8 +6,9 @@ RxLua - [unsubscribe](#unsubscribe) - [CompositeSubscription](#compositesubscription) - [create](#createsubscriptions) - - [add](#addsubscriptions) - [unsubscribe](#unsubscribe) + - [add](#addsubscriptions) + - [clear](#clear) - [Observer](#observer) - [create](#createonnext-onerror-oncompleted) - [onNext](#onnextvalues) @@ -149,9 +150,15 @@ Creates a new CompositeSubscription. It may be initialized empty or with a set o --- +#### `:unsubscribe()` + +Unsubscribes all subscriptions that were added to this CompositeSubscription and removes them from this CompositeSubscription. + +--- + #### `:add(subscriptions)` -Adds one or more Subscriptions to this CompositeSubscription. +Adds one or more Subscriptions to this CompositeSubscription. If this subscription has already unsubscribed, then any added subscriptions will be immediately disposed. | Name | Type | Default | Description | |------|------|---------|-------------| @@ -159,9 +166,9 @@ Adds one or more Subscriptions to this CompositeSubscription. --- -#### `:unsubscribe()` +#### `:clear()` -Unsubscribes all subscriptions that were added to this CompositeSubscription and removes them from this CompositeSubscription. +Removes all subscriptions from this CompositeSubscription and calls `Subscription:unsubscribe()` on each one. More subscriptions can be added to this CompositeSubscription in the future. # Observer diff --git a/rx.lua b/rx.lua index 80e1a0d..8756ec4 100644 --- a/rx.lua +++ b/rx.lua @@ -51,7 +51,7 @@ end --- @class CompositeSubscription -- @description A Subscription that is composed of other subscriptions and can be used to -- unsubscribe multiple subscriptions at once. -local CompositeSubscription = {} +local CompositeSubscription = setmetatable({}, Subscription) CompositeSubscription.__index = CompositeSubscription CompositeSubscription.__tostring = util.constant('CompositeSubscription') @@ -61,28 +61,47 @@ CompositeSubscription.__tostring = util.constant('CompositeSubscription') function CompositeSubscription.create(...) local self = { subscriptions = {...}, + unsubscribed = false, } return setmetatable(self, CompositeSubscription) end ---- Adds one or more Subscriptions to this CompositeSubscription. +--- Unsubscribes all subscriptions that were added to this CompositeSubscription and removes them +-- from this CompositeSubscription. +-- @returns {nil} +function CompositeSubscription:unsubscribe() + if not self.unsubscribed then + self.unsubscribed = true + for _,subscription in ipairs(self.subscriptions) do + subscription:unsubscribe() + end + self.subscriptions = {} + end +end + +--- Adds one or more Subscriptions to this CompositeSubscription. If this subscription has already +-- unsubscribed, then any added subscriptions will be immediately disposed. -- @arg {Subscription...} subscriptions - The list of Subscriptions to add. -- @returns {nil} function CompositeSubscription:add(...) for _,subscription in ipairs({...}) do - table.insert(self.subscriptions, subscription) + if not self.unsubscribed then + table.insert(self.subscriptions, subscription) + else + subscription:unsubscribe() + end end end ---- Unsubscribes all subscriptions that were added to this CompositeSubscription and removes them --- from this CompositeSubscription. +--- Removes all subscriptions from this CompositeSubscription and calls `Subscription:unsubscribe()` +-- on each one. More subscriptions can be added to this CompositeSubscription in the future. -- @returns {nil} -function CompositeSubscription:unsubscribe() +function CompositeSubscription:clear() for _,subscription in ipairs(self.subscriptions) do subscription:unsubscribe() + self.subscriptions = {} end - self.subscriptions = {} end --- @class Observer diff --git a/src/subscriptions/compositesubscription.lua b/src/subscriptions/compositesubscription.lua index 4014908..501fd35 100644 --- a/src/subscriptions/compositesubscription.lua +++ b/src/subscriptions/compositesubscription.lua @@ -20,6 +20,19 @@ function CompositeSubscription.create(...) return setmetatable(self, CompositeSubscription) end +--- Unsubscribes all subscriptions that were added to this CompositeSubscription and removes them +-- from this CompositeSubscription. +-- @returns {nil} +function CompositeSubscription:unsubscribe() + if not self.unsubscribed then + self.unsubscribed = true + for _,subscription in ipairs(self.subscriptions) do + subscription:unsubscribe() + end + self.subscriptions = {} + end +end + --- Adds one or more Subscriptions to this CompositeSubscription. If this subscription has already -- unsubscribed, then any added subscriptions will be immediately disposed. -- @arg {Subscription...} subscriptions - The list of Subscriptions to add. @@ -43,16 +56,3 @@ function CompositeSubscription:clear() self.subscriptions = {} end end - ---- Unsubscribes all subscriptions that were added to this CompositeSubscription and removes them --- from this CompositeSubscription. --- @returns {nil} -function CompositeSubscription:unsubscribe() - if not self.unsubscribed then - self.unsubscribed = true - for _,subscription in ipairs(self.subscriptions) do - subscription:unsubscribe() - end - self.subscriptions = {} - end -end diff --git a/tests/compositesubscription.lua b/tests/compositesubscription.lua index fe81365..9553256 100644 --- a/tests/compositesubscription.lua +++ b/tests/compositesubscription.lua @@ -1,9 +1,3 @@ -subscriptionSpy = function() - local subscription = Rx.Subscription.create(function() end) - local unsubscribe = spy(subscription, 'unsubscribe') - return subscription, unsubscribe -end - describe('CompositeSubscription', function() describe('create', function() it('returns a CompositeSubscription', function() @@ -13,58 +7,161 @@ describe('CompositeSubscription', function() end) describe('unsubscribe', function() - it('unsubscribes all subscriptions it was created with', function() - local subscription1, unsubscribe1 = subscriptionSpy() - local subscription2, unsubscribe2 = subscriptionSpy() - local compositeSubscription = Rx.CompositeSubscription.create(subscription1, subscription2) - compositeSubscription:unsubscribe() - expect(#unsubscribe1).to.equal(1) - expect(#unsubscribe2).to.equal(1) + describe('with subscriptions composed at initialization', function() + it('unsubscribes composed subscriptions', function() + local subscriptions = { + Rx.Subscription.create(function() end), + Rx.Subscription.create(function() end), + } + local spies = { + spy(subscriptions[1], 'unsubscribe'), + spy(subscriptions[2], 'unsubscribe'), + } + + local compositeSubscription = Rx.CompositeSubscription.create(unpack(subscriptions)) + compositeSubscription:unsubscribe() + + expect(#spies[1]).to.equal(1) + expect(#spies[2]).to.equal(1) + end) + + it('only invokes unsubscribe once', function() + local subscriptions = { + Rx.Subscription.create(function() end), + Rx.Subscription.create(function() end), + } + local spies = { + spy(subscriptions[1], 'unsubscribe'), + spy(subscriptions[2], 'unsubscribe'), + } + + local compositeSubscription = Rx.CompositeSubscription.create(unpack(subscriptions)) + compositeSubscription:unsubscribe() + compositeSubscription:unsubscribe() + + expect(#spies[1]).to.equal(1) + expect(#spies[2]).to.equal(1) + end) end) - it('removes all subscriptions it was created with', function() - local subscription1, unsubscribe1 = subscriptionSpy() - local subscription2, unsubscribe2 = subscriptionSpy() - local compositeSubscription = Rx.CompositeSubscription.create(subscription1, subscription2) - compositeSubscription:unsubscribe() - compositeSubscription:unsubscribe() - expect(#unsubscribe1).to.equal(1) - expect(#unsubscribe2).to.equal(1) + describe('with subscriptions composed dynamically', function() + it('unsubscribes composed subscriptions', function() + local subscriptions = { + Rx.Subscription.create(function() end), + Rx.Subscription.create(function() end), + } + local spies = { + spy(subscriptions[1], 'unsubscribe'), + spy(subscriptions[2], 'unsubscribe'), + } + + local compositeSubscription = Rx.CompositeSubscription.create() + compositeSubscription:add(unpack(subscriptions)) + compositeSubscription:unsubscribe() + + expect(#spies[1]).to.equal(1) + expect(#spies[2]).to.equal(1) + end) + + it('only invokes unsubscribe once', function() + local subscriptions = { + Rx.Subscription.create(function() end), + Rx.Subscription.create(function() end), + } + local spies = { + spy(subscriptions[1], 'unsubscribe'), + spy(subscriptions[2], 'unsubscribe'), + } + + local compositeSubscription = Rx.CompositeSubscription.create() + compositeSubscription:add(unpack(subscriptions)) + compositeSubscription:unsubscribe() + compositeSubscription:unsubscribe() + + expect(#spies[1]).to.equal(1) + expect(#spies[2]).to.equal(1) + end) end) + end) + + describe('add', function() + it('does not unsubscribe composed subscriptions', function() + local subscriptions = { + Rx.Subscription.create(function() end), + Rx.Subscription.create(function() end), + } + local spies = { + spy(subscriptions[1], 'unsubscribe'), + spy(subscriptions[2], 'unsubscribe'), + } - it('unsubscribes all subscriptions that were added to it', function() - local subscription1, unsubscribe1 = subscriptionSpy() - local subscription2, unsubscribe2 = subscriptionSpy() local compositeSubscription = Rx.CompositeSubscription.create() - compositeSubscription:add(subscription1, subscription2) - compositeSubscription:unsubscribe() - expect(#unsubscribe1).to.equal(1) - expect(#unsubscribe2).to.equal(1) + compositeSubscription:add(unpack(subscriptions)) + + expect(#spies[1]).to.equal(0) + expect(#spies[2]).to.equal(0) end) - it('removes all subscriptions that were added to it', function() - local subscription1, unsubscribe1 = subscriptionSpy() - local subscription2, unsubscribe2 = subscriptionSpy() - local compositeSubscription = Rx.CompositeSubscription.create() - compositeSubscription:add(subscription1, subscription2) - compositeSubscription:unsubscribe() - compositeSubscription:unsubscribe() - expect(#unsubscribe1).to.equal(1) - expect(#unsubscribe2).to.equal(1) + describe('if CompositeSubscription is already unsubscribed', function() + it('immediately unsubscribes subscriptions', function() + local subscriptions = { + Rx.Subscription.create(function() end), + Rx.Subscription.create(function() end), + } + local spies = { + spy(subscriptions[1], 'unsubscribe'), + spy(subscriptions[2], 'unsubscribe'), + } + + local compositeSubscription = Rx.CompositeSubscription.create() + compositeSubscription:unsubscribe() + + compositeSubscription:add(unpack(subscriptions)) + + expect(#spies[1]).to.equal(1) + expect(#spies[2]).to.equal(1) + end) end) - end) - it('can be reused after it has been unsubscribed', function() - local compositeSubscription = Rx.CompositeSubscription.create() + describe('if CompositeSubscription was cleared', function() + it('does not unsubscribe composed subscriptions', function() + local subscriptions = { + Rx.Subscription.create(function() end), + Rx.Subscription.create(function() end), + } + local spies = { + spy(subscriptions[1], 'unsubscribe'), + spy(subscriptions[2], 'unsubscribe'), + } - local subscription1 = subscriptionSpy() - compositeSubscription:add(subscription1) - compositeSubscription:unsubscribe() + local compositeSubscription = Rx.CompositeSubscription.create( + Rx.Subscription.create(function() end)) + compositeSubscription:clear() - local subscription2, unsubscribe2 = subscriptionSpy() - compositeSubscription:add(subscription2) - compositeSubscription:unsubscribe() + compositeSubscription:add(unpack(subscriptions)) - expect(#unsubscribe2).to.equal(1) + expect(#spies[1]).to.equal(0) + expect(#spies[2]).to.equal(0) + end) + end) + end) + + describe('clear', function() + it('unsubscribes composed subscriptions', function() + local subscriptions = { + Rx.Subscription.create(function() end), + Rx.Subscription.create(function() end), + } + local spies = { + spy(subscriptions[1], 'unsubscribe'), + spy(subscriptions[2], 'unsubscribe'), + } + + local compositeSubscription = Rx.CompositeSubscription.create(unpack(subscriptions)) + compositeSubscription:clear() + + expect(#spies[1]).to.equal(1) + expect(#spies[2]).to.equal(1) + end) end) end) \ No newline at end of file From c27a2f5b70484f43efb884456d946a572c174821 Mon Sep 17 00:00:00 2001 From: Daniel Andrus Date: Sat, 4 Apr 2020 19:54:29 -0700 Subject: [PATCH 7/7] Fix broken test using unpack --- rx.lua | 4 ++-- src/subscriptions/compositesubscription.lua | 4 ++-- tests/compositesubscription.lua | 17 +++++++++-------- 3 files changed, 13 insertions(+), 12 deletions(-) diff --git a/rx.lua b/rx.lua index 8756ec4..083ec40 100644 --- a/rx.lua +++ b/rx.lua @@ -60,7 +60,7 @@ CompositeSubscription.__tostring = util.constant('CompositeSubscription') -- @returns {CompositeSubscription} function CompositeSubscription.create(...) local self = { - subscriptions = {...}, + subscriptions = util.pack(...), unsubscribed = false, } @@ -85,7 +85,7 @@ end -- @arg {Subscription...} subscriptions - The list of Subscriptions to add. -- @returns {nil} function CompositeSubscription:add(...) - for _,subscription in ipairs({...}) do + for _,subscription in ipairs(util.pack(...)) do if not self.unsubscribed then table.insert(self.subscriptions, subscription) else diff --git a/src/subscriptions/compositesubscription.lua b/src/subscriptions/compositesubscription.lua index 501fd35..2744d86 100644 --- a/src/subscriptions/compositesubscription.lua +++ b/src/subscriptions/compositesubscription.lua @@ -13,7 +13,7 @@ CompositeSubscription.__tostring = util.constant('CompositeSubscription') -- @returns {CompositeSubscription} function CompositeSubscription.create(...) local self = { - subscriptions = {...}, + subscriptions = util.pack(...), unsubscribed = false, } @@ -38,7 +38,7 @@ end -- @arg {Subscription...} subscriptions - The list of Subscriptions to add. -- @returns {nil} function CompositeSubscription:add(...) - for _,subscription in ipairs({...}) do + for _,subscription in ipairs(util.pack(...)) do if not self.unsubscribed then table.insert(self.subscriptions, subscription) else diff --git a/tests/compositesubscription.lua b/tests/compositesubscription.lua index 9553256..afce236 100644 --- a/tests/compositesubscription.lua +++ b/tests/compositesubscription.lua @@ -18,7 +18,8 @@ describe('CompositeSubscription', function() spy(subscriptions[2], 'unsubscribe'), } - local compositeSubscription = Rx.CompositeSubscription.create(unpack(subscriptions)) + local compositeSubscription = Rx.CompositeSubscription.create( + subscriptions[1], subscriptions[2]) compositeSubscription:unsubscribe() expect(#spies[1]).to.equal(1) @@ -35,7 +36,7 @@ describe('CompositeSubscription', function() spy(subscriptions[2], 'unsubscribe'), } - local compositeSubscription = Rx.CompositeSubscription.create(unpack(subscriptions)) + local compositeSubscription = Rx.CompositeSubscription.create(subscriptions[1], subscriptions[2]) compositeSubscription:unsubscribe() compositeSubscription:unsubscribe() @@ -56,7 +57,7 @@ describe('CompositeSubscription', function() } local compositeSubscription = Rx.CompositeSubscription.create() - compositeSubscription:add(unpack(subscriptions)) + compositeSubscription:add(subscriptions[1], subscriptions[2]) compositeSubscription:unsubscribe() expect(#spies[1]).to.equal(1) @@ -74,7 +75,7 @@ describe('CompositeSubscription', function() } local compositeSubscription = Rx.CompositeSubscription.create() - compositeSubscription:add(unpack(subscriptions)) + compositeSubscription:add(subscriptions[1], subscriptions[2]) compositeSubscription:unsubscribe() compositeSubscription:unsubscribe() @@ -96,7 +97,7 @@ describe('CompositeSubscription', function() } local compositeSubscription = Rx.CompositeSubscription.create() - compositeSubscription:add(unpack(subscriptions)) + compositeSubscription:add(subscriptions[1], subscriptions[2]) expect(#spies[1]).to.equal(0) expect(#spies[2]).to.equal(0) @@ -116,7 +117,7 @@ describe('CompositeSubscription', function() local compositeSubscription = Rx.CompositeSubscription.create() compositeSubscription:unsubscribe() - compositeSubscription:add(unpack(subscriptions)) + compositeSubscription:add(subscriptions[1], subscriptions[2]) expect(#spies[1]).to.equal(1) expect(#spies[2]).to.equal(1) @@ -138,7 +139,7 @@ describe('CompositeSubscription', function() Rx.Subscription.create(function() end)) compositeSubscription:clear() - compositeSubscription:add(unpack(subscriptions)) + compositeSubscription:add(subscriptions[1], subscriptions[2]) expect(#spies[1]).to.equal(0) expect(#spies[2]).to.equal(0) @@ -157,7 +158,7 @@ describe('CompositeSubscription', function() spy(subscriptions[2], 'unsubscribe'), } - local compositeSubscription = Rx.CompositeSubscription.create(unpack(subscriptions)) + local compositeSubscription = Rx.CompositeSubscription.create(subscriptions[1], subscriptions[2]) compositeSubscription:clear() expect(#spies[1]).to.equal(1)