Skip to content

Commit

Permalink
[bugfix beta] Fetch cancels unload
Browse files Browse the repository at this point in the history
When fetching records, cancel internal model destruction if it is
scheduled, ie if the model is dematerializing because it was unloaded.

It is not possible to construct snapshots for dematerializing internal
models.  Prior to this commit, this could happen when fetching a record
in the same runloop that it was unloaded.  A straightfoward way of
getting into this state was via

```
store.findRecord('book', 1).then(b => b.unloadRecord)
```

when the model is already cached in the store.  Under these conditions,
the fetch is scheduled, then the promise fulfills with the cached record
and is unloaded and *then* the scheduled fetch is flushed.

[fix #5343]
  • Loading branch information
hjdivad committed Mar 13, 2018
1 parent c609ef1 commit 1ae5181
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 0 deletions.
7 changes: 7 additions & 0 deletions addon/-private/system/store.js
Original file line number Diff line number Diff line change
Expand Up @@ -793,6 +793,10 @@ Store = Service.extend({
assert(`You tried to find a record but you have no adapter (for ${modelName})`, adapter);
assert(`You tried to find a record but your adapter (for ${modelName}) does not implement 'findRecord'`, typeof adapter.findRecord === 'function');

if (internalModel.hasScheduledDestroy()) {
internalModel.cancelDestroy();
}

return _find(adapter, this, internalModel.type, internalModel.id, internalModel, options);
},

Expand Down Expand Up @@ -921,6 +925,9 @@ Store = Service.extend({
// will once again convert the records to snapshots for adapter.findMany()
let snapshots = new Array(totalItems);
for (let i = 0; i < totalItems; i++) {
if (internalModels[i].hasScheduledDestroy()) {
internalModels[i].cancelDestroy();
}
snapshots[i] = internalModels[i].createSnapshot();
}

Expand Down
28 changes: 28 additions & 0 deletions tests/integration/records/unload-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -2008,3 +2008,31 @@ test('1 sync : many async unload sync side', function(assert) {
})
);
});

test('fetching records cancels unloading', function(assert) {
env.adapter.findRecord = (store, type, id) => {
assert.equal(type, Person, 'findRecord(_, type) is correct');
assert.deepEqual(id, '1', 'findRecord(_, _, id) is correct');

return {
data: {
id: 1,
type: 'person'
}
}
};

run(() =>
env.store.push({
data: {
id: 1,
type: 'person'
}
})
);

return run(() =>
env.store.findRecord('person', 1, { backgroundReload: true })
.then(person => person.unloadRecord())
);
});

0 comments on commit 1ae5181

Please sign in to comment.