From d564c874a3aef6317e8a32c607fd5fc5225c65ab Mon Sep 17 00:00:00 2001 From: Linus Rath <139418639+rathlinus@users.noreply.github.com> Date: Wed, 3 Jun 2026 21:20:25 +0200 Subject: [PATCH] feat: migrate legacy flat-named Files into real hierarchy on load #379 --- app/(main)/[locale]/files/page.tsx | 37 +++++++++- lib/demo/demo-client.ts | 12 ++++ lib/jmap/client-interface.ts | 1 + lib/jmap/client.ts | 33 +++++++++ locales/en/common.json | 4 +- stores/__tests__/file-store.test.ts | 68 +++++++++++++++++++ stores/file-store.ts | 102 ++++++++++++++++++++++++++++ 7 files changed, 253 insertions(+), 4 deletions(-) diff --git a/app/(main)/[locale]/files/page.tsx b/app/(main)/[locale]/files/page.tsx index 10aa3a52..d1fe7ce8 100644 --- a/app/(main)/[locale]/files/page.tsx +++ b/app/(main)/[locale]/files/page.tsx @@ -27,7 +27,7 @@ import { FilePreviewModal } from "@/components/files/file-preview-modal"; import { loadFilesSettings } from "@/components/files/files-settings-dialog"; import type { FolderLayout } from "@/components/files/files-settings-dialog"; import { AppTopBannerSlot } from "@/components/plugins/app-top-banner-slot"; -import { AlertTriangle } from "lucide-react"; +import { AlertTriangle, Loader2 } from "lucide-react"; export default function FilesPage() { const router = useRouter(); @@ -48,9 +48,11 @@ export default function FilesPage() { supportsFiles, selectedResources, uploadProgress, + migrationProgress, clipboard, initClient, checkSupport, + migrateLegacyFlatNodes, navigate, navigateByPath, refresh, @@ -160,13 +162,16 @@ export default function FilesPage() { const storeClient = useFileStore(s => s.client); useEffect(() => { if (storeClient && supportsFiles === null) { - checkSupport().then((supported) => { + checkSupport().then(async (supported) => { if (supported) { + // Upgrade any files created by older builds (flat path-encoded names) + // into the real FileNode hierarchy before the first listing. + await migrateLegacyFlatNodes(); navigate(null); } }); } - }, [storeClient, supportsFiles, checkSupport, navigate]); + }, [storeClient, supportsFiles, checkSupport, migrateLegacyFlatNodes, navigate]); const handleNavigate = useCallback((path: string, resourceId?: string | null) => { // Pro shell only: the Account breadcrumb segment signals "go to this @@ -573,6 +578,32 @@ export default function FilesPage() { /> )} + {/* Legacy file migration progress (issue #379) */} + {migrationProgress && ( +
+
+
+ +
+

{t("migration_title")}

+

{t("migration_description")}

+
+
+
+
0 + ? `${(migrationProgress.current / migrationProgress.total) * 100}%` + : '0%' }} + /> +
+

+ {migrationProgress.current} / {migrationProgress.total} +

+
+
+ )} +
diff --git a/lib/demo/demo-client.ts b/lib/demo/demo-client.ts index d50af4c7..38d2a2a6 100644 --- a/lib/demo/demo-client.ts +++ b/lib/demo/demo-client.ts @@ -936,6 +936,18 @@ export class DemoJMAPClient implements IJMAPClient { if (node) Object.assign(node, updates, { updated: new Date().toISOString() }); } + async updateFileNodes(updates: Record>>): Promise<{ updated: string[]; notUpdated: Record }> { + const updated: string[] = []; + for (const [id, patch] of Object.entries(updates)) { + const node = this.data.fileNodes.find(n => n.id === id); + if (node) { + Object.assign(node, patch, { updated: new Date().toISOString() }); + updated.push(id); + } + } + return { updated, notUpdated: {} }; + } + async destroyFileNodes(ids: string[]): Promise<{ destroyed: string[]; notDestroyed: string[] }> { const idSet = new Set(ids); this.data.fileNodes = this.data.fileNodes.filter(n => !idSet.has(n.id)); diff --git a/lib/jmap/client-interface.ts b/lib/jmap/client-interface.ts index 19da1cb0..e635765d 100644 --- a/lib/jmap/client-interface.ts +++ b/lib/jmap/client-interface.ts @@ -311,6 +311,7 @@ export interface IJMAPClient { createFileDirectory(name: string, parentId: string | null): Promise; createFileNode(name: string, blobId: string, type: string, size: number, parentId: string | null): Promise; updateFileNode(id: string, updates: Partial>): Promise; + updateFileNodes(updates: Record>>): Promise<{ updated: string[]; notUpdated: Record }>; destroyFileNodes(ids: string[]): Promise<{ destroyed: string[]; notDestroyed: string[] }>; copyFileNode(id: string, newName: string, parentId: string | null): Promise; diff --git a/lib/jmap/client.ts b/lib/jmap/client.ts index ffeda113..c8ce07ab 100644 --- a/lib/jmap/client.ts +++ b/lib/jmap/client.ts @@ -5182,6 +5182,39 @@ export class JMAPClient implements IJMAPClient { } } + /** + * Update many FileNodes in a single FileNode/set call. Returns the ids that + * were updated and a map of id -> error for any the server rejected. Never + * throws for per-node failures (only for a whole-method error). + */ + async updateFileNodes(updates: Record>>): Promise<{ updated: string[]; notUpdated: Record }> { + const ids = Object.keys(updates); + if (ids.length === 0) return { updated: [], notUpdated: {} }; + const accountId = this.getFilesAccountId(); + + const response = await this.request( + [["FileNode/set", { accountId, update: updates }, "fns0"]], + this.fileUsing(), + ); + + const result = response.methodResponses?.[0]; + if (!result || result[0] === "error") { + throw new Error(result?.[1]?.description || "FileNode/set update failed"); + } + + const updatedMap: Record = result[1].updated || {}; + const notUpdatedMap: Record = result[1].notUpdated || {}; + const notUpdated: Record = {}; + for (const id of Object.keys(notUpdatedMap)) { + notUpdated[id] = notUpdatedMap[id]?.description || 'not updated'; + } + // Servers may omit the `updated` map; treat anything not rejected as updated. + const updated = Object.keys(updatedMap).length > 0 + ? Object.keys(updatedMap) + : ids.filter(id => !(id in notUpdated)); + return { updated, notUpdated }; + } + async destroyFileNodes(ids: string[]): Promise<{ destroyed: string[]; notDestroyed: string[] }> { const accountId = this.getFilesAccountId(); diff --git a/locales/en/common.json b/locales/en/common.json index a9e89736..394ba782 100644 --- a/locales/en/common.json +++ b/locales/en/common.json @@ -2943,7 +2943,9 @@ "settings_folder_layout_sidebar": "Sidebar", "disabled_title": "Files feature is disabled by your administrator", "disabled_description": "Large file uploads via WebDAV can cause Stalwart/RocksDB instability, including out-of-memory crashes and unrecoverable disk usage. Deleted files may not be immediately purged from blob storage. This feature is not recommended for production environments.", - "stability_warning": "Large file uploads can cause server instability. Deleted files may not be immediately purged from storage. Use with caution." + "stability_warning": "Large file uploads can cause server instability. Deleted files may not be immediately purged from storage. Use with caution.", + "migration_title": "Updating your files…", + "migration_description": "Organising folders and files into their proper structure. This happens only once." }, "smime": { "your_certificates": "Your Certificates", diff --git a/stores/__tests__/file-store.test.ts b/stores/__tests__/file-store.test.ts index 926166c5..2972909c 100644 --- a/stores/__tests__/file-store.test.ts +++ b/stores/__tests__/file-store.test.ts @@ -37,6 +37,14 @@ function makeMockClient(initial: FileNode[] = []) { const node = nodes.find(n => n.id === id); if (node) Object.assign(node, updates); }, + async updateFileNodes(updates: Record>>) { + const updated: string[] = []; + for (const [id, patch] of Object.entries(updates)) { + const node = nodes.find(n => n.id === id); + if (node) { Object.assign(node, patch); updated.push(id); } + } + return { updated, notUpdated: {} as Record }; + }, async destroyFileNodes(ids: string[]) { // Emulate Stalwart's onDestroyRemoveChildren: removing a directory also // removes its whole subtree. @@ -172,6 +180,66 @@ describe('file-store hierarchy (issue #379)', () => { expect(node.parentId).toBeNull(); }); + it('migrates legacy flat path-encoded nodes into the real hierarchy', async () => { + const SEP = '∕'; + // Exactly the broken shape from issue #379: everything flat at the root + // with the Unicode separator baked into the names. + const client = makeMockClient([ + dir('stuff', 'Stuff', null), + dir('nonsense', `Stuff${SEP}Nonsense`, null), + file('notes', `Stuff${SEP}Nonsense${SEP}Notes.md`, null), + file('other', `Stuff${SEP}Nonsense${SEP}Other.md`, null), + ]); + useFileStore.getState().initClient(client); + + const migrated = await useFileStore.getState().migrateLegacyFlatNodes(); + expect(migrated).toBe(true); + + const byId = Object.fromEntries(client._nodes().map(n => [n.id, n])); + expect(byId.stuff).toMatchObject({ name: 'Stuff', parentId: null }); + expect(byId.nonsense).toMatchObject({ name: 'Nonsense', parentId: 'stuff' }); + expect(byId.notes).toMatchObject({ name: 'Notes.md', parentId: 'nonsense' }); + expect(byId.other).toMatchObject({ name: 'Other.md', parentId: 'nonsense' }); + + // Browsing now reflects the real tree. + await useFileStore.getState().navigate(null); + expect(useFileStore.getState().resources.map(r => r.name)).toEqual(['Stuff']); + await useFileStore.getState().navigate('nonsense', 'Nonsense'); + expect(useFileStore.getState().resources.map(r => r.name).sort()).toEqual(['Notes.md', 'Other.md']); + }); + + it('migrates legacy nodes that used a plain "/" separator (issue #379 data)', async () => { + // A folder upload of a git checkout, stored flat with "/" in the names. + const client = makeMockClient([ + dir('root', 'night-medieval-castle-lake', null), + dir('branding', 'night-medieval-castle-lake/branding', null), + dir('git', 'night-medieval-castle-lake/branding/.git', null), + file('cfg', 'night-medieval-castle-lake/branding/.git/config', null), + file('logo', 'kunden/logo.svg', null), + dir('kunden', 'kunden', null), + ]); + useFileStore.getState().initClient(client); + + expect(await useFileStore.getState().migrateLegacyFlatNodes()).toBe(true); + + const byId = Object.fromEntries(client._nodes().map(n => [n.id, n])); + expect(byId.root).toMatchObject({ name: 'night-medieval-castle-lake', parentId: null }); + expect(byId.branding).toMatchObject({ name: 'branding', parentId: 'root' }); + expect(byId.git).toMatchObject({ name: '.git', parentId: 'branding' }); + expect(byId.cfg).toMatchObject({ name: 'config', parentId: 'git' }); + expect(byId.logo).toMatchObject({ name: 'logo.svg', parentId: 'kunden' }); + + await useFileStore.getState().navigate(null); + expect(useFileStore.getState().resources.map(r => r.name).sort()) + .toEqual(['kunden', 'night-medieval-castle-lake']); + }); + + it('migration is a no-op when no legacy nodes exist', async () => { + const client = makeMockClient([dir('a', 'A', null), file('f', 'f.txt', 'a')]); + useFileStore.getState().initClient(client); + expect(await useFileStore.getState().migrateLegacyFlatNodes()).toBe(false); + }); + it('deletes a folder and the server removes its subtree', async () => { const client = makeMockClient([ dir('stuff', 'Stuff', null), diff --git a/stores/file-store.ts b/stores/file-store.ts index daa2349b..6c59c205 100644 --- a/stores/file-store.ts +++ b/stores/file-store.ts @@ -47,6 +47,8 @@ interface FileState { supportsFiles: boolean | null; selectedResources: Set; uploadProgress: UploadProgress | null; + /** Progress of the one-time legacy flat-node migration; null when idle. */ + migrationProgress: { current: number; total: number } | null; client: IJMAPClient | null; /** Which connected account's files are being browsed. Pro shell only - null in single-account contexts. */ currentAccountId: string | null; @@ -61,6 +63,13 @@ interface FileState { /** Detach the current client and reset browse state. Used by the Pro shell to return to the cross-account picker. */ clearClient: () => void; checkSupport: () => Promise; + /** + * One-time upgrade of files created by older Bulwark builds, which encoded + * the folder tree into flat node names with a Unicode separator. Reparents + * those nodes into the real FileNode hierarchy. No-op once migrated. + * Returns true if any node was migrated. + */ + migrateLegacyFlatNodes: () => Promise; navigate: (parentId: string | null, name?: string) => Promise; navigateByPath: (path: string) => Promise; navigateUp: () => Promise; @@ -98,6 +107,22 @@ interface FileState { const DIRECTORY_TYPES = new Set(['d', 'application/x-directory', 'text/directory', 'httpd/unix-directory', 'inode/directory']); +// Legacy builds encoded the folder hierarchy into flat node names using a path +// separator. Depending on the build / how the data was created (folder upload, +// WebDAV) this is either a plain "/" or the Unicode DIVISION SLASH (U+2215) that +// older webmail used to dodge Stalwart's "/" rejection. We accept both so the +// one-time migration into the real parentId hierarchy can't miss data (#379). +const LEGACY_PATH_SEPS = ['∕', '/', '⁄', '／']; + +function lastLegacySepIndex(name: string): number { + let idx = -1; + for (const sep of LEGACY_PATH_SEPS) { + const i = name.lastIndexOf(sep); + if (i > idx) idx = i; + } + return idx; +} + function isDirectoryType(type: string | undefined): boolean { if (!type) return false; return DIRECTORY_TYPES.has(type) || type.includes('directory'); @@ -171,6 +196,7 @@ export const useFileStore = create((set, get) => ({ supportsFiles: null, selectedResources: new Set(), uploadProgress: null, + migrationProgress: null, client: null, currentAccountId: null, clipboard: null, @@ -219,6 +245,82 @@ export const useFileStore = create((set, get) => ({ return supported; }, + migrateLegacyFlatNodes: async () => { + const { client } = get(); + if (!client) return false; + + let allNodes: FileNode[]; + try { + allNodes = await client.listAllFileNodes(); + } catch { + return false; + } + + const legacy = allNodes.filter(n => lastLegacySepIndex(n.name) >= 0); + if (legacy.length === 0) return false; + + // Resolve parents by their original encoded name. Building the map up front + // means the order in which we rewrite nodes does not matter: each node's + // parent is located by id, captured before any rename happens. + const idByEncodedName = new Map(); + for (const n of allNodes) idByEncodedName.set(n.name, n.id); + + // Build the per-node patches (new leaf name + resolved parentId). + const patches: { id: string; name: string; parentId: string | null }[] = []; + let skipped = 0; + for (const node of legacy) { + const idx = lastLegacySepIndex(node.name); + const parentName = node.name.slice(0, idx); + const leafName = node.name.slice(idx + 1); + const parentId = idByEncodedName.get(parentName); + // Skip orphans (missing parent) to avoid name collisions at the root; + // they keep their encoded name and remain accessible. + if (!parentId) { + skipped++; + console.warn('[Files] migration: no parent for', JSON.stringify(node.name), '(expected', JSON.stringify(parentName) + ')'); + continue; + } + patches.push({ id: node.id, name: leafName, parentId }); + } + + set({ migrationProgress: { current: 0, total: patches.length } }); + + let migrated = 0; + let firstError: string | null = null; + const CHUNK = 100; + try { + for (let i = 0; i < patches.length; i += CHUNK) { + const slice = patches.slice(i, i + CHUNK); + const updates: Record = {}; + for (const p of slice) updates[p.id] = { name: p.name, parentId: p.parentId }; + try { + const { updated, notUpdated } = await client.updateFileNodes(updates); + migrated += updated.length; + const failedIds = Object.keys(notUpdated); + if (failedIds.length > 0 && !firstError) firstError = notUpdated[failedIds[0]]; + for (const id of failedIds) { + console.error('[Files] migration: server rejected node', id, '→', notUpdated[id]); + } + } catch (err) { + const msg = err instanceof Error ? err.message : String(err); + if (!firstError) firstError = msg; + console.error('[Files] migration: batch failed →', msg); + } + set({ migrationProgress: { current: Math.min(i + CHUNK, patches.length), total: patches.length } }); + } + } finally { + set({ migrationProgress: null }); + } + + if (migrated === 0) { + console.error(`[Files] migration found ${legacy.length} legacy node(s) but reparented none ` + + `(skipped ${skipped}, first error: ${firstError ?? 'none'}).`); + } else { + console.info(`[Files] migration reparented ${migrated}/${legacy.length} legacy node(s).`); + } + return migrated > 0; + }, + navigate: async (parentId: string | null, name?: string) => { const { client, pathStack } = get(); if (!client) return;