From 19d2ba3259ac3d40a9e8783d3ff4db0144d9d24b Mon Sep 17 00:00:00 2001 From: Morgan Roderick Date: Tue, 11 Feb 2014 21:51:19 +0100 Subject: [PATCH 1/4] Whitespace --- test/test-subscribe.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/test/test-subscribe.js b/test/test-subscribe.js index bf52529..f3053f3 100644 --- a/test/test-subscribe.js +++ b/test/test-subscribe.js @@ -7,7 +7,7 @@ */ (function( global ){ "use strict"; - + var PubSub = global.PubSub || require("../src/pubsub"), TestHelper = global.TestHelper || require("../test/helper"), assert = buster.assert, @@ -22,7 +22,7 @@ assert.isString( token ); }, - + "should return new token for several subscriptions with same function" : function(){ var func = function(){}, tokens = [], @@ -37,7 +37,7 @@ // make sure all tokens are different TestHelper.assertAllTokensDifferent( tokens ); }, - + "should return unique tokens for each namespaced subscription" : function(){ var func = function(){}, tokens = [], @@ -51,13 +51,13 @@ // make sure all tokens are different TestHelper.assertAllTokensDifferent( tokens ); }, - + "should return unique token for unique functions" : function(){ var tokens = [], iterations = 10, message = TestHelper.getUniqueString(), i; - + function bakeFunc( value ){ return function(){ return value; From 751b0c4e5aa1c1a4a55ab32a4fe9e59e7f37cca0 Mon Sep 17 00:00:00 2001 From: Morgan Roderick Date: Tue, 11 Feb 2014 21:53:32 +0100 Subject: [PATCH 2/4] Test for #43 Assert that all subscribers are called, even if some are unsubscribed during publishing --- test/test-publish.js | 30 +++++++++++++++++++++++++++++- 1 file changed, 29 insertions(+), 1 deletion(-) diff --git a/test/test-publish.js b/test/test-publish.js index ca9574d..9222a90 100644 --- a/test/test-publish.js +++ b/test/test-publish.js @@ -165,7 +165,35 @@ // make sure we restore PubSub to it's original state delete PubSub.immediateExceptions; + }, + + "publish should call all subscribers, even when there are unsubscriptions within" : function(done){ + var topic = TestHelper.getUniqueString(), + spy1 = this.spy(), + func1 = function func1(){ + PubSub.unsubscribe(func1); + spy1(); + }, + + spy2 = this.spy(), + func2 = function func2(){ + PubSub.unsubscribe(func2); + spy2(); + }, + + clock = this.useFakeTimers(); + + PubSub.subscribe(topic, func1); + PubSub.subscribe(topic, func2); + + PubSub.publish(topic, 'some data'); + clock.tick(1); + + assert(spy1.called, 'expected spy1 to be called'); + assert(spy2.called, 'expected spy2 to be called'); + + clock.restore(); + done(); } }); - }(this)); From 033294ab8d7866412b58ebf9e57d44b7cb20045f Mon Sep 17 00:00:00 2001 From: Morgan Roderick Date: Tue, 11 Feb 2014 21:54:16 +0100 Subject: [PATCH 3/4] Fix for #43 Refactor the internals to not use arrays, so we are free to modify the list of subscribers without causing problems. --- src/pubsub.js | 53 ++++++++++++++++++++++++++++++++------------------- 1 file changed, 33 insertions(+), 20 deletions(-) diff --git a/src/pubsub.js b/src/pubsub.js index a4696f6..acfea3d 100755 --- a/src/pubsub.js +++ b/src/pubsub.js @@ -35,6 +35,17 @@ https://github.com/mroderick/PubSubJS messages = {}, lastUid = -1; + function hasKeys(obj){ + var key; + + for (key in obj){ + if ( obj.hasOwnProperty(key) ){ + return true; + } + } + return false; + } + /** * Returns a function that throws the passed exception, for use as argument for setTimeout * @param { Object } ex An Error object @@ -60,17 +71,16 @@ https://github.com/mroderick/PubSubJS function deliverMessage( originalMessage, matchedMessage, data, immediateExceptions ){ var subscribers = messages[matchedMessage], callSubscriber = immediateExceptions ? callSubscriberWithImmediateExceptions : callSubscriberWithDelayedExceptions, - i, j; + s; if ( !messages.hasOwnProperty( matchedMessage ) ) { return; } - // do not cache the length of the subscribers array, as it might change if there are unsubscribtions - // by subscribers during delivery of a topic - // see https://github.com/mroderick/PubSubJS/issues/26 - for ( i = 0; i < subscribers.length; i++ ){ - callSubscriber( subscribers[i].func, originalMessage, data ); + for (s in subscribers){ + if ( subscribers.hasOwnProperty(s)){ + callSubscriber( subscribers[s], originalMessage, data ); + } } } @@ -93,13 +103,13 @@ https://github.com/mroderick/PubSubJS function messageHasSubscribers( message ){ var topic = String( message ), - found = messages.hasOwnProperty( topic ) && messages[topic].length, + found = Boolean(messages.hasOwnProperty( topic ) && hasKeys(messages[topic])), position = topic.lastIndexOf( '.' ); while ( !found && position !== -1 ){ topic = topic.substr( 0, position ); position = topic.lastIndexOf( '.' ); - found = messages.hasOwnProperty( topic ) && messages[topic].length; + found = Boolean(messages.hasOwnProperty( topic ) && hasKeys(messages[topic])); } return found; @@ -155,13 +165,13 @@ https://github.com/mroderick/PubSubJS // message is not registered yet if ( !messages.hasOwnProperty( message ) ){ - messages[message] = []; + messages[message] = {}; } // forcing token as String, to allow for future expansions without breaking usage // and allow for easy use as key names for the 'messages' object - var token = String(++lastUid); - messages[message].push( { token : token, func : func } ); + var token = 'uid_' + String(++lastUid); + messages[message][token] = func; // return token for unsubscribing return token; @@ -179,18 +189,21 @@ https://github.com/mroderick/PubSubJS succesfulReturnValue = isToken ? tokenOrFunction : true, result = false, - m, i; + m, message, t, token; for ( m in messages ){ if ( messages.hasOwnProperty( m ) ){ - for ( i = messages[m].length-1 ; i >= 0; i-- ){ - if ( messages[m][i][key] === tokenOrFunction ){ - messages[m].splice( i, 1 ); - result = succesfulReturnValue; - - // tokens are unique, so we can just return here - if ( isToken ){ - return result; + message = messages[m]; + + if ( isToken && message[tokenOrFunction] ){ + delete message[tokenOrFunction]; + // tokens are unique, so we can just return here + return tokenOrFunction; + } else if (!isToken) { + for ( t in message ){ + if (message.hasOwnProperty(t) && message[t] === tokenOrFunction){ + delete message[t]; + result = succesfulReturnValue; } } } From 7843f931ac7c33fa84120c3cb734ad9112688399 Mon Sep 17 00:00:00 2001 From: Morgan Roderick Date: Tue, 11 Feb 2014 22:12:13 +0100 Subject: [PATCH 4/4] Fix linting --- src/pubsub.js | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/pubsub.js b/src/pubsub.js index acfea3d..d2b3f34 100755 --- a/src/pubsub.js +++ b/src/pubsub.js @@ -185,9 +185,6 @@ https://github.com/mroderick/PubSubJS **/ PubSub.unsubscribe = function( tokenOrFunction ){ var isToken = typeof tokenOrFunction === 'string', - key = isToken ? 'token' : 'func', - succesfulReturnValue = isToken ? tokenOrFunction : true, - result = false, m, message, t, token; @@ -197,13 +194,14 @@ https://github.com/mroderick/PubSubJS if ( isToken && message[tokenOrFunction] ){ delete message[tokenOrFunction]; - // tokens are unique, so we can just return here - return tokenOrFunction; + result = tokenOrFunction; + // tokens are unique, so we can just stop here + break; } else if (!isToken) { for ( t in message ){ if (message.hasOwnProperty(t) && message[t] === tokenOrFunction){ delete message[t]; - result = succesfulReturnValue; + result = true; } } }