diff --git a/addon/-private/system/model/internal-model.js b/addon/-private/system/model/internal-model.js index 5bc9e621528..878c749c4f6 100644 --- a/addon/-private/system/model/internal-model.js +++ b/addon/-private/system/model/internal-model.js @@ -59,6 +59,16 @@ function extractPivotName(name) { ); } +function areAllModelsUnloaded(internalModels) { + for (let i=0; i { + if (this._relationships.has(key)) { + let relationship = this._relationships.get(key); + let localRelationships = relationship.members.toArray(); + let serverRelationships = relationship.canonicalMembers.toArray(); + + array = array.concat(localRelationships, serverRelationships); + } + }); + return array; + } + + + /** + Computes the set of internal models reachable from this internal model. + + Reachability is determined over the relationship graph (ie a graph where + nodes are internal models and edges are belongs to or has many + relationships). + + @return {Array} An array including `this` and all internal models reachable + from `this`. + */ + _allRelatedInternalModels() { + let array = []; + let queue = []; + let bfsId = nextBfsId++; + queue.push(this); + this._bfsId = bfsId; + while (queue.length > 0) { + let node = queue.shift(); + array.push(node); + let related = node._directlyRelatedInternalModels(); + for (let i=0; i { + if (this._relationships.has(name)) { + let rel = this._relationships.get(name); + rel.destroy(); + } + }); + Object.keys(this._implicitRelationships).forEach((key) => { + this._implicitRelationships[key].destroy(); + }); + } + /* When a find request is triggered on the store, the user can optionally pass in attributes and relationships to be preloaded. These are meant to behave as if they diff --git a/addon/-private/system/model/model.js b/addon/-private/system/model/model.js index cecaf6ed9d0..da32cd604bc 100644 --- a/addon/-private/system/model/model.js +++ b/addon/-private/system/model/model.js @@ -807,14 +807,6 @@ const Model = Ember.Object.extend(Ember.Evented, { this._super(...arguments); }, - willDestroy() { - //TODO Move! - this._super(...arguments); - this._internalModel.clearRelationships(); - this._internalModel.recordObjectWillDestroy(); - //TODO should we set internalModel to null here? - }, - // This is a temporary solution until we refactor DS.Model to not // rely on the data property. willMergeMixin(props) { diff --git a/addon/-private/system/model/states.js b/addon/-private/system/model/states.js index ae542b53897..0843589314c 100644 --- a/addon/-private/system/model/states.js +++ b/addon/-private/system/model/states.js @@ -459,10 +459,6 @@ const RootState = { // you out of the in-flight state. rolledBack() { }, unloadRecord(internalModel) { - // clear relationships before moving to deleted state - // otherwise it fails - internalModel.clearRelationships(); - internalModel.transitionTo('deleted.saved'); }, propertyWasReset() { }, @@ -573,10 +569,6 @@ const RootState = { }, unloadRecord(internalModel) { - // clear relationships before moving to deleted state - // otherwise it fails - internalModel.clearRelationships(); - internalModel.transitionTo('deleted.saved'); }, didCommit() {}, @@ -680,8 +672,6 @@ const RootState = { setup(internalModel) { internalModel.clearRelationships(); - var store = internalModel.store; - store._dematerializeRecord(internalModel); }, invokeLifecycleCallbacks(internalModel) { diff --git a/addon/-private/system/record-array-manager.js b/addon/-private/system/record-array-manager.js index 79fc0c02445..0df3350f4a1 100644 --- a/addon/-private/system/record-array-manager.js +++ b/addon/-private/system/record-array-manager.js @@ -117,8 +117,15 @@ export default class RecordArrayManager { for (let i = 0, l = updated.length; i < l; i++) { let internalModel = updated[i]; - if (internalModel.isDestroyed || - internalModel.currentState.stateName === 'root.deleted.saved') { + // During dematerialization we don't want to rematerialize the record. + // recordWasDeleted can cause other records to rematerialize because it + // removes the internal model from the array and Ember arrays will always + // `objectAt(0)` and `objectAt(len -1)` to check whether `firstObject` or + // `lastObject` have changed. When this happens we don't want those + // models to rematerialize their records. + if (internalModel._isDematerializing || + internalModel.isDestroyed || + internalModel.currentState.stateName === 'root.deleted.saved') { this._recordWasDeleted(internalModel); } else { this._recordWasChanged(internalModel); diff --git a/addon/-private/system/record-map.js b/addon/-private/system/record-map.js index 7633531f511..d91756214e9 100644 --- a/addon/-private/system/record-map.js +++ b/addon/-private/system/record-map.js @@ -120,7 +120,6 @@ export default class RecordMap { for (let i = 0; i < records.length; i++) { record = records[i]; record.unloadRecord(); - record.destroy(); // maybe within unloadRecord } } diff --git a/addon/-private/system/relationships/state/belongs-to.js b/addon/-private/system/relationships/state/belongs-to.js index 2b6089d828a..7f4b2dbf1ac 100644 --- a/addon/-private/system/relationships/state/belongs-to.js +++ b/addon/-private/system/relationships/state/belongs-to.js @@ -46,6 +46,10 @@ export default class BelongsToRelationship extends Relationship { super.addCanonicalRecord(newRecord); } + inverseDidDematerialize() { + this.notifyBelongsToChanged(); + } + flushCanonical() { //temporary fix to not remove newly created records if server returned null. //TODO remove once we have proper diffing @@ -54,7 +58,7 @@ export default class BelongsToRelationship extends Relationship { } if (this.inverseRecord !== this.canonicalState) { this.inverseRecord = this.canonicalState; - this.internalModel.notifyBelongsToChanged(this.key); + this.notifyBelongsToChanged(); } super.flushCanonical(); @@ -71,7 +75,7 @@ export default class BelongsToRelationship extends Relationship { this.inverseRecord = newRecord; super.addRecord(newRecord); - this.internalModel.notifyBelongsToChanged(this.key); + this.notifyBelongsToChanged(); } setRecordPromise(newPromise) { @@ -84,6 +88,10 @@ export default class BelongsToRelationship extends Relationship { if (!this.members.has(record)) { return;} this.inverseRecord = null; super.removeRecordFromOwn(record); + this.notifyBelongsToChanged(); + } + + notifyBelongsToChanged() { this.internalModel.notifyBelongsToChanged(this.key); } diff --git a/addon/-private/system/relationships/state/has-many.js b/addon/-private/system/relationships/state/has-many.js index 7bfd7aaff89..6cda156f3cf 100644 --- a/addon/-private/system/relationships/state/has-many.js +++ b/addon/-private/system/relationships/state/has-many.js @@ -30,8 +30,10 @@ export default class ManyRelationship extends Relationship { } destroy() { + super.destroy(); if (this._manyArray) { this._manyArray.destroy(); + this._manyArray = null; } } @@ -54,6 +56,14 @@ export default class ManyRelationship extends Relationship { super.addCanonicalRecord(record, idx); } + inverseDidDematerialize() { + if (this._manyArray) { + this._manyArray.destroy(); + this._manyArray = null; + } + this.notifyHasManyChanged(); + } + addRecord(record, idx) { if (this.members.has(record)) { return; diff --git a/addon/-private/system/relationships/state/relationship.js b/addon/-private/system/relationships/state/relationship.js index eb7fc9b5c34..44c1367644b 100644 --- a/addon/-private/system/relationships/state/relationship.js +++ b/addon/-private/system/relationships/state/relationship.js @@ -83,7 +83,26 @@ export default class Relationship { return this.internalModel.modelName; } - destroy() { } + destroy() { + if (!this.inverseKey) { return; } + + let allMembers = + // we actually want a union of members and canonicalMembers + // they should be disjoint but currently are not due to a bug + this.members.toArray().concat(this.canonicalMembers.toArray()); + + allMembers.forEach(inverseInternalModel => { + let relationship = inverseInternalModel._relationships.get(this.inverseKey); + // TODO: there is always a relationship in this case; this guard exists + // because there are tests that fail in teardown after putting things in + // invalid state + if (relationship) { + relationship.inverseDidDematerialize(); + } + }); + } + + inverseDidDematerialize() {} updateMeta(meta) { heimdall.increment(updateMeta); @@ -92,13 +111,18 @@ export default class Relationship { clear() { heimdall.increment(clear); - var members = this.members.list; - var member; + let members = this.members.list; while (members.length > 0) { - member = members[0]; + let member = members[0]; this.removeRecord(member); } + + let canonicalMembers = this.canonicalMembers.list; + while (canonicalMembers.length > 0) { + let member = canonicalMembers[0]; + this.removeCanonicalRecord(member); + } } removeRecords(records) { diff --git a/addon/-private/system/store.js b/addon/-private/system/store.js index 2dfa8acbf85..64b62c0f602 100644 --- a/addon/-private/system/store.js +++ b/addon/-private/system/store.js @@ -2541,16 +2541,14 @@ Store = Service.extend({ When a record is destroyed, this un-indexes it and removes it from any record arrays so it can be GCed. - @method _dematerializeRecord + @method _removeFromIdMap @private @param {InternalModel} internalModel */ - _dematerializeRecord(internalModel) { + _removeFromIdMap(internalModel) { let recordMap = this._recordMapFor(internalModel.modelName); let id = internalModel.id; - internalModel.updateRecordArrays(); - recordMap.remove(internalModel, id); }, diff --git a/tests/integration/adapter/find-all-test.js b/tests/integration/adapter/find-all-test.js index b9ba670bbe6..cc4a081bc53 100644 --- a/tests/integration/adapter/find-all-test.js +++ b/tests/integration/adapter/find-all-test.js @@ -19,6 +19,7 @@ module("integration/adapter/find_all - Finding All Records of a Type", { firstName: attr('string'), lastName: attr('string') }); + Person.reopenClass({ toString() { return 'Person'; } }); allRecords = null; diff --git a/tests/integration/records/delete-record-test.js b/tests/integration/records/delete-record-test.js index 78d90206222..f877e09ca28 100644 --- a/tests/integration/records/delete-record-test.js +++ b/tests/integration/records/delete-record-test.js @@ -17,6 +17,7 @@ module("integration/deletedRecord - Deleting Records", { Person = DS.Model.extend({ name: attr('string') }); + Person.toString = () => { return 'Person'; }; env = setupStore({ person: Person @@ -78,6 +79,7 @@ test('deleting a record that is part of a hasMany removes it from the hasMany re const Group = DS.Model.extend({ people: DS.hasMany('person', { inverse: null, async: false }) }); + Group.toString = () => { return 'Group'; } env.adapter.deleteRecord = function() { return Ember.RSVP.Promise.resolve(); diff --git a/tests/integration/records/load-test.js b/tests/integration/records/load-test.js index d3e4b96b2fe..02e070c0c1c 100644 --- a/tests/integration/records/load-test.js +++ b/tests/integration/records/load-test.js @@ -25,19 +25,14 @@ module("integration/load - Loading Records", { } }); -test("When loading a record fails, the isLoading is set to false", function(assert) { +test("When loading a record fails, the record is not left behind", function(assert) { env.adapter.findRecord = function(store, type, id, snapshot) { return Ember.RSVP.reject(); }; run(function() { env.store.findRecord('post', 1).then(null, assert.wait(function() { - // store.recordForId is private, but there is currently no other way to - // get the specific record instance, since it is not passed to this - // rejection handler - var post = env.store.recordForId('post', 1); - - assert.equal(post.get("isLoading"), false, "post is not loading anymore"); + assert.equal(env.store.hasRecordForId('post', 1), false); })); }); }); diff --git a/tests/integration/records/rematerialize-test.js b/tests/integration/records/rematerialize-test.js new file mode 100644 index 00000000000..9780a60750a --- /dev/null +++ b/tests/integration/records/rematerialize-test.js @@ -0,0 +1,245 @@ +/*eslint no-unused-vars: ["error", { "varsIgnorePattern": "(adam|bob|dudu)" }]*/ + +import setupStore from 'dummy/tests/helpers/store'; +import Ember from 'ember'; + +import {module, test} from 'qunit'; + +import DS from 'ember-data'; + +let attr = DS.attr; +let belongsTo = DS.belongsTo; +let hasMany = DS.hasMany; +let run = Ember.run; +let env; + +let Person = DS.Model.extend({ + name: attr('string'), + cars: hasMany('car', { async: false }), + boats: hasMany('boat', { async: true }) +}); +Person.reopenClass({ toString() { return 'Person'; } }); + +let Group = DS.Model.extend({ + people: hasMany('person', { async: false }) +}); +Group.reopenClass({ toString() { return 'Group'; } }); + +let Car = DS.Model.extend({ + make: attr('string'), + model: attr('string'), + person: belongsTo('person', { async: false }) +}); +Car.reopenClass({ toString() { return 'Car'; } }); + +let Boat = DS.Model.extend({ + name: attr('string'), + person: belongsTo('person', { async: false }) +}); +Boat.toString = function() { return 'Boat'; }; + +module("integration/unload - Unloading Records", { + beforeEach() { + env = setupStore({ + adapter: DS.JSONAPIAdapter, + person: Person, + car: Car, + group: Group, + boat: Boat + }); + }, + + afterEach() { + Ember.run(function() { + env.container.destroy(); + }); + } +}); + +test("a sync belongs to relationship to an unloaded record can restore that record", function(assert) { + let adam, bob; + + // disable background reloading so we do not re-create the relationship. + env.adapter.shouldBackgroundReloadRecord = () => false; + + run(function() { + env.store.push({ + data: { + type: 'person', + id: '1', + attributes: { + name: 'Adam Sunderland' + }, + relationships: { + cars: { + data: [ + { type: 'car', id: '1' } + ] + } + } + } + }); + adam = env.store.peekRecord('person', 1); + }); + + run(function() { + env.store.push({ + data: { + type: 'car', + id: '1', + attributes: { + make: "Lotus", + model: "Exige" + }, + relationships: { + person: { + data: { type: 'person', id: '1' } + } + } + } + }); + bob = env.store.peekRecord('car', 1); + }); + + let person = env.store.peekRecord('person', 1); + assert.equal(person.get('cars.length'), 1, 'The inital length of cars is correct'); + + assert.equal(env.store.hasRecordForId('person', 1), true, 'The person is in the store'); + assert.equal(env.store._recordMapFor('person').has(1), true, 'The person internalModel is loaded'); + + run(function() { + person.unloadRecord(); + }); + + assert.equal(env.store.hasRecordForId('person', 1), false, 'The person is unloaded'); + assert.equal(env.store._recordMapFor('person').has(1), true, 'The person internalModel is retained'); + + run(() => { + env.store.push({ + data: { + type: 'person', + id: '1', + attributes: { + name: 'Adam Sunderland' + }, + relationships: { + cars: { + data: [ + { type: 'car', id: '1' } + ] + } + } + } + }); + }); + + let rematerializedPerson = bob.get('person'); + assert.equal(rematerializedPerson.get('id'), '1'); + // the person is rematerialized; the previous person is *not* re-used + assert.notEqual(rematerializedPerson, adam, 'the person is rematerialized, not recycled'); +}); + +test("an async has many relationship to an unloaded record can restore that record", function(assert) { + assert.expect(13); + + // disable background reloading so we do not re-create the relationship. + env.adapter.shouldBackgroundReloadRecord = () => false; + + env.adapter.findRecord = function() { + assert.ok('adapter called'); + return Ember.RSVP.Promise.resolve({ + data: { + type: 'boat', + id: '1', + attributes: { + name: "Boaty McBoatface" + }, + relationships: { + person: { + data: { type: 'person', id: '1' } + } + } + } + }); + } + + run(function() { + env.store.push({ + data: { + type: 'person', + id: '1', + attributes: { + name: 'Adam Sunderland' + }, + relationships: { + boats: { + data: [ + { type: 'boat', id: '2' }, + { type: 'boat', id: '1' } + ] + } + } + } + }); + }); + + run(function() { + env.store.push({ + data: [{ + type: 'boat', + id: '2', + attributes: { + name: 'Some other boat' + }, + relationships: { + person: { + data: { type: 'person', id: '1' } + } + } + }, { + type: 'boat', + id: '1', + attributes: { + name: 'Boaty McBoatface' + }, + relationships: { + person: { + data: { type: 'person', id: '1' } + } + } + }] + }); + }); + + let adam = env.store.peekRecord('person', 1); + let boaty = env.store.peekRecord('boat', 1); + + assert.equal(env.store.hasRecordForId('person', 1), true, 'The person is in the store'); + assert.equal(env.store._recordMapFor('person').has(1), true, 'The person internalModel is loaded'); + assert.equal(env.store.hasRecordForId('boat', 1), true, 'The boat is in the store'); + assert.equal(env.store._recordMapFor('boat').has(1), true, 'The boat internalModel is loaded'); + + let boats = run(() => { + return adam.get('boats'); + }); + + assert.equal(boats.get('length'), 2, 'Before unloading boats.length is correct'); + + run(function() { + boaty.unloadRecord(); + }); + + assert.equal(env.store.hasRecordForId('boat', 1), false, 'The boat is unloaded'); + assert.equal(env.store._recordMapFor('boat').has(1), true, 'The boat internalModel is retained'); + + let rematerializedBoaty = run(() => { + return rematerializedBoaty = adam.get('boats').objectAt(1); + }); + + assert.equal(adam.get('boats.length'), 2, 'boats.length correct after rematerialization'); + assert.equal(rematerializedBoaty.get('id'), '1'); + assert.notEqual(rematerializedBoaty, boaty, 'the boat is rematerialized, not recycled'); + + assert.equal(env.store.hasRecordForId('boat', 1), true, 'The boat is loaded'); + assert.equal(env.store._recordMapFor('boat').has(1), true, 'The boat internalModel is retained'); +}); diff --git a/tests/integration/records/unload-test.js b/tests/integration/records/unload-test.js index b97a79797cc..63e8e616055 100644 --- a/tests/integration/records/unload-test.js +++ b/tests/integration/records/unload-test.js @@ -7,33 +7,45 @@ import {module, test} from 'qunit'; import DS from 'ember-data'; -var attr = DS.attr; -var belongsTo = DS.belongsTo; -var hasMany = DS.hasMany; -var run = Ember.run; -var env; +let attr = DS.attr; +let belongsTo = DS.belongsTo; +let hasMany = DS.hasMany; +let run = Ember.run; +let env; -var Person = DS.Model.extend({ +let Person = DS.Model.extend({ name: attr('string'), - cars: hasMany('car', { async: false }) + cars: hasMany('car', { async: false }), + boats: hasMany('boat', { async: true }) }); +Person.reopenClass({ toString() { return 'Person'; } }); -var Group = DS.Model.extend({ +let Group = DS.Model.extend({ people: hasMany('person', { async: false }) }); +Group.reopenClass({ toString() { return 'Group'; } }); -var Car = DS.Model.extend({ +let Car = DS.Model.extend({ make: attr('string'), model: attr('string'), person: belongsTo('person', { async: false }) }); +Car.reopenClass({ toString() { return 'Car'; } }); + +let Boat = DS.Model.extend({ + name: attr('string'), + person: belongsTo('person', { async: false }) +}); +Boat.toString = function() { return 'Boat'; }; module("integration/unload - Unloading Records", { beforeEach() { env = setupStore({ + adapter: DS.JSONAPIAdapter, person: Person, car: Car, - group: Group + group: Group, + boat: Boat }); }, @@ -45,7 +57,7 @@ module("integration/unload - Unloading Records", { }); test("can unload a single record", function(assert) { - var adam; + let adam; run(function() { env.store.push({ data: { @@ -59,17 +71,21 @@ test("can unload a single record", function(assert) { adam = env.store.peekRecord('person', 1); }); + assert.equal(env.store.peekAll('person').get('length'), 1, 'one person record loaded'); + assert.equal(env.store._recordMapFor('person').length, 1, 'one person internalModel loaded'); + Ember.run(function() { adam.unloadRecord(); }); - assert.equal(env.store.peekAll('person').get('length'), 0); + assert.equal(env.store.peekAll('person').get('length'), 0, 'no person records'); + assert.equal(env.store._recordMapFor('person').length, 0, 'no person internalModels'); }); test("can unload all records for a given type", function(assert) { - assert.expect(2); + assert.expect(8); - var adam, bob, dudu; + let adam, bob, dudu; run(function() { env.store.push({ data: [{ @@ -107,18 +123,25 @@ test("can unload all records for a given type", function(assert) { dudu = bob = env.store.peekRecord('car', 1); }); + assert.equal(env.store.peekAll('person').get('length'), 2, 'two person records loaded'); + assert.equal(env.store._recordMapFor('person').length, 2, 'two person internalModels loaded'); + assert.equal(env.store.peekAll('car').get('length'), 1, 'one car record loaded'); + assert.equal(env.store._recordMapFor('car').length, 1, 'one car internalModel loaded'); + Ember.run(function() { env.store.unloadAll('person'); }); assert.equal(env.store.peekAll('person').get('length'), 0); assert.equal(env.store.peekAll('car').get('length'), 1); + assert.equal(env.store._recordMapFor('person').length, 0, 'zero person internalModels loaded'); + assert.equal(env.store._recordMapFor('car').length, 1, 'one car internalModel loaded'); }); test("can unload all records", function(assert) { - assert.expect(2); + assert.expect(8); - var adam, bob, dudu; + let adam, bob, dudu; run(function() { env.store.push({ data: [{ @@ -156,16 +179,25 @@ test("can unload all records", function(assert) { dudu = bob = env.store.peekRecord('car', 1); }); + assert.equal(env.store.peekAll('person').get('length'), 2, 'two person records loaded'); + assert.equal(env.store._recordMapFor('person').length, 2, 'two person internalModels loaded'); + assert.equal(env.store.peekAll('car').get('length'), 1, 'one car record loaded'); + assert.equal(env.store._recordMapFor('car').length, 1, 'one car internalModel loaded'); + Ember.run(function() { env.store.unloadAll(); }); assert.equal(env.store.peekAll('person').get('length'), 0); assert.equal(env.store.peekAll('car').get('length'), 0); + assert.equal(env.store._recordMapFor('person').length, 0, 'zero person internalModels loaded'); + assert.equal(env.store._recordMapFor('car').length, 0, 'zero car internalModels loaded'); }); test("removes findAllCache after unloading all records", function(assert) { - var adam, bob; + assert.expect(4); + + let adam, bob; run(function() { env.store.push({ data: [{ @@ -186,16 +218,20 @@ test("removes findAllCache after unloading all records", function(assert) { bob = env.store.peekRecord('person', 2); }); + assert.equal(env.store.peekAll('person').get('length'), 2, 'two person records loaded'); + assert.equal(env.store._recordMapFor('person').length, 2, 'two person internalModels loaded'); + Ember.run(function() { env.store.peekAll('person'); env.store.unloadAll('person'); }); - assert.equal(env.store.peekAll('person').get('length'), 0); + assert.equal(env.store.peekAll('person').get('length'), 0, 'zero person records loaded'); + assert.equal(env.store._recordMapFor('person').length, 0, 'zero person internalModels loaded'); }); test("unloading all records also updates record array from peekAll()", function(assert) { - var adam, bob; + let adam, bob; run(function() { env.store.push({ data: [{ @@ -215,52 +251,48 @@ test("unloading all records also updates record array from peekAll()", function( adam = env.store.peekRecord('person', 1); bob = env.store.peekRecord('person', 2); }); - var all = env.store.peekAll('person'); + let all = env.store.peekAll('person'); assert.equal(all.get('length'), 2); + Ember.run(function() { env.store.unloadAll('person'); }); - assert.equal(all.get('length'), 0); }); - -test("unloading a record also clears its relationship", function(assert) { - var adam, bob; - - // disable background reloading so we do not re-create the relationship. +test('unloading a disconnected subgraph clears the relevant internal models', function(assert) { env.adapter.shouldBackgroundReloadRecord = () => false; - run(function() { + run(() => { env.store.push({ data: { type: 'person', id: '1', attributes: { - name: 'Adam Sunderland' + name: 'Could be Anybody' }, relationships: { cars: { data: [ - { type: 'car', id: '1' } + { type: 'car', id: '1' }, + { type: 'car', id: '2' } ] } } } }); - adam = env.store.peekRecord('person', 1); }); - run(function() { + run(() => { env.store.push({ data: { type: 'car', id: '1', attributes: { - make: "Lotus", - model: "Exige" + make: 'Nissan', + model: 'Altima' }, relationships: { person: { @@ -269,66 +301,70 @@ test("unloading a record also clears its relationship", function(assert) { } } }); - bob = env.store.peekRecord('car', 1); }); - run(function() { - env.store.findRecord('person', 1).then(function(person) { - assert.equal(person.get('cars.length'), 1, 'The inital length of cars is correct'); - - run(function() { - person.unloadRecord(); - }); - - assert.equal(person.get('cars.length'), undefined); - }); - }); -}); - -test("unloading a record also clears the implicit inverse relationships", function(assert) { - var adam, bob; - // disable background reloading so we do not re-create the relationship. - env.adapter.shouldBackgroundReloadRecord = () => false; - - run(function() { + run(() => { env.store.push({ data: { - type: 'person', - id: '1', + type: 'car', + id: '2', attributes: { - name: 'Adam Sunderland' - } - } - }); - adam = env.store.peekRecord('person', 1); - }); - - run(function() { - env.store.push({ - data: { - type: 'group', - id: '1', + make: 'Tesla', + model: 'S' + }, relationships: { - people: { - data: [ - { type: 'person', id: '1' } - ] + person: { + data: { type: 'person', id: '1' } } } } }); - bob = env.store.peekRecord('group', 1); }); - run(function() { - env.store.findRecord('group', 1).then(function(group) { - assert.equal(group.get('people.length'), 1, 'The inital length of people is correct'); - var person = env.store.peekRecord('person', 1); - run(function() { - person.unloadRecord(); - }); - - assert.equal(group.get('people.length'), 0, 'Person was removed from the people array'); - }); + assert.equal( + env.store._recordMapFor('person').records.length, + 1, + 'one person record is loaded' + ); + assert.equal( + env.store._recordMapFor('car').records.length, + 2, + 'two car records are loaded' + ); + assert.equal(env.store.hasRecordForId('person', 1), true); + assert.equal(env.store.hasRecordForId('car', 1), true); + assert.equal(env.store.hasRecordForId('car', 2), true); + + let checkOrphanCalls = 0; + let cleanupOrphanCalls = 0; + + function countOrphanCalls(record) { + let origCheck = record._internalModel._checkForOrphanedInternalModels; + let origCleanup = record._internalModel._cleanupOrphanedInternalModels; + + record._internalModel._checkForOrphanedInternalModels = function () { + ++checkOrphanCalls; + return origCheck.apply(record._internalModel, arguments); + }; + + record._internalModel._cleanupOrphanedInternalModels = function () { + ++cleanupOrphanCalls; + return origCleanup.apply(record._internalModel, arguments); + }; + } + countOrphanCalls(env.store.peekRecord('person', 1)); + countOrphanCalls(env.store.peekRecord('car', 1)); + countOrphanCalls(env.store.peekRecord('car', 2)); + + run(() => { + env.store.peekRecord('person', 1).unloadRecord(); + env.store.peekRecord('car', 1).unloadRecord(); + env.store.peekRecord('car', 2).unloadRecord(); }); + + assert.equal(env.store._recordMapFor('person').records.length, 0); + assert.equal(env.store._recordMapFor('car').records.length, 0); + + assert.equal(checkOrphanCalls, 3, 'each internalModel checks for cleanup'); + assert.equal(cleanupOrphanCalls, 1, 'cleanup only happens once'); }); diff --git a/tests/integration/relationships/has-many-test.js b/tests/integration/relationships/has-many-test.js index bf49959d009..74592311f49 100644 --- a/tests/integration/relationships/has-many-test.js +++ b/tests/integration/relationships/has-many-test.js @@ -29,44 +29,53 @@ module("integration/relationships/has_many - Has-Many Relationships", { Contact = DS.Model.extend({ user: belongsTo('user', { async: false }) }); + Contact.reopenClass({ toString: () => 'Contact' }); Email = Contact.extend({ email: attr('string') }); + Email.reopenClass({ toString: () => 'Email' }); Phone = Contact.extend({ number: attr('string') }); + Phone.reopenClass({ toString: () => 'Phone' }); Message = DS.Model.extend({ user: belongsTo('user', { async: false }), created_at: attr('date') }); + Message.reopenClass({ toString: () => 'Message' }); Post = Message.extend({ title: attr('string'), comments: hasMany('comment', { async: false }) }); + Post.reopenClass({ toString: () => 'Post' }); Comment = Message.extend({ body: DS.attr('string'), message: DS.belongsTo('post', { polymorphic: true, async: true }) }); + Comment.reopenClass({ toString: () => 'Comment' }); Book = DS.Model.extend({ title: attr(), chapters: hasMany('chapter', { async: true }) }); + Book.reopenClass({ toString: () => 'Book' }); Chapter = DS.Model.extend({ title: attr(), pages: hasMany('page', { async: false }) }); + Chapter.reopenClass({ toString: () => 'Chapter' }); Page = DS.Model.extend({ number: attr('number'), chapter: belongsTo('chapter', { async: false }) }); + Page.reopenClass({ toString: () => 'Page' }); env = setupStore({ user: User, @@ -2197,10 +2206,12 @@ test("adding and removing records from hasMany relationship #2666", function(ass var Post = DS.Model.extend({ comments: DS.hasMany('comment', { async: true }) }); + Post.reopenClass({ toString: () => 'Post' }); var Comment = DS.Model.extend({ post: DS.belongsTo('post', { async: false }) }); + Comment.reopenClass({ toString: () => 'Comment' }); env = setupStore({ post: Post, @@ -2776,7 +2787,7 @@ test("unloading and reloading a record with hasMany relationship - #3084", funct user = env.store.peekRecord('user', 'user-1'); - assert.equal(get(user, 'messages.firstObject.id'), 'message-1'); - assert.equal(get(message, 'user.id'), 'user-1'); + assert.equal(get(user, 'messages.firstObject.id'), 'message-1', 'user points to message'); + assert.equal(get(message, 'user.id'), 'user-1', 'message points to user'); }); }); diff --git a/tests/integration/relationships/many-to-many-test.js b/tests/integration/relationships/many-to-many-test.js index f133d1bea4e..9b000618661 100644 --- a/tests/integration/relationships/many-to-many-test.js +++ b/tests/integration/relationships/many-to-many-test.js @@ -525,7 +525,7 @@ test("Deleting a record that has a hasMany relationship removes it from the othe user.rollbackAttributes(); }); assert.equal(account.get('users.length'), 0, 'Users got removed'); - assert.equal(user.get('accounts.length'), undefined, 'Accounts got rolledback correctly'); + assert.equal(user.get('accounts.length'), 0, 'Accounts got rolledback correctly'); }); diff --git a/tests/integration/relationships/one-to-many-test.js b/tests/integration/relationships/one-to-many-test.js index 6225205f555..59a5f5feb7e 100644 --- a/tests/integration/relationships/one-to-many-test.js +++ b/tests/integration/relationships/one-to-many-test.js @@ -1449,6 +1449,6 @@ test("Rollbacking attributes of a created record works correctly when the belong user.get('accounts').pushObject(account); }); run(user, 'rollbackAttributes'); - assert.equal(user.get('accounts.length'), undefined, "User does not have the account anymore"); + assert.equal(user.get('accounts.length'), 0, "User does not have the account anymore"); assert.equal(account.get('user'), null, 'Account does not have the user anymore'); }); diff --git a/tests/integration/store-test.js b/tests/integration/store-test.js index 176de164571..c5767002db5 100644 --- a/tests/integration/store-test.js +++ b/tests/integration/store-test.js @@ -13,6 +13,7 @@ var Person = DS.Model.extend({ name: DS.attr('string'), cars: DS.hasMany('car', { async: false }) }); +Person.reopenClass({ toString: () => 'Person' }); var run = Ember.run; @@ -21,6 +22,7 @@ var Car = DS.Model.extend({ model: DS.attr('string'), person: DS.belongsTo('person', { async: false }) }); +Car.reopenClass({ toString: () => 'Car' }); function initializeStore(adapter) { env = setupStore({ @@ -195,11 +197,8 @@ test("destroying the store correctly cleans everything up", function(assert) { assert.equal(car.get('person'), person, "expected car's person to be the correct person"); assert.equal(person.get('cars.firstObject'), car, " expected persons cars's firstRecord to be the correct car"); - Ember.run(person, person.destroy); Ember.run(store, 'destroy'); - assert.equal(car.get('person'), null, "expected car.person to no longer be present"); - assert.equal(personWillDestroy.called.length, 1, 'expected person to have recieved willDestroy once'); assert.equal(carWillDestroy.called.length, 1, 'expected car to recieve willDestroy once'); assert.equal(carsWillDestroy.called.length, 1, 'expected cars to recieve willDestroy once'); diff --git a/tests/unit/many-array-test.js b/tests/unit/many-array-test.js index cd2fd323cf5..115efadbb04 100644 --- a/tests/unit/many-array-test.js +++ b/tests/unit/many-array-test.js @@ -19,11 +19,13 @@ module("unit/many_array - DS.ManyArray", { title: attr('string'), tags: hasMany('tag', { async: false }) }); + Post.reopenClass({ toString: () => 'Post'}); Tag = DS.Model.extend({ name: attr('string'), post: belongsTo('post', { async: false }) }); + Tag.reopenClass({ toString: () => 'Tag'}); env = setupStore({ post: Post, @@ -88,7 +90,7 @@ test("manyArray.save() calls save() on all records", function(assert) { }); test("manyArray trigger arrayContentChange functions with the correct values", function(assert) { - assert.expect(12); + assert.expect(6); var willChangeStartIdx; var willChangeRemoveAmt; var willChangeAddAmt; diff --git a/tests/unit/model/rollback-attributes-test.js b/tests/unit/model/rollback-attributes-test.js index 9c58865b958..178e4499607 100644 --- a/tests/unit/model/rollback-attributes-test.js +++ b/tests/unit/model/rollback-attributes-test.js @@ -15,6 +15,7 @@ module("unit/model/rollbackAttributes - model.rollbackAttributes()", { firstName: DS.attr(), lastName: DS.attr() }); + Person.reopenClass({ toString() { return 'Person'; } }); env = setupStore({ person: Person }); store = env.store; diff --git a/tests/unit/store/unload-test.js b/tests/unit/store/unload-test.js index 9bcd77696c5..fc6a4875be6 100644 --- a/tests/unit/store/unload-test.js +++ b/tests/unit/store/unload-test.js @@ -17,6 +17,8 @@ module("unit/store/unload - Store unloading records", { title: DS.attr('string'), wasFetched: DS.attr('boolean') }); + Record.reopenClass({ toString: () => 'Record'}); + store = createStore({ adapter: DS.Adapter.extend({ findRecord(store, type, id, snapshot) { @@ -47,26 +49,25 @@ testInDebug("unload a dirty record asserts", function(assert) { } }); - store.findRecord('record', 1).then(function(record) { - record.set('title', 'toto2'); - record._internalModel.send('willCommit'); + let record = store.peekRecord('record', 1); + record.set('title', 'toto2'); + record._internalModel.send('willCommit'); - assert.equal(get(record, 'hasDirtyAttributes'), true, "record is dirty"); + assert.equal(get(record, 'hasDirtyAttributes'), true, "record is dirty"); - assert.expectAssertion(function() { - record.unloadRecord(); - }, "You can only unload a record which is not inFlight. `" + record._internalModel.toString() + "`", "can not unload dirty record"); + assert.expectAssertion(function() { + record.unloadRecord(); + }, "You can only unload a record which is not inFlight. `" + record._internalModel.toString() + "`", "can not unload dirty record"); - // force back into safe to unload mode. - run(function() { - record._internalModel.transitionTo('deleted.saved'); - }); + // force back into safe to unload mode. + run(function() { + record._internalModel.transitionTo('deleted.saved'); }); }); }); -test("unload a record", function(assert) { - assert.expect(5); +test('unload a record', function(assert) { + assert.expect(2); run(function() { store.push({ @@ -80,15 +81,11 @@ test("unload a record", function(assert) { }); store.findRecord('record', 1).then(function(record) { assert.equal(get(record, 'id'), 1, "found record with id 1"); - assert.equal(get(record, 'hasDirtyAttributes'), false, "record is not dirty"); run(function() { store.unloadRecord(record); }); - assert.equal(get(record, 'hasDirtyAttributes'), false, "record is not dirty"); - assert.equal(get(record, 'isDeleted'), true, "record is deleted"); - tryToFind = false; return store.findRecord('record', 1).then(function() { assert.equal(tryToFind, true, "not found record with id 1"); @@ -103,22 +100,23 @@ module("DS.Store - unload record with relationships"); test("can commit store after unload record with relationships", function(assert) { assert.expect(1); - var like, product; - - var Brand = DS.Model.extend({ + let Brand = DS.Model.extend({ name: DS.attr('string') }); + Brand.reopenClass({ toString: () => 'Brand'}); - var Product = DS.Model.extend({ + let Product = DS.Model.extend({ description: DS.attr('string'), brand: DS.belongsTo('brand', { async: false }) }); + Product.reopenClass({ toString: () => 'Product'}); - var Like = DS.Model.extend({ + let Like = DS.Model.extend({ product: DS.belongsTo('product', { async: false }) }); + Like.reopenClass({ toString: () => 'Like'}); - var store = createStore({ + let store = createStore({ adapter: DS.Adapter.extend({ findRecord(store, type, id, snapshot) { return Ember.RSVP.resolve({ id: 1, description: 'cuisinart', brand: 1 }); @@ -131,9 +129,8 @@ test("can commit store after unload record with relationships", function(assert) product: Product, like: Like }); - var asyncRecords; - run(function() { + return run(() => { store.push({ data: [{ type: 'brand', @@ -155,20 +152,17 @@ test("can commit store after unload record with relationships", function(assert) }] }); - asyncRecords = Ember.RSVP.hash({ - brand: store.findRecord('brand', 1), - product: store.findRecord('product', 1) - }); - asyncRecords.then(function(records) { - like = store.createRecord('like', { id: 1, product: product }); - records.like = like.save(); - return Ember.RSVP.hash(records); - }).then(function(records) { - store.unloadRecord(records.product); - return store.findRecord('product', 1); - }).then(function(product) { - assert.equal(product.get('description'), 'cuisinart', "The record was unloaded and the adapter's `findRecord` was called"); - store.destroy(); + let product = store.peekRecord('product', 1); + let like = store.createRecord('like', { id: 1, product: product }); + + return like.save(); + }).then(() => { + return run(() => { + store.unloadRecord(store.peekRecord('product', 1)); }); + }).then(() => { + return store.findRecord('product', 1); + }).then((product) => { + assert.equal(product.get('description'), 'cuisinart', "The record was unloaded and the adapter's `findRecord` was called"); }); });