Merge pull request #5097 from emberjs/release-unload-and-create

[BUGFIX release] restore unloadRecord + createRecord
emberjs · Jul 27, 2017 · bb3c60b · bb3c60b
2 parents ac644e7 + 22d84ca
commit bb3c60b
Show file tree

Hide file tree

Showing 3 changed files with 45 additions and 3 deletions.
diff --git a/addon/-private/system/model/internal-model.js b/addon/-private/system/model/internal-model.js
@@ -493,6 +493,10 @@ export default class InternalModel {
     }
   }
 
+  hasScheduledDestroy() {
+    return !!this._scheduledDestroy;
+  }
+
   cancelDestroy() {
     assert(`You cannot cancel the destruction of an InternalModel once it has already been destroyed`, !this.isDestroyed);
 
@@ -501,6 +505,24 @@ export default class InternalModel {
     this._scheduledDestroy = null;
   }
 
+  // typically, we prefer to async destroy this lets us batch cleanup work.
+  // Unfortunately, some scenarios where that is not possible. Such as:
+  //
+  // ```js
+  // const record = store.find(‘record’, 1);
+  // record.unloadRecord();
+  // store.createRecord(‘record’, 1);
+  // ```
+  //
+  // In those scenarios, we make that model's cleanup work, sync.
+  //
+  destroySync() {
+    if (this._isDematerializing) {
+      this.cancelDestroy();
+    }
+    this._checkForOrphanedInternalModels();
+  }
+
   _checkForOrphanedInternalModels() {
     this._isDematerializing = false;
     this._scheduledDestroy = null;

diff --git a/addon/-private/system/store.js b/addon/-private/system/store.js
@@ -2566,15 +2566,23 @@ Store = Service.extend({
 
     assert(`You can no longer pass a modelClass as the first argument to store._buildInternalModel. Pass modelName instead.`, typeof modelName === 'string');
 
-    let recordMap = this._internalModelsFor(modelName);
+    let internalModels = this._internalModelsFor(modelName);
+    let existingInternalModel = internalModels.get(id);
 
-    assert(`The id ${id} has already been used with another record for modelClass '${modelName}'.`, !id || !recordMap.get(id));
+    if (existingInternalModel && existingInternalModel.hasScheduledDestroy()) {
+      // unloadRecord is async, if one attempts to unload + then sync create,
+      // we must ensure the unload is complete before starting the create
+      existingInternalModel.destroySync();
+      existingInternalModel = null;
+    }
+
+    assert(`The id ${id} has already been used with another record for modelClass '${modelName}'.`, !existingInternalModel);
 
     // lookupFactory should really return an object that creates
     // instances with the injections applied
     let internalModel = new InternalModel(modelName, id, this, data);
 
-    recordMap.add(internalModel, id);
+    internalModels.add(internalModel, id);
 
     return internalModel;
   },

diff --git a/tests/unit/store/unload-test.js b/tests/unit/store/unload-test.js
@@ -100,6 +100,18 @@ test('unload a record', function(assert) {
   });
 });
 
+test('unload followed by create of the same type + id', function(assert) {
+  let record = run(() => store.createRecord('record', { id: 1 }));
+
+  assert.ok(store.recordForId('record', 1) === record, 'record should exactly equal');
+
+  return run(() => {
+    record.unloadRecord();
+    let createdRecord = store.createRecord('record', { id: 1 });
+    assert.ok(record !== createdRecord, 'newly created record is fresh (and was created)');
+  });
+});
+
 module("DS.Store - unload record with relationships");
 
 test('can commit store after unload record with relationships', function(assert) {