Skip to content

Commit

Permalink
Further changes to unregister child records when a record is unloaded.
Browse files Browse the repository at this point in the history
  • Loading branch information
Greg Fairbanks authored and publickeating committed Aug 30, 2014
1 parent 1df7ec8 commit fcd514a
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 53 deletions.
33 changes: 19 additions & 14 deletions frameworks/datastore/system/store.js
Original file line number Diff line number Diff line change
Expand Up @@ -735,6 +735,7 @@ SC.Store = SC.Object.extend( /** @scope SC.Store.prototype */ {
this.dataHashes = {} ;
this.revisions = {} ;
this.statuses = {} ;
this.records = {};
this.childRecords = {};
this.parentRecords = {};

Expand Down Expand Up @@ -1357,6 +1358,8 @@ SC.Store = SC.Object.extend( /** @scope SC.Store.prototype */ {
that.unloadRecord(null, null, storeKey, newStatus);
});

this.unregisterChildFromParent(storeKey, YES);

return this ;
},

Expand Down Expand Up @@ -1543,31 +1546,33 @@ SC.Store = SC.Object.extend( /** @scope SC.Store.prototype */ {
/**
Unregister the Child Record from its Parent. This will cause the Child
Record to be removed from the store.
@param {Number} childStoreKey storeKey to unregister
*/
unregisterChildFromParent: function(childStoreKey) {
var childRecords, oldPk, storeKeys,
recordType = this.recordTypeFor(childStoreKey),
id = this.idFor(childStoreKey),
that = this;

// Check the child to see if it has a parent
// Check the child to see if it has a parent
childRecords = this.childRecords;

// Remove the parent's connection to the child. This doesn't remove the
// parent store key from the cache of parent store keys if the parent
// no longer has any other registered children, because the amount of effort
// to determine that would not be worth the miniscule memory savings.
// Remove the parent's connection to the child. This doesn't remove the
// parent store key from the cache of parent store keys if the parent
// no longer has any other registered children, because the amount of effort
// to determine that would not be worth the miniscule memory savings.
oldPk = childRecords[childStoreKey];
if (oldPk) {
delete this.parentRecords[oldPk][childStoreKey];
}
if (oldPk) {
delete this.parentRecords[oldPk][childStoreKey];
}

// Remove the child.
// 1. from the cache of data hashes
// 2. from the cache of record objects
// 3. from the cache of child record store keys
this.removeDataHash(childStoreKey);
delete this.records[childStoreKey];
// Remove the child.
// 1. from the cache of data hashes
// 2. from the cache of record objects
// 3. from the cache of child record store keys
this.removeDataHash(childStoreKey);
delete this.records[childStoreKey];
delete childRecords[childStoreKey];

// 4. from the cache of ids
Expand Down
40 changes: 2 additions & 38 deletions frameworks/datastore/tests/models/nested_records/nested_record.js
Original file line number Diff line number Diff line change
Expand Up @@ -425,15 +425,6 @@ test("Writing over a child record should remove caches in the store.", function(
for (key in store.dataHashes) { if (store.dataHashes[key] !== null) cacheLength += 1; }
equals(cacheLength, 4, 'there should be four non-null datahashes in the store');

cacheLength = 0;
for (idx = 0; idx < storeKeys.length; ++idx) { if (!SC.none(SC.Store.idsByStoreKey[storeKeys[idx]])) { cacheLength += 1; } }
equals(cacheLength, 1, 'there should be one id cached in the store');

cacheLength = 0;
sks = NestedRecord.ChildRecordTest.storeKeysById();
for (idx = 0; idx < ids.length; ++idx) { if (!SC.none(sks[ids[idx]])) { cacheLength += 1; } }
equals(cacheLength, 1, 'there should be one store key cached in the store');

// Overwrite the child record with a new child record with the same guid.
testParent.set('info', {type: 'ChildRecordTest', name: 'New Child Name', value: 'Red Goo', guid: '5001'});
cr = testParent.get('info');
Expand All @@ -458,15 +449,6 @@ test("Writing over a child record should remove caches in the store.", function(
for (key in store.dataHashes) { if (store.dataHashes[key] !== null) cacheLength += 1; }
equals(cacheLength, 4, 'there should be four non-null datahashes in the store after replacing child record once');

cacheLength = 0;
for (idx = 0; idx < storeKeys.length; ++idx) { if (!SC.none(SC.Store.idsByStoreKey[storeKeys[idx]])) { cacheLength += 1; } }
equals(cacheLength, 1, 'there should be one id cached in the store');

cacheLength = 0;
sks = NestedRecord.ChildRecordTest.storeKeysById();
for (idx = 0; idx < ids.length; ++idx) { if (!SC.none(sks[ids[idx]])) { cacheLength += 1; } }
equals(cacheLength, 1, 'there should be one store key cached in the store');

// Overwrite the child record with a new child record with the same guid.
testParent.set('info', store.createRecord(NestedRecord.ChildRecordTest, {type: 'ChildRecordTest', name: 'New Child Name', value: 'Orange Goo', guid: '6001'}));
cr = testParent.get('info');
Expand All @@ -491,15 +473,6 @@ test("Writing over a child record should remove caches in the store.", function(
for (key in store.dataHashes) { if (store.dataHashes[key] !== null) cacheLength += 1; }
equals(cacheLength, 4, 'there should be four non-null datahashes in the store after replacing child record twice');

cacheLength = 0;
for (idx = 0; idx < storeKeys.length; ++idx) { if (!SC.none(SC.Store.idsByStoreKey[storeKeys[idx]])) { cacheLength += 1; } }
equals(cacheLength, 1, 'there should be one id cached in the store');

cacheLength = 0;
sks = NestedRecord.ChildRecordTest.storeKeysById();
for (idx = 0; idx < ids.length; ++idx) { if (!SC.none(sks[ids[idx]])) { cacheLength += 1; } }
equals(cacheLength, 1, 'there should be one store key cached in the store');

// Make sure you can set the child to null.
testParent.set('info', null);
cr = testParent.get('info');
Expand All @@ -520,15 +493,6 @@ test("Writing over a child record should remove caches in the store.", function(
cacheLength = 0;
for (key in store.dataHashes) { if (store.dataHashes[key] !== null) cacheLength += 1; }
equals(cacheLength, 3, 'there should be three non-null datahashes in the store after removing child record');

cacheLength = 0;
for (idx = 0; idx < storeKeys.length; ++idx) { if (!SC.none(SC.Store.idsByStoreKey[storeKeys[idx]])) { cacheLength += 1; } }
equals(cacheLength, 0, 'there should be zero ids cached in the store');

cacheLength = 0;
sks = NestedRecord.ChildRecordTest.storeKeysById();
for (idx = 0; idx < ids.length; ++idx) { if (!SC.none(sks[ids[idx]])) { cacheLength += 1; } }
equals(cacheLength, 0, 'there should be zero store keys cached in the store');
});

test("Child Status Changed", function() {
Expand Down Expand Up @@ -653,11 +617,11 @@ test("Reloading the parent record uses same child record.", function() {

cacheLength = 0;
for (key in store.childRecords) { cacheLength += 1; }
equals(cacheLength, 1, 'there should only be one child record registered in the store');
equals(cacheLength, 0, 'there should be zero child records registered in the store');

cacheLength = 0;
for (key in store.records) { cacheLength += 1; }
equals(cacheLength, 4, 'there should be four records cached in the store');
equals(cacheLength, 3, 'there should be three records cached in the store');

cacheLength = 0;
for (key in store.dataHashes) { if (store.dataHashes[key] !== null) cacheLength += 1; }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -246,6 +246,7 @@ function() {

// Test Child Record creation
oldP = testParent.get('person');
oldKey = oldP.get('id');
testParent.set('person', {
type: 'Person',
name: 'Al Gore',
Expand All @@ -266,7 +267,6 @@ function() {
storeRef = store.find(NestedRecord.Person, key);
ok(storeRef, 'after a set() with an object, checking that the store has the instance of the child record with proper primary key');
equals(p, storeRef, "after a set with an object, checking the parent reference is the same as the direct store reference");
oldKey = oldP.get('id');
ok((oldKey === key), 'check to see that the old child record has the same key as the new child record');

// Check for changes on the child bubble to the parent.
Expand Down

0 comments on commit fcd514a

Please sign in to comment.