diff --git a/TESTING.md b/TESTING.md index cf02b9d..3ee1330 100644 --- a/TESTING.md +++ b/TESTING.md @@ -1685,6 +1685,47 @@ Excalidraw ist MIT. draw.io waere die Alternative (Apache-2.0), zielt aber auf Flussdiagramme und UML statt auf freies Zeichnen – und bringt eine deutlich schwerere Oberflaeche mit. +### Feinschliff: zwei stille Wege in den Datenverlust (30.09.2026) + +Beim Durchlesen mit fremdem Blick gefunden, beide nachgestellt. + +**Ein Fehler beim Laden ergab ein leeres Zeichenbrett.** `fetch` wirft bei 404 +oder 403 **nicht**. Der Rumpf `{"error":"file not found"}` ist gueltiges JSON, +`JSON.parse` gelang, `elements` war `undefined` – und die Zeichnung ging leer +auf. Sie *sah aus*, als waere sie verloren. Wer dann speichert, macht es wahr. + +Nachgestellt gegen die laufende Instanz: + + GET /api/v1/files//download?inline=1 + Status: 404 + Rumpf : {"error":"file not found"} + JSON.parse gelingt -> ['error'] d.elements -> None + +**Weiterzeichnen waehrend des Speicherns gingen verloren.** Der Upload lief mit +dem Stand von Sekunde 0; die Fassungsnummer wurde hinterher aus +`aktuell.current` gelesen – und das war inzwischen ein anderer Stand. Die neuen +Striche galten als gespeichert, ohne je hochgegangen zu sein, und verschwanden +beim Schliessen ohne Rueckfrage. + +Jetzt wird der Stand **einmal festgehalten**: Er geht hoch, seine Nummer wird +vermerkt, und das Vorschaubild zeigt denselben. Ist inzwischen weitergezeichnet +worden, steht „Unsaved" sofort wieder da – richtig so. + +Das Laden steht dafuer jetzt in `ladeZeichnung()` statt im Effekt: Der Weg hat +drei Fallen, und zwei enden in verlorener Arbeit – das gehoert pruefbar. + +- [x] Vier Tests fuer das Laden, unter Mutation nachgewiesen: Ohne die + Statuspruefung werden zwei davon rot. + +### Was bewusst so bleibt + +**Leserecht faellt erst beim Speichern auf.** `FileItem` traegt kein +Rechtefeld; die Oberflaeche kann vorher nicht unterscheiden. Wer eine geliehene +Zeichnung nur lesen darf, kann darin zeichnen und erfaehrt es beim Speichern – +mit dem Satz des Servers. Das Zeichenbrett bleibt dabei offen, die Arbeit ist +also nicht weg. Genauso verhaelt sich der Text-Editor seit jeher; es einzeln zu +loesen hiesse, einen Rechte-Endpunkt nur fuer diesen Fall zu bauen. + ### Drei Nachbesserungen aus dem Testen (29.09.2026) **„Close without saving?" kam, obwohl gerade gespeichert war.** Der Schalter diff --git a/frontend/src/components/DrawingEditor.tsx b/frontend/src/components/DrawingEditor.tsx index 48cd70b..62e4a16 100644 --- a/frontend/src/components/DrawingEditor.tsx +++ b/frontend/src/components/DrawingEditor.tsx @@ -1,7 +1,7 @@ import { useCallback, useEffect, useRef, useState } from 'react' import { Excalidraw, exportToBlob, getSceneVersion, restore, serializeAsJSON } from '@excalidraw/excalidraw' import '@excalidraw/excalidraw/index.css' -import { filesApi, type FileItem } from '../api/client' +import { APIError, filesApi, type FileItem } from '../api/client' import { useBackClose } from '../hooks/useBackClose' import { FileTypeIcon } from './icons' import ConfirmDialog from './ConfirmDialog' @@ -53,6 +53,39 @@ export function leereZeichnung(): string { export const drawingExtension = '.excalidraw' +/** + * Eine Zeichnung holen und in einen Zustand verwandeln, den das Zeichenbrett + * annimmt. + * + * Steht hier und nicht im Effekt, damit es sich prüfen lässt – der Weg hat + * drei Fallen, und zwei davon enden in verlorener Arbeit. + */ +export async function ladeZeichnung(url: string) { + const r = await fetch(url, { credentials: 'include' }) + // **`fetch` wirft bei 404 oder 403 nicht.** Ohne diese Zeile käme der + // Fehlerrumpf `{"error":"file not found"}` weiter, `JSON.parse` gelänge – + // es ist gültiges JSON – und `elements` wäre `undefined`. Das Zeichenbrett + // ginge **leer** auf; es sähe aus, als sei die Zeichnung verloren. Wer dann + // speichert, macht es wahr. + if (!r.ok) throw new APIError(r.status, 'could not load the drawing') + + const text = await r.text() + // Eine frisch angelegte Zeichnung ist leer – das ist kein Fehler. + const d = JSON.parse(text.trim() || leereZeichnung()) + + // `restore` säubert und ergänzt Fehlendes. Es ist der Weg, den Excalidraw + // dafür vorsieht – selbst zu prüfen hiesse, seine Feldliste nachzubauen und + // bei jeder Fassung nachzuziehen. + // + // **Eine Sache erledigt es allerdings nicht** (nachgemessen): Ein bereits + // verunglücktes `collaborators: {}` reicht es unverändert durch – die Map + // entsteht nur, wenn das Feld ganz fehlt. Neue Dateien haben es nicht mehr, + // weil `serializeAsJSON` es gar nicht erst schreibt; die paar aus der ersten + // Stunde wären sonst dauerhaft kaputt. + if (d?.appState) delete d.appState.collaborators + return restore(d, null, null) +} + interface Props { file: FileItem onClose: () => void @@ -91,31 +124,8 @@ export default function DrawingEditor({ file, onClose, onSaved }: Props) { useEffect(() => { let lebt = true - fetch(filesApi.inline(file.id), { credentials: 'include' }) - .then(r => r.text()) - .then(text => { - if (!lebt) return - // Eine frisch angelegte Zeichnung ist leer – das ist kein Fehler. - const d = JSON.parse(text.trim() || leereZeichnung()) - // **Über `restore`, nicht roh hineinreichen.** Der `appState` enthält - // Dinge, die kein JSON überlebt – `collaborators` ist eine Map und - // kommt als `{}` zurück; Excalidraw ruft darauf `forEach` und bricht - // ab. Das Zeichenbrett blieb dann beim **zweiten** Öffnen leer, beim - // ersten nicht, weil da noch nichts gespeichert war. - // - // `restore` säubert und ergänzt Fehlendes. Es ist der Weg, den - // Excalidraw dafür vorsieht – selbst zu prüfen hiesse, seine - // Feldliste nachzubauen und bei jeder Fassung nachzuziehen. - // - // **Eine Sache erledigt `restore` allerdings nicht** (nachgemessen): - // Ein bereits verunglücktes `collaborators: {}` reicht es unverändert - // durch – die Map entsteht nur, wenn das Feld ganz fehlt. Neue - // Dateien haben es nicht mehr, weil `serializeAsJSON` es gar nicht - // erst schreibt. Die paar, die die erste Fassung angelegt hat, wären - // sonst dauerhaft kaputt. Also weg damit, bevor `restore` läuft. - if (d?.appState) delete d.appState.collaborators - setSzene(restore(d, null, null)) - }) + ladeZeichnung(filesApi.inline(file.id)) + .then(szene => { if (lebt) setSzene(szene) }) .catch(err => { if (lebt) setFehler(fehlertext(err, 'Could not open this drawing')) }) return () => { lebt = false } }, [file.id]) @@ -158,21 +168,28 @@ export default function DrawingEditor({ file, onClose, onSaved }: Props) { return } + // **Einmal festhalten, was hochgeht.** Während der Upload läuft, kann + // weitergezeichnet werden – `aktuell.current` ist dann ein anderer + // Stand. Läse man die Fassungsnummer hinterher von dort, gälten die + // neuen Striche als gespeichert, obwohl sie nie hochgingen. Beim + // Schliessen wären sie lautlos weg. + const stand = aktuell.current + // Ebenfalls über das Paket: `serializeAsJSON` schreibt nur die Felder, // die eine Zeichnung ausmachen – nicht den Laufzeitzustand. const inhalt = serializeAsJSON( - aktuell.current.elements as never, - aktuell.current.appState as never, - aktuell.current.files as never, - 'local') + stand.elements as never, stand.appState as never, stand.files as never, 'local') const blob = new File([inhalt], file.name, { type: 'application/json' }) const neu = await filesApi.upload(blob, file.folder_id ?? undefined) basis.current = neu.version - if (aktuell.current) gespeicherteFassung.current = getSceneVersion(aktuell.current.elements as never) - setGeaendert(false) + gespeicherteFassung.current = getSceneVersion(stand.elements as never) + // Ist inzwischen weitergezeichnet worden, steht „Unsaved" sofort wieder + // da – richtig so, es ist ja etwas offen. + const jetzt = aktuell.current ? getSceneVersion(aktuell.current.elements as never) : null + setGeaendert(jetzt !== null && jetzt !== gespeicherteFassung.current) onSaved?.(neu) toast.info('Drawing saved') - void vorschaubild(neu) + void vorschaubild(neu, stand) } catch (err) { toast.error(fehlertext(err, 'Could not save the drawing')) } finally { @@ -192,13 +209,15 @@ export default function DrawingEditor({ file, onClose, onSaved }: Props) { * scheitern zu lassen. Es bleibt deshalb auch stumm – die Zeichnung selbst * ist heil, und eine rote Meldung dafür wäre verwirrend. */ - async function vorschaubild(neu: FileItem) { - if (!aktuell.current) return + async function vorschaubild(neu: FileItem, stand: NonNullable) { try { const bild = await exportToBlob({ - elements: aktuell.current.elements as never, - appState: { ...(aktuell.current.appState as object), exportBackground: true } as never, - files: aktuell.current.files as never, + // Derselbe Stand, der hochgegangen ist – nicht der, der gerade auf + // dem Zeichenbrett liegt. Sonst zeigte die Kachel etwas, das so in + // keiner Fassung steht. + elements: stand.elements as never, + appState: { ...(stand.appState as object), exportBackground: true } as never, + files: stand.files as never, mimeType: 'image/png', exportPadding: 16, // Der Server verkleinert ohnehin auf Kachelgröße. Grösser hochzuladen diff --git a/frontend/src/components/drawing.test.ts b/frontend/src/components/drawing.test.ts index ccd7ff1..3e8a776 100644 --- a/frontend/src/components/drawing.test.ts +++ b/frontend/src/components/drawing.test.ts @@ -12,6 +12,7 @@ HTMLCanvasElement.prototype.getContext = (() => ({ })) as never const { restore, serializeAsJSON } = await import('@excalidraw/excalidraw') +const { ladeZeichnung } = await import('./DrawingEditor') /** * Der Fehler, den dieser Test festhält, ist echt passiert (29.09.2026): @@ -96,4 +97,43 @@ describe('Zeichnungen: Speichern und wieder Öffnen', () => { expect(leer.elements).toHaveLength(0) expect(leer.appState.collaborators instanceof Map).toBe(true) }) + + describe('Laden', () => { + function antwort(status: number, rumpf: string) { + vi.stubGlobal('fetch', async () => ({ + ok: status >= 200 && status < 300, + status, + text: async () => rumpf, + })) + } + + it('macht aus einem Fehlerrumpf KEINE leere Zeichnung', async () => { + // Das war der Fehler: `fetch` wirft bei 404 nicht, der Rumpf + // `{"error":"file not found"}` ist gueltiges JSON, `elements` waere + // undefined – und das Zeichenbrett ginge leer auf. Wer dann speichert, + // ueberschreibt die echte Zeichnung mit nichts. + antwort(404, '{"error":"file not found"}') + await expect(ladeZeichnung('/egal')).rejects.toBeTruthy() + }) + + it('auch 403 fuehrt nicht zu einem leeren Brett', async () => { + antwort(403, '{"error":"you only have read access to this file"}') + await expect(ladeZeichnung('/egal')).rejects.toBeTruthy() + }) + + it('eine leere Datei ist dagegen in Ordnung', async () => { + // Eine frisch angelegte Zeichnung hat noch keinen Inhalt. + antwort(200, '') + const szene = await ladeZeichnung('/egal') + expect(szene.elements).toHaveLength(0) + }) + + it('und eine echte Zeichnung kommt vollstaendig an', async () => { + antwort(200, serializeAsJSON(beispiel as never, { viewBackgroundColor: '#123456' } as never, {}, 'local')) + const szene = await ladeZeichnung('/egal') + expect(szene.elements).toHaveLength(1) + expect(szene.appState.viewBackgroundColor).toBe('#123456') + expect(szene.appState.collaborators instanceof Map).toBe(true) + }) + }) })