[BUGFIX release] fixes an issue with sync dematerialization followed …

…by a findRecord, adds test coverage (#5012)

addon/-private/system/model/internal-model.js

@@ -25,6 +25,7 @@ const {
   isEmpty,
   isEqual,
   setOwner,
+  run,
   RSVP,
   RSVP: { Promise }
 } = Ember;
@@ -137,6 +138,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();
 
@@ -477,11 +479,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();

addon/-private/system/store.js

@@ -1129,6 +1129,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;

tests/integration/records/unload-test.js

@@ -368,3 +368,160 @@ test('unloading a disconnected subgraph clears the relevant internal models', fu
   assert.equal(checkOrphanCalls, 3, 'each internalModel checks for cleanup');
   assert.equal(cleanupOrphanCalls, 1, 'cleanup only happens once');
 });
+
+
+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');
+});