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]

(cherry picked from commit 7444444)
  • Loading branch information
hjdivad authored and bmac committed Mar 19, 2018
1 parent 651f00b commit ba990cf
Show file tree
Hide file tree
Showing 2 changed files with 121 additions and 0 deletions.
12 changes: 12 additions & 0 deletions addon/-private/system/store.js
Original file line number Diff line number Diff line change
Expand Up @@ -857,6 +857,18 @@ Store = Service.extend({
seeking[internalModel.id] = pendingItem;
}

for (let i = 0; i < totalItems; i++) {
let internalModel = internalModels[i];
// We may have unloaded the record after scheduling this fetch, in which
// case we must cancel the destory. This is because we require a record
// to build a snapshot. This is not fundamental: this cancelation code
// can be removed when snapshots can be created for internal models that
// have no records.
if (internalModel.hasScheduledDestroy()) {
internalModels[i].cancelDestroy();
}
}

function _fetchRecord(recordResolverPair) {
let recordFetch = store._fetchRecord(
recordResolverPair.internalModel,
Expand Down
109 changes: 109 additions & 0 deletions tests/integration/records/unload-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -2008,3 +2008,112 @@ test('1 sync : many async unload sync side', function(assert) {
})
);
});

test('unload invalidates link promises', function(assert) {
let isUnloaded = false;
env.adapter.coalesceFindRequests = false;

env.adapter.findRecord = (/* store, type, id */) => {
assert.notOk('Records only expected to be loaded via link');
};

env.adapter.findHasMany = (store, snapshot, link) => {
assert.equal(snapshot.modelName, 'person', 'findHasMany(_, snapshot) is correct');
assert.equal(link, 'boats', 'findHasMany(_, _, link) is correct');

let relationships = {
person: {
data: {
type: 'person',
id: 1
}
}
};

let data = [
{
id: 3,
type: 'boat',
relationships
}
];

if (!isUnloaded) {
data.unshift({
id: 2,
type: 'boat',
relationships
});
}

return {
data
};
};

let person = run(() =>
env.store.push({
data: {
id: 1,
type: 'person',
relationships: {
boats: {
links: { related: 'boats' }
}
}
}
})
);
let boats, boat2, boat3;

return run(() =>
person.get('boats').then((asyncRecords) => {
boats = asyncRecords;
[boat2, boat3] = boats.toArray();
}).then(() => {
assert.deepEqual(person.hasMany('boats').ids(), ['2', '3'], 'initially relationship established rhs');
assert.equal(boat2.belongsTo('person').id(), '1', 'initially relationship established rhs');
assert.equal(boat3.belongsTo('person').id(), '1', 'initially relationship established rhs');

isUnloaded = true;
run(() => {
boat2.unloadRecord();
person.get('boats');
});

assert.deepEqual(boats.mapBy('id'), ['3'], 'unloaded boat is removed from ManyArray');
}).then(() => {
return run(() => person.get('boats'));
}).then(newBoats => {
assert.equal(newBoats.length, 1, 'new ManyArray has only 1 boat after unload');
})
);
});

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 ba990cf

Please sign in to comment.