From d0b64ee1a6db0a0080cd07a212362a13ba8c33e2 Mon Sep 17 00:00:00 2001 From: Chris Thoburn Date: Fri, 16 Jun 2017 12:10:46 -0700 Subject: [PATCH] [BUGFIX] fixes an issue with sync dematerialization followed by a findRecord, adds test coverage --- addon/-private/system/model/internal-model.js | 15 +- addon/-private/system/store.js | 4 + tests/integration/records/unload-test.js | 157 ++++++++++++++++++ 3 files changed, 175 insertions(+), 1 deletion(-) diff --git a/addon/-private/system/model/internal-model.js b/addon/-private/system/model/internal-model.js index 4460d103e3f..e1cdbd5a9b4 100644 --- a/addon/-private/system/model/internal-model.js +++ b/addon/-private/system/model/internal-model.js @@ -24,6 +24,7 @@ const { isEmpty, isEqual, setOwner, + run, RSVP, RSVP: { Promise } } = Ember; @@ -136,6 +137,7 @@ export default class InternalModel { // `objectAt(len - 1)` to test whether or not `firstObject` or `lastObject` // have changed. this._isDematerializing = false; + this._scheduledDestroy = null; this.resetRecord(); @@ -490,11 +492,22 @@ export default class InternalModel { unloadRecord() { this.send('unloadRecord'); this.dematerializeRecord(); - Ember.run.schedule('destroy', this, '_checkForOrphanedInternalModels'); + if (this._scheduledDestroy === null) { + this._scheduledDestroy = run.schedule('destroy', this, '_checkForOrphanedInternalModels'); + } + } + + cancelDestroy() { + assert(`You cannot cancel the destruction of an InternalModel once it has already been destroyed`, !this.isDestroyed); + + this._isDematerializing = false; + run.cancel(this._scheduledDestroy); + this._scheduledDestroy = null; } _checkForOrphanedInternalModels() { this._isDematerializing = false; + this._scheduledDestroy = null; if (this.isDestroyed) { return; } this._cleanupOrphanedInternalModels(); diff --git a/addon/-private/system/store.js b/addon/-private/system/store.js index 3c01a50e429..0c45cae4d15 100644 --- a/addon/-private/system/store.js +++ b/addon/-private/system/store.js @@ -1127,6 +1127,10 @@ Store = Service.extend({ if (!internalModel) { internalModel = this._buildInternalModel(modelName, trueId); + } else { + // if we already have an internalModel, we need to ensure any async teardown is cancelled + // since we want it again. + internalModel.cancelDestroy(); } return internalModel; diff --git a/tests/integration/records/unload-test.js b/tests/integration/records/unload-test.js index 038436c76e5..df15382eda2 100644 --- a/tests/integration/records/unload-test.js +++ b/tests/integration/records/unload-test.js @@ -419,3 +419,160 @@ test('unloading a disconnected subgraph clears the relevant internal models', fu assert.equal(relPayloads.get('person', 1, 'cars'), null, 'person - cars relationship payload unloaded'); }); + + +test("Unloading a record twice only schedules destroy once", function(assert) { + const store = env.store; + let record; + + // populate initial record + run(function() { + record = store.push({ + data: { + type: 'person', + id: '1', + attributes: { + name: 'Adam Sunderland' + } + } + }); + }); + + const internalModel = record._internalModel; + + run(function() { + store.unloadRecord(record); + store.unloadRecord(record); + internalModel.cancelDestroy(); + }); + + assert.equal(internalModel.isDestroyed, false, 'We cancelled destroy'); +}); + +test("Cancelling destroy leaves the record in the empty state", function(assert) { + const store = env.store; + let record; + + // populate initial record + run(function() { + record = store.push({ + data: { + type: 'person', + id: '1', + attributes: { + name: 'Adam Sunderland' + } + } + }); + }); + + const internalModel = record._internalModel; + assert.equal(internalModel.currentState.stateName, 'root.loaded.saved', 'We are loaded initially'); + + run(function() { + store.unloadRecord(record); + assert.equal(record.isDestroying, true, 'the record is destroying'); + assert.equal(internalModel.isDestroyed, false, 'the internal model is not destroyed'); + assert.equal(internalModel._isDematerializing, true, 'the internal model is dematerializing'); + internalModel.cancelDestroy(); + assert.equal(internalModel.currentState.stateName, 'root.empty', 'We are unloaded after unloadRecord'); + }); + + assert.equal(internalModel.isDestroyed, false, 'the internal model was not destroyed'); + assert.equal(internalModel._isDematerializing, false, 'the internal model is no longer dematerializing'); + assert.equal(internalModel.currentState.stateName, 'root.empty', 'We are still unloaded after unloadRecord'); +}); + +test("after unloading a record, the record can be fetched again immediately", function(assert) { + const store = env.store; + let record; + + // stub findRecord + env.adapter.findRecord = () => { + return Ember.RSVP.Promise.resolve({ + data: { + type: 'person', + id: '1', + attributes: { + name: 'Adam Sunderland' + } + } + }); + }; + + // populate initial record + run(function() { + record = store.push({ + data: { + type: 'person', + id: '1', + attributes: { + name: 'Adam Sunderland' + } + } + }); + }); + + const internalModel = record._internalModel; + assert.equal(internalModel.currentState.stateName, 'root.loaded.saved', 'We are loaded initially'); + + // we test that we can sync call unloadRecord followed by findRecord + run(function() { + store.unloadRecord(record); + assert.equal(record.isDestroying, true, 'the record is destroying'); + assert.equal(internalModel.currentState.stateName, 'root.empty', 'We are unloaded after unloadRecord'); + store.findRecord('person', '1'); + }); + + assert.equal(internalModel.currentState.stateName, 'root.loaded.saved', 'We are loaded after findRecord'); +}); + + +test("after unloading a record, the record can be fetched again soon there after", function(assert) { + const store = env.store; + let record; + + // stub findRecord + env.adapter.findRecord = () => { + return Ember.RSVP.Promise.resolve({ + data: { + type: 'person', + id: '1', + attributes: { + name: 'Adam Sunderland' + } + } + }); + }; + + // populate initial record + run(function() { + record = store.push({ + data: { + type: 'person', + id: '1', + attributes: { + name: 'Adam Sunderland' + } + } + }); + }); + + let internalModel = record._internalModel; + assert.equal(internalModel.currentState.stateName, 'root.loaded.saved', 'We are loaded initially'); + + run(function() { + store.unloadRecord(record); + assert.equal(record.isDestroying, true, 'the record is destroying'); + assert.equal(internalModel.currentState.stateName, 'root.empty', 'We are unloaded after unloadRecord'); + }); + + run(function() { + store.findRecord('person', '1'); + }); + + record = store.peekRecord('person', '1'); + internalModel = record._internalModel; + + assert.equal(internalModel.currentState.stateName, 'root.loaded.saved', 'We are loaded after findRecord'); +});