Fixed bug in moemoized function. Since I actually need

per-instance caching, I used a regular caching object instead.
marques-work · Jul 5, 2014 · 626cd30 · 626cd30
1 parent cf9051e
commit 626cd30
Show file tree

Hide file tree

Showing 23 changed files with 4,560 additions and 399 deletions.
diff --git a/dist/asteroid.js b/dist/asteroid.js
@@ -143,6 +143,7 @@ var Asteroid = function (host, ssl, socketInterceptFunction) {
 	// Reference containers
 	this.collections = {};
 	this.subscriptions = {};
+	this._subscriptionsCache = {};
 	// Init the instance
 	this._init();
 };
@@ -987,27 +988,21 @@ Subscription.prototype._onError = function (err) {
 // Subscribe method //
 //////////////////////
 
-Asteroid.prototype.subscribe = (function () {
-	// Memoize calls to the method, since subscribing to
-	// a resource twice with the same arguments yields the
-	// same results.
-	var calls = {};
-	// Actual subscribe function
-	return function (name /* , param1, param2, ... */) {
+Asteroid.prototype.subscribe = function (name /* , param1, param2, ... */) {
 		// Assert arguments type
 		must.beString(name);
-		// Hash the arguments (using JSON.stringify as hash function)
+		// Hash the arguments to get a key for _subscriptionsCache
 		var hash = JSON.stringify(arguments);
 		// Only subscribe if there is no cached subscription
-		if (!calls[hash]) {
+		if (!this._subscriptionsCache[hash]) {
 			// Collect arguments into array
 			var params = Array.prototype.slice.call(arguments, 1);
-			calls[hash] = new Subscription(name, params, this);
-			this.subscriptions[sub.id] = calls[hash];	
+			var sub = new Subscription(name, params, this);
+			this._subscriptionsCache[hash] = sub;
+			this.subscriptions[sub.id] = sub;
 		}
-		return calls[hash];
-	};
-})();
+		return this._subscriptionsCache[hash];
+};
 
 Asteroid.prototype._reEstablishSubscriptions = function () {
 	var subs = this.subscriptions;

diff --git a/dist/asteroid.min.js b/dist/asteroid.min.js
diff --git a/dist/node.asteroid.js b/dist/node.asteroid.js
@@ -132,6 +132,7 @@ var Asteroid = function (host, ssl, socketInterceptFunction) {
 	// Reference containers
 	this.collections = {};
 	this.subscriptions = {};
+	this._subscriptionsCache = {};
 	// Init the instance
 	this._init();
 };
@@ -826,27 +827,21 @@ Subscription.prototype._onError = function (err) {
 // Subscribe method //
 //////////////////////
 
-Asteroid.prototype.subscribe = (function () {
-	// Memoize calls to the method, since subscribing to
-	// a resource twice with the same arguments yields the
-	// same results.
-	var calls = {};
-	// Actual subscribe function
-	return function (name /* , param1, param2, ... */) {
+Asteroid.prototype.subscribe = function (name /* , param1, param2, ... */) {
 		// Assert arguments type
 		must.beString(name);
-		// Hash the arguments (using JSON.stringify as hash function)
+		// Hash the arguments to get a key for _subscriptionsCache
 		var hash = JSON.stringify(arguments);
 		// Only subscribe if there is no cached subscription
-		if (!calls[hash]) {
+		if (!this._subscriptionsCache[hash]) {
 			// Collect arguments into array
 			var params = Array.prototype.slice.call(arguments, 1);
-			calls[hash] = new Subscription(name, params, this);
-			this.subscriptions[sub.id] = calls[hash];	
+			var sub = new Subscription(name, params, this);
+			this._subscriptionsCache[hash] = sub;
+			this.subscriptions[sub.id] = sub;
 		}
-		return calls[hash];
-	};
-})();
+		return this._subscriptionsCache[hash];
+};
 
 Asteroid.prototype._reEstablishSubscriptions = function () {
 	var subs = this.subscriptions;

diff --git a/gulpfile.js b/gulpfile.js
@@ -69,7 +69,7 @@ gulp.task("default", function () {
 	console.log("");
 	console.log("Available tasks:");
 	console.log("  buildBrowser     build the sources into dist/asteroid.js and dist/asteroid.min.js");
-	console.log("  buildNode     build the sources into dist/node.asteroid.js and dist/node.asteroid.min.js");
+	console.log("  buildNode        build the sources into dist/node.asteroid.js and dist/node.asteroid.min.js");
 	console.log("  demo	            sets up a demo server");
 	console.log("  dev              init dev environment with automatic test running");
 	console.log("  test-node        run tests with mocha");

diff --git a/package.json b/package.json
@@ -47,7 +47,7 @@
     "tiny-lr": "0.0.5"
   },
   "dependencies": {
-    "ddp.js": "^0.4.5",
+    "ddp.js": "^0.5.0",
     "faye-websocket": "^0.7.2",
     "q": "^1.0.1"
   }

diff --git a/src/asteroid.js b/src/asteroid.js
@@ -47,6 +47,7 @@ var Asteroid = function (host, ssl, socketInterceptFunction) {
 	// Reference containers
 	this.collections = {};
 	this.subscriptions = {};
+	this._subscriptionsCache = {};
 	// Init the instance
 	this._init();
 };

diff --git a/src/subscription.js b/src/subscription.js
@@ -42,27 +42,21 @@ Subscription.prototype._onError = function (err) {
 // Subscribe method //
 //////////////////////
 
-Asteroid.prototype.subscribe = (function () {
-	// Memoize calls to the method, since subscribing to
-	// a resource twice with the same arguments yields the
-	// same results.
-	var calls = {};
-	// Actual subscribe function
-	return function (name /* , param1, param2, ... */) {
+Asteroid.prototype.subscribe = function (name /* , param1, param2, ... */) {
 		// Assert arguments type
 		must.beString(name);
-		// Hash the arguments (using JSON.stringify as hash function)
+		// Hash the arguments to get a key for _subscriptionsCache
 		var hash = JSON.stringify(arguments);
 		// Only subscribe if there is no cached subscription
-		if (!calls[hash]) {
+		if (!this._subscriptionsCache[hash]) {
 			// Collect arguments into array
 			var params = Array.prototype.slice.call(arguments, 1);
-			calls[hash] = new Subscription(name, params, this);
-			this.subscriptions[sub.id] = calls[hash];	
+			var sub = new Subscription(name, params, this);
+			this._subscriptionsCache[hash] = sub;
+			this.subscriptions[sub.id] = sub;
 		}
-		return calls[hash];
-	};
-})();
+		return this._subscriptionsCache[hash];
+};
 
 Asteroid.prototype._reEstablishSubscriptions = function () {
 	var subs = this.subscriptions;

diff --git a/test/asteroid.unit.js b/test/asteroid.unit.js
@@ -47,22 +47,22 @@ describe("The Asteroid constructor", function () {
 		ceres = new Asteroid("example.com", true, true);
 		ceres._host.should.equal("https://example.com");
 		ceres._ddpOptions.endpoint.should.equal("wss://example.com/websocket");
-		ceres._ddpOptions.debug.should.equal(true);
+		ceres._ddpOptions.socketInterceptFunction.should.equal(true);
 
 		ceres = new Asteroid("example.com", false, true);
 		ceres._host.should.equal("http://example.com");
 		ceres._ddpOptions.endpoint.should.equal("ws://example.com/websocket");
-		ceres._ddpOptions.debug.should.equal(true);
+		ceres._ddpOptions.socketInterceptFunction.should.equal(true);
 
 		ceres = new Asteroid("example.com", true);
 		ceres._host.should.equal("https://example.com");
 		ceres._ddpOptions.endpoint.should.equal("wss://example.com/websocket");
-		_.isUndefined(ceres._ddpOptions.debug).should.equal(true);
+		_.isUndefined(ceres._ddpOptions.socketInterceptFunction).should.equal(true);
 
 		ceres = new Asteroid("example.com");
 		ceres._host.should.equal("http://example.com");
 		ceres._ddpOptions.endpoint.should.equal("ws://example.com/websocket");
-		_.isUndefined(ceres._ddpOptions.debug).should.equal(true);
+		_.isUndefined(ceres._ddpOptions.socketInterceptFunction).should.equal(true);
 
 		Asteroid.prototype._init.restore();
 
@@ -1170,10 +1170,32 @@ describe("The Asteroid.subscribe method", function () {
 		var p2 = {};
 		// ...
 		var ceres = new Asteroid("example.com");	
-		var promise = ceres.subscribe("sub", p0, p1, p2);
+		var sub = ceres.subscribe("sub", p0, p1, p2);
 		ceres.ddp.params[0].should.equal(p0);
 		ceres.ddp.params[1].should.equal(p1);
 		ceres.ddp.params[2].should.equal(p2);
 	});
 
+	it("should cache identical calls", function () {
+		var p0 = {};
+		var p1 = {};
+		var p2 = {};
+		// ...
+		var ceres = new Asteroid("example.com");	
+		var sub0 = ceres.subscribe("sub", p0, p1, p2);
+		var sub1 = ceres.subscribe("sub", p0, p1, p2);
+		sub0.should.equal(sub1);
+	});
+
+	it("should not cache non identical calls", function () {
+		var p = {};
+		var p0 = {a: 0};
+		var p1 = {a: 1};
+		// ...
+		var ceres = new Asteroid("example.com");	
+		var sub0 = ceres.subscribe("sub", p, p0);
+		var sub1 = ceres.subscribe("sub", p, p1);
+		sub0.should.not.equal(sub1);
+	});
+
 });