mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-10-16 16:03:14 +00:00
fix: keep an in-flight patch's fields on every write of the deployed side
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
1a50765295
commit
89170ab042
@@ -193,8 +193,7 @@ class Entry<V> {
|
||||
waitingOn: Entry<any>[] = []
|
||||
handles = new Set<Handle<V>>()
|
||||
stopWatch: (() => void) | undefined
|
||||
/** Fields a `patch` has put on the deployed side ahead of the server; a save landing
|
||||
* meanwhile keeps them rather than rolling the baseline back past them. */
|
||||
/** Fields a `patch` has put on the deployed side ahead of the server. */
|
||||
private patched: Record<string, unknown> = {}
|
||||
/** The value as last observed, serialized: `touch` acts only on a real change. */
|
||||
private seen: string | undefined
|
||||
@@ -256,6 +255,14 @@ class Entry<V> {
|
||||
this.ports.write(key, desired === null ? null : snapshot(this.value))
|
||||
}
|
||||
|
||||
/** A deployed side the server gave, with the fields a patch is still ahead of it on. Every
|
||||
* write of that side goes through here: rolling the baseline back past an optimistic toggle
|
||||
* makes the value differ from it, which is a draft row of a change nobody made. */
|
||||
private withPatched(deployed: V | undefined): V | undefined {
|
||||
if (deployed === undefined) return undefined
|
||||
return { ...deployed, ...this.patched } as V
|
||||
}
|
||||
|
||||
private replaceValue(value: V | undefined): void {
|
||||
this.value = snapshot(value)
|
||||
this.seen = serialize(this.value)
|
||||
@@ -335,7 +342,7 @@ class Entry<V> {
|
||||
if (!keepValue) this.replaceValue(res.template)
|
||||
return { ok: true }
|
||||
}
|
||||
this.deployed = snapshot(res.deployed)
|
||||
this.deployed = this.withPatched(snapshot(res.deployed))
|
||||
this.origin = res.deployed !== undefined ? 'deployed' : 'draft'
|
||||
this.row = serialize(res.draft) ?? null
|
||||
this.ports.seedSync(key, res.draftSavedAt)
|
||||
@@ -428,7 +435,7 @@ class Entry<V> {
|
||||
wrote = true
|
||||
this.error = undefined
|
||||
if (held !== sent) this.adopt(sent, held)
|
||||
this.deployed = { ...held, ...this.patched } as V
|
||||
this.deployed = this.withPatched(held)
|
||||
this.origin = 'deployed'
|
||||
this.template = undefined
|
||||
if (moved) this.moveTo(to)
|
||||
@@ -575,7 +582,10 @@ class Entry<V> {
|
||||
}
|
||||
settled()
|
||||
this.error = undefined
|
||||
if (this[baseKey] !== undefined) this[baseKey] = { ...this[baseKey], ...sent } as V
|
||||
// `settled` has dropped from `patched` whatever this patch owned, so what is left is a
|
||||
// newer patch of the same field, whose value stands over this one's.
|
||||
if (this[baseKey] !== undefined)
|
||||
this[baseKey] = this.withPatched({ ...this[baseKey], ...sent } as V)
|
||||
this.touch()
|
||||
this.reconcile()
|
||||
return { ok: true }
|
||||
|
||||
@@ -251,6 +251,64 @@ describe('item store: commands', () => {
|
||||
expect(rows.writes.at(-1)).toEqual({ path: 's', value: null })
|
||||
})
|
||||
|
||||
it('lets the later of two overlapping toggles stand, without a row in between', async () => {
|
||||
type Sched = { path: string; enabled: boolean; summary: string }
|
||||
const rows = fakeRows()
|
||||
const first = deferred()
|
||||
const store = createItemStore(rows.port)
|
||||
const { handle: item } = store.acquire(
|
||||
{ workspace: 'w', kind: 'trigger_schedule', path: 's' },
|
||||
{ workspace: 'w', path: 's' },
|
||||
{
|
||||
load: async () => ({ deployed: { path: 's', enabled: false, summary: 'a' } })
|
||||
} as ItemAdapter<Sched>
|
||||
)
|
||||
await settle()
|
||||
const enabling = item.patch({ enabled: true }, () => first.promise)
|
||||
const disabling = item.patch({ enabled: false }, async () => {})
|
||||
first.resolve()
|
||||
|
||||
expect(await enabling).toEqual({ ok: true })
|
||||
// The first request landing must not put its own `enabled` back over the second's.
|
||||
expect(item.dirty).toBe(false)
|
||||
expect(await disabling).toEqual({ ok: true })
|
||||
expect(item.deployed).toEqual({ path: 's', enabled: false, summary: 'a' })
|
||||
expect(item.dirty).toBe(false)
|
||||
expect(rows.writes).toEqual([])
|
||||
})
|
||||
|
||||
it('keeps a toggle made during a read the server answered before it', async () => {
|
||||
type Sched = { path: string; enabled: boolean; summary: string }
|
||||
const rows = fakeRows()
|
||||
const reading = deferred()
|
||||
let reads = 0
|
||||
const store = createItemStore(rows.port)
|
||||
const { handle: item } = store.acquire(
|
||||
{ workspace: 'w', kind: 'trigger_schedule', path: 's' },
|
||||
{ workspace: 'w', path: 's' },
|
||||
{
|
||||
load: async () => {
|
||||
// The read in flight predates the toggle, so it still reports it disabled.
|
||||
if (reads++ > 0) await reading.promise
|
||||
return { deployed: { path: 's', enabled: false, summary: 'a' } }
|
||||
}
|
||||
} as ItemAdapter<Sched>
|
||||
)
|
||||
await settle()
|
||||
const reloading = item.reload()
|
||||
await settle()
|
||||
const toggling = item.patch({ enabled: true }, async () => {})
|
||||
reading.resolve()
|
||||
|
||||
expect(await reloading).toEqual({ ok: true })
|
||||
// The read must not put the schedule back to disabled under the toggle: that reads as an
|
||||
// unsaved change to `enabled` and would be written as a draft.
|
||||
expect(item.dirty).toBe(false)
|
||||
expect(await toggling).toEqual({ ok: true })
|
||||
expect(item.deployed).toEqual({ path: 's', enabled: true, summary: 'a' })
|
||||
expect(rows.writes).toEqual([])
|
||||
})
|
||||
|
||||
it('holds a toggle behind a save, and keeps an unrelated edit through the toggle', async () => {
|
||||
type Sched = { path: string; enabled: boolean; summary: string }
|
||||
const rows = fakeRows()
|
||||
|
||||
Reference in New Issue
Block a user