From 6842203bfbb805a4e055f9ce44b941cd0a6669ff Mon Sep 17 00:00:00 2001 From: Edouard Vanbelle Date: Fri, 4 Sep 2026 23:44:49 +0200 Subject: [PATCH] fix(nfc migrate): fix the cli command line --- src/cli/migrate.rs | 450 ++++++++++++++++++++++++++++++++++++++++----- src/main.rs | 18 ++ 2 files changed, 420 insertions(+), 48 deletions(-) diff --git a/src/cli/migrate.rs b/src/cli/migrate.rs index 2ff35f4d..89f6e06a 100644 --- a/src/cli/migrate.rs +++ b/src/cli/migrate.rs @@ -6,22 +6,44 @@ //! runtime, or historical schema-drift cleanup). //! //! Currently ships one action: `nfc-filenames` — cleans up NFD/NFC -//! filename collisions in databases populated before the June 2026 -//! write-time fix at `src/domain/services/path_service.rs::normalize_storage_name` -//! (called from `src/infrastructure/repositories/pg/file_blob_read_repository.rs` -//! during file operations). New installs never need this migration; -//! only pre-June-2026 databases do. +//! name collisions in databases populated before the write-time fix +//! landed at the repository layer (see +//! `folder_db_repository::create_folder`, `file_blob_write_repository` +//! ingest paths, `drive_pg_repository::create_shared_drive_atomic`). +//! New installs get NFC on every ingest and never accumulate drift. +//! +//! Covers BOTH `storage.files.name` and `storage.folders.name`. The +//! folder pass was added 2026-09-04 in response to +//! AtalayaLabs/OxiCloud#706 (macOS Finder folder upload landed NFD; +//! the file-only migrate did nothing for the reporter). Folders have +//! no `blob_hash`, so the collision branch is "older keeps NFC name, +//! newer becomes `.duplicate[-N]`" only — no dedup-by-trash arm, +//! because trashing a folder strands its subtree. //! //! Previously lived in a standalone `migrate-nfc-filenames` binary //! before the v0.9.0 CLI/server merge — see docs/plan/bundled-binary.md § 1b. //! The 149-line body of `main()` moved here as `run_nfc_filenames()` //! with `env::args()` parsing replaced by clap. //! -//! Future removal target: v1.0. Databases upgraded through v0.9.0 -//! will have run this migration (or been unaffected because they were -//! post-fix installs); by v1.0 no user should still need it. Drop -//! the `NfcFilenames` variant + this module's `run_nfc_filenames()` -//! function together at that point. +//! **Retention: indefinite.** An earlier version of this doc set a +//! "future removal target: v1.0" — retracted 2026-09-04 for three +//! reasons: +//! +//! 1. The pre-2026-09 write-side normalization was DEAD CODE +//! (invariants at `File::new` / `Folder::new_folder` entity +//! constructors that the create path bypassed), so every +//! OxiCloud version shipped before that date accumulated NFD +//! content and has a real remediation need. Many self-hosters +//! won't upgrade for months. +//! 2. Prior versions of THIS migrate command referenced the D7- +//! dropped `user_id` column and errored on first run, so users +//! who tried to apply it never got anywhere. The 2026-09-04 fix +//! makes it work again — but re-applying to instances that were +//! "already migrated" (they weren't) is now the only remediation +//! path for their historical NFD content. +//! 3. Post-fix installs run it as a no-op (all `already_nfc`), so +//! the cost of shipping it forever is zero and the safety it +//! offers for late-upgraders is real. use std::env; @@ -34,17 +56,25 @@ use crate::domain::services::path_service::normalize_storage_name; #[derive(Subcommand)] pub enum Action { - /// NFC-normalize storage.files.name across the instance. + /// NFC-normalize `storage.files.name` AND `storage.folders.name` + /// across the instance. /// - /// Historical cleanup for databases populated before June 2026. - /// New installs (post-`normalize_storage_name` write-time fix) - /// never need this — file operations already write NFC form. + /// Historical cleanup for databases with rows written before the + /// repo-level write-time normalization landed (see module doc for + /// the exact repo methods). Post-fix installs run this as a + /// harmless no-op — every row reports `already_nfc`. /// - /// Collision handling: + /// Collision handling (files): /// * No collision → UPDATE row name to NFC. /// * Same blob content → trash the newer row. /// * Different content → rename the newer to `{name}.duplicate[-N]`. /// + /// Collision handling (folders): + /// * No collision → UPDATE row name to NFC. + /// * Collision → rename the newer to `{name}.duplicate[-N]`; the + /// dedup-by-trash arm from the file path is deliberately absent + /// because trashing a folder strands its subtree. + /// /// In all collision cases, the surviving (older) row's name is /// also normalized to NFC. NfcFilenames { @@ -64,12 +94,36 @@ pub async fn run(action: Action) -> u8 { struct FileRow { id: Uuid, folder_id: Option, - user_id: Uuid, + /// §14 provenance — the user who created the row. Pre-D7 this + /// lived on `user_id`; post-D7 it's `created_by` and `user_id` + /// no longer exists. Not part of the collision scope (the DB + /// unique index is `(folder_id, name) WHERE NOT is_trashed` — + /// no user column in it), but surfaced in the log lines so an + /// operator triaging a large migration output can spot rows + /// owned by a specific principal without a separate query. + created_by: Option, name: String, blob_hash: String, created_at: DateTime, } +/// Structural sibling of [`FileRow`] for `storage.folders`. Folders +/// have no `blob_hash` — there is no "same content dedup" branch on +/// collision, only "keep older, rename newer to .duplicate". Added to +/// close the AtalayaLabs/OxiCloud#706 recovery gap: pre-fix DBs with +/// NFD-named folders (macOS Finder / NC desktop upload from macOS) +/// were unreachable via NFC-normalizing clients, and the file-only +/// migrate did nothing for them. +#[derive(Debug, Clone)] +struct FolderRow { + id: Uuid, + parent_id: Option, + /// §14 provenance — see [`FileRow::created_by`]. + created_by: Option, + name: String, + created_at: DateTime, +} + #[derive(Default)] struct Stats { scanned: u64, @@ -77,6 +131,13 @@ struct Stats { normalized_in_place: u64, deduped_same_content: u64, renamed_duplicate: u64, + // Folder stats — deliberately separate so operators reading the + // summary see "X files, Y folders" instead of one blended count + // that hides the fact that a run touched both scopes. + folders_scanned: u64, + folders_already_nfc: u64, + folders_normalized_in_place: u64, + folders_renamed_duplicate: u64, } async fn run_nfc_filenames(dry_run: bool) -> u8 { @@ -129,8 +190,12 @@ async fn run_nfc_filenames(dry_run: bool) -> u8 { } // Row is in non-NFC form. Look for a collision in the same - // (folder_id, user_id) scope, including rows that may also - // be non-NFC but happen to normalize to the same NFC value. + // folder scope (the DB's unique-index scope for storage.files — + // `(folder_id, name) WHERE NOT is_trashed`), including rows + // that may also be non-NFC but happen to normalize to the same + // NFC value. Pre-D7 this scope included user_id; the column + // has since been dropped (`docs/plan/drive.md` §D7), so the + // scope now matches today's unique constraint verbatim. let collision = match find_collision(&pool, row, &nfc_name).await { Ok(c) => c, Err(e) => { @@ -145,8 +210,14 @@ async fn run_nfc_filenames(dry_run: bool) -> u8 { match collision { None => { println!( - "NORMALIZE {} user={} '{}' → '{}'", - row.id, row.user_id, row.name, nfc_name + "NORMALIZE file={} folder={:?} created_by={:?} '{}' ({}B) → '{}' ({}B)", + row.id, + row.folder_id, + row.created_by, + row.name, + row.name.len(), + nfc_name, + nfc_name.len(), ); if !dry_run && let Err(e) = sqlx::query("UPDATE storage.files SET name = $1 WHERE id = $2") @@ -172,10 +243,11 @@ async fn run_nfc_filenames(dry_run: bool) -> u8 { // Same content → trash the newer; promote older's // name to NFC if it isn't already. println!( - "DEDUP newer={} (trash, same blob) older={} user={} hash={}", + "DEDUP newer={} (trash, same blob) older={} folder={:?} created_by={:?} hash={}", newer.id, older.id, - older.user_id, + older.folder_id, + older.created_by, &older.blob_hash[..16.min(older.blob_hash.len())] ); if !dry_run { @@ -217,8 +289,14 @@ async fn run_nfc_filenames(dry_run: bool) -> u8 { } }; println!( - "RENAME newer={} (different blob) older={} '{}' → '{}'", - newer.id, older.id, newer.name, disambiguated + "RENAME newer={} (different blob) older={} created_by={:?} '{}' ({}B) → '{}' ({}B)", + newer.id, + older.id, + newer.created_by, + newer.name, + newer.name.len(), + disambiguated, + disambiguated.len(), ); if !dry_run { if let Err(e) = @@ -248,8 +326,18 @@ async fn run_nfc_filenames(dry_run: bool) -> u8 { } } + // Second pass: folders. Same shape as the file loop but no dedup + // branch (folders have no `blob_hash`). Added to close + // AtalayaLabs/OxiCloud#706 — a reported macOS-Finder folder upload + // with an NFD name was unreachable via NFC-normalizing clients and + // this migration was the operator's documented recovery path. + if let Err(code) = run_folders(&pool, dry_run, &mut stats).await { + return code; + } + println!(); println!("=== Summary ==="); + println!(" --- storage.files ---"); println!(" scanned : {}", stats.scanned); println!( " already in NFC : {}", @@ -267,6 +355,23 @@ async fn run_nfc_filenames(dry_run: bool) -> u8 { " renamed to .duplicate : {}", stats.renamed_duplicate ); + println!(" --- storage.folders ---"); + println!( + " scanned : {}", + stats.folders_scanned + ); + println!( + " already in NFC : {}", + stats.folders_already_nfc + ); + println!( + " normalized in place (no collision) : {}", + stats.folders_normalized_in_place + ); + println!( + " renamed to .duplicate : {}", + stats.folders_renamed_duplicate + ); if dry_run { println!(); println!("DRY RUN — no rows were written. Re-run without --dry-run to apply."); @@ -277,7 +382,7 @@ async fn run_nfc_filenames(dry_run: bool) -> u8 { async fn load_non_trashed_files(pool: &PgPool) -> Result, Box> { let raw = sqlx::query( - "SELECT id, folder_id, user_id, name, blob_hash, created_at + "SELECT id, folder_id, created_by, name, blob_hash, created_at FROM storage.files WHERE NOT is_trashed ORDER BY created_at", @@ -290,7 +395,7 @@ async fn load_non_trashed_files(pool: &PgPool) -> Result, Box Result, Box Result, Box> { + let raw = sqlx::query( + "SELECT id, parent_id, created_by, name, created_at + FROM storage.folders + WHERE NOT is_trashed + ORDER BY created_at", + ) + .fetch_all(pool) + .await?; + + let mut out = Vec::with_capacity(raw.len()); + for r in raw { + out.push(FolderRow { + id: r.try_get("id")?, + parent_id: r.try_get("parent_id")?, + created_by: r.try_get("created_by")?, + name: r.try_get("name")?, + created_at: r.try_get("created_at")?, + }); + } + Ok(out) +} + +/// Looks for a file in the same folder scope whose CURRENT name +/// equals `nfc_name`, excluding the row being processed. The other +/// row may itself be in non-NFC form whose normalized representation +/// happens to differ from `nfc_name`; the collision check is +/// intentionally based on stored bytes (matching the UNIQUE-index +/// semantics that this migration is repairing). async fn find_collision( pool: &PgPool, row: &FileRow, nfc_name: &str, ) -> Result, Box> { let result = sqlx::query( - "SELECT id, folder_id, user_id, name, blob_hash, created_at + "SELECT id, folder_id, created_by, name, blob_hash, created_at FROM storage.files WHERE name = $1 - AND user_id = $2 - AND ($3::uuid IS NULL AND folder_id IS NULL - OR folder_id = $3::uuid) - AND id <> $4 + AND ($2::uuid IS NULL AND folder_id IS NULL + OR folder_id = $2::uuid) + AND id <> $3 AND NOT is_trashed LIMIT 1", ) .bind(nfc_name) - .bind(row.user_id) .bind(row.folder_id) .bind(row.id) .fetch_optional(pool) @@ -331,17 +464,49 @@ async fn find_collision( Ok(result.map(|r| FileRow { id: r.get("id"), folder_id: r.get("folder_id"), - user_id: r.get("user_id"), + created_by: r.get("created_by"), name: r.get("name"), blob_hash: r.get("blob_hash"), created_at: r.get("created_at"), })) } +/// Folder-side sibling of [`find_collision`]. Same shape but keyed on +/// `parent_id` — the natural uniqueness scope for `storage.folders`. +async fn find_folder_collision( + pool: &PgPool, + row: &FolderRow, + nfc_name: &str, +) -> Result, Box> { + let result = sqlx::query( + "SELECT id, parent_id, created_by, name, created_at + FROM storage.folders + WHERE name = $1 + AND ($2::uuid IS NULL AND parent_id IS NULL + OR parent_id = $2::uuid) + AND id <> $3 + AND NOT is_trashed + LIMIT 1", + ) + .bind(nfc_name) + .bind(row.parent_id) + .bind(row.id) + .fetch_optional(pool) + .await?; + + Ok(result.map(|r| FolderRow { + id: r.get("id"), + parent_id: r.get("parent_id"), + created_by: r.get("created_by"), + name: r.get("name"), + created_at: r.get("created_at"), + })) +} + /// Finds a free name in the form `{nfc_name}.duplicate` or /// `{nfc_name}.duplicate-N` for `N >= 1`, scoped to the row's -/// `(folder_id, user_id)`. Returns the first candidate that does -/// not currently exist as a non-trashed row. +/// folder. Returns the first candidate that does not currently +/// exist as a non-trashed row. async fn find_free_duplicate_name( pool: &PgPool, row: &FileRow, @@ -359,14 +524,12 @@ async fn find_free_duplicate_name( "SELECT EXISTS( SELECT 1 FROM storage.files WHERE name = $1 - AND user_id = $2 - AND ($3::uuid IS NULL AND folder_id IS NULL - OR folder_id = $3::uuid) - AND id <> $4 + AND ($2::uuid IS NULL AND folder_id IS NULL + OR folder_id = $2::uuid) + AND id <> $3 AND NOT is_trashed)", ) .bind(&candidate) - .bind(row.user_id) .bind(row.folder_id) .bind(row.id) .fetch_one(pool) @@ -379,8 +542,52 @@ async fn find_free_duplicate_name( // Safety bound — should never trigger under realistic data. if suffix > 10_000 { return Err(format!( - "Exhausted .duplicate-N suffixes for '{}' in scope (user={}, folder_id={:?})", - nfc_name, row.user_id, row.folder_id + "Exhausted .duplicate-N suffixes for '{}' in scope (folder_id={:?})", + nfc_name, row.folder_id + ) + .into()); + } + } +} + +/// Folder-side sibling. Same shape as [`find_free_duplicate_name`] +/// but keyed on `parent_id`. +async fn find_free_folder_duplicate_name( + pool: &PgPool, + row: &FolderRow, + nfc_name: &str, +) -> Result> { + let mut suffix: u32 = 0; + loop { + let candidate = if suffix == 0 { + format!("{}.duplicate", nfc_name) + } else { + format!("{}.duplicate-{}", nfc_name, suffix) + }; + + let taken: bool = sqlx::query_scalar( + "SELECT EXISTS( + SELECT 1 FROM storage.folders + WHERE name = $1 + AND ($2::uuid IS NULL AND parent_id IS NULL + OR parent_id = $2::uuid) + AND id <> $3 + AND NOT is_trashed)", + ) + .bind(&candidate) + .bind(row.parent_id) + .bind(row.id) + .fetch_one(pool) + .await?; + + if !taken { + return Ok(candidate); + } + suffix = suffix.saturating_add(1); + if suffix > 10_000 { + return Err(format!( + "Exhausted .duplicate-N suffixes for '{}' in scope (parent_id={:?})", + nfc_name, row.parent_id ) .into()); } @@ -404,3 +611,150 @@ async fn normalize_survivor_name( .await?; Ok(()) } + +/// Folder-side sibling of [`normalize_survivor_name`]. If the older +/// folder we kept was itself in non-NFC form, promote it to the NFC +/// name we just picked as canonical. +async fn normalize_folder_survivor_name( + pool: &PgPool, + survivor: &FolderRow, + nfc_name: &str, +) -> Result<(), Box> { + if survivor.name == nfc_name { + return Ok(()); + } + sqlx::query("UPDATE storage.folders SET name = $1 WHERE id = $2") + .bind(nfc_name) + .bind(survivor.id) + .execute(pool) + .await?; + Ok(()) +} + +/// Process every non-trashed folder, mirroring the file loop's shape. +/// Folders have no `blob_hash` so the "same content → dedup" branch is +/// absent: on collision the older folder wins its NFC name, the newer +/// gets renamed to `{nfc_name}.duplicate[-N]`. Never trashes a folder +/// — trashing would strand its subtree, and we cannot know without +/// inspection whether the newer folder was a broken second attempt +/// or an intentional sibling containing different files. Renaming is +/// the conservative choice. +async fn run_folders(pool: &PgPool, dry_run: bool, stats: &mut Stats) -> Result<(), u8> { + let rows = match load_non_trashed_folders(pool).await { + Ok(r) => r, + Err(e) => { + eprintln!("migrate nfc-filenames: folder scan failed: {e}"); + return Err(1); + } + }; + println!("Loaded {} non-trashed folder rows", rows.len()); + println!(); + + stats.folders_scanned = rows.len() as u64; + + for row in &rows { + let nfc_name = normalize_storage_name(&row.name); + if nfc_name == row.name { + stats.folders_already_nfc += 1; + continue; + } + + let collision = match find_folder_collision(pool, row, &nfc_name).await { + Ok(c) => c, + Err(e) => { + eprintln!( + "migrate nfc-filenames: folder collision query failed for {}: {e}", + row.id + ); + return Err(1); + } + }; + + match collision { + None => { + println!( + "NORMALIZE folder={} parent={:?} created_by={:?} '{}' ({}B) → '{}' ({}B)", + row.id, + row.parent_id, + row.created_by, + row.name, + row.name.len(), + nfc_name, + nfc_name.len(), + ); + if !dry_run + && let Err(e) = + sqlx::query("UPDATE storage.folders SET name = $1 WHERE id = $2") + .bind(&nfc_name) + .bind(row.id) + .execute(pool) + .await + { + eprintln!( + "migrate nfc-filenames: folder rename failed for {}: {e}", + row.id + ); + return Err(1); + } + stats.folders_normalized_in_place += 1; + } + Some(other) => { + // Older wins the canonical NFC slot; newer gets a + // `.duplicate[-N]` suffix. No dedup branch here — see + // the doc comment above. + let (older, newer) = if row.created_at <= other.created_at { + (row, &other) + } else { + (&other, row) + }; + + let disambiguated = match find_free_folder_duplicate_name(pool, newer, &nfc_name) + .await + { + Ok(n) => n, + Err(e) => { + eprintln!( + "migrate nfc-filenames: folder duplicate-name search failed for {}: {e}", + newer.id + ); + return Err(1); + } + }; + println!( + "RENAME folder-newer={} older={} created_by={:?} '{}' ({}B) → '{}' ({}B)", + newer.id, + older.id, + newer.created_by, + newer.name, + newer.name.len(), + disambiguated, + disambiguated.len(), + ); + if !dry_run { + if let Err(e) = + sqlx::query("UPDATE storage.folders SET name = $1 WHERE id = $2") + .bind(&disambiguated) + .bind(newer.id) + .execute(pool) + .await + { + eprintln!( + "migrate nfc-filenames: folder disambiguation rename failed for {}: {e}", + newer.id + ); + return Err(1); + } + if let Err(e) = normalize_folder_survivor_name(pool, older, &nfc_name).await { + eprintln!( + "migrate nfc-filenames: folder survivor rename failed for {}: {e}", + older.id + ); + return Err(1); + } + } + stats.folders_renamed_duplicate += 1; + } + } + } + Ok(()) +} diff --git a/src/main.rs b/src/main.rs index 0fc27f83..792cfa12 100644 --- a/src/main.rs +++ b/src/main.rs @@ -141,6 +141,24 @@ fn main() -> Result<(), Box> { if let Some(first) = std::env::args().nth(1) && matches!(first.as_str(), "opaque" | "migrate" | "storage") { + // Load `.env` from CWD before dispatching so subcommands see the + // same `DATABASE_URL` / `OXICLOUD_*` variables the server-startup + // path does. Without this, `oxicloud migrate nfc-filenames` + // errors out with `DATABASE_URL not set` for any operator who + // keeps their config in `.env` (i.e. every self-host on a + // homelab, per the standard project layout). + // + // Non-overriding `dotenv()` — a live shell export still wins, + // matching the server path's default-branch behaviour at + // line ~219 below. `--config ` (line ~177+ below) is + // NOT yet supported for subcommands — that would require + // hoisting the `--config` parse above this dispatch and is + // tracked as follow-up work. Operators needing pinned config + // for a subcommand today: `env $(cat prod.env | xargs) + // oxicloud migrate nfc-filenames`, or run under a systemd + // EnvironmentFile= directive. + dotenvy::dotenv().ok(); + // `oxicloud::cli::run()` returns a plain `u8` exit-code, which // widens exactly into `i32` for `std::process::exit`. Values are // 0/1/2 today; the widening is loss-free by construction.