diff --git a/packages/host/app/services/store.ts b/packages/host/app/services/store.ts index 20357df552..50519821ee 100644 --- a/packages/host/app/services/store.ts +++ b/packages/host/app/services/store.ts @@ -965,6 +965,28 @@ export default class StoreService extends Service implements StoreInterface { if (opts?.doNotPersist) { await this.stopAutoSaving(instance); } + // Resolve any linked-card relationships first. This can require a + // network fetch, and a sibling task elsewhere may mutate other fields on + // this same live instance while we wait. Snapshotting the instance below + // (for the merge + write-back further down) only after this resolves + // keeps that snapshot from going stale and reverting the sibling's write. + let linkedCards = await this.loadPatchedInstances(patch, instance.id); + for (let [field, value] of Object.entries(linkedCards)) { + if (field.includes('.')) { + let parts = field.split('.'); + let leaf = parts.pop(); + if (!leaf) { + throw new Error(`bug: error in field name "${field}"`); + } + let inner = instance; + for (let part of parts) { + inner = (inner as any)[part]; + } + (inner as any)[leaf.match(/^\d+$/) ? Number(leaf) : leaf] = value; + } else { + (instance as any)[field] = value; + } + } let doc = await this.cardService.serializeCard(instance, { omitQueryFields: true, }); @@ -988,23 +1010,6 @@ export default class StoreService extends Service implements StoreInterface { if (patch.meta) { doc.data.meta = merge(doc.data.meta, patch.meta); } - let linkedCards = await this.loadPatchedInstances(patch, instance.id); - for (let [field, value] of Object.entries(linkedCards)) { - if (field.includes('.')) { - let parts = field.split('.'); - let leaf = parts.pop(); - if (!leaf) { - throw new Error(`bug: error in field name "${field}"`); - } - let inner = instance; - for (let part of parts) { - inner = (inner as any)[part]; - } - (inner as any)[leaf.match(/^\d+$/) ? Number(leaf) : leaf] = value; - } else { - (instance as any)[field] = value; - } - } let api = await this.cardService.getAPI(); await api.updateFromSerialized(instance, doc, this.store); let shouldPersist = !opts?.doNotPersist; diff --git a/packages/host/tests/integration/store-test.gts b/packages/host/tests/integration/store-test.gts index e5fd238da1..a28e29282f 100644 --- a/packages/host/tests/integration/store-test.gts +++ b/packages/host/tests/integration/store-test.gts @@ -1593,6 +1593,65 @@ module('Integration | Store', function (hooks) { ); }); + test('a concurrent field write during store.patch is not clobbered by the patch’s stale snapshot', async function (assert) { + let targetId = `${testRealmURL}Person/hassan`; + let instance = (await storeService.get(targetId)) as any; + + let gate = new Deferred(); + let enteredGate = new Deferred(); + let originalLoadPatchedInstances = (storeService as any) + .loadPatchedInstances; + (storeService as any).loadPatchedInstances = async function ( + this: unknown, + ...args: any[] + ) { + let result = await originalLoadPatchedInstances.apply(this, args); + // simulate a slow relationship load (e.g. fetching a not-yet-cached + // linked card) so a concurrent field write on the same live instance + // can land in the window between the patch's snapshot and its + // eventual write-back + enteredGate.fulfill(); + await gate.promise; + return result; + }; + + try { + let patchPromise = storeService.patch(targetId, { + relationships: { + bestFriend: { + links: { self: `${testRealmURL}Person/jade` }, + }, + }, + }); + + // wait until the patch has actually reached the gated relationship + // load before mutating a sibling field, so the write lands squarely in + // the window the patch's snapshot needs to still be sensitive to + await enteredGate.promise; + + // a sibling background task (e.g. an unrelated auto-linking step, as in + // the catalog listing-create flow) mutates a different field on the + // same live instance while the patch above is still in flight + instance.name = 'Race Winner'; + + gate.fulfill(); + await patchPromise; + } finally { + (storeService as any).loadPatchedInstances = originalLoadPatchedInstances; + } + + assert.strictEqual( + instance.name, + 'Race Winner', + "the concurrent field write is not clobbered by the patch's stale snapshot", + ); + assert.strictEqual( + instance.bestFriend?.id, + `${testRealmURL}Person/jade`, + 'the patch itself was still applied', + ); + }); + test('loads FileDef links from included resources', async function (assert) { await testRealm.writeMany( new Map([