mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-09-21 00:02:30 +00:00
fix: settle the key before a forced resolution, and cover quiesce with tests
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
20e4424925
commit
0b3fa66b64
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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<T = void>() {
|
||||
let resolve!: (v: T) => void
|
||||
const promise = new Promise<T>((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()
|
||||
})
|
||||
})
|
||||
Reference in New Issue
Block a user