From 43498037beaaa0a005f2dcecbc25811fac0a2054 Mon Sep 17 00:00:00 2001 From: stroblme Date: Thu, 15 Oct 2026 10:07:50 +0200 Subject: [PATCH] PUT /panels/ carries a version and refuses a stale one with 409 The Panels dialog re-reads the list on 409. A panels.json without a version reads as 0. --- backend/fluksio/api/routes/panels.py | 25 ++++++++++-- backend/fluksio/flow/panels.py | 3 ++ backend/tests/api/routes/test_panels.py | 39 ++++++++++++++++++- docs/code/api.md | 2 +- frontend/src/client/schemas.gen.ts | 10 +++++ frontend/src/client/sdk.gen.ts | 5 +++ frontend/src/client/types.gen.ts | 2 + .../src/components/Dashboard/PanelsDialog.tsx | 14 +++++-- frontend/src/components/Dashboard/queries.ts | 13 ++++++- frontend/tests/panel.spec.ts | 5 ++- 10 files changed, 108 insertions(+), 10 deletions(-) diff --git a/backend/fluksio/api/routes/panels.py b/backend/fluksio/api/routes/panels.py index 56ef9c7..8cb6b78 100644 --- a/backend/fluksio/api/routes/panels.py +++ b/backend/fluksio/api/routes/panels.py @@ -269,6 +269,8 @@ class PanelsPublic(BaseModel): """ panels: list[PanelDef] = Field(default_factory=list) + #: What a save of this list has to send back. + version: int = 0 #: Whatever this instance was told it is reachable at. The same setting #: the password-reset links are built from, so an instance that has it #: wrong has it wrong in both places. @@ -277,7 +279,9 @@ class PanelsPublic(BaseModel): def _public(config: PanelsConfig) -> PanelsPublic: return PanelsPublic( - panels=config.panels, frontend_host=settings.FRONTEND_HOST.rstrip("/") + panels=config.panels, + version=config.version, + frontend_host=settings.FRONTEND_HOST.rstrip("/"), ) @@ -298,6 +302,11 @@ async def save_panels(body: PanelsConfig) -> Any: A panel that disappears here takes its credential with it, so this is also how a device is unpaired along with its assignment. Re-pairing one screen and keeping the assignment is ``/{panel_id}/unpair`` below. + + The body carries the ``version`` it was based on, and a save someone else + has moved past is refused with 409, as a dashboard's is. Two clients each + writing back the whole list would otherwise drop each other's panels, and + a dropped panel revokes its screen at once. """ seen = set() for panel in body.panels: @@ -314,9 +323,19 @@ async def save_panels(body: PanelsConfig) -> Any: # saving an arrangement — and reading it in a separate step from # writing it is exactly how an unpair in between was undone. with edit_lock: - stored = {p.id: p.nonce for p in read_config().panels} + stored = read_config() + if body.version != stored.version: + raise HTTPException( + status_code=409, + detail={ + "message": "Someone else saved the panels first", + "current_version": stored.version, + }, + ) + nonces = {p.id: p.nonce for p in stored.panels} for panel in body.panels: - panel.nonce = stored.get(panel.id, 0) + panel.nonce = nonces.get(panel.id, 0) + body.version = stored.version + 1 write_config(body) await run_in_threadpool(_save) diff --git a/backend/fluksio/flow/panels.py b/backend/fluksio/flow/panels.py index f8f62a9..91bc4f4 100644 --- a/backend/fluksio/flow/panels.py +++ b/backend/fluksio/flow/panels.py @@ -96,6 +96,9 @@ class PanelsConfig(BaseModel): """Every panel this instance knows about.""" panels: list[PanelDef] = Field(default_factory=list) + #: Bumped on every save; a save based on an older one is refused, as a + #: dashboard's is. A file written before there was one reads as 0. + version: int = 0 #: Held across a read-modify-write of the panels file. diff --git a/backend/tests/api/routes/test_panels.py b/backend/tests/api/routes/test_panels.py index a2622ed..bde5a16 100644 --- a/backend/tests/api/routes/test_panels.py +++ b/backend/tests/api/routes/test_panels.py @@ -11,7 +11,11 @@ DASHBOARDS = f"{settings.API_V1_STR}/dashboards" def _panels(client: TestClient, headers: dict[str, str], config: dict) -> None: - response = client.put(f"{PREFIX}/", headers=headers, json=config) + """Replace the panels, based on whatever is stored now.""" + version = client.get(f"{PREFIX}/", headers=headers).json()["version"] + response = client.put( + f"{PREFIX}/", headers=headers, json={**config, "version": version} + ) assert response.status_code == 200, response.text @@ -130,6 +134,39 @@ def test_duplicate_panel_is_refused( assert response.status_code == 422 +def test_a_stale_save_is_refused( + client: TestClient, superuser_token_headers: dict[str, str] +) -> None: + """Two clients writing back the whole list must not drop each other's panels.""" + read = client.get(f"{PREFIX}/", headers=superuser_token_headers).json() + base = read["version"] + + saved = client.put( + f"{PREFIX}/", + headers=superuser_token_headers, + json={"panels": [*read["panels"], {"id": "mine"}], "version": base}, + ) + assert saved.status_code == 200, saved.text + assert saved.json()["version"] == base + 1 + + stale = client.put( + f"{PREFIX}/", + headers=superuser_token_headers, + json={"panels": [*read["panels"], {"id": "theirs"}], "version": base}, + ) + assert stale.status_code == 409 + assert stale.json()["detail"]["current_version"] == base + 1 + + ids = [ + p["id"] + for p in client.get(f"{PREFIX}/", headers=superuser_token_headers).json()[ + "panels" + ] + ] + assert "mine" in ids + assert "theirs" not in ids + + def test_polling_needs_the_secret(client: TestClient) -> None: started = client.post(f"{PREFIX}/pair").json() assert len(started["code"]) == 6 diff --git a/docs/code/api.md b/docs/code/api.md index a418e09..16a8a50 100644 --- a/docs/code/api.md +++ b/docs/code/api.md @@ -177,7 +177,7 @@ stored once. Node code uses `save_artifact` and `load_artifact` instead. | `POST` | `/dashboards/from-flow/{flow}` | create a batch flow's results dashboard | | `PUT` | `/dashboards/{name}` | save a draft | | `POST` | `/dashboards/{name}/publish`, `/dashboards/{name}/discard`, `/dashboards/{name}/rename` | publish, discard, or rename | -| `GET`, `PUT` | `/panels/` | read or update panel assignments | +| `GET`, `PUT` | `/panels/` | read or replace panel assignments; send the current `version` | | `POST` | `/panels/pair` | start device pairing | | `GET` | `/panels/pair/{code}` | poll pairing with its `secret` | | `POST` | `/panels/{id}/pair` | approve the pairing | diff --git a/frontend/src/client/schemas.gen.ts b/frontend/src/client/schemas.gen.ts index 46f8c6a..9276ad3 100644 --- a/frontend/src/client/schemas.gen.ts +++ b/frontend/src/client/schemas.gen.ts @@ -2933,6 +2933,11 @@ export const PanelsConfigSchema = { }, type: 'array', title: 'Panels' + }, + version: { + type: 'integer', + title: 'Version', + default: 0 } }, type: 'object', @@ -2949,6 +2954,11 @@ export const PanelsPublicSchema = { type: 'array', title: 'Panels' }, + version: { + type: 'integer', + title: 'Version', + default: 0 + }, frontend_host: { type: 'string', title: 'Frontend Host', diff --git a/frontend/src/client/sdk.gen.ts b/frontend/src/client/sdk.gen.ts index 66ce387..48b1be5 100644 --- a/frontend/src/client/sdk.gen.ts +++ b/frontend/src/client/sdk.gen.ts @@ -1946,6 +1946,11 @@ export class PanelsService { * A panel that disappears here takes its credential with it, so this is also * how a device is unpaired along with its assignment. Re-pairing one screen * and keeping the assignment is ``/{panel_id}/unpair`` below. + * + * The body carries the ``version`` it was based on, and a save someone else + * has moved past is refused with 409, as a dashboard's is. Two clients each + * writing back the whole list would otherwise drop each other's panels, and + * a dropped panel revokes its screen at once. * @param data The data for the request. * @param data.requestBody * @returns PanelsPublic Successful Response diff --git a/frontend/src/client/types.gen.ts b/frontend/src/client/types.gen.ts index 7258c8b..5e02060 100644 --- a/frontend/src/client/types.gen.ts +++ b/frontend/src/client/types.gen.ts @@ -994,6 +994,7 @@ export type direction = 'in' | 'out'; */ export type PanelsConfig = { panels?: Array; + version?: number; }; /** @@ -1009,6 +1010,7 @@ export type PanelsConfig = { */ export type PanelsPublic = { panels?: Array; + version?: number; frontend_host?: string; }; diff --git a/frontend/src/components/Dashboard/PanelsDialog.tsx b/frontend/src/components/Dashboard/PanelsDialog.tsx index 255d3f0..6eba6e1 100644 --- a/frontend/src/components/Dashboard/PanelsDialog.tsx +++ b/frontend/src/components/Dashboard/PanelsDialog.tsx @@ -107,9 +107,17 @@ export function PanelsDialog() { const host = config?.frontend_host ?? "" const write = (next: PanelsConfig) => - save.mutate(next, { - onError: (error) => handleError.call(showErrorToast, error as ApiError), - }) + save.mutate( + { ...next, version: config?.version }, + { + onError: (error) => + (error as ApiError).status === 409 + ? showErrorToast( + "Someone else changed the panels first. Their version is shown now, so make the change again.", + ) + : handleError.call(showErrorToast, error as ApiError), + }, + ) const replace = (id: string, panel: PanelDef) => write({ panels: panels.map((p) => (p.id === id ? panel : p)) }) diff --git a/frontend/src/components/Dashboard/queries.ts b/frontend/src/components/Dashboard/queries.ts index fffca1d..4cc77bb 100644 --- a/frontend/src/components/Dashboard/queries.ts +++ b/frontend/src/components/Dashboard/queries.ts @@ -1,6 +1,7 @@ import { useMutation, useQueryClient } from "@tanstack/react-query" import { + ApiError, type DashboardDef_Input, DashboardsService, MessagesService, @@ -55,7 +56,13 @@ export const panelQueryOptions = (id: string) => ({ queryFn: () => PanelsService.readPanel({ panelId: id }), }) -/** Replace the panels. Superuser-only on the server. */ +/** + * Replace the panels, carrying the version they were read at. Superuser-only + * on the server. + * + * A 409 means someone else saved first, so the list is read again and the + * next change is based on theirs. + */ export function useSavePanels() { const queryClient = useQueryClient() return useMutation({ @@ -65,6 +72,10 @@ export function useSavePanels() { queryClient.setQueryData(panelKeys.all, saved) queryClient.invalidateQueries({ queryKey: panelKeys.all }) }, + onError: (error) => { + if (error instanceof ApiError && error.status === 409) + queryClient.invalidateQueries({ queryKey: panelKeys.all }) + }, }) } diff --git a/frontend/tests/panel.spec.ts b/frontend/tests/panel.spec.ts index 24f5da3..8632086 100644 --- a/frontend/tests/panel.spec.ts +++ b/frontend/tests/panel.spec.ts @@ -107,6 +107,7 @@ test.beforeAll(async ({ browser }) => { ...existing, { id: flowName, title: "Hall", dashboards: [first, second] }, ], + version: config.version, }, }) await page.close() @@ -116,7 +117,8 @@ test.afterAll(async ({ browser }) => { // Restoring the list is also what removes this spec's own panel. if (panels) { const page = await apiPage(browser) - await api(page, "/panels/", { method: "PUT", data: { panels } }) + const { version } = await (await api(page, "/panels/")).json() + await api(page, "/panels/", { method: "PUT", data: { panels, version } }) await page.close() } await deleteAll(browser, [ @@ -240,6 +242,7 @@ async function setTouch(page: Page, touch: boolean) { panels: (config.panels ?? []).map((panel: PanelDef) => panel.id === flowName ? { ...panel, touch } : panel, ), + version: config.version, }, }) }