mirror of
https://github.com/olehomelchenko/astrolabe.git
synced 2026-08-08 02:02:33 +00:00
Roll back import atomically on storage-quota failure (M6, §08)
This commit is contained in:
@@ -426,6 +426,27 @@ user-fixable (storage full → next step, no diagnostic) from not (blocked stora
|
|||||||
explanation **+** a reportable `detail`); and a blocked store at startup **warns and runs
|
explanation **+** a reportable `detail`); and a blocked store at startup **warns and runs
|
||||||
in memory** rather than rejecting into the void.
|
in memory** rather than rejecting into the void.
|
||||||
|
|
||||||
|
### Multi-record writes (import): atomicity at the service boundary
|
||||||
|
|
||||||
|
`db.ts` exposes only per-store request/transaction helpers — there is **no wrapper for a
|
||||||
|
single transaction spanning many records across stores**. So a bulk operation like **import**
|
||||||
|
(`services/transfer.ts`) cannot be truly atomic at the IDB layer; it achieves atomicity at
|
||||||
|
the **service boundary** instead: write the new records to IndexedDB first, tracking what
|
||||||
|
succeeded, and only on full success commit to the Zustand stores. On any write failure
|
||||||
|
(typically `QuotaExceededError`) it **rolls back best-effort** — deletes the records written
|
||||||
|
so far (`Promise.allSettled`) and removes any datasets already added to the store — so the
|
||||||
|
spec §08 "no partial import is committed" contract holds and the user gets an actionable
|
||||||
|
"storage full, delete and retry" message.
|
||||||
|
|
||||||
|
> **Rule:** for any operation that persists multiple records, write-then-commit and roll
|
||||||
|
> back on failure — never mutate the in-memory stores before the writes are known to have
|
||||||
|
> landed (a half-merged workspace is worse than a failed import).
|
||||||
|
> **Limit:** rollback is best-effort, not transactional; if the rollback deletes themselves
|
||||||
|
> fail, orphan records can remain (invisible to the app — never added to a store — and
|
||||||
|
> cleaned up on the next successful write). True cross-record atomicity would require
|
||||||
|
> exposing a raw multi-store transaction from `db.ts`; defer that until a second multi-record
|
||||||
|
> writer needs it.
|
||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
## 7. Checklist for Adding a New Persisted Entity
|
## 7. Checklist for Adding a New Persisted Entity
|
||||||
|
|||||||
@@ -7,9 +7,23 @@ vi.mock('../infrastructure/file-transfer', () => ({
|
|||||||
readTextFile: vi.fn(),
|
readTextFile: vi.fn(),
|
||||||
}));
|
}));
|
||||||
|
|
||||||
|
// The IDB adapter is mocked so tests are deterministic and don't rely on
|
||||||
|
// happy-dom's IndexedDB. saveSnippet and deleteSnippet are replaced with
|
||||||
|
// in-memory no-ops by default; individual tests override them to simulate
|
||||||
|
// quota failures.
|
||||||
|
vi.mock('../infrastructure/snippet-store', async (importOriginal) => {
|
||||||
|
const original = await importOriginal<typeof import('../infrastructure/snippet-store')>();
|
||||||
|
return {
|
||||||
|
...original,
|
||||||
|
saveSnippet: vi.fn().mockResolvedValue(undefined),
|
||||||
|
deleteSnippet: vi.fn().mockResolvedValue(undefined),
|
||||||
|
};
|
||||||
|
});
|
||||||
|
|
||||||
import { createDataset } from '@core/dataset';
|
import { createDataset } from '@core/dataset';
|
||||||
import { createSnippet } from '@core/snippet';
|
import { createSnippet } from '@core/snippet';
|
||||||
import { downloadJson, readTextFile } from '../infrastructure/file-transfer';
|
import { downloadJson, readTextFile } from '../infrastructure/file-transfer';
|
||||||
|
import { deleteSnippet, saveSnippet, StorageQuotaError } from '../infrastructure/snippet-store';
|
||||||
import { useDatasetStore } from '../stores/DatasetStore';
|
import { useDatasetStore } from '../stores/DatasetStore';
|
||||||
import { useNotificationStore } from '../stores/NotificationStore';
|
import { useNotificationStore } from '../stores/NotificationStore';
|
||||||
import { useSnippetStore } from '../stores/SnippetStore';
|
import { useSnippetStore } from '../stores/SnippetStore';
|
||||||
@@ -17,6 +31,8 @@ import { exportWorkspace, importWorkspace } from './transfer';
|
|||||||
|
|
||||||
const mockedDownload = vi.mocked(downloadJson);
|
const mockedDownload = vi.mocked(downloadJson);
|
||||||
const mockedRead = vi.mocked(readTextFile);
|
const mockedRead = vi.mocked(readTextFile);
|
||||||
|
const mockedSave = vi.mocked(saveSnippet);
|
||||||
|
const mockedDelete = vi.mocked(deleteSnippet);
|
||||||
|
|
||||||
/** The most recent notification raised. */
|
/** The most recent notification raised. */
|
||||||
function lastNote() {
|
function lastNote() {
|
||||||
@@ -153,4 +169,114 @@ describe('importWorkspace', () => {
|
|||||||
expect(imported.id).not.toBe('s1');
|
expect(imported.id).not.toBe('s1');
|
||||||
expect(snippets.find((s) => s.name === 'Existing')!.id).toBe('s1');
|
expect(snippets.find((s) => s.name === 'Existing')!.id).toBe('s1');
|
||||||
});
|
});
|
||||||
|
|
||||||
|
describe('atomic rollback on storage-quota failure', () => {
|
||||||
|
it('leaves the snippet store and IDB unchanged when a quota error fires on the first write', async () => {
|
||||||
|
// Seed one pre-existing snippet.
|
||||||
|
useSnippetStore.getState().hydrate([createSnippet({ id: 'pre1', name: 'Pre-existing' })]);
|
||||||
|
// Every saveSnippet call throws a quota error.
|
||||||
|
mockedSave.mockRejectedValueOnce(new StorageQuotaError());
|
||||||
|
|
||||||
|
await importJson(
|
||||||
|
JSON.stringify({
|
||||||
|
version: '1.0',
|
||||||
|
snippets: [
|
||||||
|
{
|
||||||
|
id: 'imp1',
|
||||||
|
created: '2026-01-01T00:00:00.000Z',
|
||||||
|
name: 'Incoming',
|
||||||
|
spec: '{"mark":"bar"}',
|
||||||
|
},
|
||||||
|
],
|
||||||
|
}),
|
||||||
|
);
|
||||||
|
|
||||||
|
// The Zustand store must be unchanged — no partial import visible.
|
||||||
|
const snippets = useSnippetStore.getState().snippets;
|
||||||
|
expect(snippets).toHaveLength(1);
|
||||||
|
expect(snippets[0].id).toBe('pre1');
|
||||||
|
|
||||||
|
// deleteSnippet must NOT have been called (nothing was written before the
|
||||||
|
// first save threw, so there is nothing to clean up).
|
||||||
|
expect(mockedDelete).not.toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
|
||||||
|
it('rolls back IDB records already written before the failing call', async () => {
|
||||||
|
// saveSnippet succeeds for the first snippet, then throws on the second.
|
||||||
|
mockedSave.mockResolvedValueOnce(undefined);
|
||||||
|
mockedSave.mockRejectedValueOnce(new StorageQuotaError());
|
||||||
|
|
||||||
|
await importJson(
|
||||||
|
JSON.stringify({
|
||||||
|
version: '1.0',
|
||||||
|
snippets: [
|
||||||
|
{ id: 'a', created: '2026-01-01T00:00:00.000Z', name: 'A', spec: '{"mark":"bar"}' },
|
||||||
|
{ id: 'b', created: '2026-01-01T00:00:00.000Z', name: 'B', spec: '{"mark":"point"}' },
|
||||||
|
],
|
||||||
|
}),
|
||||||
|
);
|
||||||
|
|
||||||
|
// The store must still be empty — addSnippets was never called.
|
||||||
|
expect(useSnippetStore.getState().snippets).toHaveLength(0);
|
||||||
|
|
||||||
|
// deleteSnippet must be called once to clean up the record that was written
|
||||||
|
// before the second save failed (the first snippet's id).
|
||||||
|
expect(mockedDelete).toHaveBeenCalledTimes(1);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('rolls back datasets added to the store before the snippet write failed', async () => {
|
||||||
|
// The import contains both a dataset and a snippet; the snippet write fails.
|
||||||
|
mockedSave.mockRejectedValueOnce(new StorageQuotaError());
|
||||||
|
|
||||||
|
await importJson(
|
||||||
|
JSON.stringify({
|
||||||
|
version: '1.0',
|
||||||
|
snippets: [
|
||||||
|
{ id: 's1', created: '2026-01-01T00:00:00.000Z', name: 'S', spec: '{"mark":"bar"}' },
|
||||||
|
],
|
||||||
|
datasets: [{ id: 1, name: 'DS', data: [{ x: 1 }], format: 'json', source: 'inline' }],
|
||||||
|
}),
|
||||||
|
);
|
||||||
|
|
||||||
|
// Neither snippets nor datasets should persist in the store.
|
||||||
|
expect(useSnippetStore.getState().snippets).toHaveLength(0);
|
||||||
|
expect(useDatasetStore.getState().datasets).toHaveLength(0);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('surfaces a quota error as a clear actionable notification without a detail field', async () => {
|
||||||
|
mockedSave.mockRejectedValueOnce(new StorageQuotaError());
|
||||||
|
|
||||||
|
await importJson(
|
||||||
|
JSON.stringify({
|
||||||
|
version: '1.0',
|
||||||
|
snippets: [{ id: 'x', created: '2026-01-01T00:00:00.000Z', name: 'X', spec: '{}' }],
|
||||||
|
}),
|
||||||
|
);
|
||||||
|
|
||||||
|
const note = lastNote();
|
||||||
|
expect(note.kind).toBe('error');
|
||||||
|
expect(note.title).toContain('storage full');
|
||||||
|
// Plain-language copy: names the problem and the fix, no error codes.
|
||||||
|
expect(note.message.toLowerCase()).toContain('delete');
|
||||||
|
expect(note.message.toLowerCase()).toContain('try');
|
||||||
|
// User-fixable failure: no diagnostic detail (nothing to report).
|
||||||
|
expect(note.detail).toBeUndefined();
|
||||||
|
});
|
||||||
|
|
||||||
|
it('surfaces a non-quota storage error with a diagnostic detail', async () => {
|
||||||
|
mockedSave.mockRejectedValueOnce(new DOMException('disk I/O error', 'UnknownError'));
|
||||||
|
|
||||||
|
await importJson(
|
||||||
|
JSON.stringify({
|
||||||
|
version: '1.0',
|
||||||
|
snippets: [{ id: 'x', created: '2026-01-01T00:00:00.000Z', name: 'X', spec: '{}' }],
|
||||||
|
}),
|
||||||
|
);
|
||||||
|
|
||||||
|
const note = lastNote();
|
||||||
|
expect(note.kind).toBe('error');
|
||||||
|
expect(note.detail).toBeDefined();
|
||||||
|
expect(note.detail).toContain('UnknownError');
|
||||||
|
});
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -8,6 +8,14 @@
|
|||||||
* Order matters on import (spec §08 → Merge): datasets are committed **before**
|
* Order matters on import (spec §08 → Merge): datasets are committed **before**
|
||||||
* snippets so a snippet's by-name reference resolves against the just-added
|
* snippets so a snippet's by-name reference resolves against the just-added
|
||||||
* dataset (whose name may have been auto-suffixed to avoid a clash).
|
* dataset (whose name may have been auto-suffixed to avoid a clash).
|
||||||
|
*
|
||||||
|
* Atomicity (spec §08 → Storage limit handling / spec §10 → Non-destructive
|
||||||
|
* import): snippet writes are performed directly to IDB before touching the
|
||||||
|
* Zustand store. If any write fails the already-written IDB records are deleted
|
||||||
|
* and the just-added datasets are rolled back from the store, so neither the
|
||||||
|
* store nor IDB retains a partial import. The write-through subscriber sees
|
||||||
|
* store changes only after all IDB writes succeed; because `put` is idempotent
|
||||||
|
* the second (subscriber) write of each record is a harmless no-op.
|
||||||
*/
|
*/
|
||||||
|
|
||||||
import { buildExportEnvelope, exportFilename, exportSummaryMessage } from '@core/export-envelope';
|
import { buildExportEnvelope, exportFilename, exportSummaryMessage } from '@core/export-envelope';
|
||||||
@@ -20,6 +28,7 @@ import {
|
|||||||
} from '@core/import-normalize';
|
} from '@core/import-normalize';
|
||||||
import { snippetSizeBytes } from '@core/snippet';
|
import { snippetSizeBytes } from '@core/snippet';
|
||||||
import { downloadJson, readTextFile } from '../infrastructure/file-transfer';
|
import { downloadJson, readTextFile } from '../infrastructure/file-transfer';
|
||||||
|
import { deleteSnippet, saveSnippet, StorageQuotaError } from '../infrastructure/snippet-store';
|
||||||
import { notify } from '../stores/NotificationStore';
|
import { notify } from '../stores/NotificationStore';
|
||||||
import { useDatasetStore } from '../stores/DatasetStore';
|
import { useDatasetStore } from '../stores/DatasetStore';
|
||||||
import { useSnippetStore } from '../stores/SnippetStore';
|
import { useSnippetStore } from '../stores/SnippetStore';
|
||||||
@@ -122,6 +131,9 @@ export async function importWorkspace(file: File): Promise<void> {
|
|||||||
const finalSnippets = reassignCollidingSnippetIds(existingSnippetIds, renamedSnippets);
|
const finalSnippets = reassignCollidingSnippetIds(existingSnippetIds, renamedSnippets);
|
||||||
|
|
||||||
// Commit datasets BEFORE snippets so by-name references resolve (spec §08).
|
// Commit datasets BEFORE snippets so by-name references resolve (spec §08).
|
||||||
|
// Record which dataset ids existed before the add so we can roll them back if
|
||||||
|
// the subsequent snippet writes fail.
|
||||||
|
const datasetIdsBefore = new Set(useDatasetStore.getState().datasets.map((d) => d.id));
|
||||||
useDatasetStore.getState().addDatasets(dedupedDatasets);
|
useDatasetStore.getState().addDatasets(dedupedDatasets);
|
||||||
|
|
||||||
// Storage budget pre-check (spec §08 → Storage limit handling): warn on overage
|
// Storage budget pre-check (spec §08 → Storage limit handling): warn on overage
|
||||||
@@ -132,12 +144,58 @@ export async function importWorkspace(file: File): Promise<void> {
|
|||||||
const incomingBytes = finalSnippets.reduce((n, s) => n + snippetSizeBytes(s), 0);
|
const incomingBytes = finalSnippets.reduce((n, s) => n + snippetSizeBytes(s), 0);
|
||||||
const overage = existingBytes + incomingBytes - SNIPPET_BUDGET_BYTES;
|
const overage = existingBytes + incomingBytes - SNIPPET_BUDGET_BYTES;
|
||||||
|
|
||||||
// TODO (spec §08 → Storage limit handling): a *hard* quota failure on save
|
// Atomic snippet write (spec §08 → Storage limit handling / spec §10 →
|
||||||
// should commit no partial snippet import. The write-through subscriber
|
// Non-destructive import): write every snippet to IDB before updating the
|
||||||
// currently surfaces such a failure as a per-record error toast, but the
|
// Zustand store. A quota failure mid-write is caught here, the already-written
|
||||||
// in-memory store keeps the records (they vanish on reload). True atomic
|
// records are deleted, the just-added datasets are removed from the store, and
|
||||||
// rollback needs a write-before-store import path — a persistence-architecture
|
// we surface a clear actionable error — leaving the workspace exactly as before.
|
||||||
// change deferred until the M6 storage monitor (web.dev quota estimate) lands.
|
// After all writes succeed, `addSnippets` makes them visible in the store; the
|
||||||
|
// write-through subscriber re-runs `saveSnippet` for each, but `put` is
|
||||||
|
// idempotent so those second writes are harmless.
|
||||||
|
const writtenSnippetIds: string[] = [];
|
||||||
|
try {
|
||||||
|
for (const snippet of finalSnippets) {
|
||||||
|
await saveSnippet(snippet);
|
||||||
|
writtenSnippetIds.push(snippet.id);
|
||||||
|
}
|
||||||
|
} catch (err) {
|
||||||
|
// Roll back: delete every IDB record we already wrote.
|
||||||
|
await Promise.allSettled(writtenSnippetIds.map((id) => deleteSnippet(id)));
|
||||||
|
|
||||||
|
// Roll back: remove the datasets we just added to the store so neither
|
||||||
|
// store nor IDB retains any trace of this import.
|
||||||
|
const addedDatasets = useDatasetStore
|
||||||
|
.getState()
|
||||||
|
.datasets.filter((d) => !datasetIdsBefore.has(d.id));
|
||||||
|
for (const d of addedDatasets) {
|
||||||
|
useDatasetStore.getState().remove(d.id);
|
||||||
|
}
|
||||||
|
|
||||||
|
// Surface a clear, actionable error (spec §08 "Quota failure"; NN/g #9 /
|
||||||
|
// GOV.UK plain language — no codes, tell the user what to do next).
|
||||||
|
if (err instanceof StorageQuotaError) {
|
||||||
|
notify({
|
||||||
|
kind: 'error',
|
||||||
|
title: 'Import failed — storage full',
|
||||||
|
message:
|
||||||
|
'The import could not be saved because snippet storage is full. ' +
|
||||||
|
'Delete snippets you no longer need to free up space, then try importing again.',
|
||||||
|
});
|
||||||
|
} else {
|
||||||
|
notify({
|
||||||
|
kind: 'error',
|
||||||
|
title: 'Import failed',
|
||||||
|
message:
|
||||||
|
'A storage error prevented the import from completing. The workspace was not changed. ' +
|
||||||
|
'If this keeps happening, your browser may be blocking local storage.',
|
||||||
|
detail:
|
||||||
|
err instanceof Error ? `Snippet save failed: ${err.name}: ${err.message}` : String(err),
|
||||||
|
});
|
||||||
|
}
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
|
// All IDB writes succeeded — now make the snippets visible in the store.
|
||||||
useSnippetStore.getState().addSnippets(finalSnippets);
|
useSnippetStore.getState().addSnippets(finalSnippets);
|
||||||
|
|
||||||
// Feedback (spec §08 → Feedback): one summary toast; a warning when datasets were
|
// Feedback (spec §08 → Feedback): one summary toast; a warning when datasets were
|
||||||
|
|||||||
Reference in New Issue
Block a user