diff --git a/backend/.sqlx/query-2cb84c274a3e8f7c6ec91c5d86b885dac4de6171463ac0d1c45909da9f91b3a4.json b/backend/.sqlx/query-2cb84c274a3e8f7c6ec91c5d86b885dac4de6171463ac0d1c45909da9f91b3a4.json new file mode 100644 index 0000000000..19fbdd36f9 --- /dev/null +++ b/backend/.sqlx/query-2cb84c274a3e8f7c6ec91c5d86b885dac4de6171463ac0d1c45909da9f91b3a4.json @@ -0,0 +1,59 @@ +{ + "db_name": "PostgreSQL", + "query": "DELETE FROM draft\n WHERE workspace_id = $1\n AND email = $2\n AND path = $3\n AND typ = $4\n AND ($6::bool = true\n OR $5::timestamptz IS NULL\n OR created_at <= $5::timestamptz)\n RETURNING now() as \"now!\"", + "describe": { + "columns": [ + { + "ordinal": 0, + "name": "now!", + "type_info": "Timestamptz" + } + ], + "parameters": { + "Left": [ + "Text", + "Text", + "Text", + { + "Custom": { + "name": "draft_kind", + "kind": { + "Enum": [ + "script", + "flow", + "app", + "raw_app", + "resource", + "variable", + "trigger_schedule", + "trigger_webhook", + "trigger_default_email", + "trigger_email", + "trigger_http", + "trigger_websocket", + "trigger_postgres", + "trigger_kafka", + "trigger_nats", + "trigger_mqtt", + "trigger_sqs", + "trigger_gcp", + "trigger_azure", + "trigger_poll", + "trigger_cli", + "trigger_nextcloud", + "trigger_google", + "trigger_github" + ] + } + } + }, + "Timestamptz", + "Bool" + ] + }, + "nullable": [ + null + ] + }, + "hash": "2cb84c274a3e8f7c6ec91c5d86b885dac4de6171463ac0d1c45909da9f91b3a4" +} diff --git a/backend/.sqlx/query-8be291d84471ff742a3c2a9d53cda55f57b4de71db778642ed72654bf47d26a5.json b/backend/.sqlx/query-8be291d84471ff742a3c2a9d53cda55f57b4de71db778642ed72654bf47d26a5.json new file mode 100644 index 0000000000..33e7bfe8ec --- /dev/null +++ b/backend/.sqlx/query-8be291d84471ff742a3c2a9d53cda55f57b4de71db778642ed72654bf47d26a5.json @@ -0,0 +1,20 @@ +{ + "db_name": "PostgreSQL", + "query": "SELECT now() as \"now!\"", + "describe": { + "columns": [ + { + "ordinal": 0, + "name": "now!", + "type_info": "Timestamptz" + } + ], + "parameters": { + "Left": [] + }, + "nullable": [ + null + ] + }, + "hash": "8be291d84471ff742a3c2a9d53cda55f57b4de71db778642ed72654bf47d26a5" +} diff --git a/backend/.sqlx/query-b7cc4786681bc370f9e5bc7bbf8eea7af6cc345d3d2b6bcf35921c6620635de4.json b/backend/.sqlx/query-e0cc7528f34cca1a65bcff355805057b1c974a9a947bda133c155503dac1f545.json similarity index 73% rename from backend/.sqlx/query-b7cc4786681bc370f9e5bc7bbf8eea7af6cc345d3d2b6bcf35921c6620635de4.json rename to backend/.sqlx/query-e0cc7528f34cca1a65bcff355805057b1c974a9a947bda133c155503dac1f545.json index 9f996a8276..c077375ef4 100644 --- a/backend/.sqlx/query-b7cc4786681bc370f9e5bc7bbf8eea7af6cc345d3d2b6bcf35921c6620635de4.json +++ b/backend/.sqlx/query-e0cc7528f34cca1a65bcff355805057b1c974a9a947bda133c155503dac1f545.json @@ -1,6 +1,6 @@ { "db_name": "PostgreSQL", - "query": "INSERT INTO draft (workspace_id, email, path, typ, value, created_at)\n VALUES ($1, $2, $3, $4, $5::text::json, now())\n ON CONFLICT (workspace_id, path, typ, email) WHERE email IS NOT NULL\n DO UPDATE SET value = EXCLUDED.value, created_at = now()\n WHERE $7::bool = true\n OR $6::timestamptz IS NULL\n OR draft.created_at <= $6::timestamptz\n RETURNING created_at", + "query": "INSERT INTO draft (workspace_id, email, path, typ, value, created_at)\n VALUES ($1, $2, $3, $4, $5::text::json, now())\n ON CONFLICT (workspace_id, path, typ, email) WHERE email IS NOT NULL\n DO UPDATE SET value = EXCLUDED.value, created_at = now()\n WHERE $7::bool = true\n OR $6::timestamptz IS NULL\n OR draft.created_at <= $6::timestamptz\n RETURNING created_at", "describe": { "columns": [ { @@ -56,5 +56,5 @@ false ] }, - "hash": "b7cc4786681bc370f9e5bc7bbf8eea7af6cc345d3d2b6bcf35921c6620635de4" + "hash": "e0cc7528f34cca1a65bcff355805057b1c974a9a947bda133c155503dac1f545" } diff --git a/backend/.sqlx/query-d3b5502806d9184b9404c3576c17ff4696b9e2016f31e56beb883e8bef34663e.json b/backend/.sqlx/query-ee783aeeb2eba7446995ca8467ff271cc64e5a3c9901c2e2f806ab3acbb4aa75.json similarity index 85% rename from backend/.sqlx/query-d3b5502806d9184b9404c3576c17ff4696b9e2016f31e56beb883e8bef34663e.json rename to backend/.sqlx/query-ee783aeeb2eba7446995ca8467ff271cc64e5a3c9901c2e2f806ab3acbb4aa75.json index bb05f3e555..b30207b10e 100644 --- a/backend/.sqlx/query-d3b5502806d9184b9404c3576c17ff4696b9e2016f31e56beb883e8bef34663e.json +++ b/backend/.sqlx/query-ee783aeeb2eba7446995ca8467ff271cc64e5a3c9901c2e2f806ab3acbb4aa75.json @@ -1,6 +1,6 @@ { "db_name": "PostgreSQL", - "query": "SELECT created_at FROM draft\n WHERE workspace_id = $1 AND email = $2 AND path = $3 AND typ = $4", + "query": "SELECT created_at FROM draft\n WHERE workspace_id = $1 AND email = $2 AND path = $3 AND typ = $4", "describe": { "columns": [ { @@ -53,5 +53,5 @@ false ] }, - "hash": "d3b5502806d9184b9404c3576c17ff4696b9e2016f31e56beb883e8bef34663e" + "hash": "ee783aeeb2eba7446995ca8467ff271cc64e5a3c9901c2e2f806ab3acbb4aa75" } diff --git a/backend/windmill-api/openapi.yaml b/backend/windmill-api/openapi.yaml index 8c20a3e9ae..d06fe2f4bd 100644 --- a/backend/windmill-api/openapi.yaml +++ b/backend/windmill-api/openapi.yaml @@ -7897,7 +7897,9 @@ paths: schema: type: object properties: - value: {} + value: + nullable: true + description: Draft content to save. `null` (or omitted) signals a delete — the row is removed under the same conflict rules. last_sync: type: string format: date-time @@ -7905,7 +7907,6 @@ paths: force: type: boolean description: Skip the conflict check and overwrite the server copy. - required: [value] responses: "200": description: save result diff --git a/backend/windmill-api/src/drafts.rs b/backend/windmill-api/src/drafts.rs index 6ff0b016d4..77c6d0fc25 100644 --- a/backend/windmill-api/src/drafts.rs +++ b/backend/windmill-api/src/drafts.rs @@ -34,7 +34,10 @@ pub fn workspaced_service() -> Router { #[derive(Deserialize, Debug)] pub struct SaveDraftRequest { - pub value: sqlx::types::Json>, + /// Draft content to save. `null` (or omitted) signals a delete — the + /// row is removed under the same conflict rules as an upsert. + #[serde(default)] + pub value: Option>>, /// Server timestamp of the client's last known sync for this draft. When /// present and `force` is false, the save is rejected if the server's /// `created_at` is more recent (i.e. another writer moved the row @@ -57,17 +60,19 @@ pub enum SaveDraftStatus { #[derive(Serialize, Debug)] pub struct SaveDraftResponse { pub status: SaveDraftStatus, - /// On `saved`: the new row's `created_at` (the client should remember - /// it as the next `last_sync`). On `conflict`: the existing row's - /// `created_at`, so the client knows what the server has. + /// On `saved`: the timestamp at which the change was applied (the + /// client should remember it as the next `last_sync`). On `conflict`: + /// the existing row's `created_at`, so the client knows what the + /// server has. pub current_timestamp: chrono::DateTime, } -/// Upsert the current user's draft at (workspace, kind, path). Conflict -/// check is done inline via a `WHERE` clause on `DO UPDATE` — when the -/// existing row is newer than `last_sync` (and `force` is false), the -/// statement is a no-op and `RETURNING` yields nothing. The handler then -/// reads the existing row's timestamp to report it back. +/// Apply the current user's draft at (workspace, kind, path). With a +/// non-null `value` this upserts; with `null` (or omitted) it deletes. +/// Either way, the same conflict rule applies: when the existing row is +/// newer than `last_sync` (and `force` is false), the operation is +/// skipped and the response carries `status = conflict` + the server's +/// current timestamp so the client can rebase. async fn save_draft( authed: ApiAuthed, Extension(db): Extension, @@ -77,48 +82,88 @@ async fn save_draft( let email = &authed.email; let path = path.to_path(); - let saved_at = sqlx::query_scalar!( - r#"INSERT INTO draft (workspace_id, email, path, typ, value, created_at) - VALUES ($1, $2, $3, $4, $5::text::json, now()) - ON CONFLICT (workspace_id, path, typ, email) WHERE email IS NOT NULL - DO UPDATE SET value = EXCLUDED.value, created_at = now() - WHERE $7::bool = true - OR $6::timestamptz IS NULL - OR draft.created_at <= $6::timestamptz - RETURNING created_at"#, + let applied_at = if let Some(value) = &req.value { + // Upsert branch. Conflict check rides on a WHERE clause attached + // to DO UPDATE — when the existing row is newer than `last_sync`, + // the statement is a no-op and RETURNING yields nothing. + sqlx::query_scalar!( + r#"INSERT INTO draft (workspace_id, email, path, typ, value, created_at) + VALUES ($1, $2, $3, $4, $5::text::json, now()) + ON CONFLICT (workspace_id, path, typ, email) WHERE email IS NOT NULL + DO UPDATE SET value = EXCLUDED.value, created_at = now() + WHERE $7::bool = true + OR $6::timestamptz IS NULL + OR draft.created_at <= $6::timestamptz + RETURNING created_at"#, + &w_id, + email, + path, + kind as UserDraftItemKind, + serde_json::to_string(value).unwrap(), + req.last_sync, + req.force, + ) + .fetch_optional(&db) + .await? + } else { + // Delete branch. Same conflict rule lifted into the WHERE clause. + // Returns NULL when the row was either too new (conflict) OR + // already absent (idempotent delete) — disambiguated below. + sqlx::query_scalar!( + r#"DELETE FROM draft + WHERE workspace_id = $1 + AND email = $2 + AND path = $3 + AND typ = $4 + AND ($6::bool = true + OR $5::timestamptz IS NULL + OR created_at <= $5::timestamptz) + RETURNING now() as "now!""#, + &w_id, + email, + path, + kind as UserDraftItemKind, + req.last_sync, + req.force, + ) + .fetch_optional(&db) + .await? + }; + + if let Some(ts) = applied_at { + return Ok(Json(SaveDraftResponse { + status: SaveDraftStatus::Saved, + current_timestamp: ts, + })); + } + + // No row affected. Either: + // - the existing row was newer than `last_sync` (conflict), or + // - this was a delete request and no row existed (idempotent ok). + let existing = sqlx::query_scalar!( + r#"SELECT created_at FROM draft + WHERE workspace_id = $1 AND email = $2 AND path = $3 AND typ = $4"#, &w_id, email, path, kind as UserDraftItemKind, - serde_json::to_string(&req.value).unwrap(), - req.last_sync, - req.force, ) .fetch_optional(&db) .await?; - match saved_at { - Some(created_at) => Ok(Json(SaveDraftResponse { - status: SaveDraftStatus::Saved, - current_timestamp: created_at, + match existing { + Some(ts) => Ok(Json(SaveDraftResponse { + status: SaveDraftStatus::Conflict, + current_timestamp: ts, })), + // Delete + nothing-was-there ⇒ report success with server's NOW(). None => { - // No row returned ⇒ the existing row was newer than `last_sync` - // and `force` was false. Surface the server's timestamp so the - // client can rebase its draft (or retry with `force`). - let existing = sqlx::query_scalar!( - r#"SELECT created_at FROM draft - WHERE workspace_id = $1 AND email = $2 AND path = $3 AND typ = $4"#, - &w_id, - email, - path, - kind as UserDraftItemKind, - ) - .fetch_one(&db) - .await?; + let now = sqlx::query_scalar!(r#"SELECT now() as "now!""#) + .fetch_one(&db) + .await?; Ok(Json(SaveDraftResponse { - status: SaveDraftStatus::Conflict, - current_timestamp: existing, + status: SaveDraftStatus::Saved, + current_timestamp: now, })) } } diff --git a/frontend/src/lib/userDraft.svelte.ts b/frontend/src/lib/userDraft.svelte.ts index cd7ba04a14..3ebebc2cd1 100644 --- a/frontend/src/lib/userDraft.svelte.ts +++ b/frontend/src/lib/userDraft.svelte.ts @@ -3,6 +3,8 @@ import { onDestroy, untrack } from 'svelte' import { deepEqual } from 'fast-equals' import { workspaceStore } from './stores' import { useLocalStorageValue } from './svelte5Utils.svelte' +import { readFieldsRecursively } from './utils' +import { UserDraftDbSyncer } from './userDraftDbSyncer.svelte' import type { UserDraftItemKind } from './gen' export type { UserDraftItemKind } @@ -121,6 +123,14 @@ type DraftEntry = { itemKind: UserDraftItemKind path: string state: DraftState + /** + * Single-shot flag consumed by the reactive sync effect in + * `acquireEntry`. Callers that already pushed the right thing to the + * server (e.g. `discard` → explicit `value: null` POST) set this + * before the reactive write so the effect doesn't fire a second, + * incorrect POST. + */ + skipNextSync: boolean /** * Tears down the `$effect.root` scope that owns the entry's * `useLocalStorageValue` reactivity — its `$state` cell and the persist @@ -367,7 +377,9 @@ export const UserDraft = { const entry = entries.get(mk) if (entry) { // Static writes are external mutations. Update live observers and - // force the storage slot to match. + // force the storage slot to match. The DB sync rides on the + // reactive effect in `acquireEntry`, which fires off + // `setWithoutPersist`'s `stateRef.val` update. const current = untrack(() => entry.state.val as StoredDraft | undefined) const meta = extractMeta(current) entry.state.setWithoutPersist(wrap(value, meta)) @@ -384,6 +396,7 @@ export const UserDraft = { } catch (e) { console.error('UserDraft.save: localStorage write failed', e) } + void UserDraftDbSyncer.save({ workspace: ws, itemKind, path, value }) } }, @@ -400,12 +413,18 @@ export const UserDraft = { if (entry) { // Static writes represent explicit external draft mutations. A // freshly acquired live entry may still have the initial-write skip - // armed, so force the storage slot to match the live value. + // armed, so force the storage slot to match the live value. The DB + // sync rides on the reactive effect in `acquireEntry`. entry.state.setWithoutPersist(wrap(value, meta)) persistDirect(localStorageKey(ws, itemKind, path), value, meta) return } persistDirect(localStorageKey(ws, itemKind, path), value, meta) + // `save_draft` requires a value — skip the sync on `undefined` + // (a delete-via-static-write), which the server route can't represent. + if (value !== undefined) { + void UserDraftDbSyncer.save({ workspace: ws, itemKind, path, value }) + } }, /** @@ -461,13 +480,22 @@ export const UserDraft = { const mk = mapKey(ws, itemKind, path) const entry = entries.get(mk) if (entry) { + // Goes through `entry.state.val =` (not `setWithoutPersist`), so + // the reactive effect in `acquireEntry` picks this up and syncs. const current = untrack(() => entry.state.val as StoredDraft | undefined) if (current === undefined) return entry.state.val = wrap(current.value, meta) + return } const existing = readPersisted(localStorageKey(ws, itemKind, path)) if (existing === undefined) return persistDirect(localStorageKey(ws, itemKind, path), existing.value, meta) + void UserDraftDbSyncer.save({ + workspace: ws, + itemKind, + path, + value: existing.value + }) }, /** @@ -503,6 +531,9 @@ export const UserDraft = { } catch (e) { console.error('UserDraft.remove: localStorage remove failed', e) } + // `remove` doesn't touch the reactive cell, so the `acquireEntry` + // effect won't fire. Push the delete explicitly. + void UserDraftDbSyncer.save({ workspace: ws, itemKind, path, value: null }) }, clear(itemKind: UserDraftItemKind, path: string, opts?: UserDraftOptions): void { @@ -608,6 +639,13 @@ export const UserDraft = { // resetting the in-memory value. Otherwise a timer from the old // entry can outlive unmount and later delete a freshly written // draft for the same key. + // + // `fallback` is the deployed baseline — when defined, it lands + // in the reactive cell so the editor sees the unmodified value. + // Arm `skipNextSync` first so the cell write doesn't fire a + // stray `save_draft({value: fallback})` from the reactive sync + // effect, racing the explicit delete below. + entry.skipNextSync = true entry.state.setWithoutPersist(wrap(fallback) as StoredDraft | undefined) } try { @@ -615,6 +653,7 @@ export const UserDraft = { } catch (e) { console.error('UserDraft.discard: localStorage remove failed', e) } + void UserDraftDbSyncer.save({ workspace: ws, itemKind, path, value: null }) }, use( @@ -743,6 +782,36 @@ function acquireEntry( undefined, useLocalStorageOptions ) + // Mirror every observable change of `stateRef.val` to the DB syncer. + // Reading `stateRef.val` alone only subscribes to the proxy root, so + // deep mutations (`handle.draft.content = '...'`) would slip past; + // `readFieldsRecursively` walks the value so the effect re-fires on + // nested writes too. Skips the very first run because the initial + // `stateRef.val` came from localStorage or `defaultValue`, not from + // a user edit. `stored === undefined` is the delete signal — the + // server route accepts `value: null` for that. `skipNextSync` lets + // callers that already POSTed (e.g. `discard`) suppress a duplicate + // fire from their own reactive write. + let firstRun = true + $effect(() => { + const stored = stateRef!.val + if (stored !== undefined) readFieldsRecursively(stored.value) + if (firstRun) { + firstRun = false + return + } + const entry = entries.get(mk) + if (entry?.skipNextSync) { + entry.skipNextSync = false + return + } + void UserDraftDbSyncer.save({ + workspace, + itemKind, + path, + value: stored === undefined ? null : stored.value + }) + }) }) if (stateRef) { entries.set(mk, { @@ -751,6 +820,7 @@ function acquireEntry( itemKind, path, state: stateRef, + skipNextSync: false, destroyRoot }) return @@ -763,7 +833,7 @@ function acquireEntry( undefined, useLocalStorageOptions ) - entries.set(mk, { count: 1, workspace, itemKind, path, state }) + entries.set(mk, { count: 1, workspace, itemKind, path, state, skipNextSync: false }) } function releaseEntry(mk: string): void { diff --git a/frontend/src/lib/userDraftDbSyncer.svelte.ts b/frontend/src/lib/userDraftDbSyncer.svelte.ts new file mode 100644 index 0000000000..33610fef21 --- /dev/null +++ b/frontend/src/lib/userDraftDbSyncer.svelte.ts @@ -0,0 +1,42 @@ +import { DraftService, type UserDraftItemKind } from './gen' + +export type UserDraftDbSyncerSaveOpts = { + workspace: string + itemKind: UserDraftItemKind + path: string + /** `null` signals a delete — the server removes the row under the same + * conflict rules as an upsert. */ + value: unknown | null +} + +/** + * Server-side persistence for `UserDraft`. `UserDraft` owns the localStorage + * cache; this module forwards each write through to `POST /drafts/save_draft` + * so the per-user `draft` table on the server stays in sync. + * + * Kept as a separate module so the two halves stay decoupled — `UserDraft` + * just calls `UserDraftDbSyncer.save(...)` and doesn't reach into the + * generated client. Adding conflict handling (`last_sync` + a reject UI) + * later means changing this file, not every editor. + * + * NOTE: every save currently uses `force: true`, so the server copy is + * unconditionally overwritten. This is intentional for the first cut — + * we'll thread `last_sync` through once the client side is settled. + */ +export const UserDraftDbSyncer = { + async save(opts: UserDraftDbSyncerSaveOpts): Promise { + try { + await DraftService.saveDraft({ + workspace: opts.workspace, + kind: opts.itemKind, + path: opts.path, + requestBody: { + value: opts.value as any, + force: true + } + }) + } catch (e) { + console.error('UserDraftDbSyncer.save failed', e) + } + } +}