[BUGFIX release] unloadRecord + findRecord + orphaned relationships

Given unloadRecord followed by a synchronous findRecord, we should be sure any orphaned relationships are discarded.
emberjs · Jul 29, 2017 · 27c58e5 · 27c58e5
1 parent bb3c60b
commit 27c58e5
Show file tree

Hide file tree

Showing 3 changed files with 89 additions and 12 deletions.
diff --git a/addon/-private/system/model/internal-model.js b/addon/-private/system/model/internal-model.js
@@ -521,6 +521,11 @@ export default class InternalModel {
       this.cancelDestroy();
     }
     this._checkForOrphanedInternalModels();
+    if (this.isDestroyed || this.isDestroying) { return; }
+
+    // just in-case we are not one of the orphaned, we should still
+    // still destroy ourselves
+    this.destroy();
   }
 
   _checkForOrphanedInternalModels() {

diff --git a/addon/-private/system/store.js b/addon/-private/system/store.js
@@ -1127,15 +1127,16 @@ Store = Service.extend({
     let trueId = coerceId(id);
     let internalModel = this._internalModelsFor(modelName).get(trueId);
 
-    if (!internalModel) {
-      internalModel = this._buildInternalModel(modelName, trueId);
+    if (internalModel) {
+      if (internalModel.hasScheduledDestroy()) {
+        internalModel.destroySync();
+        return this._buildInternalModel(modelName, trueId);
+      } else {
+        return internalModel;
+      }
     } else {
-      // if we already have an internalModel, we need to ensure any async teardown is cancelled
-      //   since we want it again.
-      internalModel.cancelDestroy();
+      return this._buildInternalModel(modelName, trueId);
     }
-
-    return internalModel;
   },
 
   _internalModelDidReceiveRelationshipData(modelName, id, relationshipData) {

diff --git a/tests/integration/records/unload-test.js b/tests/integration/records/unload-test.js
@@ -16,7 +16,8 @@ let env;
 let Person = DS.Model.extend({
   name: attr('string'),
   cars: hasMany('car', { async: false }),
-  boats: hasMany('boat', { async: true })
+  boats: hasMany('boat', { async: true }),
+  bike: belongsTo('boat', { async: false, inverse: null })
 });
 Person.reopenClass({ toString() { return 'Person'; } });
 
@@ -38,14 +39,20 @@ let Boat = DS.Model.extend({
 });
 Boat.toString = function() { return 'Boat'; };
 
+let Bike = DS.Model.extend({
+  name: DS.attr()
+});
+Bike.toString = function() { return 'Bike'; };
+
 module("integration/unload - Unloading Records", {
   beforeEach() {
     env = setupStore({
       adapter: DS.JSONAPIAdapter,
       person: Person,
       car: Car,
       group: Group,
-      boat: Boat
+      boat: Boat,
+      bike: Bike
     });
   },
 
@@ -517,16 +524,80 @@ test("after unloading a record, the record can be fetched again immediately", fu
   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() {
+  return run(() => {
     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');
+    return store.findRecord('person', '1').then(newRecord => {
+      assert.equal(internalModel.currentState.stateName, 'root.empty', 'the old internalModel is discarded');
+      assert.equal(newRecord._internalModel.currentState.stateName, 'root.loaded.saved', 'We are loaded after findRecord');
+    });
   });
 
-  assert.equal(internalModel.currentState.stateName, 'root.loaded.saved', 'We are loaded after findRecord');
 });
 
+test("after unloading a record, the record can be fetched again immediately (with relationships)", function(assert) {
+  const store = env.store;
+  // stub findRecord
+  env.adapter.findRecord = () => {
+    return {
+      data: {
+        type: 'person',
+        id: '1',
+        attributes: {
+          name: 'Adam Sunderland'
+        }
+      }
+    };
+  };
+
+  // populate initial record
+  let record = run(() => {
+    return store.push({
+      data: {
+        type: 'person',
+        id: '1',
+        relationships: {
+          bike: {
+            data: { type: 'bike', id: '1' }
+          }
+        }
+      },
+
+      included: [
+        {
+          id: '1',
+          type: 'bike',
+          attributes: {
+            name: 'mr bike'
+          }
+        }
+      ]
+    });
+  });
+
+  const internalModel = record._internalModel;
+  assert.equal(internalModel.currentState.stateName, 'root.loaded.saved', 'We are loaded initially');
+
+  assert.equal(record.get('bike.name'), 'mr bike');
+
+  // we test that we can sync call unloadRecord followed by findRecord
+  let wait = run(() => {
+    store.unloadRecord(record);
+    assert.equal(record.isDestroying, true, 'the record is destroying');
+    assert.equal(record.isDestroyed, false, 'the record is NOT YET destroyed');
+    assert.equal(internalModel.currentState.stateName, 'root.empty', 'We are unloaded after unloadRecord');
+    let wait = store.findRecord('person', '1').then(newRecord => {
+      assert.equal(record.isDestroyed, false, 'the record is NOT YET destroyed');
+      assert.ok(newRecord.get('bike') === null, 'the newRecord should NOT have had a bike');
+    });
+    assert.equal(record.isDestroyed, false, 'the record is NOT YET destroyed');
+    return wait;
+  });
+
+  assert.equal(record.isDestroyed, true, 'the record IS destroyed');
+  return wait;
+});
 
 test("after unloading a record, the record can be fetched again soon there after", function(assert) {
   const store = env.store;