diff --git a/addon/-private/system/model/internal-model.js b/addon/-private/system/model/internal-model.js index 236dd1617af..9004bd03b78 100644 --- a/addon/-private/system/model/internal-model.js +++ b/addon/-private/system/model/internal-model.js @@ -95,20 +95,18 @@ export default function InternalModel(type, id, store, _, data) { this.store = store; this._data = data || new EmptyObject(); this.modelName = type.modelName; - this.dataHasInitialized = false; + this._loadingPromise = null; //Look into making this lazy this._deferredTriggers = []; - this._attributes = new EmptyObject(); - this._inFlightAttributes = new EmptyObject(); this._relationships = new Relationships(this); this._recordArrays = undefined; - this.currentState = RootState.empty; this.recordReference = new RecordReference(store, this); this.references = {}; - this.isReloading = false; this._isDestroyed = false; this.isError = false; - this.error = null; + this._resetRecord(); + this._attributes = new EmptyObject(); + this._inFlightAttributes = new EmptyObject(); this.__ember_meta__ = null; this[Ember.GUID_KEY] = Ember.guidFor(this); /* @@ -184,6 +182,14 @@ InternalModel.prototype = { this.record = null; }, + dematerializeRecord() { + if (this.record) { + this.record.destroy(); + + this.updateRecordArrays(); + } + }, + deleteRecord() { this.send('deleteRecord'); }, @@ -235,8 +241,68 @@ InternalModel.prototype = { return this.record; }, - unloadRecord() { + directlyRelatedInternalModels() { + var array = []; + this.type.eachRelationship((key, relationship) => { + if (this._relationships.has(key)) { + var related = this._relationships.get(key).members.toArray(); + array.push(...related); + } + }); + return array; + }, + + allRelatedInternalModels() { + let array = []; + let queue = []; + let seen = []; + queue.push(this); + seen.push(this); + while (queue.length > 0) { + let node = queue.shift(); + array.push(node); + let related = node.directlyRelatedInternalModels(); + related.forEach(function(internalModel) { + if (seen.indexOf(internalModel) === -1) { + queue.push(internalModel); + seen.push(internalModel); + } + }); + } + return array; + }, + + unloadRecord: function() { this.send('unloadRecord'); + this.dematerializeRecord(); + var relatedInternalModels = this.allRelatedInternalModels(); + var allUnloaded = true; + var record; + for (var i=0; i < relatedInternalModels.length; i++) { + record = relatedInternalModels[i].record; + if (record && !(record.get('isDestroyed') || record.get('isDestroying'))) { + allUnloaded = false; + break; + } + } + if (allUnloaded) { + relatedInternalModels.forEach((internalModel) => { + if (!internalModel.isDestroyed) { + internalModel.destroy(); + } + }); + } + }, + + destroy: function() { + this._isDestroyed = true; + Ember.run.schedule('destroy', this, this.willDestroy, this); + }, + + willDestroy() { + Ember.assert("Cannot destroy an internalModel while its record is materialized", !this.record); + this.store._removeFromIdMap(this); + //other cleanup }, eachRelationship(callback, binding) { @@ -277,14 +343,7 @@ InternalModel.prototype = { return this._isDestroyed; }, - destroy() { - this._isDestroyed = true; - if (this.record) { - return this.record.destroy(); - } - }, - - /* + /** @method createSnapshot @private */ @@ -324,6 +383,7 @@ InternalModel.prototype = { @private */ pushedData() { + this._isDestroyed = false; this.send('pushedData'); }, @@ -584,6 +644,15 @@ InternalModel.prototype = { }); }, + _resetRecord() { + this.dataHasInitialized = false; + this.currentState = RootState.empty; + this.isReloading = false; + this.isError = false; + this.error = null; + this.record = null; + }, + /* 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 cddcd8693f6..ad2b379f667 100644 --- a/addon/-private/system/model/model.js +++ b/addon/-private/system/model/model.js @@ -803,9 +803,9 @@ var Model = Ember.Object.extend(Ember.Evented, { willDestroy() { //TODO Move! this._super(...arguments); - this._internalModel.clearRelationships(); - this._internalModel.recordObjectWillDestroy(); - //TODO should we set internalModel to null here? + + this._internalModel._resetRecord(); + this._internalModel = null; }, // This is a temporary solution until we refactor DS.Model to not diff --git a/addon/-private/system/model/states.js b/addon/-private/system/model/states.js index 23fddd3ae71..2db5a6ab2a4 100644 --- a/addon/-private/system/model/states.js +++ b/addon/-private/system/model/states.js @@ -461,10 +461,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() { }, @@ -575,10 +571,6 @@ const RootState = { }, unloadRecord(internalModel) { - // clear relationships before moving to deleted state - // otherwise it fails - internalModel.clearRelationships(); - internalModel.transitionTo('deleted.saved'); }, didCommit(internalModel) { diff --git a/addon/-private/system/record-array-manager.js b/addon/-private/system/record-array-manager.js index 1b44f1279d3..16e232692fe 100644 --- a/addon/-private/system/record-array-manager.js +++ b/addon/-private/system/record-array-manager.js @@ -100,7 +100,6 @@ export default Ember.Object.extend({ updateRecordArrays() { heimdall.increment(updateRecordArrays); this.changedRecords.forEach(internalModel => { - if (internalModel.isDestroyed || internalModel.currentState.stateName === 'root.deleted.saved') { this._recordWasDeleted(internalModel); diff --git a/addon/-private/system/store.js b/addon/-private/system/store.js index 8953f59f7dc..51b6abbf032 100644 --- a/addon/-private/system/store.js +++ b/addon/-private/system/store.js @@ -1609,7 +1609,7 @@ Store = Service.extend({ let types = new Array(keys.length); for (let i = 0; i < keys.length; i++) { - types[i] = typeMaps[keys[i]]['type'].modelName; + types[i] = typeMaps[keys[i]].type.modelName; } types.forEach(this.unloadAll, this); @@ -1617,12 +1617,11 @@ Store = Service.extend({ let typeClass = this.modelFor(modelName); let typeMap = this.typeMapFor(typeClass); let records = typeMap.records.slice(); - let record; + let internalModel; for (let i = 0; i < records.length; i++) { - record = records[i]; - record.unloadRecord(); - record.destroy(); // maybe within unloadRecord + internalModel = records[i]; + internalModel.unloadRecord(); } typeMap.metadata = new EmptyObject(); @@ -2430,12 +2429,14 @@ Store = Service.extend({ @param {InternalModel} internalModel */ _dematerializeRecord(internalModel) { + internalModel.dematerializeRecord(); + }, + + _removeFromIdMap(internalModel) { var type = internalModel.type; var typeMap = this.typeMapFor(type); var id = internalModel.id; - internalModel.updateRecordArrays(); - if (id) { delete typeMap.idToRecord[id]; } 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/unload-test.js b/tests/integration/records/unload-test.js index 50733e3805f..acea33ad8de 100644 --- a/tests/integration/records/unload-test.js +++ b/tests/integration/records/unload-test.js @@ -236,7 +236,7 @@ test("unloading all records also updates record array from peekAll()", function( }); -test("unloading a record also clears its relationship", function(assert) { +test("a relationship to an unloaded record can be restored", function(assert) { var adam, bob; // disable background reloading so we do not re-create the relationship. @@ -289,55 +289,108 @@ test("unloading a record also clears its relationship", function(assert) { person.unloadRecord(); }); - assert.equal(person.get('cars.length'), undefined); + assert.expectAssertion(function () { + bob.get('person'); + }, /You looked up the 'person' relationship on a 'car' with id 1 but some of the associated records were not loaded/); + + adam = person; }); }); -}); - -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(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() { + var 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); +}); + + +test('unloading a disconnected subgraph clears the relevant internal models', function(assert) { + env.adapter.shouldBackgroundReloadRecord = () => false; + + run(function () { env.store.push({ data: { - type: 'group', + type: 'person', id: '1', + attributes: { + name: 'Could be Anybody' + }, relationships: { - people: { + cars: { data: [ - { type: 'person', id: '1' } + { type: 'car', id: '1' }, + { type: 'car', id: '2' } ] } } } }); - 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(); - }); + run(function () { + env.store.push({ + data: { + type: 'car', + id: '1', + attributes: { + make: 'Nissan', + model: 'Altima' + }, + relationships: { + person: { + data: { type: 'person', id: '1' } + } + } + } + }); + }); - assert.equal(group.get('people.length'), 0, 'Person was removed from the people array'); + run(function () { + env.store.push({ + data: { + type: 'car', + id: '2', + attributes: { + make: 'Tesla', + model: 'S' + }, + relationships: { + person: { + data: { type: 'person', id: '1' } + } + } + } }); }); + + assert.equal(env.store.typeMapFor(env.store.modelFor('person')).records.length, 1); + assert.equal(env.store.typeMapFor(env.store.modelFor('car')).records.length, 2); + + run(function () { + env.store.peekRecord('person', 1).unloadRecord(); + env.store.peekRecord('car', 1).unloadRecord(); + env.store.peekRecord('car', 2).unloadRecord(); + }); + + assert.equal(env.store.typeMapFor(env.store.modelFor('person')).records.length, 0); + assert.equal(env.store.typeMapFor(env.store.modelFor('car')).records.length, 0); }); diff --git a/tests/integration/store-test.js b/tests/integration/store-test.js index 4f9d4fbbacc..2b6b3c8b66a 100644 --- a/tests/integration/store-test.js +++ b/tests/integration/store-test.js @@ -157,7 +157,6 @@ test("destroying the store correctly cleans everything up", function(assert) { var personWillDestroy = tap(person, 'willDestroy'); var carWillDestroy = tap(car, 'willDestroy'); - var carsWillDestroy = tap(car.get('person.cars'), 'willDestroy'); env.adapter.query = function() { return [{ @@ -186,7 +185,6 @@ test("destroying the store correctly cleans everything up", function(assert) { assert.equal(personWillDestroy.called.length, 0, 'expected person.willDestroy to not have been called'); assert.equal(carWillDestroy.called.length, 0, 'expected car.willDestroy to not have been called'); - assert.equal(carsWillDestroy.called.length, 0, 'expected cars.willDestroy to not have been called'); assert.equal(adapterPopulatedPeopleWillDestroy.called.length, 0, 'expected adapterPopulatedPeople.willDestroy to not have been called'); assert.equal(filterdPeopleWillDestroy.called.length, 0, 'expected filterdPeople.willDestroy to not have been called'); @@ -195,14 +193,10 @@ 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'); assert.equal(adapterPopulatedPeopleWillDestroy.called.length, 1, 'expected adapterPopulatedPeople to recieve willDestroy once'); assert.equal(filterdPeopleWillDestroy.called.length, 1, 'expected filterdPeople.willDestroy to have been called once'); }); diff --git a/tests/unit/many-array-test.js b/tests/unit/many-array-test.js index 10208f32dd7..9bc23470504 100644 --- a/tests/unit/many-array-test.js +++ b/tests/unit/many-array-test.js @@ -90,7 +90,9 @@ test("manyArray.save() calls save() on all records", function(assert) { }); test("manyArray trigger arrayContentChange functions with the correct values", function(assert) { - assert.expect(12); + // post.tags once for tag1 and once for tag 2, 3 assertions each + // 2 * 3 = 6 + assert.expect(6); var willChangeStartIdx; var willChangeRemoveAmt; var willChangeAddAmt; diff --git a/tests/unit/store/unload-test.js b/tests/unit/store/unload-test.js index c220ec95653..38a6aca9e52 100644 --- a/tests/unit/store/unload-test.js +++ b/tests/unit/store/unload-test.js @@ -49,7 +49,7 @@ testInDebug("unload a dirty record asserts", function(assert) { } }); - store.findRecord('record', 1).then(function(record) { + store.findRecord('record', 1, { backgroundReload: false }).then(function(record) { record.set('title', 'toto2'); record._internalModel.send('willCommit'); @@ -67,8 +67,8 @@ testInDebug("unload a dirty record asserts", function(assert) { }); }); -test("unload a record", function(assert) { - assert.expect(5); +test('unload a record', function(assert) { + assert.expect(2); run(function() { store.push({ @@ -82,15 +82,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"); @@ -105,25 +101,25 @@ module("DS.Store - unload record with relationships"); test("can commit store after unload record with relationships", function(assert) { assert.expect(1); - var like, product; + let 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 }); @@ -136,9 +132,9 @@ test("can commit store after unload record with relationships", function(assert) product: Product, like: Like }); - var asyncRecords; + let asyncRecords; - run(function() { + let promise = run(function() { store.push({ data: [{ type: 'brand', @@ -162,18 +158,23 @@ 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) + product: store.findRecord('product', 1, { backgroundReload: false }) }); - asyncRecords.then(function(records) { + + return 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); + }); + }); + + run(function () { + promise.then(function () { return store.findRecord('product', 1); - }).then(function(product) { + }).then(function (product) { assert.equal(product.get('description'), 'cuisinart', "The record was unloaded and the adapter's `findRecord` was called"); - store.destroy(); }); }); });