Merge pull request #404 from canjs/reference-list-instances

constructor/store: add references to instances in list when adding reference to list

can/map/map_test.js

@@ -34,6 +34,8 @@ var logErrorAndStart = function(e){
 	start();
 };
 
+constructorStore.requestCleanupDelay = 1;
+
 QUnit.module("can-connect/can/map/map",{
 	setup: function(){
 

can/model/model.js

@@ -228,6 +228,7 @@ var CanModel = CanMap.extend({
 				)
 			)
 		);
+		this.connection.init();
 
 		this.store = this.connection.instanceStore;
 		// map static stuff to crud .. but we don't want this inherited by the next thing'

can/model/model_test.js

@@ -9,14 +9,15 @@ var canEvent = require("can-event");
 var CanMap = require("can-map");
 var CanList = require("can-list");
 var Observation = require("can-observation");
-
+var constructorStore = require("can-connect/constructor/store/store");
 var assign = require("can-util/js/assign/assign");
 
 var logErrorAndStart = function(e){
 	ok(false,"Error "+e);
 	start();
 };
 
+constructorStore.requestCleanupDelay = 1;
 
 (function () {
 	QUnit.module('can-connect/can/model', {

constructor/store/store.js

@@ -107,7 +107,10 @@ var requests = {
 		if(pendingRequests === 0) {
 			noRequestsTimer = setTimeout(function(){
 				requests.dispatch("end");
-			},10);
+			},module.exports.requestCleanupDelay);
+		}
+		if(pendingRequests < 0) {
+			pendingRequests = 0;
 		}
 	},
 	count: function(){
@@ -161,21 +164,39 @@ var constructorStore = connect.behavior("constructor/store",function(baseConnect
 		 * ```
 		 */
 		listStore: new WeakReferenceMap(),
-		_requestInstances: {},
-		_requestLists: {},
-		_finishedRequest: function(){
-			var id;
-			requests.decrement(this);
-			if(requests.count() === 0) {
+		 // Set up the plain objects for tracking requested lists and instances for this connection,
+		 // and add a handler to the requests counter to flush list and instance references when all
+		 // requests have completed
+		 //
+		 // This function is called automatically when connect() is called on this behavior,
+		 // and should not need to be called manually.
+		init: function() {
+			if(baseConnection.init) {
+				baseConnection.init.apply(this, arguments);
+			}
+
+			if(!this.hasOwnProperty("_requestInstances")) {
+				this._requestInstances = {};
+			}
+			if(!this.hasOwnProperty("_requestLists")) {
+				this._requestLists = {};
+			}
+
+			requests.on("end", function(){
+				var id;
 				for(id in this._requestInstances) {
 					this.instanceStore.deleteReference(id);
 				}
 				this._requestInstances = {};
 				for(id in this._requestLists) {
 					this.listStore.deleteReference(id);
+					this._requestLists[id].forEach(this.deleteInstanceReference.bind(this));
 				}
 				this._requestLists = {};
-			}
+			}.bind(this));
+		},
+		_finishedRequest: function(){
+			requests.decrement(this);
 		},
 
 		/**
@@ -416,6 +437,9 @@ var constructorStore = connect.behavior("constructor/store",function(baseConnect
 			var id = sortedSetJSON( set || this.listSet(list) );
 			if(id) {
 				this.listStore.addReference( id, list );
+				list.forEach(function(instance) {
+					this.addInstanceReference(instance);
+				}.bind(this));
 			}
 		},
 		/**
@@ -451,6 +475,7 @@ var constructorStore = connect.behavior("constructor/store",function(baseConnect
 			var id = sortedSetJSON( set || this.listSet(list) );
 			if(id) {
 				this.listStore.deleteReference( id, list );
+				list.forEach(this.deleteInstanceReference.bind(this));
 			}
 		},
 		/**
@@ -691,13 +716,53 @@ var constructorStore = connect.behavior("constructor/store",function(baseConnect
 			});
 			return promise;
 
+		},
+		/**
+		 * @function can-connect/constructor/store/store.updatedList updatedList
+		 * @parent can-connect/constructor/store/store.callbacks
+		 *
+		 * Extends the underlying [can-connect/connection.updatedList] so any instances that have been added or removed
+		 * from the list have their reference counts updated accordingly.
+		 *
+		 * @signature `connection.updatedList( list, listData, set )`
+		 * Increments an internal request counter so instances on this list during this request will be stored, and decrements
+		 * the same counter for all items previously on the list (found in `listData.data`).
+		 *
+		 * @param {can-connect.List} list a typed list of instances being updated
+		 * @param {Object} listData an object representing the previous state of the list
+		 * @param {Object} set the retrieval set used to get the list
+		 */
+		updatedList: function(list, listData, set) {
+			var oldList = list.slice(0);
+			if(!listData.data && typeof listData.length === "number") {
+				listData = { data: listData };
+			}
+			if(baseConnection.updatedList) {
+				baseConnection.updatedList.call(this, list, listData, set);
+				list.forEach(function(instance) {
+					this.addInstanceReference(instance);
+				}.bind(this));
+			} else if(listData.data) {
+				listData.data.forEach(function(instance) {
+					this.addInstanceReference(instance);
+				}.bind(this));
+			}
+			oldList.forEach(this.deleteInstanceReference.bind(this));
 		}
 	};
 
 	return behavior;
 
 });
 constructorStore.requests = requests;
+// The number of ms to wait after all known requests have finished,
+//  before starting request cleanup.
+// If a new request comes in before timeout, wait until that request
+//  has finished (+ delay) before starting cleanup.
+// This is configurable, for use cases where more waiting is desired,
+//  or for the can-connect tests which expect everything to clean up
+//  in 1ms.
+constructorStore.requestCleanupDelay = 10;
 
 module.exports = constructorStore;
 

constructor/store/store_test.js

@@ -8,7 +8,7 @@ var connect = require("can-connect/can-connect");
 var testHelpers = require("can-connect/test-helpers");
 var assign = require("can-util/js/assign/assign");
 
-
+instanceStore.requestCleanupDelay = 1;
 
 // connects the "raw" data to a a constructor function
 // creates ways to CRUD the instances
@@ -296,3 +296,72 @@ QUnit.asyncTest("instances bound before create are moved to instance store (#296
 
 
 });
+
+QUnit.asyncTest("instanceStore adds instance references for list membership.", function() {
+	var connection = connect([
+		function(){
+			return {
+				getListData: function(){
+					return Promise.resolve([{name: "test store", id: "abc"}]);
+				}
+			};
+		},
+		constructor,
+		instanceStore],
+	{});
+
+	connection.getList({}).then(function(list) {
+		var instanceRef = connection.instanceStore.get("abc");
+		QUnit.ok(instanceRef, "instance reference exists in store");
+		QUnit.equal(connection.instanceStore.referenceCount("abc"), 2, "reference count reflects that instance is being loaded");
+		connection.addListReference(list);
+		QUnit.equal(connection.instanceStore.referenceCount("abc"), 3, "reference count reflects that instance is in reffed list");
+
+		return new Promise(function(resolve) {
+			setTimeout(function() {
+				QUnit.equal(connection.instanceStore.referenceCount("abc"), 1, "finished requests reduce instance counts to 1");
+				connection.deleteListReference(list);
+				QUnit.ok(!connection.instanceStore.has("abc"), "instance removed from store after last list ref removed");
+				resolve()
+			}, 1);
+		});
+	}).then(QUnit.start.bind(QUnit, null), QUnit.start.bind(QUnit, null));
+});
+
+QUnit.asyncTest("instanceStore adds/removes instances based on list updates.", function() {
+	var connection = connect([
+		function(){
+			var calls = 0;
+			return {
+				getListData: function(){
+					if(calls) {
+						return Promise.resolve([{name: "test store", id: "def"}]);
+					} else {
+						calls++;
+						return Promise.resolve([{name: "test store", id: "abc"}]);
+					}
+				}
+			};
+		},
+		constructor,
+		instanceStore],
+	{});
+
+	connection.getList({}).then(function(list) {
+		connection.addListReference(list);
+		return new Promise(function(resolve) {
+			setTimeout(function() {
+				QUnit.ok(connection.instanceStore.get("abc"), "first item is in instance store");
+				resolve(connection.getList({}));
+			}, 1);
+		});
+	}).then(function(list) {
+		return new Promise(function(resolve) {
+			setTimeout(function() {
+				QUnit.ok(!connection.instanceStore.get("abc"), "first item is not in instance store");
+				QUnit.ok(connection.instanceStore.get("def"), "second item is in instance store");
+				resolve();
+			}, 10);
+		});
+	}).then(QUnit.start.bind(QUnit, null), QUnit.start.bind(QUnit, null));
+});

fall-through-cache/fall-through-cache_test.js

@@ -12,6 +12,8 @@ var getId = function(d){
 	return d.id;
 };
 
+constructorStore.requestCleanupDelay = 1;
+
 QUnit.module("can-connect/fall-through-cache");
 
 QUnit.test("basics", function(){

helpers/map-deep-merge-test.js

@@ -10,6 +10,7 @@ var canMap = require('can-connect/can/map/map');
 var dataUrl = require('can-connect/data/url/url');
 var constructor = require('can-connect/constructor/constructor');
 var constructorStore = require('can-connect/constructor/store/store');
+constructorStore.requestCleanupDelay = 1;
 
 var smartMerge = require('./map-deep-merge');
 var applyPatch = require('./map-deep-merge').applyPatch;

real-time/real-time_test.js

@@ -26,6 +26,7 @@ var logErrorAndStart = function(e){
 	start();
 };
 
+constructorStore.requestCleanupDelay = 1;
 
 QUnit.test("basics", function(){
 	// get two lists