diff --git a/frontend/src/lib/components/ResourceEditor.svelte b/frontend/src/lib/components/ResourceEditor.svelte index e9e8adfcaa..3e6253382c 100644 --- a/frontend/src/lib/components/ResourceEditor.svelte +++ b/frontend/src/lib/components/ResourceEditor.svelte @@ -287,10 +287,20 @@ resolvingConflict = true try { if (keepMine) { + // Settle the key first: an ordinary autosave still queued would displace the forced + // write below, and being conditional it would be refused — so "Keep mine" would + // finish without keeping anything and leave the alert standing. + await UserDraftDbSyncer.quiesce(query) + if (selected !== ws || initialPath !== p) return // Forced, so it goes over the row that refused us, and its response reseeds // `last_sync` so the next ordinary save is conditional again. const mine = states[ws]?.draft if (mine) await UserDraftDbSyncer.overwrite({ ...query, value: $state.snapshot(mine) }) + // Say so rather than leave the alert up with no explanation: a write displaced by + // something typed meanwhile can still lose the race. + if (UserDraftDbSyncer.getConflict(query).conflict) { + sendUserToast('Could not keep your version — try again', true) + } return } // Read BEFORE giving anything up: until the server has answered, the refused payload is diff --git a/frontend/src/lib/components/VariableEditor.svelte b/frontend/src/lib/components/VariableEditor.svelte index 9356701d02..2664d7094d 100644 --- a/frontend/src/lib/components/VariableEditor.svelte +++ b/frontend/src/lib/components/VariableEditor.svelte @@ -150,10 +150,20 @@ resolvingConflict = true try { if (keepMine) { + // Settle the key first: an ordinary autosave still queued would displace the forced + // write below, and being conditional it would be refused — so "Keep mine" would + // finish without keeping anything and leave the alert standing. + await UserDraftDbSyncer.quiesce(query) + if (selected !== ws || editPath !== p) return // Forced, so it goes over the row that refused us, and its response reseeds // `last_sync` so the next ordinary save is conditional again. const mine = states[ws]?.draft if (mine) await UserDraftDbSyncer.overwrite({ ...query, value: $state.snapshot(mine) }) + // Say so rather than leave the alert up with no explanation: a write displaced by + // something typed meanwhile can still lose the race. + if (UserDraftDbSyncer.getConflict(query).conflict) { + sendUserToast('Could not keep your version — try again', true) + } return } // Read BEFORE giving anything up: until the server has answered, the refused payload is diff --git a/frontend/src/lib/userDraftQuiesce.test.ts b/frontend/src/lib/userDraftQuiesce.test.ts new file mode 100644 index 0000000000..3801af55d5 --- /dev/null +++ b/frontend/src/lib/userDraftQuiesce.test.ts @@ -0,0 +1,81 @@ +import { describe, it, expect, afterEach, vi } from 'vitest' + +// Mocked so a test can hold a POST in flight: that window is the whole point of `quiesce`, +// which exists to wait one out rather than return while it is still going. +const updateDraft = vi.fn(async (..._args: any[]) => ({ + status: 'saved' as const, + current_timestamp: '2020-01-01T00:00:00Z' +})) + +vi.mock('./gen', () => ({ + DraftService: { updateDraft: (...a: unknown[]) => updateDraft(...(a as [])) } +})) +vi.mock('./gen/core/OpenAPI', () => ({ OpenAPI: { BASE: '' } })) +vi.mock('./localDraftHints.svelte', () => ({ setLocalDraftHint: vi.fn() })) + +import { UserDraftDbSyncer } from './userDraftDbSyncer.svelte' + +function deferred() { + let resolve!: (v: T) => void + const promise = new Promise((res) => (resolve = res)) + return { promise, resolve } +} + +afterEach(() => { + vi.clearAllMocks() + updateDraft.mockResolvedValue({ status: 'saved', current_timestamp: '2020-01-01T00:00:00Z' }) +}) + +/** + * Resolving a draft conflict installs a fresh baseline. Anything of the old version still in the + * air when that lands would be made acceptable by it, and a rejected one re-raises the conflict + * just resolved — so the resolution waits for the key to go quiet first. + */ +describe('UserDraftDbSyncer.quiesce', () => { + it('waits for a save already in flight, and drops what it parks', async () => { + const q = { workspace: 'w', itemKind: 'variable' as const, path: 'u/me/quiesce_a' } + const inFlight = deferred() + let settledBeforeQuiesceReturned = false + updateDraft.mockImplementationOnce(async () => { + await inFlight.promise + settledBeforeQuiesceReturned = true + return { status: 'saved', current_timestamp: '2020-01-01T00:00:00Z' } + }) + + // In flight, not awaited: the POST is held open by the mock above. + const saving = UserDraftDbSyncer.save({ ...q, value: { v: 'in flight' }, immediate: true }) + + let quiesced = false + const quiescing = UserDraftDbSyncer.quiesce(q).then(() => (quiesced = true)) + // A full macrotask, not a microtask: anything that merely yields would have resolved by + // now, so this is what tells "waited for the POST" apart from "waited for nothing". + await new Promise((r) => setTimeout(r, 0)) + expect(quiesced).toBe(false) + + inFlight.resolve() + await quiescing + await saving + expect(settledBeforeQuiesceReturned).toBe(true) + + // Whatever that save left parked belonged to the version being replaced, so a later flush + // has nothing to send — the observable form of "dropped", since the parked payload itself + // is private to the syncer. + updateDraft.mockClear() + await UserDraftDbSyncer.flush(q) + expect(updateDraft).not.toHaveBeenCalled() + }) + + it('leaves nothing queued for a later flush to send', async () => { + const q = { workspace: 'w', itemKind: 'variable' as const, path: 'u/me/quiesce_b' } + // Debounced rather than immediate: this is the autosave a resolution has to call off. + void UserDraftDbSyncer.save({ ...q, value: { v: 'queued' } }) + + await UserDraftDbSyncer.quiesce(q) + updateDraft.mockClear() + + // A flush after quiescing has nothing to send: the queued payload is gone, not merely + // deferred, so it cannot land on top of the version the user chose. + await UserDraftDbSyncer.flush(q) + expect(updateDraft).not.toHaveBeenCalled() + }) +})