Skip to content

Commit

Permalink
[BUGFIX release] restore unloadRecord + createRecord
Browse files Browse the repository at this point in the history
The following should be possible:

```js
const record = store.find(‘record’, 1);
record.unloadRecord();
store.createRecord(‘record’, 1);
```

before this commit, the `createRecord` would fail,
as the `record` would not yet be fully unloaded, 
and its ID would still be reserved.
  • Loading branch information
stefanpenner committed Jul 27, 2017
1 parent 4a366a1 commit e19de57
Show file tree
Hide file tree
Showing 3 changed files with 45 additions and 3 deletions.
22 changes: 22 additions & 0 deletions addon/-private/system/model/internal-model.js
Original file line number Diff line number Diff line change
Expand Up @@ -497,6 +497,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);

Expand All @@ -505,6 +509,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;
Expand Down
14 changes: 11 additions & 3 deletions addon/-private/system/store.js
Original file line number Diff line number Diff line change
Expand Up @@ -2558,15 +2558,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;
},
Expand Down
12 changes: 12 additions & 0 deletions tests/unit/store/unload-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down

0 comments on commit e19de57

Please sign in to comment.