diff --git a/Cargo.lock b/Cargo.lock index 19b57a59..00cfd3d6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3856,6 +3856,7 @@ dependencies = [ "tower-http", "tracing", "tracing-subscriber", + "unicode-normalization", "urlencoding", "utoipa", "uuid", diff --git a/Cargo.toml b/Cargo.toml index 6843402e..42fa985e 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -44,6 +44,7 @@ mp3-duration = "0.1" kamadak-exif = "0.6.1" md-5 = "0.11.0" sha2 = "0.11.0" +unicode-normalization = "0.1.24" blake3 = { version = "1.8.4", features = ["rayon", "mmap"] } hex = "0.4.3" http-body-util = "0.1.3" @@ -87,6 +88,10 @@ unexpected_cfgs = { level = "warn", check-cfg = ['cfg(integration_tests)'] } name = "generate-openapi" path = "src/bin/generate-openapi.rs" +[[bin]] +name = "migrate-nfc-filenames" +path = "src/bin/migrate-nfc-filenames.rs" + [build-dependencies] oxc_allocator = "0.125.0" oxc_parser = "0.125.0" diff --git a/Dockerfile b/Dockerfile index bce8aa0b..59dc48e2 100644 --- a/Dockerfile +++ b/Dockerfile @@ -14,6 +14,7 @@ COPY static static RUN mkdir -p src/bin && \ echo 'fn main() { println!("Dummy build for caching dependencies"); }' > src/main.rs && \ echo 'fn main() {}' > src/bin/generate-openapi.rs && \ + echo 'fn main() {}' > src/bin/migrate-nfc-filenames.rs && \ cargo build --release && \ rm -rf src static-dist target/release/deps/oxicloud* target/release/build/oxicloud-* @@ -56,6 +57,12 @@ RUN apk --no-cache upgrade && \ # Copy the compiled binary and entrypoint (--chmod avoids extra RUN chmod layers) COPY --from=builder --chmod=755 /app/target/release/oxicloud /usr/local/bin/ +# Ship the NFC filename migration binary alongside the server so +# operators can run it inside the container without a separate Rust +# toolchain — `docker exec migrate-nfc-filenames --dry-run` +# to preview, drop `--dry-run` to execute. One-shot tool, safe to +# ship; it only mutates `storage.files` rows whose name ≠ NFC(name). +COPY --from=builder --chmod=755 /app/target/release/migrate-nfc-filenames /usr/local/bin/ COPY entrypoint.sh /usr/local/bin/entrypoint.sh RUN sed -i 's/\r//' /usr/local/bin/entrypoint.sh && \ chmod 755 /usr/local/bin/entrypoint.sh diff --git a/migrations/20260625000000_folder_tree_modified_at.sql b/migrations/20260625000000_folder_tree_modified_at.sql new file mode 100644 index 00000000..881d320d --- /dev/null +++ b/migrations/20260625000000_folder_tree_modified_at.sql @@ -0,0 +1,104 @@ +-- Folder rollup ETag: introduce `storage.folders.tree_modified_at`, +-- which is bumped whenever any descendant (file or folder) changes. +-- +-- Motivation: WebDAV / NextCloud sync clients use a collection's ETag +-- to decide "did anything change inside this folder since I last +-- looked?". Until now `Folder::etag()` returned the folder UUID +-- (constant for the row's life), which made the answer always "no" — +-- forcing clients to do periodic deep PROPFIND walks to discover new +-- files. With this column, `Folder::etag()` becomes +-- `{id_short}-{tree_modified_at}` and clients can do O(changed) +-- recursion instead of O(tree). +-- +-- The two triggers cascade an update timestamp up the ltree ancestor +-- chain on every file write and every folder mutation. Performance +-- ceiling: O(depth) row updates per mutation; deep concurrent writes +-- to the same root subtree can contend on the root row. + +ALTER TABLE storage.folders + ADD COLUMN tree_modified_at TIMESTAMPTZ NOT NULL DEFAULT NOW(); + +-- Backfill existing rows: collapse the rollup timestamp to the +-- per-folder updated_at. Clients re-walking after deploy will see +-- one batch of "looks new to me" responses, which they handle as a +-- content-match-no-download — the expected one-time resync wave. +UPDATE storage.folders SET tree_modified_at = updated_at; + + +-- File-side trigger: any INSERT/UPDATE/DELETE on storage.files +-- bumps the file's parent folder + all its ancestors in the ltree. +-- Root-level files (folder_id IS NULL) have no ancestors and do not +-- trigger any folder bump — the root listing isn't an etag-emitting +-- collection in OxiCloud's model (no virtual root folder row). +CREATE OR REPLACE FUNCTION storage.bump_folder_tree_from_file() +RETURNS TRIGGER LANGUAGE plpgsql AS $$ +DECLARE + target_folder_id UUID; + target_lpath ltree; +BEGIN + target_folder_id := COALESCE(NEW.folder_id, OLD.folder_id); + IF target_folder_id IS NULL THEN + RETURN COALESCE(NEW, OLD); + END IF; + + SELECT lpath INTO target_lpath + FROM storage.folders + WHERE id = target_folder_id; + + IF target_lpath IS NULL THEN + RETURN COALESCE(NEW, OLD); + END IF; + + -- `lpath @> target_lpath` matches the target folder AND every + -- ancestor up to the root. The GiST index on lpath keeps this + -- to an index range scan even on deep trees. + UPDATE storage.folders + SET tree_modified_at = NOW() + WHERE lpath @> target_lpath; + + RETURN COALESCE(NEW, OLD); +END; +$$; + +CREATE TRIGGER files_bump_folder_tree_etag + AFTER INSERT OR UPDATE OR DELETE ON storage.files + FOR EACH ROW EXECUTE FUNCTION storage.bump_folder_tree_from_file(); + + +-- Folder-side trigger: covers creates, deletes, renames, and moves. +-- A folder move changes its lpath — the OLD chain and NEW chain +-- both need bumping (old parents lost a child, new parents gained +-- one). Self-exclusion (id <> the changed row) avoids the row +-- bumping itself, which is meaningless and would amplify +-- contention on hot paths. +-- +-- The `pg_trigger_depth() > 1` guard breaks recursion: when this +-- trigger UPDATEs ancestor rows below, those UPDATEs would fire +-- the same trigger again. Without the guard, a single child +-- creation would cascade an unbounded number of upward writes. +CREATE OR REPLACE FUNCTION storage.bump_folder_tree_from_folder() +RETURNS TRIGGER LANGUAGE plpgsql AS $$ +BEGIN + IF pg_trigger_depth() > 1 THEN + RETURN COALESCE(NEW, OLD); + END IF; + + IF TG_OP IN ('DELETE', 'UPDATE') AND OLD.lpath IS NOT NULL THEN + UPDATE storage.folders + SET tree_modified_at = NOW() + WHERE lpath @> OLD.lpath AND id <> OLD.id; + END IF; + + IF TG_OP IN ('INSERT', 'UPDATE') AND NEW.lpath IS NOT NULL THEN + UPDATE storage.folders + SET tree_modified_at = NOW() + WHERE lpath @> NEW.lpath AND id <> NEW.id; + END IF; + + RETURN COALESCE(NEW, OLD); +END; +$$; + +CREATE TRIGGER folders_bump_folder_tree_etag + AFTER INSERT OR UPDATE OR DELETE ON storage.folders + FOR EACH ROW EXECUTE FUNCTION storage.bump_folder_tree_from_folder(); diff --git a/src/application/adapters/webdav_adapter.rs b/src/application/adapters/webdav_adapter.rs index da4f1ccb..24dbde25 100644 --- a/src/application/adapters/webdav_adapter.rs +++ b/src/application/adapters/webdav_adapter.rs @@ -444,9 +444,11 @@ impl WebDavAdapter { xml_writer.write_event(Event::Text(BytesText::new(&modified_at.to_rfc2822())))?; xml_writer.write_event(Event::End(BytesEnd::new("D:getlastmodified")))?; - // Other standard properties + // ETag — routes through `FolderDto::etag` (= `Folder::etag()`) + // so every WebDAV emitter and HEAD response agree on a single + // value for the same folder. xml_writer.write_event(Event::Start(BytesStart::new("D:getetag")))?; - xml_writer.write_event(Event::Text(BytesText::new(&format!("\"{}\"", folder.id))))?; + xml_writer.write_event(Event::Text(BytesText::new(&format!("\"{}\"", folder.etag))))?; xml_writer.write_event(Event::End(BytesEnd::new("D:getetag")))?; // Content length (0 for directories) @@ -505,9 +507,11 @@ impl WebDavAdapter { xml_writer.write_event(Event::Text(BytesText::new(&modified_at.to_rfc2822())))?; xml_writer.write_event(Event::End(BytesEnd::new("D:getlastmodified")))?; - // ETag + // ETag — routes through `FileDto::etag` (= `File::etag()`) so + // PROPFIND, GET, HEAD, PUT-response, and MOVE all emit + // byte-identical values for the same file. xml_writer.write_event(Event::Start(BytesStart::new("D:getetag")))?; - xml_writer.write_event(Event::Text(BytesText::new(&format!("\"{}\"", file.id))))?; + xml_writer.write_event(Event::Text(BytesText::new(&format!("\"{}\"", file.etag))))?; xml_writer.write_event(Event::End(BytesEnd::new("D:getetag")))?; Ok(()) @@ -589,7 +593,7 @@ impl WebDavAdapter { xml_writer.write_event(Event::Start(BytesStart::new("D:getetag")))?; xml_writer.write_event(Event::Text(BytesText::new(&format!( "\"{}\"", - folder.id + folder.etag ))))?; xml_writer.write_event(Event::End(BytesEnd::new("D:getetag")))?; } @@ -685,7 +689,7 @@ impl WebDavAdapter { xml_writer.write_event(Event::Start(BytesStart::new("D:getetag")))?; xml_writer.write_event(Event::Text(BytesText::new(&format!( "\"{}\"", - file.id + file.etag ))))?; xml_writer.write_event(Event::End(BytesEnd::new("D:getetag")))?; } diff --git a/src/application/dtos/favorites_dto.rs b/src/application/dtos/favorites_dto.rs index 19d09cef..80852772 100644 --- a/src/application/dtos/favorites_dto.rs +++ b/src/application/dtos/favorites_dto.rs @@ -125,6 +125,10 @@ pub struct FavoriteResourceRow { pub resource_created_at: DateTime, pub modified_at: DateTime, pub owner_id: Uuid, + /// Raw BLAKE3 content hash. `Some(_)` for file rows, `None` for + /// folder rows. Routes into `FileDto::content_hash` and feeds + /// `File::compute_etag` to populate `FileDto::etag`. + pub blob_hash: Option, /// `true` when `owner_id == requesting user_id`. pub is_owner: bool, pub favorited_at: DateTime, diff --git a/src/application/dtos/file_dto.rs b/src/application/dtos/file_dto.rs index bc6af74c..b55031f7 100644 --- a/src/application/dtos/file_dto.rs +++ b/src/application/dtos/file_dto.rs @@ -62,14 +62,31 @@ pub struct FileDto { #[serde(skip_serializing_if = "Option::is_none")] pub sort_date: Option, - /// Content-addressable ETag (= blob_hash). Changes on every content write. - /// Used for WebDAV/Nextcloud ETag headers. Omitted from REST API JSON. - #[serde(skip)] + /// Raw BLAKE3 content hash. Populated from `File::content_hash()`. + /// Exposed in REST JSON so API consumers can use it for + /// content-addressable URLs, dedup verification, and integrity + /// audits. Distinct from `etag` (which is an HTTP-only cache + /// token whose formula may grow to include `modified_at` etc.). + pub content_hash: String, + + /// Opaque HTTP ETag. Populated from `File::etag()`. Used by + /// WebDAV/NextCloud handlers when emitting `ETag` headers and + /// also exposed in REST JSON so frontends can pass it back + /// through `If-Match` / `If-None-Match` on download / mutation + /// endpoints without a separate HEAD round-trip. pub etag: String, } impl From for FileDto { fn from(file: File) -> Self { + // Compute the HTTP ETag BEFORE consuming the entity — + // `File::etag()` derives from `blob_hash` + `modified_at`, + // so it must run against the live entity, not against + // already-extracted parts. `content_hash` is just the raw + // blob hash; `etag` is the cache token derived from it. + let etag = file.etag(); + let content_hash = file.content_hash().to_string(); + // Consume the entity by moving all fields — zero heap allocations // for id, name, path, folder_id, owner_id (previously 5× .to_string()). let parts = file.into_parts(); @@ -95,7 +112,8 @@ impl From for FileDto { size_formatted, owner_id: parts.owner_id.map(|u| u.to_string()), sort_date: None, - etag: parts.etag, + content_hash, + etag, } } } @@ -150,6 +168,7 @@ impl FileDto { category: Arc::from("Document"), size_formatted: "0 Bytes".to_string(), owner_id: None, + content_hash: String::new(), etag: String::new(), sort_date: None, } diff --git a/src/application/dtos/folder_dto.rs b/src/application/dtos/folder_dto.rs index e411fab6..1cd5ca3a 100644 --- a/src/application/dtos/folder_dto.rs +++ b/src/application/dtos/folder_dto.rs @@ -73,11 +73,19 @@ pub struct FolderDto { /// Human-readable category (always "Folder") #[schema(value_type = String)] pub category: Arc, + + /// Opaque ETag for HTTP responses. Populated from `Folder::etag()` + /// at conversion time so every WebDAV / NextCloud handler emits + /// the same value, and exposed in REST JSON so the frontend can + /// pass it back through `If-Match` on rename / move endpoints + /// without a separate HEAD round-trip. + pub etag: String, } impl From for FolderDto { fn from(folder: Folder) -> Self { let is_root = folder.parent_id().is_none(); + let etag = folder.etag().to_string(); Self { id: folder.id().to_string(), @@ -91,6 +99,7 @@ impl From for FolderDto { icon_class: Arc::from("fas fa-folder"), icon_special_class: Arc::from("folder-icon"), category: Arc::from("Folder"), + etag, } } } @@ -141,6 +150,7 @@ impl FolderDto { icon_class: Arc::from("fas fa-folder"), icon_special_class: Arc::from("folder-icon"), category: Arc::from("Folder"), + etag: String::new(), } } } @@ -171,6 +181,11 @@ pub struct FolderResourceRow { pub created_at: DateTime, pub modified_at: DateTime, pub owner_id: Uuid, + /// Raw BLAKE3 content hash. `Some(_)` for file rows, `None` for + /// folder rows. Populates `FileDto::content_hash` + `FileDto::etag` + /// on the REST `/api/folders/{id}/resources` listing so API + /// consumers can issue conditional requests against listed files. + pub blob_hash: Option, // Pre-computed sort fields — returned by the SQL for cursor construction. /// `LOWER(name)` used by `name`/`type` sorts. pub sort_str: String, diff --git a/src/application/dtos/recent_dto.rs b/src/application/dtos/recent_dto.rs index cf3ca378..3eee591b 100644 --- a/src/application/dtos/recent_dto.rs +++ b/src/application/dtos/recent_dto.rs @@ -105,6 +105,10 @@ pub struct RecentResourceRow { pub resource_created_at: DateTime, pub modified_at: DateTime, pub owner_id: Uuid, + /// Raw BLAKE3 content hash. `Some(_)` for file rows, `None` for + /// folder rows. Feeds `File::compute_etag` so this listing's + /// `etag` matches GET/HEAD/PROPFIND for the same file. + pub blob_hash: Option, /// `true` when `owner_id == requesting user_id`. pub is_owner: bool, pub accessed_at: DateTime, diff --git a/src/application/dtos/search_dto.rs b/src/application/dtos/search_dto.rs index 24f4af9b..5713f712 100644 --- a/src/application/dtos/search_dto.rs +++ b/src/application/dtos/search_dto.rs @@ -127,6 +127,13 @@ pub struct SearchFileResultDto { pub icon_special_class: String, /// Content category: "document", "image", "video", "audio", "archive", "code", "other" pub category: String, + /// Raw BLAKE3 content hash. Feeds `FileDto::content_hash` and + /// `File::compute_etag` when search results are converted to + /// `FileDto` (NC REPORT/SEARCH response). Defaults to `String::new()` + /// for backward-compatible deserialisation of cached results + /// that pre-date the column. + #[serde(default)] + pub blob_hash: String, } /// A folder search result enriched with server-computed metadata diff --git a/src/application/dtos/trash_dto.rs b/src/application/dtos/trash_dto.rs index 5715c9ac..2cbe2e60 100644 --- a/src/application/dtos/trash_dto.rs +++ b/src/application/dtos/trash_dto.rs @@ -61,6 +61,12 @@ pub struct TrashResourceRow { pub resource_created_at: DateTime, pub modified_at: DateTime, pub owner_id: Uuid, + /// Raw BLAKE3 content hash. `Some(_)` for file rows, `None` for + /// folder rows. Feeds `File::compute_etag` so the trash listing's + /// `etag` matches what GET/HEAD/PROPFIND would return for the + /// same file (restorable trash items are conditional-request + /// targets too). + pub blob_hash: Option, pub trashed_at: DateTime, pub deletion_date: DateTime, /// Original location path (for folders: `path`; for files: `parent.path || '/' || name`). diff --git a/src/application/services/search_service.rs b/src/application/services/search_service.rs index 1adaff83..5ce252ab 100644 --- a/src/application/services/search_service.rs +++ b/src/application/services/search_service.rs @@ -172,6 +172,10 @@ impl SearchService { icon_class: get_icon_class(&file.name, &file.mime_type), icon_special_class: get_icon_special_class(&file.name, &file.mime_type), category: get_category(&file.name, &file.mime_type), + // Carry the content hash through so REPORT/SEARCH + // responses on the NC surface can emit the same ETag + // (`File::compute_etag`) as PROPFIND/GET would. + blob_hash: file.content_hash.clone(), } } diff --git a/src/application/services/trash_service.rs b/src/application/services/trash_service.rs index 073aae40..935f294e 100644 --- a/src/application/services/trash_service.rs +++ b/src/application/services/trash_service.rs @@ -17,6 +17,7 @@ use crate::application::ports::file_lifecycle::FileLifecycleHook; use crate::application::ports::storage_ports::{FileReadPort, FileWritePort}; use crate::application::ports::trash_ports::TrashUseCase; use crate::common::errors::{DomainError, ErrorKind, Result}; +use crate::domain::entities::file::File; use crate::domain::entities::trashed_item::{TrashedItem, TrashedItemType}; use crate::domain::repositories::folder_repository::FolderRepository; use crate::domain::repositories::trash_repository::TrashRepository; @@ -809,8 +810,10 @@ fn build_trash_cursor(row: &TrashResourceRow, order_by: &str, reverse: bool) -> fn row_to_item_dto(row: TrashResourceRow) -> TrashResourceItemDto { let path = row.path.clone().unwrap_or_default(); if row.resource_type == "folder" { + let resource_id = row.resource_id.to_string(); let dto = FolderDto { - id: row.resource_id.to_string(), + etag: resource_id.clone(), + id: resource_id, name: row.name.clone(), path, parent_id: row.parent_id.map(|u| u.to_string()), @@ -834,6 +837,16 @@ fn row_to_item_dto(row: TrashResourceRow) -> TrashResourceItemDto { .as_deref() .unwrap_or("application/octet-stream"); let size_bytes = row.size.max(0) as u64; + // Route ETag through `File::compute_etag` so trash items + // match GET/HEAD/PROPFIND ETags — a client restoring a + // file may conditional-request it immediately after. + let modified_at_u = row.modified_at.timestamp() as u64; + let content_hash = row.blob_hash.clone().unwrap_or_default(); + let etag = if content_hash.is_empty() { + String::new() + } else { + File::compute_etag(&content_hash, modified_at_u) + }; let dto = FileDto { id: row.resource_id.to_string(), name: row.name.clone(), @@ -842,14 +855,15 @@ fn row_to_item_dto(row: TrashResourceRow) -> TrashResourceItemDto { mime_type: std::sync::Arc::from(mime), folder_id: row.parent_id.map(|u| u.to_string()), created_at: row.resource_created_at.timestamp() as u64, - modified_at: row.modified_at.timestamp() as u64, + modified_at: modified_at_u, icon_class: std::sync::Arc::from(icon_class_for(&row.name, mime)), icon_special_class: std::sync::Arc::from(icon_special_class_for(&row.name, mime)), category: std::sync::Arc::from(category_for(&row.name, mime)), size_formatted: format_file_size(size_bytes), owner_id: Some(row.owner_id.to_string()), sort_date: None, - etag: String::new(), + content_hash, + etag, }; TrashResourceItemDto { resource_type: ResourceTypeDto::File, diff --git a/src/bin/migrate-nfc-filenames.rs b/src/bin/migrate-nfc-filenames.rs new file mode 100644 index 00000000..270eddb7 --- /dev/null +++ b/src/bin/migrate-nfc-filenames.rs @@ -0,0 +1,326 @@ +//! `migrate-nfc-filenames` — one-shot CLI to NFC-normalize +//! `storage.files.name` across an OxiCloud instance. +//! +//! Why: PostgreSQL compares bytes literally and the `UNIQUE` +//! index on `(folder_id, name, user_id) WHERE NOT is_trashed` +//! does not catch Unicode normalization differences. macOS APFS +//! stores filenames in NFD; browsers post NFC. A file uploaded +//! from the web ("café.txt", NFC) and the same name re-uploaded +//! from a NextCloud desktop client on macOS (round-tripped to +//! NFD: `e` + combining acute) lands as two distinct rows, both +//! visible in the listing, both pointing at the same blob. +//! +//! What this does: +//! +//! 1. Scans every non-trashed file row. +//! 2. For each row whose name ≠ NFC(name): +//! - If no other row in the same `(folder_id, user_id)` already +//! holds the NFC form → UPDATE the row's name to NFC. +//! - If a collision exists with **same blob_hash**: trash the +//! newer of the two (`is_trashed = true`, `trashed_at = NOW()`). +//! User can restore from the trash UI if needed. +//! - If a collision exists with **different blob_hash**: rename +//! the newer row to `{nfc_name}.duplicate`, incrementing the +//! suffix (`.duplicate-1`, `.duplicate-2`, …) until a free name +//! is found. Preserves both files; user can inspect and resolve. +//! - In both collision cases, the surviving (older) row's name +//! is also normalized to NFC. +//! +//! Run: +//! `cargo run --bin migrate-nfc-filenames -- --dry-run` +//! `cargo run --bin migrate-nfc-filenames` +//! +//! Folder rows are NOT touched in this pass — trashing a folder +//! affects descendants; that pass is deferred to a follow-up. + +use chrono::{DateTime, Utc}; +use sqlx::{PgPool, Row}; +use std::env; +use uuid::Uuid; + +use oxicloud::domain::services::path_service::normalize_storage_name; + +#[derive(Debug, Clone)] +struct FileRow { + id: Uuid, + folder_id: Option, + user_id: Uuid, + name: String, + blob_hash: String, + created_at: DateTime, +} + +#[derive(Default)] +struct Stats { + scanned: u64, + already_nfc: u64, + normalized_in_place: u64, + deduped_same_content: u64, + renamed_duplicate: u64, +} + +#[tokio::main] +async fn main() -> Result<(), Box> { + let args: Vec = env::args().collect(); + let dry_run = args.iter().any(|a| a == "--dry-run"); + + let database_url = + env::var("DATABASE_URL").expect("DATABASE_URL must be set in the environment"); + + let pool = PgPool::connect(&database_url).await?; + + println!( + "=== NFC filename migration ({}) ===", + if dry_run { + "DRY RUN — no writes" + } else { + "EXECUTING" + } + ); + println!(); + + let rows = load_non_trashed_files(&pool).await?; + println!("Loaded {} non-trashed file rows", rows.len()); + println!(); + + let mut stats = Stats { + scanned: rows.len() as u64, + ..Default::default() + }; + + for row in &rows { + let nfc_name = normalize_storage_name(&row.name); + if nfc_name == row.name { + stats.already_nfc += 1; + continue; + } + + // 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. + let collision = find_collision(&pool, row, &nfc_name).await?; + + match collision { + None => { + println!( + "NORMALIZE {} user={} '{}' → '{}'", + row.id, row.user_id, row.name, nfc_name + ); + if !dry_run { + sqlx::query("UPDATE storage.files SET name = $1 WHERE id = $2") + .bind(&nfc_name) + .bind(row.id) + .execute(&pool) + .await?; + } + stats.normalized_in_place += 1; + } + Some(other) => { + // Pick winner/loser by `created_at` — older wins. + let (older, newer) = if row.created_at <= other.created_at { + (row, &other) + } else { + (&other, row) + }; + + if older.blob_hash == newer.blob_hash { + // 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={}", + newer.id, + older.id, + older.user_id, + &older.blob_hash[..16.min(older.blob_hash.len())] + ); + if !dry_run { + sqlx::query( + "UPDATE storage.files + SET is_trashed = TRUE, + trashed_at = NOW() + WHERE id = $1", + ) + .bind(newer.id) + .execute(&pool) + .await?; + normalize_survivor_name(&pool, older, &nfc_name).await?; + } + stats.deduped_same_content += 1; + } else { + // Different content → rename newer to a free + // `{nfc_name}.duplicate[-N]`; promote older to NFC. + let disambiguated = find_free_duplicate_name(&pool, newer, &nfc_name).await?; + println!( + "RENAME newer={} (different blob) older={} '{}' → '{}'", + newer.id, older.id, newer.name, disambiguated + ); + if !dry_run { + sqlx::query("UPDATE storage.files SET name = $1 WHERE id = $2") + .bind(&disambiguated) + .bind(newer.id) + .execute(&pool) + .await?; + normalize_survivor_name(&pool, older, &nfc_name).await?; + } + stats.renamed_duplicate += 1; + } + } + } + } + + println!(); + println!("=== Summary ==="); + println!(" scanned : {}", stats.scanned); + println!( + " already in NFC : {}", + stats.already_nfc + ); + println!( + " normalized in place (no collision) : {}", + stats.normalized_in_place + ); + println!( + " dedup-trashed (same content) : {}", + stats.deduped_same_content + ); + println!( + " renamed to .duplicate : {}", + stats.renamed_duplicate + ); + if dry_run { + println!(); + println!("DRY RUN — no rows were written. Re-run without --dry-run to apply."); + } + + Ok(()) +} + +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 + FROM storage.files + 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(FileRow { + id: r.try_get("id")?, + folder_id: r.try_get("folder_id")?, + user_id: r.try_get("user_id")?, + name: r.try_get("name")?, + blob_hash: r.try_get("blob_hash")?, + created_at: r.try_get("created_at")?, + }); + } + Ok(out) +} + +/// Looks for a row in the same `(folder_id, user_id)` 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 + 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 NOT is_trashed + LIMIT 1", + ) + .bind(nfc_name) + .bind(row.user_id) + .bind(row.folder_id) + .bind(row.id) + .fetch_optional(pool) + .await?; + + Ok(result.map(|r| FileRow { + id: r.get("id"), + folder_id: r.get("folder_id"), + user_id: r.get("user_id"), + name: r.get("name"), + blob_hash: r.get("blob_hash"), + 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. +async fn find_free_duplicate_name( + pool: &PgPool, + row: &FileRow, + 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.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 NOT is_trashed)", + ) + .bind(&candidate) + .bind(row.user_id) + .bind(row.folder_id) + .bind(row.id) + .fetch_one(pool) + .await?; + + if !taken { + return Ok(candidate); + } + suffix = suffix.saturating_add(1); + // 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 + ) + .into()); + } + } +} + +/// If the surviving (older) row's stored name is not yet in NFC, +/// UPDATE it now that the collision has been resolved. +async fn normalize_survivor_name( + pool: &PgPool, + survivor: &FileRow, + nfc_name: &str, +) -> Result<(), Box> { + if survivor.name == nfc_name { + return Ok(()); + } + sqlx::query("UPDATE storage.files SET name = $1 WHERE id = $2") + .bind(nfc_name) + .bind(survivor.id) + .execute(pool) + .await?; + Ok(()) +} diff --git a/src/domain/entities/file.rs b/src/domain/entities/file.rs index 57728d4d..f42147f9 100644 --- a/src/domain/entities/file.rs +++ b/src/domain/entities/file.rs @@ -1,6 +1,8 @@ use uuid::Uuid; -use crate::domain::services::path_service::{StoragePath, validate_storage_name}; +use crate::domain::services::path_service::{ + StoragePath, normalize_storage_name, validate_storage_name, +}; // Re-export entity errors from the centralized module pub use super::entity_errors::{FileError, FileResult}; @@ -21,7 +23,8 @@ pub struct FileParts { pub created_at: u64, pub modified_at: u64, pub owner_id: Option, - pub etag: String, + /// BLAKE3 content hash. See [`File::content_hash`] for semantics. + pub blob_hash: String, } /** @@ -66,8 +69,14 @@ pub struct File { /// Owner user ID (from storage.files.user_id) owner_id: Option, - /// Content-addressable ETag (= blob_hash). Changes on every content write. - etag: String, + /// BLAKE3 content hash. Stable across renames/moves, changes only + /// when the file's content bytes change. Source of truth for both + /// content-addressable storage and the HTTP ETag (via + /// [`File::etag`]). Exposed publicly via [`File::content_hash`] + /// so the REST API can surface it as a distinct concept from the + /// ETag (the ETag formula may grow to include `modified_at` etc., + /// but `content_hash` remains the raw hash). + blob_hash: String, } // We no longer need this module, now we use a String directly @@ -85,7 +94,7 @@ impl Default for File { created_at: 0, modified_at: 0, owner_id: None, - etag: String::new(), + blob_hash: String::new(), } } } @@ -100,6 +109,7 @@ impl File { mime_type: String, folder_id: Option, ) -> FileResult { + let name = normalize_storage_name(&name); if let Err(reason) = validate_storage_name(&name) { return Err(FileError::InvalidFileName(format!("{name}: {reason}"))); } @@ -123,7 +133,7 @@ impl File { created_at: now, modified_at: now, owner_id: None, - etag: String::new(), + blob_hash: String::new(), }) } @@ -136,6 +146,7 @@ impl File { created_at: u64, modified_at: u64, ) -> FileResult { + let name = normalize_storage_name(&name); if let Err(reason) = validate_storage_name(&name) { return Err(FileError::InvalidFileName(format!("{name}: {reason}"))); } @@ -154,7 +165,7 @@ impl File { created_at, modified_at, owner_id: None, - etag: String::new(), + blob_hash: String::new(), }) } @@ -170,7 +181,7 @@ impl File { modified_at: u64, owner_id: Option, ) -> FileResult { - Self::with_timestamps_and_etag( + Self::with_timestamps_and_blob_hash( id, name, storage_path, @@ -185,7 +196,7 @@ impl File { } #[allow(clippy::too_many_arguments)] - pub fn with_timestamps_and_etag( + pub fn with_timestamps_and_blob_hash( id: String, name: String, storage_path: StoragePath, @@ -195,8 +206,9 @@ impl File { created_at: u64, modified_at: u64, owner_id: Option, - etag: String, + blob_hash: String, ) -> FileResult { + let name = normalize_storage_name(&name); if let Err(reason) = validate_storage_name(&name) { return Err(FileError::InvalidFileName(format!("{name}: {reason}"))); } @@ -215,7 +227,7 @@ impl File { created_at, modified_at, owner_id, - etag, + blob_hash, }) } @@ -235,12 +247,67 @@ impl File { created_at: self.created_at, modified_at: self.modified_at, owner_id: self.owner_id, - etag: self.etag, + blob_hash: self.blob_hash, } } - pub fn etag(&self) -> &str { - &self.etag + /// Raw BLAKE3 content hash — the cryptographic identity of the + /// file's bytes. Stable across renames, moves, and metadata + /// updates. Changes only when the underlying content changes. + /// + /// This is **distinct from [`File::etag`]**: the ETag is an HTTP + /// cache token that may incorporate non-content signals (mtime, + /// permissions, …) in future revisions; `content_hash` is the + /// raw hash, suitable for content-addressable URLs, dedup + /// verification, and integrity audits. Keep both accessible — + /// the API layer can choose to expose `content_hash` even when + /// `etag` grows additional inputs. + pub fn content_hash(&self) -> &str { + &self.blob_hash + } + + /// Opaque HTTP ETag string (raw, NOT HTTP-quoted). Handlers wrap + /// in `"…"` themselves at the HTTP boundary. + /// + /// This is a thin instance-method wrapper around + /// [`File::compute_etag`] — see that function for the full + /// formula, rationale, and the "single source of truth" + /// guarantee that lets raw-row listings (`/api/folders/{id}/resources`, + /// favorites, trash, recents, REPORT/SEARCH) compute the same + /// value without constructing a full `File` entity. + pub fn etag(&self) -> String { + Self::compute_etag(&self.blob_hash, self.modified_at) + } + + /// Pure formula for the file ETag, exposed as a static method so + /// listing handlers that operate on raw SQL rows (rather than + /// fully-constructed `File` entities) route through the same + /// definition. + /// + /// **Formula**: `{blob_hash[..16]}-{modified_at}`. + /// + /// - The 16-char BLAKE3 prefix is the content identity (64 bits + /// ≈ 10⁻⁹ collision probability over 10M files). + /// - `modified_at` (Unix seconds) catches the `x-oc-mtime` + /// case: NextCloud preserves the client-side mtime on upload, + /// so a "touch-then-resync" of unchanged content still bumps + /// the mtime — without the suffix the ETag wouldn't change + /// and clients would serve stale metadata. + /// - When `blob_hash` is shorter than 16 chars (test fixtures, + /// stub entities) the prefix is just the whole value. + /// - Folder ETags follow a separate formula — see + /// [`crate::domain::entities::folder::Folder::compute_etag`]. + /// + /// Every handler that emits a file ETag header MUST go through + /// this function (directly or via [`File::etag`] / + /// `FileDto::etag`) so `GET`, `HEAD`, `PROPFIND`, `PUT` + /// response, `MOVE`, and every JSON listing return + /// byte-identical values for the same file. Changing the + /// formula here changes it everywhere — that is the property + /// we want. + pub fn compute_etag(blob_hash: &str, modified_at: u64) -> String { + let prefix: String = blob_hash.chars().take(16).collect(); + format!("{}-{}", prefix, modified_at) } // Getters @@ -298,7 +365,11 @@ impl File { // Create storage_path from string let storage_path = StoragePath::from_string(&path); - // Create directly without validation to avoid errors in DTO conversions + // Create directly without validation to avoid errors in DTO + // conversions. Still NFC-normalize so even DTO-reconstructed + // entities maintain the storage invariant. + let name = normalize_storage_name(&name); + Self { id, name, @@ -310,7 +381,7 @@ impl File { created_at, modified_at, owner_id: None, - etag: String::new(), + blob_hash: String::new(), } } @@ -318,6 +389,7 @@ impl File { /// Creates a new version of the file with updated name pub fn with_name(&self, new_name: String) -> FileResult { + let new_name = normalize_storage_name(&new_name); if let Err(reason) = validate_storage_name(&new_name) { return Err(FileError::InvalidFileName(format!("{new_name}: {reason}"))); } @@ -348,7 +420,7 @@ impl File { created_at: self.created_at, modified_at: now, owner_id: self.owner_id, - etag: self.etag.clone(), + blob_hash: self.blob_hash.clone(), }) } @@ -383,7 +455,7 @@ impl File { created_at: self.created_at, modified_at: now, owner_id: self.owner_id, - etag: self.etag.clone(), + blob_hash: self.blob_hash.clone(), }) } @@ -405,7 +477,7 @@ impl File { created_at: self.created_at, modified_at: now, owner_id: self.owner_id, - etag: self.etag.clone(), + blob_hash: self.blob_hash.clone(), } } } @@ -467,4 +539,77 @@ mod tests { assert_eq!(renamed.name(), "newname.txt"); assert_eq!(renamed.id(), "123"); // The ID does not change } + + /// The ETag formula is `{blob_hash[..16]}-{modified_at}`. Two + /// fixtures with identical content + mtime must produce + /// byte-identical ETags — that's the invariant every handler + /// relies on when comparing a cached client ETag against a + /// freshly-loaded one. + #[test] + fn test_etag_combines_blob_hash_prefix_and_mtime() { + let file = File::with_timestamps_and_blob_hash( + "id-1".to_string(), + "file.txt".to_string(), + StoragePath::from_string("/file.txt"), + 42, + "text/plain".to_string(), + None, + 1_000, + 2_000, + None, + "abcdef0123456789ZZZZZZZZ".to_string(), + ) + .unwrap(); + + // content_hash stays raw — full blob hash, no truncation. + assert_eq!(file.content_hash(), "abcdef0123456789ZZZZZZZZ"); + // etag is the 16-char prefix + "-" + mtime. + assert_eq!(file.etag(), "abcdef0123456789-2000"); + } + + /// When the blob hash is shorter than 16 chars (test fixtures, + /// stub entities), the prefix degrades to "whatever is there". + /// Production blob hashes are always full BLAKE3 hex (64 chars). + #[test] + fn test_etag_short_blob_hash_uses_full_value() { + let file = File::with_timestamps_and_blob_hash( + "id-1".to_string(), + "file.txt".to_string(), + StoragePath::from_string("/file.txt"), + 42, + "text/plain".to_string(), + None, + 1_000, + 2_000, + None, + "shorthash".to_string(), + ) + .unwrap(); + + assert_eq!(file.etag(), "shorthash-2000"); + } + + /// `content_hash` is the cryptographic identity of the bytes — + /// it must NEVER change because of metadata operations like + /// rename. The ETag is allowed to change (because `with_name` + /// bumps `modified_at`), but the content hash is not. + #[test] + fn test_content_hash_stable_across_rename() { + let file = File::with_timestamps_and_blob_hash( + "id-1".to_string(), + "file.txt".to_string(), + StoragePath::from_string("/file.txt"), + 42, + "text/plain".to_string(), + None, + 1_000, + 2_000, + None, + "stable-content-hash".to_string(), + ) + .unwrap(); + + let renamed = file.with_name("renamed.txt".to_string()).unwrap(); + assert_eq!(renamed.content_hash(), "stable-content-hash"); + } } diff --git a/src/domain/entities/folder.rs b/src/domain/entities/folder.rs index d4ebe33c..b08c0c76 100644 --- a/src/domain/entities/folder.rs +++ b/src/domain/entities/folder.rs @@ -1,6 +1,8 @@ use uuid::Uuid; -use crate::domain::services::path_service::{StoragePath, validate_storage_name}; +use crate::domain::services::path_service::{ + StoragePath, normalize_storage_name, validate_storage_name, +}; // Re-export entity errors from the centralized module pub use super::entity_errors::{FolderError, FolderResult}; @@ -30,8 +32,17 @@ pub struct Folder { /// Creation timestamp created_at: u64, - /// Last modification timestamp + /// Last modification timestamp of THIS folder row (rename, move, + /// metadata change). Does NOT bump when descendants change — + /// that signal lives on `tree_modified_at`. modified_at: u64, + + /// Latest `modified_at`-equivalent across the entire descendant + /// subtree. Bumped by a PostgreSQL trigger on any file or folder + /// write under this folder's ltree subtree. Source of the + /// HTTP ETag emitted in PROPFIND/GET/HEAD responses — see + /// [`Folder::etag`] for the formula and rationale. + tree_modified_at: u64, } // We no longer need this module, now we use a String directly @@ -47,6 +58,7 @@ impl Default for Folder { owner_id: None, created_at: 0, modified_at: 0, + tree_modified_at: 0, } } } @@ -70,6 +82,7 @@ impl Folder { parent_id: Option, owner_id: Option, ) -> FolderResult { + let name = normalize_storage_name(&name); // Validate folder name if let Err(reason) = validate_storage_name(&name) { return Err(FolderError::InvalidFolderName(format!("{name}: {reason}"))); @@ -92,10 +105,15 @@ impl Folder { owner_id, created_at: now, modified_at: now, + tree_modified_at: now, }) } - /// Creates a folder with specific timestamps (for reconstruction) + /// Creates a folder with specific timestamps (for reconstruction). + /// `tree_modified_at` defaults to `modified_at` — appropriate for + /// in-memory construction; database loads should always go via + /// [`Folder::with_timestamps_and_tree`] so the rollup value + /// reflects DB reality. pub fn with_timestamps( id: String, name: String, @@ -104,7 +122,7 @@ impl Folder { created_at: u64, modified_at: u64, ) -> FolderResult { - Self::with_timestamps_and_owner( + Self::with_timestamps_and_tree( id, name, storage_path, @@ -112,10 +130,15 @@ impl Folder { None, created_at, modified_at, + modified_at, ) } - /// Creates a folder with specific timestamps and owner (for DB reconstruction) + /// Creates a folder with specific timestamps and owner (legacy + /// constructor — `tree_modified_at` defaults to `modified_at`). + /// Prefer [`Folder::with_timestamps_and_tree`] for DB reconstruction + /// so the rollup ETag reflects descendant activity, not just this + /// row's own metadata. pub fn with_timestamps_and_owner( id: String, name: String, @@ -125,12 +148,37 @@ impl Folder { created_at: u64, modified_at: u64, ) -> FolderResult { - // Validate folder name + Self::with_timestamps_and_tree( + id, + name, + storage_path, + parent_id, + owner_id, + created_at, + modified_at, + modified_at, + ) + } + + /// Full constructor used by the PG repository when reading rows. + /// `tree_modified_at` comes from the trigger-maintained column on + /// `storage.folders` and feeds [`Folder::etag`]. + #[allow(clippy::too_many_arguments)] + pub fn with_timestamps_and_tree( + id: String, + name: String, + storage_path: StoragePath, + parent_id: Option, + owner_id: Option, + created_at: u64, + modified_at: u64, + tree_modified_at: u64, + ) -> FolderResult { + let name = normalize_storage_name(&name); if let Err(reason) = validate_storage_name(&name) { return Err(FolderError::InvalidFolderName(format!("{name}: {reason}"))); } - // Store the path string for serialization compatibility let path_string = storage_path.to_string(); Ok(Self { @@ -142,6 +190,7 @@ impl Folder { owner_id, created_at, modified_at, + tree_modified_at, }) } @@ -178,6 +227,54 @@ impl Folder { self.owner_id } + /// Latest descendant-write timestamp, maintained by a Postgres + /// trigger that walks the ltree ancestor chain on every file or + /// folder write inside this folder's subtree. See migration + /// `20260625000000_folder_tree_modified_at.sql` for the trigger + /// definition. + pub fn tree_modified_at(&self) -> u64 { + self.tree_modified_at + } + + /// Opaque HTTP ETag string (raw, NOT HTTP-quoted). Handlers wrap + /// in `"…"` themselves at the HTTP boundary. + /// + /// Thin instance-method wrapper around [`Folder::compute_etag`] + /// — see that function for the formula and the rationale. + /// Raw-row listings (favorites, trash, recents, search) call + /// the static form so the same formula governs every code path. + pub fn etag(&self) -> String { + Self::compute_etag(&self.id, self.tree_modified_at) + } + + /// Pure formula for the folder ETag, exposed as a static method + /// so callers that don't have a fully-constructed `Folder` (raw + /// SQL rows in listing handlers, search results, etc.) route + /// through the same definition. + /// + /// **Formula**: `{id[..16]}-{tree_modified_at}`. + /// + /// - The 16-char UUID prefix gives the folder its identity + /// component — keeps two empty same-mtime folders distinct. + /// - `tree_modified_at` (Unix seconds) is the actual signal: + /// bumped by trigger whenever ANY descendant (file or + /// sub-folder, at any depth) is created, modified, deleted, + /// or moved. This is the contract NextCloud's sync engine + /// relies on — "did anything change inside this collection + /// since I last looked?". Until this column existed, the + /// answer was always "no" because the folder UUID never + /// changed; clients had to do periodic deep PROPFIND walks + /// to discover web-uploaded files. + /// - Renaming the folder itself does NOT change the etag's + /// identity portion (UUID is stable across renames). The + /// trigger does bump `tree_modified_at` on rename via the + /// folder-side trigger, so the etag still changes — which is + /// correct, the parent collection's listing changed. + pub fn compute_etag(id: &str, tree_modified_at: u64) -> String { + let prefix: String = id.chars().take(16).collect(); + format!("{}-{}", prefix, tree_modified_at) + } + /// Creates a new Folder instance from a DTO /// This function is primarily for conversions in batch handlers pub fn from_dto( @@ -191,7 +288,14 @@ impl Folder { // Create storage_path from the string let storage_path = StoragePath::from_string(&path); - // Create directly without validation to avoid errors in DTO conversions + // Create directly without validation to avoid errors in DTO + // conversions. Still NFC-normalize so DTO-reconstructed + // entities maintain the storage invariant. + // `tree_modified_at` defaults to `modified_at`: DTO + // round-trips lose the real rollup signal, so callers that + // need a freshly-rolled-up etag must reload from the + // repository. + let name = normalize_storage_name(&name); Self { id, name, @@ -201,6 +305,7 @@ impl Folder { owner_id: None, created_at, modified_at, + tree_modified_at: modified_at, } } @@ -208,6 +313,7 @@ impl Folder { /// Creates a new version of the folder with updated name pub fn with_name(&self, new_name: String) -> FolderResult { + let new_name = normalize_storage_name(&new_name); if let Err(reason) = validate_storage_name(&new_name) { return Err(FolderError::InvalidFolderName(format!( "{new_name}: {reason}" @@ -238,6 +344,10 @@ impl Folder { owner_id: self.owner_id, created_at: self.created_at, modified_at: now, + // Renaming bumps both self and descendant rollup — + // ancestors' listings now show a new name, so the + // collection has materially changed. + tree_modified_at: now, }) } @@ -270,6 +380,7 @@ impl Folder { owner_id: self.owner_id, created_at: self.created_at, modified_at: now, + tree_modified_at: now, }) } @@ -343,4 +454,90 @@ mod tests { assert_eq!(renamed.name(), "new_name"); assert_eq!(renamed.id(), "123"); // The ID doesn't change } + + /// The folder ETag is `{id[..16]}-{tree_modified_at}`. Two + /// fixtures with identical id-prefix + tree_modified_at must + /// produce byte-identical ETags — that's what NC's incremental + /// sync relies on across PROPFIND cycles. + #[test] + fn test_etag_combines_id_prefix_and_tree_modified_at() { + let folder = Folder::with_timestamps_and_tree( + "0123456789abcdefZZZZZZZZ".to_string(), + "folder".to_string(), + StoragePath::from_string("/folder"), + None, + None, + 1_000, + 2_000, + 5_000, + ) + .unwrap(); + + assert_eq!(folder.tree_modified_at(), 5_000); + assert_eq!(folder.etag(), "0123456789abcdef-5000"); + } + + /// Two folders with the same `tree_modified_at` but different + /// IDs must NOT collide on ETag — the id prefix is the identity + /// portion that keeps them distinct. + #[test] + fn test_etag_distinct_folders_same_tree_mtime() { + let a = Folder::with_timestamps_and_tree( + "aaaaaaaaaaaaaaaaZZZZZZZZ".to_string(), + "a".to_string(), + StoragePath::from_string("/a"), + None, + None, + 0, + 0, + 42, + ) + .unwrap(); + let b = Folder::with_timestamps_and_tree( + "bbbbbbbbbbbbbbbbZZZZZZZZ".to_string(), + "b".to_string(), + StoragePath::from_string("/b"), + None, + None, + 0, + 0, + 42, + ) + .unwrap(); + + assert_ne!(a.etag(), b.etag()); + } + + /// `tree_modified_at` is the actual change-detection signal — + /// the trigger bumps it for descendant writes. Renaming the + /// folder bumps both `modified_at` and `tree_modified_at` + /// (the parent collection's listing changed), and the etag + /// must reflect that — otherwise NC won't notice the rename. + #[test] + fn test_etag_changes_when_tree_modified_at_changes() { + let folder_a = Folder::with_timestamps_and_tree( + "abcd1234efgh5678ZZZZZZZZ".to_string(), + "folder".to_string(), + StoragePath::from_string("/folder"), + None, + None, + 1_000, + 2_000, + 3_000, + ) + .unwrap(); + let folder_b = Folder::with_timestamps_and_tree( + "abcd1234efgh5678ZZZZZZZZ".to_string(), + "folder".to_string(), + StoragePath::from_string("/folder"), + None, + None, + 1_000, + 2_000, + 4_000, + ) + .unwrap(); + + assert_ne!(folder_a.etag(), folder_b.etag()); + } } diff --git a/src/domain/services/path_service.rs b/src/domain/services/path_service.rs index 5c147ba7..70fa4b4c 100644 --- a/src/domain/services/path_service.rs +++ b/src/domain/services/path_service.rs @@ -5,6 +5,29 @@ //! infrastructure/services/path_service.rs because it has file system dependencies. use std::path::PathBuf; +use unicode_normalization::UnicodeNormalization; + +/// NFC-normalize a single file or folder name component. +/// +/// The storage layer (PostgreSQL `storage.files.name` and +/// `storage.folders.name`) compares bytes literally — there is no +/// Unicode-aware collation in either UNIQUE index. macOS APFS stores +/// filenames in NFD (decomposed: `é` = `e` + U+0301), while browsers +/// and most other clients post NFC (`é` = U+00E9). Without +/// normalization, the same logical filename can land as two distinct +/// rows: one from a web upload, one from a NextCloud desktop client +/// re-upload of the round-tripped name. The UNIQUE index does not +/// catch it because the bytes differ. +/// +/// This function is called at every name-receiving boundary (entity +/// constructors, repository path lookups) so the database invariant +/// becomes "every stored name is NFC". A one-shot migration +/// (`migrate-nfc-filenames`) cleans up rows that pre-date this rule. +/// +/// Pure function — no I/O, allocates one `String`. +pub fn normalize_storage_name(name: &str) -> String { + name.nfc().collect() +} /// Validates a single file or folder name component. /// @@ -251,4 +274,43 @@ mod tests { assert!(!path.segments().contains(&"..".to_string())); assert!(!path.segments().contains(&".".to_string())); } + + // ── NFC normalization tests ───────────────────────────────── + + /// Plain ASCII names must round-trip identical bytes. + #[test] + fn test_normalize_ascii_unchanged() { + assert_eq!(normalize_storage_name("file.txt"), "file.txt"); + assert_eq!(normalize_storage_name("My Documents"), "My Documents"); + } + + /// The macOS APFS / NextCloud-desktop pathological case: `é` + /// decomposed as `e` + combining acute (U+0301). Stored bytes + /// `65 cc 81` collapse to NFC `c3 a9`. + #[test] + fn test_normalize_nfd_to_nfc() { + let nfd = "caf\u{0065}\u{0301}"; + let nfc = "caf\u{00E9}"; + assert_ne!(nfd.as_bytes(), nfc.as_bytes()); + assert_eq!(normalize_storage_name(nfd), nfc); + } + + /// Already-NFC input must round-trip unchanged. This is the + /// idempotence property the boundary normalization relies on. + #[test] + fn test_normalize_nfc_idempotent() { + let nfc = "Capture d\u{2019}\u{00E9}cran.png"; + assert_eq!(normalize_storage_name(nfc), nfc); + // And applying twice is the same as once. + assert_eq!(normalize_storage_name(&normalize_storage_name(nfc)), nfc); + } + + /// Multi-codepoint NFD sequences (combining acute + grave + + /// typographic apostrophe) all converge to a single NFC form. + #[test] + fn test_normalize_mixed_accents() { + let nfd = "Capture d\u{2019}\u{0065}\u{0301}cran a\u{0300}.png"; + let nfc = "Capture d\u{2019}\u{00E9}cran \u{00E0}.png"; + assert_eq!(normalize_storage_name(nfd), nfc); + } } diff --git a/src/infrastructure/repositories/pg/favorites_pg_repository.rs b/src/infrastructure/repositories/pg/favorites_pg_repository.rs index 03d00a9e..3020722b 100644 --- a/src/infrastructure/repositories/pg/favorites_pg_repository.rs +++ b/src/infrastructure/repositories/pg/favorites_pg_repository.rs @@ -310,6 +310,7 @@ impl FavoritesRepositoryPort for FavoritesPgRepository { fld.created_at AS resource_created_at, fld.updated_at AS modified_at, fld.user_id AS owner_id, + NULL::text AS blob_hash, (fld.user_id = $1::uuid) AS is_owner, uf.created_at AS favorited_at, fld.path::text AS resource_path, @@ -332,6 +333,7 @@ impl FavoritesRepositoryPort for FavoritesPgRepository { f.created_at AS resource_created_at, f.updated_at AS modified_at, f.user_id AS owner_id, + f.blob_hash, (f.user_id = $1::uuid) AS is_owner, uf.created_at AS favorited_at, COALESCE(pfld.path::text || '/' || f.name, f.name) AS resource_path, @@ -574,6 +576,7 @@ LIMIT $6" resource_created_at: row.get("resource_created_at"), modified_at: row.get("modified_at"), owner_id: row.get("owner_id"), + blob_hash: row.try_get("blob_hash").ok(), is_owner: row.try_get("is_owner").unwrap_or(false), favorited_at: row.get("favorited_at"), path: row.try_get("resource_path").ok(), diff --git a/src/infrastructure/repositories/pg/file_blob_read_repository.rs b/src/infrastructure/repositories/pg/file_blob_read_repository.rs index 8bb55f53..3077001a 100644 --- a/src/infrastructure/repositories/pg/file_blob_read_repository.rs +++ b/src/infrastructure/repositories/pg/file_blob_read_repository.rs @@ -131,11 +131,11 @@ impl FileBlobReadRepository { mime_type: String, created_at: i64, modified_at: i64, - etag: String, + blob_hash: String, owner_id: Option, ) -> Result { let storage_path = Self::make_file_path(folder_path.as_deref(), &name); - File::with_timestamps_and_etag( + File::with_timestamps_and_blob_hash( id, name, storage_path, @@ -145,7 +145,7 @@ impl FileBlobReadRepository { created_at as u64, modified_at as u64, owner_id, - etag, + blob_hash, ) .map_err(|e| DomainError::internal_error("FileBlobRead", format!("entity: {e}"))) } @@ -226,9 +226,9 @@ impl FileBlobReadRepository { let mut files = Vec::with_capacity(rows.len()); let mut sort_dates = Vec::with_capacity(rows.len()); - for (id, name, fid, fpath, size, mime, ca, ma, etag, uid, sd) in rows { + for (id, name, fid, fpath, size, mime, ca, ma, blob_hash, uid, sd) in rows { files.push(Self::row_to_file( - id, name, fid, fpath, size, mime, ca, ma, etag, uid, + id, name, fid, fpath, size, mime, ca, ma, blob_hash, uid, )?); sort_dates.push(sd); } @@ -410,9 +410,11 @@ impl FileReadPort for FileBlobReadRepository { .map_err(|e| DomainError::internal_error("FileBlobRead", format!("list: {e}")))?; rows.into_iter() - .map(|(id, name, fid, fpath, size, mime, ca, ma, etag, uid)| { - Self::row_to_file(id, name, fid, fpath, size, mime, ca, ma, etag, uid) - }) + .map( + |(id, name, fid, fpath, size, mime, ca, ma, blob_hash, uid)| { + Self::row_to_file(id, name, fid, fpath, size, mime, ca, ma, blob_hash, uid) + }, + ) .collect() } @@ -466,9 +468,11 @@ impl FileReadPort for FileBlobReadRepository { .map_err(|e| DomainError::internal_error("FileBlobRead", format!("list_for_owner: {e}")))?; rows.into_iter() - .map(|(id, name, fid, fpath, size, mime, ca, ma, etag, uid)| { - Self::row_to_file(id, name, fid, fpath, size, mime, ca, ma, etag, uid) - }) + .map( + |(id, name, fid, fpath, size, mime, ca, ma, blob_hash, uid)| { + Self::row_to_file(id, name, fid, fpath, size, mime, ca, ma, blob_hash, uid) + }, + ) .collect() } @@ -532,9 +536,11 @@ impl FileReadPort for FileBlobReadRepository { .map_err(|e| DomainError::internal_error("FileBlobRead", format!("list_batch: {e}")))?; rows.into_iter() - .map(|(id, name, fid, fpath, size, mime, ca, ma, etag, uid)| { - Self::row_to_file(id, name, fid, fpath, size, mime, ca, ma, etag, uid) - }) + .map( + |(id, name, fid, fpath, size, mime, ca, ma, blob_hash, uid)| { + Self::row_to_file(id, name, fid, fpath, size, mime, ca, ma, blob_hash, uid) + }, + ) .collect() } @@ -598,9 +604,11 @@ impl FileReadPort for FileBlobReadRepository { })?; rows.into_iter() - .map(|(id, name, fid, fpath, size, mime, ca, ma, etag, uid)| { - Self::row_to_file(id, name, fid, fpath, size, mime, ca, ma, etag, uid) - }) + .map( + |(id, name, fid, fpath, size, mime, ca, ma, blob_hash, uid)| { + Self::row_to_file(id, name, fid, fpath, size, mime, ca, ma, blob_hash, uid) + }, + ) .collect() } @@ -699,8 +707,14 @@ impl FileReadPort for FileBlobReadRepository { return Ok(None); } - // Last segment is the filename, preceding segments are the folder path - let filename = segments[segments.len() - 1]; + // Last segment is the filename, preceding segments are the + // folder path. NFC-normalize the filename so a NextCloud + // client's NFD-encoded path still hits the NFC row stored + // by a web upload — see `normalize_storage_name` for the + // full rationale. + let filename = crate::domain::services::path_service::normalize_storage_name( + segments[segments.len() - 1], + ); let folder_path = segments[..segments.len() - 1].join("/"); let row = if folder_path.is_empty() { @@ -815,9 +829,9 @@ impl FileReadPort for FileBlobReadRepository { while let Some(row) = row_stream.try_next().await.map_err(|e| { DomainError::internal_error("FileBlobRead", format!("subtree stream: {e}")) })? { - let (id, name, fid, fpath, size, mime, ca, ma, etag, uid) = row; + let (id, name, fid, fpath, size, mime, ca, ma, blob_hash, uid) = row; let file = FileBlobReadRepository::row_to_file( - id, name, fid, fpath, size, mime, ca, ma, etag, uid, + id, name, fid, fpath, size, mime, ca, ma, blob_hash, uid, )?; yield file; } @@ -930,8 +944,8 @@ impl FileReadPort for FileBlobReadRepository { let files = rows .into_iter() .map( - |(id, name, fid, fpath, size, mime, ca, ma, etag, uid, _total)| { - Self::row_to_file(id, name, fid, fpath, size, mime, ca, ma, etag, uid) + |(id, name, fid, fpath, size, mime, ca, ma, blob_hash, uid, _total)| { + Self::row_to_file(id, name, fid, fpath, size, mime, ca, ma, blob_hash, uid) }, ) .collect::, _>>() @@ -1116,8 +1130,8 @@ impl FileReadPort for FileBlobReadRepository { let files = rows .into_iter() .map( - |(id, name, fid, fpath, size, mime, ca, ma, etag, uid, _total)| { - Self::row_to_file(id, name, fid, fpath, size, mime, ca, ma, etag, uid) + |(id, name, fid, fpath, size, mime, ca, ma, blob_hash, uid, _total)| { + Self::row_to_file(id, name, fid, fpath, size, mime, ca, ma, blob_hash, uid) }, ) .collect::, _>>() @@ -1212,9 +1226,11 @@ impl FileReadPort for FileBlobReadRepository { .map_err(|e| DomainError::internal_error("FileBlobRead", format!("suggest: {e}")))?; rows.into_iter() - .map(|(id, name, fid, fpath, size, mime, ca, ma, etag, uid)| { - Self::row_to_file(id, name, fid, fpath, size, mime, ca, ma, etag, uid) - }) + .map( + |(id, name, fid, fpath, size, mime, ca, ma, blob_hash, uid)| { + Self::row_to_file(id, name, fid, fpath, size, mime, ca, ma, blob_hash, uid) + }, + ) .collect() } } diff --git a/src/infrastructure/repositories/pg/file_blob_write_repository.rs b/src/infrastructure/repositories/pg/file_blob_write_repository.rs index 3bc1a9bd..7819f939 100644 --- a/src/infrastructure/repositories/pg/file_blob_write_repository.rs +++ b/src/infrastructure/repositories/pg/file_blob_write_repository.rs @@ -101,10 +101,10 @@ impl FileBlobWriteRepository { created_at: i64, modified_at: i64, owner_id: Option, - etag: String, + blob_hash: String, ) -> Result { let storage_path = Self::make_file_path(folder_path.as_deref(), &name); - File::with_timestamps_and_etag( + File::with_timestamps_and_blob_hash( id, name, storage_path, @@ -114,7 +114,7 @@ impl FileBlobWriteRepository { created_at as u64, modified_at as u64, owner_id, - etag, + blob_hash, ) .map_err(|e| DomainError::internal_error("FileBlobWrite", format!("entity: {e}"))) } diff --git a/src/infrastructure/repositories/pg/folder_db_repository.rs b/src/infrastructure/repositories/pg/folder_db_repository.rs index 508370c6..ea6b3466 100644 --- a/src/infrastructure/repositories/pg/folder_db_repository.rs +++ b/src/infrastructure/repositories/pg/folder_db_repository.rs @@ -20,10 +20,25 @@ use crate::domain::services::authorization::ResourceKind; use crate::domain::services::path_service::StoragePath; /// Type alias for folder metadata rows from SQL queries. -type FolderRow = (String, String, String, Option, Uuid, i64, i64); +/// Tuple order: id, name, path, parent_id, user_id, created_at, +/// modified_at, tree_modified_at. The trailing `tree_modified_at` +/// feeds [`Folder::etag`] — every SELECT here must include +/// `EXTRACT(EPOCH FROM tree_modified_at)::bigint`. +type FolderRow = (String, String, String, Option, Uuid, i64, i64, i64); -/// Type alias for paginated folder rows (includes total_count). -type FolderRowPaginated = (String, String, String, Option, Uuid, i64, i64, i64); +/// Type alias for paginated folder rows (includes total_count as +/// the last element after `tree_modified_at`). +type FolderRowPaginated = ( + String, + String, + String, + Option, + Uuid, + i64, + i64, + i64, + i64, +); /// Type alias for folder rows with optional user_id. type FolderRowOptUser = ( @@ -34,6 +49,7 @@ type FolderRowOptUser = ( Option, i64, i64, + i64, ); /// PostgreSQL-backed folder repository. @@ -68,6 +84,7 @@ impl FolderDbRepository { /// /// The `path` comes directly from the materialized `path` column — no /// extra queries needed. + #[allow(clippy::too_many_arguments)] fn row_to_folder( id: String, name: String, @@ -76,9 +93,10 @@ impl FolderDbRepository { user_id: Option, created_at: i64, modified_at: i64, + tree_modified_at: i64, ) -> Result { let storage_path = StoragePath::from_string(&path); - Folder::with_timestamps_and_owner( + Folder::with_timestamps_and_tree( id, name, storage_path, @@ -86,6 +104,7 @@ impl FolderDbRepository { user_id, created_at as u64, modified_at as u64, + tree_modified_at as u64, ) .map_err(|e| DomainError::internal_error("FolderDb", format!("entity: {e}"))) } @@ -116,14 +135,15 @@ impl FolderRepository for FolderDbRepository { )); }; - let row = sqlx::query_as::<_, (String, String, i64, i64)>( + let row = sqlx::query_as::<_, (String, String, i64, i64, i64)>( r#" INSERT INTO storage.folders (name, parent_id, user_id) VALUES ($1, $2::uuid, $3) RETURNING id::text, path, EXTRACT(EPOCH FROM created_at)::bigint, - EXTRACT(EPOCH FROM updated_at)::bigint + EXTRACT(EPOCH FROM updated_at)::bigint, + EXTRACT(EPOCH FROM tree_modified_at)::bigint "#, ) .bind(&name) @@ -143,15 +163,25 @@ impl FolderRepository for FolderDbRepository { DomainError::internal_error("FolderDb", format!("insert: {e}")) })?; - Self::row_to_folder(row.0, name, row.1, parent_id, Some(user_id), row.2, row.3) + Self::row_to_folder( + row.0, + name, + row.1, + parent_id, + Some(user_id), + row.2, + row.3, + row.4, + ) } async fn get_folder(&self, id: &str) -> Result { - let row = sqlx::query_as::<_, (String, String, String, Option, Uuid, i64, i64)>( + let row = sqlx::query_as::<_, FolderRow>( r#" SELECT id::text, name, path, parent_id::text, user_id, EXTRACT(EPOCH FROM created_at)::bigint, - EXTRACT(EPOCH FROM updated_at)::bigint + EXTRACT(EPOCH FROM updated_at)::bigint, + EXTRACT(EPOCH FROM tree_modified_at)::bigint FROM storage.folders WHERE id = $1::uuid AND NOT is_trashed "#, @@ -162,7 +192,7 @@ impl FolderRepository for FolderDbRepository { .map_err(|e| DomainError::internal_error("FolderDb", format!("get: {e}")))? .ok_or_else(|| DomainError::not_found("Folder", id))?; - Self::row_to_folder(row.0, row.1, row.2, row.3, Some(row.4), row.5, row.6) + Self::row_to_folder(row.0, row.1, row.2, row.3, Some(row.4), row.5, row.6, row.7) } async fn get_folder_by_path(&self, storage_path: &StoragePath) -> Result { @@ -174,11 +204,12 @@ impl FolderRepository for FolderDbRepository { return Err(DomainError::not_found("Folder", "empty path")); } - let row = sqlx::query_as::<_, (String, String, String, Option, Uuid, i64, i64)>( + let row = sqlx::query_as::<_, FolderRow>( r#" SELECT id::text, name, path, parent_id::text, user_id, EXTRACT(EPOCH FROM created_at)::bigint, - EXTRACT(EPOCH FROM updated_at)::bigint + EXTRACT(EPOCH FROM updated_at)::bigint, + EXTRACT(EPOCH FROM tree_modified_at)::bigint FROM storage.folders WHERE path = $1 AND NOT is_trashed "#, @@ -189,7 +220,7 @@ impl FolderRepository for FolderDbRepository { .map_err(|e| DomainError::internal_error("FolderDb", format!("path lookup: {e}")))? .ok_or_else(|| DomainError::not_found("Folder", lookup))?; - Self::row_to_folder(row.0, row.1, row.2, row.3, Some(row.4), row.5, row.6) + Self::row_to_folder(row.0, row.1, row.2, row.3, Some(row.4), row.5, row.6, row.7) } #[allow(clippy::type_complexity)] @@ -199,7 +230,8 @@ impl FolderRepository for FolderDbRepository { r#" SELECT id::text, name, path, parent_id::text, user_id, EXTRACT(EPOCH FROM created_at)::bigint, - EXTRACT(EPOCH FROM updated_at)::bigint + EXTRACT(EPOCH FROM updated_at)::bigint, + EXTRACT(EPOCH FROM tree_modified_at)::bigint FROM storage.folders WHERE parent_id = $1::uuid AND NOT is_trashed ORDER BY name @@ -213,7 +245,8 @@ impl FolderRepository for FolderDbRepository { r#" SELECT id::text, name, path, parent_id::text, user_id, EXTRACT(EPOCH FROM created_at)::bigint, - EXTRACT(EPOCH FROM updated_at)::bigint + EXTRACT(EPOCH FROM updated_at)::bigint, + EXTRACT(EPOCH FROM tree_modified_at)::bigint FROM storage.folders WHERE parent_id IS NULL AND NOT is_trashed ORDER BY name @@ -225,8 +258,8 @@ impl FolderRepository for FolderDbRepository { .map_err(|e| DomainError::internal_error("FolderDb", format!("list: {e}")))?; rows.into_iter() - .map(|(id, name, path, pid, uid, ca, ma)| { - Self::row_to_folder(id, name, path, pid, Some(uid), ca, ma) + .map(|(id, name, path, pid, uid, ca, ma, tma)| { + Self::row_to_folder(id, name, path, pid, Some(uid), ca, ma, tma) }) .collect() } @@ -242,7 +275,8 @@ impl FolderRepository for FolderDbRepository { r#" SELECT id::text, name, path, parent_id::text, user_id, EXTRACT(EPOCH FROM created_at)::bigint, - EXTRACT(EPOCH FROM updated_at)::bigint + EXTRACT(EPOCH FROM updated_at)::bigint, + EXTRACT(EPOCH FROM tree_modified_at)::bigint FROM storage.folders WHERE parent_id = $1::uuid AND user_id = $2 AND NOT is_trashed ORDER BY name @@ -257,7 +291,8 @@ impl FolderRepository for FolderDbRepository { r#" SELECT id::text, name, path, parent_id::text, user_id, EXTRACT(EPOCH FROM created_at)::bigint, - EXTRACT(EPOCH FROM updated_at)::bigint + EXTRACT(EPOCH FROM updated_at)::bigint, + EXTRACT(EPOCH FROM tree_modified_at)::bigint FROM storage.folders WHERE parent_id IS NULL AND user_id = $1 AND NOT is_trashed ORDER BY name @@ -270,8 +305,8 @@ impl FolderRepository for FolderDbRepository { .map_err(|e| DomainError::internal_error("FolderDb", format!("list_by_owner: {e}")))?; rows.into_iter() - .map(|(id, name, path, pid, uid, ca, ma)| { - Self::row_to_folder(id, name, path, pid, Some(uid), ca, ma) + .map(|(id, name, path, pid, uid, ca, ma, tma)| { + Self::row_to_folder(id, name, path, pid, Some(uid), ca, ma, tma) }) .collect() } @@ -293,6 +328,7 @@ impl FolderRepository for FolderDbRepository { SELECT id::text, name, path, parent_id::text, user_id, EXTRACT(EPOCH FROM created_at)::bigint, EXTRACT(EPOCH FROM updated_at)::bigint, + EXTRACT(EPOCH FROM tree_modified_at)::bigint, COUNT(*) OVER() AS total_count FROM storage.folders WHERE parent_id = $1::uuid AND NOT is_trashed @@ -311,6 +347,7 @@ impl FolderRepository for FolderDbRepository { SELECT id::text, name, path, parent_id::text, user_id, EXTRACT(EPOCH FROM created_at)::bigint, EXTRACT(EPOCH FROM updated_at)::bigint, + EXTRACT(EPOCH FROM tree_modified_at)::bigint, COUNT(*) OVER() AS total_count FROM storage.folders WHERE parent_id IS NULL AND NOT is_trashed @@ -327,15 +364,15 @@ impl FolderRepository for FolderDbRepository { // total_count is identical in every row; 0 when the result set is empty. let total = if include_total { - Some(rows.first().map_or(0, |r| r.7) as usize) + Some(rows.first().map_or(0, |r| r.8) as usize) } else { None }; let folders: Result, DomainError> = rows .into_iter() - .map(|(id, name, path, pid, uid, ca, ma, _total)| { - Self::row_to_folder(id, name, path, pid, Some(uid), ca, ma) + .map(|(id, name, path, pid, uid, ca, ma, tma, _total)| { + Self::row_to_folder(id, name, path, pid, Some(uid), ca, ma, tma) }) .collect(); Ok((folders?, total)) @@ -358,6 +395,7 @@ impl FolderRepository for FolderDbRepository { SELECT id::text, name, path, parent_id::text, user_id, EXTRACT(EPOCH FROM created_at)::bigint, EXTRACT(EPOCH FROM updated_at)::bigint, + EXTRACT(EPOCH FROM tree_modified_at)::bigint, COUNT(*) OVER() AS total_count FROM storage.folders WHERE parent_id = $1::uuid AND user_id = $2 AND NOT is_trashed @@ -377,6 +415,7 @@ impl FolderRepository for FolderDbRepository { SELECT id::text, name, path, parent_id::text, user_id, EXTRACT(EPOCH FROM created_at)::bigint, EXTRACT(EPOCH FROM updated_at)::bigint, + EXTRACT(EPOCH FROM tree_modified_at)::bigint, COUNT(*) OVER() AS total_count FROM storage.folders WHERE parent_id IS NULL AND user_id = $1 AND NOT is_trashed @@ -393,15 +432,15 @@ impl FolderRepository for FolderDbRepository { .map_err(|e| DomainError::internal_error("FolderDb", format!("paginate_by_owner: {e}")))?; let total = if include_total { - Some(rows.first().map_or(0, |r| r.7) as usize) + Some(rows.first().map_or(0, |r| r.8) as usize) } else { None }; let folders: Result, DomainError> = rows .into_iter() - .map(|(id, name, path, pid, uid, ca, ma, _total)| { - Self::row_to_folder(id, name, path, pid, Some(uid), ca, ma) + .map(|(id, name, path, pid, uid, ca, ma, tma, _total)| { + Self::row_to_folder(id, name, path, pid, Some(uid), ca, ma, tma) }) .collect(); Ok((folders?, total)) @@ -411,14 +450,15 @@ impl FolderRepository for FolderDbRepository { // The BEFORE UPDATE trigger recomputes path/lpath for this row; // the AFTER UPDATE cascade trigger then batch-updates all // descendants in a single UPDATE using the GiST lpath index. - let row = sqlx::query_as::<_, (String, String, String, Option, Uuid, i64, i64)>( + let row = sqlx::query_as::<_, FolderRow>( r#" UPDATE storage.folders SET name = $1, updated_at = NOW() WHERE id = $2::uuid AND NOT is_trashed RETURNING id::text, name, path, parent_id::text, user_id, EXTRACT(EPOCH FROM created_at)::bigint, - EXTRACT(EPOCH FROM updated_at)::bigint + EXTRACT(EPOCH FROM updated_at)::bigint, + EXTRACT(EPOCH FROM tree_modified_at)::bigint "#, ) .bind(&new_name) @@ -435,7 +475,7 @@ impl FolderRepository for FolderDbRepository { })? .ok_or_else(|| DomainError::not_found("Folder", id))?; - Self::row_to_folder(row.0, row.1, row.2, row.3, Some(row.4), row.5, row.6) + Self::row_to_folder(row.0, row.1, row.2, row.3, Some(row.4), row.5, row.6, row.7) } async fn move_folder( @@ -446,14 +486,15 @@ impl FolderRepository for FolderDbRepository { // The BEFORE UPDATE trigger recomputes path/lpath for this row; // the AFTER UPDATE cascade trigger then batch-updates all // descendants in a single UPDATE using the GiST lpath index. - let row = sqlx::query_as::<_, (String, String, String, Option, Uuid, i64, i64)>( + let row = sqlx::query_as::<_, FolderRow>( r#" UPDATE storage.folders SET parent_id = $1::uuid, updated_at = NOW() WHERE id = $2::uuid AND NOT is_trashed RETURNING id::text, name, path, parent_id::text, user_id, EXTRACT(EPOCH FROM created_at)::bigint, - EXTRACT(EPOCH FROM updated_at)::bigint + EXTRACT(EPOCH FROM updated_at)::bigint, + EXTRACT(EPOCH FROM tree_modified_at)::bigint "#, ) .bind(new_parent_id) @@ -463,7 +504,7 @@ impl FolderRepository for FolderDbRepository { .map_err(|e| DomainError::internal_error("FolderDb", format!("move: {e}")))? .ok_or_else(|| DomainError::not_found("Folder", id))?; - Self::row_to_folder(row.0, row.1, row.2, row.3, Some(row.4), row.5, row.6) + Self::row_to_folder(row.0, row.1, row.2, row.3, Some(row.4), row.5, row.6, row.7) } async fn delete_folder(&self, id: &str) -> Result<(), DomainError> { @@ -622,7 +663,7 @@ impl FolderRepository for FolderDbRepository { } async fn create_home_folder(&self, user_id: Uuid, name: String) -> Result { - let row = sqlx::query_as::<_, (String, String, i64, i64)>( + let row = sqlx::query_as::<_, (String, String, i64, i64, i64)>( r#" INSERT INTO storage.folders (name, parent_id, user_id) VALUES ($1, NULL, $2) @@ -630,7 +671,8 @@ impl FolderRepository for FolderDbRepository { RETURNING id::text, path, EXTRACT(EPOCH FROM created_at)::bigint, - EXTRACT(EPOCH FROM updated_at)::bigint + EXTRACT(EPOCH FROM updated_at)::bigint, + EXTRACT(EPOCH FROM tree_modified_at)::bigint "#, ) .bind(&name) @@ -640,17 +682,18 @@ impl FolderRepository for FolderDbRepository { .map_err(|e| DomainError::internal_error("FolderDb", format!("home folder: {e}")))?; match row { - Some((id, path, ca, ma)) => { - Self::row_to_folder(id, name.clone(), path, None, Some(user_id), ca, ma) + Some((id, path, ca, ma, tma)) => { + Self::row_to_folder(id, name.clone(), path, None, Some(user_id), ca, ma, tma) } None => { // Already exists — fetch it - let existing = sqlx::query_as::<_, (String, String, i64, i64)>( + let existing = sqlx::query_as::<_, (String, String, i64, i64, i64)>( r#" SELECT id::text, path, EXTRACT(EPOCH FROM created_at)::bigint, - EXTRACT(EPOCH FROM updated_at)::bigint + EXTRACT(EPOCH FROM updated_at)::bigint, + EXTRACT(EPOCH FROM tree_modified_at)::bigint FROM storage.folders WHERE name = $1 AND user_id = $2 AND parent_id IS NULL "#, @@ -668,6 +711,7 @@ impl FolderRepository for FolderDbRepository { Some(user_id), existing.2, existing.3, + existing.4, ) } } @@ -682,7 +726,8 @@ impl FolderRepository for FolderDbRepository { let sql = "SELECT fo.id::text, fo.name, fo.path, fo.parent_id::text, \ fo.user_id, \ EXTRACT(EPOCH FROM fo.created_at)::bigint, \ - EXTRACT(EPOCH FROM fo.updated_at)::bigint \ + EXTRACT(EPOCH FROM fo.updated_at)::bigint, \ + EXTRACT(EPOCH FROM fo.tree_modified_at)::bigint \ FROM storage.folders fo \ WHERE fo.is_trashed = false \ AND fo.lpath <@ (SELECT lpath FROM storage.folders WHERE id = $1::uuid) \ @@ -697,8 +742,8 @@ impl FolderRepository for FolderDbRepository { })?; rows.into_iter() - .map(|(id, name, path, pid, uid, ca, ma)| { - Self::row_to_folder(id, name, path, pid, uid, ca, ma) + .map(|(id, name, path, pid, uid, ca, ma, tma)| { + Self::row_to_folder(id, name, path, pid, uid, ca, ma, tma) }) .collect() } @@ -744,7 +789,8 @@ impl FolderRepository for FolderDbRepository { "SELECT fo.id::text, fo.name, fo.path, fo.parent_id::text, \ fo.user_id, \ EXTRACT(EPOCH FROM fo.created_at)::bigint, \ - EXTRACT(EPOCH FROM fo.updated_at)::bigint \ + EXTRACT(EPOCH FROM fo.updated_at)::bigint, \ + EXTRACT(EPOCH FROM fo.tree_modified_at)::bigint \ FROM storage.folders fo \ WHERE fo.user_id = $1 \ AND fo.is_trashed = false \ @@ -768,8 +814,8 @@ impl FolderRepository for FolderDbRepository { return rows .into_iter() - .map(|(id, name, path, pid, uid, ca, ma)| { - Self::row_to_folder(id, name, path, pid, uid, ca, ma) + .map(|(id, name, path, pid, uid, ca, ma, tma)| { + Self::row_to_folder(id, name, path, pid, uid, ca, ma, tma) }) .collect(); } @@ -780,7 +826,8 @@ impl FolderRepository for FolderDbRepository { "SELECT fo.id::text, fo.name, fo.path, fo.parent_id::text, \ fo.user_id, \ EXTRACT(EPOCH FROM fo.created_at)::bigint, \ - EXTRACT(EPOCH FROM fo.updated_at)::bigint \ + EXTRACT(EPOCH FROM fo.updated_at)::bigint, \ + EXTRACT(EPOCH FROM fo.tree_modified_at)::bigint \ FROM storage.folders fo \ WHERE fo.parent_id = $1::uuid \ AND fo.user_id = $2 \ @@ -798,7 +845,8 @@ impl FolderRepository for FolderDbRepository { "SELECT fo.id::text, fo.name, fo.path, fo.parent_id::text, \ fo.user_id, \ EXTRACT(EPOCH FROM fo.created_at)::bigint, \ - EXTRACT(EPOCH FROM fo.updated_at)::bigint \ + EXTRACT(EPOCH FROM fo.updated_at)::bigint, \ + EXTRACT(EPOCH FROM fo.tree_modified_at)::bigint \ FROM storage.folders fo \ WHERE fo.parent_id IS NULL \ AND fo.user_id = $1 \ @@ -838,8 +886,8 @@ impl FolderRepository for FolderDbRepository { .map_err(|e| DomainError::internal_error("FolderDb", format!("search_folders: {e}")))?; rows.into_iter() - .map(|(id, name, path, pid, uid, ca, ma)| { - Self::row_to_folder(id, name, path, pid, uid, ca, ma) + .map(|(id, name, path, pid, uid, ca, ma, tma)| { + Self::row_to_folder(id, name, path, pid, uid, ca, ma, tma) }) .collect() } @@ -866,7 +914,8 @@ impl FolderRepository for FolderDbRepository { "SELECT fo.id::text, fo.name, fo.path, fo.parent_id::text, \ fo.user_id, \ EXTRACT(EPOCH FROM fo.created_at)::bigint, \ - EXTRACT(EPOCH FROM fo.updated_at)::bigint \ + EXTRACT(EPOCH FROM fo.updated_at)::bigint, \ + EXTRACT(EPOCH FROM fo.tree_modified_at)::bigint \ FROM storage.folders fo \ WHERE fo.user_id = $1 \ AND fo.is_trashed = false \ @@ -893,8 +942,8 @@ impl FolderRepository for FolderDbRepository { .map_err(|e| DomainError::internal_error("FolderDb", format!("descendant search: {e}")))?; rows.into_iter() - .map(|(id, name, path, pid, uid, ca, ma)| { - Self::row_to_folder(id, name, path, pid, uid, ca, ma) + .map(|(id, name, path, pid, uid, ca, ma, tma)| { + Self::row_to_folder(id, name, path, pid, uid, ca, ma, tma) }) .collect() } @@ -914,7 +963,8 @@ impl FolderRepository for FolderDbRepository { r#" SELECT id::text, name, path, parent_id::text, user_id, EXTRACT(EPOCH FROM created_at)::bigint, - EXTRACT(EPOCH FROM updated_at)::bigint + EXTRACT(EPOCH FROM updated_at)::bigint, + EXTRACT(EPOCH FROM tree_modified_at)::bigint FROM storage.folders WHERE parent_id = $1::uuid AND NOT is_trashed @@ -939,7 +989,8 @@ impl FolderRepository for FolderDbRepository { r#" SELECT id::text, name, path, parent_id::text, user_id, EXTRACT(EPOCH FROM created_at)::bigint, - EXTRACT(EPOCH FROM updated_at)::bigint + EXTRACT(EPOCH FROM updated_at)::bigint, + EXTRACT(EPOCH FROM tree_modified_at)::bigint FROM storage.folders WHERE parent_id IS NULL AND NOT is_trashed @@ -962,8 +1013,8 @@ impl FolderRepository for FolderDbRepository { .map_err(|e| DomainError::internal_error("FolderDb", format!("suggest: {e}")))?; rows.into_iter() - .map(|(id, name, path, pid, uid, ca, ma)| { - Self::row_to_folder(id, name, path, pid, Some(uid), ca, ma) + .map(|(id, name, path, pid, uid, ca, ma, tma)| { + Self::row_to_folder(id, name, path, pid, Some(uid), ca, ma, tma) }) .collect() } @@ -1100,6 +1151,7 @@ impl FolderDbRepository { f.created_at, f.updated_at AS modified_at, f.user_id, + NULL::text AS blob_hash, LOWER(f.name) AS sort_str, 0::bigint AS type_order, 0::int AS folder_first @@ -1118,6 +1170,7 @@ impl FolderDbRepository { fm.created_at, fm.updated_at AS modified_at, fm.user_id, + fm.blob_hash, LOWER(fm.name) AS sort_str, fm.category_order::bigint AS type_order, 1::int AS folder_first @@ -1241,7 +1294,7 @@ impl FolderDbRepository { let sql = format!( "WITH resources AS ({cte_inner}) \ SELECT resource_type, id, name, folder_id, mime_type, size, \ - created_at, modified_at, user_id, sort_str, type_order, folder_first \ + created_at, modified_at, user_id, blob_hash, sort_str, type_order, folder_first \ FROM resources \ {where_clause} \ {order_clause} \ @@ -1249,7 +1302,8 @@ impl FolderDbRepository { ); // Row: (resource_type, id, name, folder_id, mime_type, size, - // created_at, modified_at, user_id, sort_str, type_order, folder_first) + // created_at, modified_at, user_id, blob_hash, + // sort_str, type_order, folder_first) type Row = ( String, Uuid, @@ -1260,6 +1314,7 @@ impl FolderDbRepository { chrono::DateTime, chrono::DateTime, Uuid, + Option, String, i64, i32, @@ -1290,9 +1345,10 @@ impl FolderDbRepository { created_at: r.6, modified_at: r.7, owner_id: r.8, - sort_str: r.9, - type_order: r.10, - folder_first: r.11, + blob_hash: r.9, + sort_str: r.10, + type_order: r.11, + folder_first: r.12, }) .collect()) } diff --git a/src/infrastructure/repositories/pg/recent_items_pg_repository.rs b/src/infrastructure/repositories/pg/recent_items_pg_repository.rs index 1a3cc33c..93e44c05 100644 --- a/src/infrastructure/repositories/pg/recent_items_pg_repository.rs +++ b/src/infrastructure/repositories/pg/recent_items_pg_repository.rs @@ -217,6 +217,7 @@ impl RecentItemsRepositoryPort for RecentItemsPgRepository { fld.created_at AS resource_created_at, fld.updated_at AS modified_at, fld.user_id AS owner_id, + NULL::text AS blob_hash, (fld.user_id = $1::uuid) AS is_owner, ur.accessed_at AS accessed_at, fld.path::text AS resource_path, @@ -239,6 +240,7 @@ impl RecentItemsRepositoryPort for RecentItemsPgRepository { f.created_at AS resource_created_at, f.updated_at AS modified_at, f.user_id AS owner_id, + f.blob_hash, (f.user_id = $1::uuid) AS is_owner, ur.accessed_at AS accessed_at, COALESCE(pfld.path::text || '/' || f.name, f.name) AS resource_path, @@ -483,6 +485,7 @@ LIMIT $6" resource_created_at: row.get("resource_created_at"), modified_at: row.get("modified_at"), owner_id: row.get("owner_id"), + blob_hash: row.try_get("blob_hash").ok(), is_owner: row.try_get("is_owner").unwrap_or(false), accessed_at: row.get("accessed_at"), path: row.try_get("resource_path").ok(), diff --git a/src/infrastructure/repositories/pg/trash_db_repository.rs b/src/infrastructure/repositories/pg/trash_db_repository.rs index 191ccecb..ab9d3741 100644 --- a/src/infrastructure/repositories/pg/trash_db_repository.rs +++ b/src/infrastructure/repositories/pg/trash_db_repository.rs @@ -261,6 +261,7 @@ impl TrashDbRepository { fld.created_at AS resource_created_at, fld.updated_at AS modified_at, fld.user_id AS owner_id, + NULL::text AS blob_hash, fld.trashed_at AS trashed_at, (fld.trashed_at + ($7::int * INTERVAL '1 day')) AS deletion_date, fld.path::text AS resource_path, @@ -286,6 +287,7 @@ impl TrashDbRepository { f.created_at AS resource_created_at, f.updated_at AS modified_at, f.user_id AS owner_id, + f.blob_hash, f.trashed_at AS trashed_at, (f.trashed_at + ($7::int * INTERVAL '1 day')) AS deletion_date, COALESCE(pfld.path::text || '/' || f.name, f.name) AS resource_path, @@ -473,6 +475,7 @@ LIMIT $6" resource_created_at: row.get("resource_created_at"), modified_at: row.get("modified_at"), owner_id: row.get("owner_id"), + blob_hash: row.try_get("blob_hash").ok(), trashed_at, deletion_date, path: row.try_get("resource_path").ok(), diff --git a/src/infrastructure/services/path_resolver_service.rs b/src/infrastructure/services/path_resolver_service.rs index 291db682..ea187db5 100644 --- a/src/infrastructure/services/path_resolver_service.rs +++ b/src/infrastructure/services/path_resolver_service.rs @@ -145,6 +145,7 @@ impl PathResolverService { match resource_type.as_str() { "folder" => Ok(ResolvedResource::Folder(FolderDto { + etag: id.clone(), id, name: name.clone(), path: res_path, @@ -160,6 +161,11 @@ impl PathResolverService { _ => { let mime = mime_type.unwrap_or_else(|| "application/octet-stream".to_string()); let sz = size.unwrap_or(0) as u64; + // `content_hash`/`etag` are empty here: this resolver + // path doesn't select `blob_hash` from SQL — callers + // are doing existence/type discrimination, not ETag + // emission. If a caller ever needs an ETag from this + // codepath, widen the SELECT and populate properly. Ok(ResolvedResource::File(FileDto { id, name: name.clone(), @@ -175,6 +181,7 @@ impl PathResolverService { size_formatted: format_file_size(sz), owner_id: uid, sort_date: None, + content_hash: String::new(), etag: String::new(), })) } diff --git a/src/interfaces/api/handlers/favorites_handler.rs b/src/interfaces/api/handlers/favorites_handler.rs index 05d21264..c32a5a20 100644 --- a/src/interfaces/api/handlers/favorites_handler.rs +++ b/src/interfaces/api/handlers/favorites_handler.rs @@ -20,6 +20,7 @@ use crate::application::dtos::folder_dto::FolderDto; use crate::application::dtos::grant_dto::{ResourceContentDto, ResourceTypeDto}; use crate::application::ports::favorites_ports::FavoritesUseCase; use crate::application::services::favorites_service::FavoritesService; +use crate::domain::entities::file::File; use crate::interfaces::errors::AppError; use crate::interfaces::middleware::auth::AuthUser; @@ -253,8 +254,10 @@ pub async fn list_favorites_resources( }; if row.resource_type == "folder" { + let resource_id = row.resource_id.to_string(); let dto = FolderDto { - id: row.resource_id.to_string(), + etag: resource_id.clone(), + id: resource_id, name: row.name.clone(), path, parent_id: row.parent_id.map(|u| u.to_string()), @@ -277,6 +280,18 @@ pub async fn list_favorites_resources( .as_deref() .unwrap_or("application/octet-stream"); let size_bytes = row.size.max(0) as u64; + // Route ETag through `File::compute_etag` so + // this listing's `etag` byte-equals what + // GET/HEAD/PROPFIND would return for the same + // file. `blob_hash` is `None` only for + // folder rows, which take the other branch. + let modified_at_u = row.modified_at.timestamp() as u64; + let content_hash = row.blob_hash.clone().unwrap_or_default(); + let etag = if content_hash.is_empty() { + String::new() + } else { + File::compute_etag(&content_hash, modified_at_u) + }; let dto = FileDto { id: row.resource_id.to_string(), name: row.name.clone(), @@ -285,7 +300,7 @@ pub async fn list_favorites_resources( mime_type: std::sync::Arc::from(mime), folder_id: row.parent_id.map(|u| u.to_string()), created_at: row.resource_created_at.timestamp() as u64, - modified_at: row.modified_at.timestamp() as u64, + modified_at: modified_at_u, icon_class: std::sync::Arc::from(icon_class_for(&row.name, mime)), icon_special_class: std::sync::Arc::from(icon_special_class_for( &row.name, mime, @@ -294,7 +309,8 @@ pub async fn list_favorites_resources( size_formatted: format_file_size(size_bytes), owner_id: Some(row.owner_id.to_string()), sort_date: None, - etag: String::new(), + content_hash, + etag, }; FavoritesResourceItemDto { resource_type: ResourceTypeDto::File, diff --git a/src/interfaces/api/handlers/file_handler.rs b/src/interfaces/api/handlers/file_handler.rs index 0a7866a9..aface7df 100644 --- a/src/interfaces/api/handlers/file_handler.rs +++ b/src/interfaces/api/handlers/file_handler.rs @@ -593,7 +593,11 @@ impl FileHandler { .into_response(); } - let etag = format!("\"{}-{}\"", id, file_dto.modified_at); + // Route through `FileDto::etag` so this REST download + // endpoint, WebDAV/NextCloud GET, HEAD, PROPFIND, and PUT all + // emit the same opaque token for the same file — see + // `File::etag` for the formula. + let etag = format!("\"{}\"", file_dto.etag); // ── ETag (304 Not Modified) ────────────────────────────────── if let Some(inm) = headers.get(header::IF_NONE_MATCH) diff --git a/src/interfaces/api/handlers/folder_handler.rs b/src/interfaces/api/handlers/folder_handler.rs index 69775bdb..da113be5 100644 --- a/src/interfaces/api/handlers/folder_handler.rs +++ b/src/interfaces/api/handlers/folder_handler.rs @@ -26,6 +26,7 @@ use crate::application::ports::folder_ports::FolderUseCase; use crate::application::ports::trash_ports::TrashUseCase; use crate::application::services::folder_service::FolderService; use crate::common::di::AppState as GlobalAppState; +use crate::domain::entities::file::File; use crate::interfaces::errors::AppError; use crate::interfaces::middleware::auth::AuthUser; @@ -692,8 +693,10 @@ pub async fn list_folder_resources( .into_iter() .map(|row| { if row.resource_type == "folder" { + let resource_id = row.id.to_string(); let dto = FolderDto { - id: row.id.to_string(), + etag: resource_id.clone(), + id: resource_id, name: row.name.clone(), path: String::new(), // cleared — share recipients must not see hierarchy parent_id: row.parent_id.map(|u| u.to_string()), @@ -715,6 +718,20 @@ pub async fn list_folder_resources( .as_deref() .unwrap_or("application/octet-stream"); let size_bytes = row.size.max(0) as u64; + // `blob_hash` is `Some(_)` for file rows in the + // UNION ALL (`NULL` for folders). Route the + // ETag formula through `File::compute_etag` — + // the single source of truth shared with + // GET/HEAD/PROPFIND/PUT response — so this + // listing's `etag` byte-equals what a + // conditional request would compare against. + let modified_at_u = row.modified_at.timestamp() as u64; + let content_hash = row.blob_hash.clone().unwrap_or_default(); + let etag = if content_hash.is_empty() { + String::new() + } else { + File::compute_etag(&content_hash, modified_at_u) + }; let dto = FileDto { id: row.id.to_string(), name: row.name.clone(), @@ -730,7 +747,8 @@ pub async fn list_folder_resources( size_formatted: format_file_size(size_bytes), owner_id: Some(row.owner_id.to_string()), sort_date: None, - etag: String::new(), + content_hash, + etag, }; FolderResourceItemDto { resource_type: ResourceTypeDto::File, diff --git a/src/interfaces/api/handlers/recent_handler.rs b/src/interfaces/api/handlers/recent_handler.rs index fe6d978a..14dcb189 100644 --- a/src/interfaces/api/handlers/recent_handler.rs +++ b/src/interfaces/api/handlers/recent_handler.rs @@ -19,6 +19,7 @@ use crate::application::dtos::recent_dto::{ }; use crate::application::ports::recent_ports::RecentItemsUseCase; use crate::application::services::recent_service::RecentService; +use crate::domain::entities::file::File; use crate::interfaces::errors::AppError; use crate::interfaces::middleware::auth::AuthUser; use uuid::Uuid; @@ -283,8 +284,10 @@ pub async fn list_recent_resources( }; if row.resource_type == "folder" { + let resource_id = row.resource_id.to_string(); let dto = FolderDto { - id: row.resource_id.to_string(), + etag: resource_id.clone(), + id: resource_id, name: row.name.clone(), path, parent_id: row.parent_id.map(|u| u.to_string()), @@ -307,6 +310,16 @@ pub async fn list_recent_resources( .as_deref() .unwrap_or("application/octet-stream"); let size_bytes = row.size.max(0) as u64; + // Route ETag through `File::compute_etag` so this + // listing matches GET/HEAD/PROPFIND byte-for-byte + // for the same file. + let modified_at_u = row.modified_at.timestamp() as u64; + let content_hash = row.blob_hash.clone().unwrap_or_default(); + let etag = if content_hash.is_empty() { + String::new() + } else { + File::compute_etag(&content_hash, modified_at_u) + }; let dto = FileDto { id: row.resource_id.to_string(), name: row.name.clone(), @@ -315,7 +328,7 @@ pub async fn list_recent_resources( mime_type: std::sync::Arc::from(mime), folder_id: row.parent_id.map(|u| u.to_string()), created_at: row.resource_created_at.timestamp() as u64, - modified_at: row.modified_at.timestamp() as u64, + modified_at: modified_at_u, icon_class: std::sync::Arc::from(icon_class_for(&row.name, mime)), icon_special_class: std::sync::Arc::from(icon_special_class_for( &row.name, mime, @@ -324,7 +337,8 @@ pub async fn list_recent_resources( size_formatted: format_file_size(size_bytes), owner_id: Some(row.owner_id.to_string()), sort_date: None, - etag: String::new(), + content_hash, + etag, }; RecentResourceItemDto { resource_type: ResourceTypeDto::File, diff --git a/src/interfaces/api/handlers/webdav_handler.rs b/src/interfaces/api/handlers/webdav_handler.rs index a0f57afe..ef339dff 100644 --- a/src/interfaces/api/handlers/webdav_handler.rs +++ b/src/interfaces/api/handlers/webdav_handler.rs @@ -69,6 +69,37 @@ pub(crate) fn encode_uri_path(path: &str) -> String { .join("/") } +/// Build the `` value for a non-collection (file) resource. +/// +/// RFC 4918 §5.2 distinguishes collection (folder) URLs from +/// non-collection URLs by a trailing `/`. Files use NO trailing +/// slash. Mirror of [`webdav_collection_href`] — keep both arms +/// of the choice on the same screen so an "is it a file or a +/// folder?" reviewer can verify both branches at once. +fn webdav_href(path: &str) -> String { + format!("/webdav/{}", encode_uri_path(path)) +} + +/// Build the `` value for a collection (folder) resource. +/// +/// Always terminates with `/` — RFC 4918 §5.2 requires collection +/// URLs to end in a slash, and strict WebDAV clients (notably the +/// NextCloud desktop sync engine, which also speaks to this +/// endpoint) abort multi-status parses with +/// `Invalid href "<…>" expected starting with ""` +/// when the response's own-entry href is missing the trailing `/`. +/// PROPPATCH and LOCK responses on folders MUST use this — using +/// [`webdav_href`] for a folder is the bug class this helper +/// exists to prevent. +fn webdav_collection_href(path: &str) -> String { + let h = webdav_href(path); + if h.ends_with('/') { + h + } else { + format!("{}/", h) + } +} + // Create a custom DAV header since it's not in the standard headers const HEADER_DAV: HeaderName = HeaderName::from_static("dav"); const HEADER_LOCK_TOKEN: HeaderName = HeaderName::from_static("lock-token"); @@ -353,6 +384,7 @@ async fn handle_propfind( // Root folder let root_folder = FolderDto { id: "root".to_string(), + etag: "root".to_string(), name: "".to_string(), path: "".to_string(), parent_id: None, @@ -604,12 +636,35 @@ async fn build_streaming_propfind_response( * @return XML response with property modification results */ async fn handle_proppatch( - _state: Arc, + state: Arc, req: Request, path: String, ) -> Result, AppError> { let _user = extract_user(&req)?; + // Resolve the target resource type BEFORE consuming the body so + // we can pick the correct href shape in the multi-status + // response. RFC 4918 §5.2 + strict WebDAV-client parser rules + // require a trailing `/` for collection hrefs; emitting + // `/webdav/foo` for a folder breaks NC-desktop / Cyberduck / + // other multi-status consumers the same way the NC PROPFIND + // bug did. An empty / `/` path is the root, always a + // collection. A path that resolves to neither file nor folder + // (e.g. PROPPATCH on a resource that doesn't exist) defaults + // to non-collection — matches the request-line shape the + // client used, since collection paths conventionally arrive + // with trailing `/` already trimmed by routing. + let is_collection = if path.is_empty() || path == "/" { + true + } else { + state + .applications + .folder_service + .get_folder_by_path(&path) + .await + .is_ok() + }; + // Read request body (XML — bounded to 1 MB) let body_bytes = body::to_bytes(req.into_body(), MAX_XML_BODY) .await @@ -635,8 +690,12 @@ async fn handle_proppatch( results.push((prop, true)); } - // Generate response - let href = format!("/webdav/{}", encode_uri_path(&path)); + // Generate response — collection vs file href chosen above. + let href = if is_collection { + webdav_collection_href(&path) + } else { + webdav_href(&path) + }; let mut response_body = Vec::new(); WebDavAdapter::generate_proppatch_response(&mut response_body, &href, &results).map_err( |e| AppError::internal_error(format!("Failed to generate PROPPATCH response: {}", e)), @@ -706,7 +765,7 @@ async fn handle_get( .status(StatusCode::OK) .header(header::CONTENT_TYPE, &*file.mime_type) .header(header::CONTENT_LENGTH, file.size) - .header(header::ETAG, format!("\"{}\"", file.id)) + .header(header::ETAG, format!("\"{}\"", file.etag)) .header( header::LAST_MODIFIED, chrono::DateTime::::from_timestamp(file.created_at as i64, 0) @@ -747,7 +806,7 @@ async fn handle_head( .status(StatusCode::OK) .header(header::CONTENT_TYPE, "httpd/unix-directory") .header(header::CONTENT_LENGTH, 0) - .header(header::ETAG, format!("\"{}\"", folder.id)) + .header(header::ETAG, format!("\"{}\"", folder.etag)) .body(Body::empty()) .unwrap()); } @@ -756,7 +815,7 @@ async fn handle_head( .status(StatusCode::OK) .header(header::CONTENT_TYPE, &*file.mime_type) .header(header::CONTENT_LENGTH, file.size) - .header(header::ETAG, format!("\"{}\"", file.id)) + .header(header::ETAG, format!("\"{}\"", file.etag)) .header( header::LAST_MODIFIED, chrono::DateTime::::from_timestamp(file.created_at as i64, 0) @@ -777,7 +836,7 @@ async fn handle_head( .status(StatusCode::OK) .header(header::CONTENT_TYPE, "httpd/unix-directory") .header(header::CONTENT_LENGTH, 0) - .header(header::ETAG, format!("\"{}\"", folder.id)) + .header(header::ETAG, format!("\"{}\"", folder.etag)) .body(Body::empty()) .unwrap()); } @@ -793,7 +852,7 @@ async fn handle_head( .status(StatusCode::OK) .header(header::CONTENT_TYPE, &*file.mime_type) .header(header::CONTENT_LENGTH, file.size) - .header(header::ETAG, format!("\"{}\"", file.id)) + .header(header::ETAG, format!("\"{}\"", file.etag)) .header( header::LAST_MODIFIED, chrono::DateTime::::from_timestamp(file.created_at as i64, 0) @@ -1686,6 +1745,24 @@ async fn handle_lock( ) -> Result, AppError> { let user = extract_user(&req)?; + // Determine collection-vs-file for href shape. Root + known + // folders → collection; everything else (existing files, + // lock-null on a non-existent path) → file. RFC 4918 §9.10.1 + // allows LOCK on a non-existent resource (the "lock-null + // resource" pattern used by Office save flows) — that arm + // falls through to the file href shape, matching the + // request-line shape clients send. + let is_collection = if path.is_empty() || path == "/" { + true + } else { + state + .applications + .folder_service + .get_folder_by_path(&path) + .await + .is_ok() + }; + // Get the headers that we need let depth = req .headers() @@ -1735,8 +1812,12 @@ async fn handle_lock( AppError::precondition_failed(format!("Lock token not found or expired: {}", token)) })?; - // Generate response - let href = format!("/webdav/{}", encode_uri_path(&path)); + // Generate response — collection vs file href chosen above. + let href = if is_collection { + webdav_collection_href(&path) + } else { + webdav_href(&path) + }; let mut response_body = Vec::new(); WebDavAdapter::generate_lock_response(&mut response_body, &entry.info, &href).map_err( |e| AppError::internal_error(format!("Failed to generate LOCK response: {}", e)), @@ -1771,8 +1852,12 @@ async fn handle_lock( )) })?; - // Generate response - let href = format!("/webdav/{}", encode_uri_path(&path)); + // Generate response — collection vs file href chosen above. + let href = if is_collection { + webdav_collection_href(&path) + } else { + webdav_href(&path) + }; let mut response_body = Vec::new(); WebDavAdapter::generate_lock_response(&mut response_body, &entry.info, &href).map_err( |e| AppError::internal_error(format!("Failed to generate LOCK response: {}", e)), @@ -1836,3 +1921,49 @@ async fn handle_unlock( .body(Body::empty()) .unwrap()) } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_webdav_href_no_trailing_slash() { + assert_eq!( + webdav_href("Documents/report.pdf"), + "/webdav/Documents/report.pdf" + ); + assert_eq!(webdav_href("file.txt"), "/webdav/file.txt"); + } + + #[test] + fn test_webdav_collection_href_appends_slash_when_missing() { + assert_eq!(webdav_collection_href("Documents"), "/webdav/Documents/"); + assert_eq!( + webdav_collection_href("Documents/subfolder"), + "/webdav/Documents/subfolder/" + ); + } + + #[test] + fn test_webdav_collection_href_idempotent_when_already_slashed() { + // `encode_uri_path` never emits a trailing `/` of its own + // because the path argument is already trimmed by routing, + // but the helper still has to be robust to a path that + // happens to end in `/` — exercise the idempotence path. + assert_eq!(webdav_collection_href("Documents/"), "/webdav/Documents/"); + } + + #[test] + fn test_webdav_href_preserves_url_encoding() { + // Spaces and Unicode must percent-encode at the segment level, + // not get a verbatim `%20` re-encoded as `%2520`. + assert_eq!( + webdav_href("My Photos/vacation pic.jpg"), + "/webdav/My%20Photos/vacation%20pic.jpg" + ); + assert_eq!( + webdav_collection_href("My Photos/2024"), + "/webdav/My%20Photos/2024/" + ); + } +} diff --git a/src/interfaces/nextcloud/basic_auth_middleware.rs b/src/interfaces/nextcloud/basic_auth_middleware.rs index 4ebeb140..52d65529 100644 --- a/src/interfaces/nextcloud/basic_auth_middleware.rs +++ b/src/interfaces/nextcloud/basic_auth_middleware.rs @@ -114,6 +114,15 @@ pub async fn basic_auth_middleware( ); return Err(NextcloudAuthError::Unauthorized); } + // Populate the deferred `user_id` field on the request + // tracing span (declared in `middleware/trace_span.rs::ClientIpMakeSpan`). + // Mirrors what `interfaces/middleware/auth.rs` does for the + // JWT path so the two auth surfaces produce log lines with + // the same structured shape — without this, every NC + // request would appear in the logs with `user_id=-`, + // making it harder to correlate WebDAV / OCS activity to + // a specific principal. + tracing::Span::current().record("user_id", user_id.to_string()); request.extensions_mut().insert(Arc::new(CurrentUser { id: user_id, username: uname, diff --git a/src/interfaces/nextcloud/report_handler.rs b/src/interfaces/nextcloud/report_handler.rs index 03311557..c59a07f2 100644 --- a/src/interfaces/nextcloud/report_handler.rs +++ b/src/interfaces/nextcloud/report_handler.rs @@ -21,6 +21,7 @@ use crate::application::ports::file_ports::FileRetrievalUseCase; use crate::application::ports::folder_ports::FolderUseCase; use crate::application::ports::inbound::SearchUseCase; use crate::common::di::AppState; +use crate::domain::entities::file::File; use crate::interfaces::errors::AppError; use crate::interfaces::middleware::auth::CurrentUser; use crate::interfaces::nextcloud::webdav_handler::{ @@ -250,6 +251,16 @@ async fn handle_search( /// Build a `FileDto` from a search file result. fn file_dto_from_search(fr: &crate::application::dtos::search_dto::SearchFileResultDto) -> FileDto { + // Route ETag through `File::compute_etag` so REPORT/SEARCH hits + // emit the same opaque token NC's sync client cached from the + // earlier PROPFIND walk — without this, NC's conditional-request + // logic on search results disagrees with its own cached state + // and triggers a spurious re-fetch. + let etag = if fr.blob_hash.is_empty() { + String::new() + } else { + File::compute_etag(&fr.blob_hash, fr.modified_at) + }; FileDto { id: fr.id.clone(), name: fr.name.clone(), @@ -267,7 +278,8 @@ fn file_dto_from_search(fr: &crate::application::dtos::search_dto::SearchFileRes size_formatted: format_file_size(fr.size), owner_id: None, sort_date: None, - etag: String::new(), + content_hash: fr.blob_hash.clone(), + etag, } } @@ -276,6 +288,7 @@ fn folder_dto_from_search( sr: &crate::application::dtos::search_dto::SearchFolderResultDto, ) -> FolderDto { FolderDto { + etag: sr.id.clone(), id: sr.id.clone(), name: sr.name.clone(), path: sr.path.clone(), diff --git a/src/interfaces/nextcloud/webdav_handler.rs b/src/interfaces/nextcloud/webdav_handler.rs index af813e8b..8be9577b 100644 --- a/src/interfaces/nextcloud/webdav_handler.rs +++ b/src/interfaces/nextcloud/webdav_handler.rs @@ -62,10 +62,32 @@ pub fn nc_to_internal_path(username: &str, subpath: &str) -> Result` doesn't end in `/` aborts the parse with +/// `Invalid href "<…>" expected starting with ""` and +/// surfaces as `Network request error "Erreur inconnue" HTTP status +/// 207` in the client log. Files use [`nc_href`] (no trailing slash). +pub fn nc_collection_href(username: &str, subpath: &str) -> String { + let h = nc_href(username, subpath); + if h.ends_with('/') { + h + } else { + format!("{}/", h) + } +} + /// Build the Nextcloud DAV href for a resource. /// /// Each path segment is URL-encoded individually so filenames with spaces, /// `#`, `%`, or non-ASCII characters produce valid PROPFIND hrefs. +/// +/// Returns NO trailing slash for non-empty subpaths. Callers rendering +/// a **collection** must use [`nc_collection_href`] (or append `/` +/// manually) to satisfy RFC 4918 §5.2 and the NC client's parser. pub fn nc_href(username: &str, subpath: &str) -> String { let subpath = subpath.trim_matches('/'); let encoded_user = urlencoding::encode(username); @@ -120,9 +142,18 @@ pub async fn handle_nc_webdav( // ──────────────────── OPTIONS ──────────────────── fn handle_options() -> Result, AppError> { + // Advertise WebDAV compliance classes 1 + 3 only. + // Class 2 (LOCK/UNLOCK) is intentionally omitted because the NC + // surface has no LOCK/UNLOCK dispatch arm — claiming class 2 + // would invite clients (notably the NC desktop sync engine) to + // start sending LOCK requests we then 405. Class 3 covers the + // weak-resource-validators behaviour PROPFIND already implements. + // If LOCK is ever wired in here, restore "1, 2, 3" in the same + // commit as the LOCK arm — never split the advertisement from + // the implementation. Ok(Response::builder() .status(StatusCode::OK) - .header(HEADER_DAV, "1, 2, 3") + .header(HEADER_DAV, "1, 3") .header( header::ALLOW, "OPTIONS, GET, HEAD, PUT, DELETE, MKCOL, MOVE, PROPFIND, PROPPATCH, REPORT, SEARCH", @@ -318,11 +349,18 @@ async fn handle_get( chrono::DateTime::::from_timestamp(timestamp_to_i64(file.modified_at), 0) .unwrap_or_else(Utc::now); + // ETag comes from `FileDto::etag` (populated from `File::etag()` + // in the `From` impl) — single source of truth, so GET, + // HEAD, PUT-response, MOVE, and PROPFIND all emit byte-identical + // values for the same file. NC's sync engine compares cached + // PROPFIND ETags against GET/HEAD responses; using `file.id` here + // (a UUID) while PROPFIND emitted the blob hash made NC see + // every file as "remotely changed" on first descent. Ok(Response::builder() .status(StatusCode::OK) .header(header::CONTENT_TYPE, file.mime_type.as_ref()) .header(header::CONTENT_LENGTH, file.size) - .header(header::ETAG, format!("\"{}\"", file.id)) + .header(header::ETAG, format!("\"{}\"", file.etag)) .header(header::LAST_MODIFIED, modified_at.to_rfc2822()) .body(Body::from_stream(std::pin::Pin::from(stream))) .unwrap()) @@ -370,11 +408,14 @@ async fn handle_head( chrono::DateTime::::from_timestamp(timestamp_to_i64(file.modified_at), 0) .unwrap_or_else(Utc::now); + // ETag comes from `FileDto::etag` — see the same comment block on + // the GET handler. HEAD and GET must agree byte-for-byte; pulling + // both from the same DTO field guarantees that. Ok(Response::builder() .status(StatusCode::OK) .header(header::CONTENT_TYPE, file.mime_type.as_ref()) .header(header::CONTENT_LENGTH, file.size) - .header(header::ETAG, format!("\"{}\"", file.id)) + .header(header::ETAG, format!("\"{}\"", file.etag)) .header(header::LAST_MODIFIED, modified_at.to_rfc2822()) .body(Body::empty()) .unwrap()) @@ -394,23 +435,40 @@ async fn handle_proppatch( let body_str = String::from_utf8_lossy(&body_bytes); + // Resolve the target resource once — needed for two things: + // 1. Applying the oc:favorite mutation when the PROPPATCH body + // carries one (`item_type` distinguishes file vs folder rows + // in the favorites table). + // 2. Picking the right `` shape in the multi-status + // response: collection (folder) hrefs MUST end in `/` per + // RFC 4918 §5.2 — see `nc_collection_href` for the full + // reasoning. Without this distinction the NC desktop client + // parser aborted on PROPFIND; PROPPATCH would hit the same + // wall the moment the user favourited a folder. + // + // When the resource is missing we tolerate it for the no-op + // PROPPATCH path (no favorite directive in the body) — matches + // the prior behaviour. A PROPPATCH that *does* try to set + // favorite on a missing resource still returns NotFound. + let internal_path = nc_to_internal_path(&user.username, subpath)?; + let file_service = &state.applications.file_retrieval_service; + let folder_service = &state.applications.folder_service; + let resource = if let Ok(file) = file_service.get_file_by_path(&internal_path).await { + Some((file.id, "file")) + } else if let Ok(folder) = folder_service.get_folder_by_path(&internal_path).await { + Some((folder.id, "folder")) + } else { + None + }; + let is_collection = matches!(resource, Some((_, "folder"))); + // Parse oc:favorite value from PROPPATCH XML. let favorite_value = parse_proppatch_favorite(&body_str); if let Some(value) = favorite_value { - let internal_path = nc_to_internal_path(&user.username, subpath)?; - let file_service = &state.applications.file_retrieval_service; - let folder_service = &state.applications.folder_service; - - // Determine item_id and item_type. - let (item_id, item_type) = - if let Ok(file) = file_service.get_file_by_path(&internal_path).await { - (file.id, "file") - } else if let Ok(folder) = folder_service.get_folder_by_path(&internal_path).await { - (folder.id, "folder") - } else { - return Err(AppError::not_found("Resource not found")); - }; + let Some((item_id, item_type)) = resource else { + return Err(AppError::not_found("Resource not found")); + }; if let Some(fav_svc) = state.favorites_service.as_ref() { if value == 1 { @@ -431,8 +489,15 @@ async fn handle_proppatch( } } - // Return 207 Multi-Status with success response using quick_xml for safe escaping. - let href = nc_href(&user.username, subpath); + // Return 207 Multi-Status with success response using quick_xml + // for safe escaping. Collection vs file href chosen by resource + // type to satisfy the RFC 4918 §5.2 trailing-slash invariant — + // see the comment block at the top of this function. + let href = if is_collection { + nc_collection_href(&user.username, subpath) + } else { + nc_href(&user.username, subpath) + }; let mut buf = Vec::new(); { let mut xml = Writer::new(&mut buf); @@ -804,9 +869,13 @@ async fn handle_move( let dest_internal = nc_to_internal_path(&user.username, &dest_subpath)?; let mut builder = Response::builder().status(StatusCode::CREATED); if let Ok(moved) = file_service.get_file_by_path(&dest_internal).await { + // Route through `FileDto::etag` so the MOVE response + // matches what a subsequent PROPFIND on the destination + // will return — `moved.id` (UUID) would differ from the + // blob hash and trigger NC's "remote changed" detection. builder = builder - .header(header::ETAG, format!("\"{}\"", moved.id)) - .header("oc-etag", format!("\"{}\"", moved.id)); + .header(header::ETAG, format!("\"{}\"", moved.etag)) + .header("oc-etag", format!("\"{}\"", moved.etag)); } return Ok(builder.body(Body::empty()).unwrap()); @@ -935,9 +1004,10 @@ async fn write_nc_multistatus( ms.push_attribute(("xmlns:ocs", "http://open-collaboration-services.org/ns")); xml.write_event(Event::Start(ms)).xml_err()?; - // Current folder entry. + // Current folder entry. Collection hrefs MUST end in `/` (RFC 4918 + // §5.2 + strict NC-client enforcement — see `nc_collection_href`). if let Some(f) = folder { - let href = nc_href(username, subpath); + let href = nc_collection_href(username, subpath); let file_id = resolve_folder_id(file_id_svc, &f.id).await; let oc_id = file_id.map(|id| format_oc_id(id, file_id_svc)); write_folder_response( @@ -981,14 +1051,14 @@ async fn write_nc_multistatus( )?; } - // Subfolders. + // Subfolders — also collections, same trailing-slash rule. for sf in subfolders { let child_sub = if subpath.is_empty() { sf.name.clone() } else { format!("{}/{}", subpath.trim_end_matches('/'), sf.name) }; - let href = format!("{}/", nc_href(username, &child_sub)); + let href = nc_collection_href(username, &child_sub); let file_id = resolve_folder_id(file_id_svc, &sf.id).await; let oc_id = file_id.map(|id| format_oc_id(id, file_id_svc)); write_folder_response( @@ -1047,7 +1117,10 @@ pub fn write_folder_response( .unwrap_or_else(Utc::now); write_text_element(xml, "d:getlastmodified", &modified_at.to_rfc2822())?; - write_text_element(xml, "d:getetag", &format!("\"{}\"", folder.id))?; + // Route through `FolderDto::etag` (= `Folder::etag()`, currently + // the folder UUID — see the entity for the documented v1 formula + // and the follow-up plan to make it descendant-aware). + write_text_element(xml, "d:getetag", &format!("\"{}\"", folder.etag))?; write_text_element(xml, "d:getcontenttype", "httpd/unix-directory")?; write_text_element(xml, "d:getcontentlength", "0")?; write_text_element(xml, "d:creationdate", &created_at.to_rfc3339())?; @@ -1277,6 +1350,41 @@ mod tests { assert!(href.contains("file%231.txt")); } + // ── nc_collection_href ── + // RFC 4918 §5.2 requires a collection URL to end in '/'. The NC + // desktop client at `networkjobs.cpp:234` aborts the PROPFIND + // parse with `Invalid href "<…>" expected starting with + // ""` if the own-entry href is missing the slash. + // These tests pin the helper's behaviour so the regression can't + // come back silently. + + #[test] + fn test_collection_href_appends_slash_when_missing() { + assert_eq!( + nc_collection_href("alice", "ext"), + "/remote.php/dav/files/alice/ext/" + ); + } + + #[test] + fn test_collection_href_idempotent_at_root() { + // Root subpath already ends in '/' — don't double-append. + assert_eq!( + nc_collection_href("alice", ""), + "/remote.php/dav/files/alice/" + ); + } + + #[test] + fn test_collection_href_preserves_encoding() { + // Wrapping must not re-encode or double-encode already-encoded + // segments. + assert_eq!( + nc_collection_href("alice", "My Photos/2024"), + "/remote.php/dav/files/alice/My%20Photos/2024/" + ); + } + // ── extract_nc_subpath_from_dest ── #[test] diff --git a/src/main.rs b/src/main.rs index 96cf18c5..192b4146 100644 --- a/src/main.rs +++ b/src/main.rs @@ -451,21 +451,20 @@ async fn main() -> Result<(), Box> { .merge(caldav_protected) .merge(carddav_protected) .merge(webdav_protected) - .merge(web_routes) - .layer( - TraceLayer::new_for_http() - .make_span_with(ClientIpMakeSpan) - .on_response(LogBadRequest), - ) - .layer(PropagateRequestIdLayer::x_request_id()) - .layer(SetRequestIdLayer::x_request_id(UuidRequestId)); + .merge(web_routes); - // Mount Nextcloud routes (uses its own Basic Auth middleware) + // Mount Nextcloud routes (uses its own Basic Auth middleware). + // **Merged BEFORE the trace + request-id layers** so NC requests + // get the same `request_id` / `user_id` / `client_ip` span + // fields as every other surface — see + // `interfaces/middleware/trace_span.rs::ClientIpMakeSpan`. if let Some(nc_router) = nextcloud_router { app = app.merge(nc_router.with_state(app_state.clone())); } - // Mount WOPI routes (protocol routes use own token auth, API routes behind auth middleware) + // Mount WOPI routes (protocol routes use own token auth, API routes behind auth middleware). + // Same reasoning as NC above: merge before the trace layer so + // WOPI requests appear in the structured log channel. if let Some((wopi_protocol, wopi_api)) = wopi_routes { let wopi_api_protected = wopi_api .layer(axum::middleware::from_fn(csrf_middleware)) @@ -477,6 +476,20 @@ async fn main() -> Result<(), Box> { .nest("/wopi", wopi_protocol) .nest("/api/wopi", wopi_api_protected); } + + // ── Trace + request-id layers applied LAST so every route + // merged above (including the conditional NC and WOPI + // surfaces) is wrapped. New protocol routers added later + // only have to be merged before this point to get tracing + // for free — no second site to remember to update. + app = app + .layer( + TraceLayer::new_for_http() + .make_span_with(ClientIpMakeSpan) + .on_response(LogBadRequest), + ) + .layer(PropagateRequestIdLayer::x_request_id()) + .layer(SetRequestIdLayer::x_request_id(UuidRequestId)); } else { // Auth disabled — no middleware applied tracing::warn!("Authentication is DISABLED — all API routes are publicly accessible"); @@ -491,7 +504,24 @@ async fn main() -> Result<(), Box> { .merge(caldav_router) .merge(carddav_router) .merge(webdav_router) - .merge(web_routes) + .merge(web_routes); + + // Mount Nextcloud routes — merged BEFORE the trace + request-id + // layers so NC requests get the same span fields as every + // other surface (matches the auth-enabled branch above). + if let Some(nc_router) = nextcloud_router { + app = app.merge(nc_router.with_state(app_state.clone())); + } + + // Mount WOPI routes (no auth middleware when auth is disabled). + // Same reasoning: merge before the trace layer. + if let Some((wopi_protocol, wopi_api)) = wopi_routes { + app = app.nest("/wopi", wopi_protocol).nest("/api/wopi", wopi_api); + } + + // ── Trace + request-id layers applied LAST. See the + // auth-enabled branch above for the rationale. + app = app .layer( TraceLayer::new_for_http() .make_span_with(ClientIpMakeSpan) @@ -499,16 +529,6 @@ async fn main() -> Result<(), Box> { ) .layer(PropagateRequestIdLayer::x_request_id()) .layer(SetRequestIdLayer::x_request_id(UuidRequestId)); - - // Mount Nextcloud routes - if let Some(nc_router) = nextcloud_router { - app = app.merge(nc_router.with_state(app_state.clone())); - } - - // Mount WOPI routes (no auth middleware when auth is disabled) - if let Some((wopi_protocol, wopi_api)) = wopi_routes { - app = app.nest("/wopi", wopi_protocol).nest("/api/wopi", wopi_api); - } } // Increase the default body limit to allow large file uploads. diff --git a/static/css/components/tooltip.css b/static/css/components/tooltip.css new file mode 100644 index 00000000..9c0aada3 --- /dev/null +++ b/static/css/components/tooltip.css @@ -0,0 +1,95 @@ +/* ── Tooltip primitive ───────────────────────────────────────────── + * + * Generic on-hover tooltip used app-wide. Two flavours: + * + * 1. Single-line label, content from `data-tooltip="…"` on the trigger. + * Use `attachTooltip(el, text)` from `static/js/utils/tooltip.js`. + * 2. Rich popover with a DOM subtree, populated lazily on first hover. + * Use `attachRichTooltip(el, populateAsync)` for things like the + * group-vignette member list. + * + * Both are portal'd to `document.body` (the JS helper creates a child + * of body and positions it with `position: fixed`) so they escape the + * `overflow: hidden` clipping that lives on lane wrappers, list rows, + * and any other "contain my children" ancestor. Without the portal, + * tooltips near the edges of those containers get cropped — exactly + * the bug that prompted this refactor. + * + * Hover-intent timing is asymmetric on purpose: + * - Entry: 250 ms delay before the fade-in starts. Short enough to + * feel responsive (much faster than the browser-native `title` + * delay, which is ~500–1500 ms), long enough to suppress flicker + * on accidental mouseovers. + * - Exit: 0 ms delay. The tooltip dismisses immediately when the + * user moves away. + * + * The class toggle is JS-driven (mouseenter/leave + focusin/out + * listeners on the trigger); the visible transition lives entirely + * in CSS so a setTimeout never gates the visual change. + */ + +.oxi-tooltip-popover { + position: fixed; + min-width: 0; + max-width: 280px; + padding: 6px 10px; + background-color: var(--color-text); + color: var(--color-bg-page); + font-size: 12px; + line-height: 1.5; + text-align: left; + border-radius: 6px; + box-shadow: 0 2px 8px var(--color-shadow); + pointer-events: none; + opacity: 0; + /* Default (no `--visible`): fast hide, no delay. */ + transition: opacity 100ms ease-out 0ms; + z-index: 10000; +} + +/* Simple (data-tooltip) flavour — keeps the label on one line so it + reads as a short caption, not a paragraph. Used by short labels + like a user vignette's email-on-hover. */ +.oxi-tooltip-popover--simple { + white-space: nowrap; + max-width: none; +} + +/* Visible state overrides the transition to add the hover-intent + delay on the *entry* path. Removing the class falls back to + the default rule above (no delay → immediate fade-out). */ +.oxi-tooltip-popover--visible { + opacity: 1; + transition: opacity 100ms ease-out 250ms; +} + +/* ── Rich-popover layout helpers ────────────────────────────────── */ + +.oxi-tooltip-popover__line { + overflow: hidden; + text-overflow: ellipsis; + white-space: nowrap; +} + +/* "+N" overflow badge — small pill at the end of a list of items when + there are more than the tooltip cares to show. Lighter background + so the count reads as meta-information, not just another line. */ +.oxi-tooltip-popover__overflow { + display: inline-block; + margin-top: 4px; + padding: 1px 7px; + font-size: 11px; + font-weight: 600; + color: var(--color-bg-page); + background-color: var(--color-text-faint); + border-radius: 9999px; +} + +/* Placeholder line shown briefly between the first hover and the + populate() callback's resolve (e.g. while `/api/groups/{id}/members` + is in flight). Italic + dimmed so the user perceives it as a + transitional state, not real content. */ +.oxi-tooltip-popover__placeholder { + color: var(--color-text-faint); + font-style: italic; +} diff --git a/static/css/components/userVignette.css b/static/css/components/userVignette.css index b4f52f5f..d4b75978 100644 --- a/static/css/components/userVignette.css +++ b/static/css/components/userVignette.css @@ -21,6 +21,12 @@ overflow: hidden; } +/* The on-hover email tooltip (used by `createUserVignette` when the + email is set) and the rich group-members popover both live in + `components/tooltip.css` and are attached at runtime via + `static/js/utils/tooltip.js` — kept generic so other surfaces + (role chips, link chips, action buttons) can opt in the same way. */ + .user-vignette__avatar { border-radius: 50%; display: flex; diff --git a/static/css/main.css b/static/css/main.css index 660aa9b7..b1ba10cd 100644 --- a/static/css/main.css +++ b/static/css/main.css @@ -23,6 +23,7 @@ @import url("./components/shareDialog.css"); @import url("./components/shareModal.css"); @import url("./components/groupsModal.css"); +@import url("./components/tooltip.css"); @import url("./components/userVignette.css"); @import url("./components/linkChip.css"); @import url("./components/uploadDropdown.css"); diff --git a/static/js/app/main.js b/static/js/app/main.js index 8e1e6678..2972b8ae 100644 --- a/static/js/app/main.js +++ b/static/js/app/main.js @@ -540,8 +540,15 @@ function initApp() { grants.fetchIncomingGrants(); grants.fetchOutgoingGrants(); - switchSectionTo(hashContext.section); if (hashContext.section === 'files') { + // Capture the URL-derived path BEFORE the section switch. + // `switchToFilesSection` resets `app.currentPath` to home + // by default and kicks off its own `loadFiles()` — if we + // wrote the hash-derived path AFTER the switch, our write + // would race the load-already-in-flight and lose, taking + // the user back to root on every other refresh. Setting + // first + `preservePath: true` makes the switch's load + // start with the correct path. if (hashContext.path) { console.log(`init: reusing folder from hash URL: ${hashContext.path}`); app.currentPath = hashContext.path; @@ -551,7 +558,11 @@ function initApp() { app.viewFile = hashContext.file; } - loadFiles(); + switchToFilesSection({ preservePath: true }); + // `switchToFilesSection` already calls `loadFiles()` — do + // not call it again here, that would race itself. + } else { + switchSectionTo(hashContext.section); } }); diff --git a/static/js/app/navigation.js b/static/js/app/navigation.js index 88b0e64a..a3791bf2 100644 --- a/static/js/app/navigation.js +++ b/static/js/app/navigation.js @@ -270,7 +270,20 @@ function switchToSharedWithMeSection() { sharedWithMeView.init(); } -function switchToFilesSection() { +/** + * Switch the UI into the Files section. + * + * @param {Object} [options] + * @param {boolean} [options.preservePath=false] + * When true, keep the current `app.currentPath` instead of resetting + * to the home folder. Used on initial page load with a hash-driven + * path (e.g. `#/files/folder/`) so the section switch doesn't + * clobber the path the caller has just set from the URL. Without + * this flag, the unconditional reset races with `loadFiles()` + * (which is also kicked off from here) and the home-folder URL + * wins, redirecting the user back to root on every other refresh. + */ +function switchToFilesSection({ preservePath = false } = {}) { if (!setCurrentSection('files')) return; // Set actions bar mode @@ -299,12 +312,13 @@ function switchToFilesSection() { //reset files view + remove any error ui.resetFilesList(); - // Reset to home folder and update breadcrumb. External users have no - // home — leave `currentPath` as the caller set it (e.g. the magic-link + // Reset to home folder unless the caller has pre-set a path (the + // `preservePath` opt-in). External users have no home — leave + // `currentPath` as the caller set it (e.g. the magic-link // landing's hash context) so loadFiles() doesn't fall through to - // `/api/folders//resources`. If `currentPath` is still empty by the - // time loadFiles() runs, it self-redirects to /#/sharedwithme. - if (!app.isExternalUser) { + // `/api/folders//resources`. If `currentPath` is still empty by + // the time loadFiles() runs, it self-redirects to /#/sharedwithme. + if (!app.isExternalUser && !preservePath) { app.currentPath = app.userHomeFolderId || ''; app.breadcrumbPath = []; } diff --git a/static/js/components/groupVignette.js b/static/js/components/groupVignette.js index b395eb56..7e1fa671 100644 --- a/static/js/components/groupVignette.js +++ b/static/js/components/groupVignette.js @@ -17,6 +17,142 @@ */ import { escapeHtml } from '../core/formatters.js'; +import { i18n } from '../core/i18n.js'; +import { groups, INTERNAL_GROUP_ID } from '../model/groups.js'; +import { systemUsers } from '../model/systemUsers.js'; +import { attachRichTooltip, OxiTooltipClass } from '../utils/tooltip.js'; + +/** Max member names displayed in the on-hover tooltip; any extra count + * surfaces as a "+N" badge on the last line. Kept small enough to fit + * inside the 280 px tooltip without scrolling, large enough to be + * informative for the typical share-with-a-team case. */ +const MAX_MEMBERS_IN_TOOLTIP = 8; + +/** + * Session-scoped cache of resolved member lists, keyed by group UUID. + * One entry per group ever hovered; the promise is reused across + * subsequent vignettes for the same group so a list of 50 rows that + * all reference the same group hits `/api/groups/{id}/members` once. + * + * @type {Map>} + */ +const _membersCache = new Map(); + +/** + * Resolve the first N direct members of a group to display names. + * Idempotent + memoised across vignettes that share a group id. + * + * Failures (403 on a group the caller can't list, 404 on a stale id, + * network errors) resolve to an empty-names + zero-total shape so the + * UI shows a graceful "no members" placeholder instead of throwing. + * + * @param {string} groupId + * @returns {Promise<{ names: string[], total: number }>} + */ +async function _resolveGroupMembers(groupId) { + const cached = _membersCache.get(groupId); + if (cached) return cached; + + const promise = (async () => { + /** @type {import('../core/types.js').GroupMemberItem[]} */ + let members; + try { + members = await groups.listMembers(groupId); + } catch { + return { names: [], total: 0 }; + } + const total = members.length; + // Only resolve the names we'll actually display — the "+N more" + // badge counts the rest from `total - MAX`. Cuts down on the + // number of /api/users/{id} backfills for big groups. + const slice = members.slice(0, MAX_MEMBERS_IN_TOOLTIP); + + // Parallelise the per-member name lookups so a 8-member group + // resolves in one round-trip's worth of latency, not eight. + const names = await Promise.all( + slice.map(async (m) => { + if (m.kind === 'user') { + try { + return await systemUsers.getDisplayName(m.id); + } catch { + return `${m.id.slice(0, 8)}…`; + } + } + // Nested group — resolve via groups model. + try { + const resolved = await groups.resolveGroups([m.id]); + const g = resolved?.[m.id]; + if (g?.name) return `👥 ${g.name}`; + } catch { + // fall through + } + return `👥 ${m.id.slice(0, 8)}…`; + }) + ); + return { names, total }; + })(); + + _membersCache.set(groupId, promise); + return promise; +} + +/** + * Attach the on-hover member popover to a group vignette. Delegates + * positioning + show/hide + portal placement to the generic + * `attachRichTooltip` helper (see `utils/tooltip.js`); this function + * just owns the per-row content — the placeholder, the member lines, + * and the "+N" overflow badge. + * + * @param {HTMLElement} vignetteEl the wrapper returned by `createGroupVignette` + * @param {string} groupId UUID of the group whose members to show + */ +function _attachMembersTooltip(vignetteEl, groupId) { + attachRichTooltip(vignetteEl, async (pop) => { + // Placeholder shown until the network resolves. Slow connections + // see "Loading members…" instead of an empty box. + const placeholder = document.createElement('div'); + placeholder.className = OxiTooltipClass.PLACEHOLDER; + placeholder.textContent = i18n.t('groups.members_loading', 'Loading members…'); + pop.appendChild(placeholder); + + const { names, total } = await _resolveGroupMembers(groupId); + pop.replaceChildren(); + + if (total === 0) { + const empty = document.createElement('div'); + empty.className = OxiTooltipClass.PLACEHOLDER; + // Special case: the built-in "Internal" virtual group has + // implicit membership — it represents every internal user + // on this server. `listMembers` returns an empty array + // because no explicit rows exist in `auth.subject_group_members`, + // but "No members" would mislead. Surface the real meaning + // instead. Future virtual groups with implicit membership + // (e.g. "Everyone") would extend this branch. + empty.textContent = + groupId === INTERNAL_GROUP_ID + ? i18n.t('groups.virtual_internal_explanation', 'Every internal user on this server') + : i18n.t('groups.members_empty', 'No members'); + pop.appendChild(empty); + return; + } + for (const name of names) { + const line = document.createElement('div'); + line.className = OxiTooltipClass.LINE; + line.textContent = name; + pop.appendChild(line); + } + if (total > MAX_MEMBERS_IN_TOOLTIP) { + const overflow = document.createElement('div'); + overflow.className = OxiTooltipClass.LINE; + // Inner badge so the "+N" reads as a count, not another name. + const badge = document.createElement('span'); + badge.className = OxiTooltipClass.OVERFLOW; + badge.textContent = `+${total - MAX_MEMBERS_IN_TOOLTIP}`; + overflow.append('… ', badge); + pop.appendChild(overflow); + } + }); +} /** * Build the inline vignette. @@ -26,17 +162,25 @@ import { escapeHtml } from '../core/formatters.js'; * @param {'xs'|'sm'|'md'|'list'} [size='sm'] * Matches the size scale of `createUserVignette`. The size class is * `user-vignette--${size}`; see `static/css/components/userVignette.css`. - * @param {{ icon?: string }} [opts] + * @param {{ icon?: string, groupId?: string }} [opts] * `icon`: FA class string without the `fa-` prefix (defaults to * `'fa-user-group'`). Used to signal virtual groups visually — see * `groupIconClass()` / `groupIconClassByVirtual()` in `./groupDisplay.js` * return a distinct icon for system-wide virtual groups (Internal, * future Everyone, …). + * + * `groupId`: UUID of the group. When supplied, hovering the vignette + * reveals a tooltip listing up to {@link MAX_MEMBERS_IN_TOOLTIP} + * member names with a `+N` badge for overflow. The member list is + * fetched lazily on first hover, memoised across all vignettes that + * share the same id, so a row of 50 grants pointing at the same + * team only hits `/api/groups/{id}/members` once. * @returns {HTMLElement} */ -export function createGroupVignette(name, size = 'sm', { icon = 'fa-user-group' } = {}) { +export function createGroupVignette(name, size = 'sm', { icon = 'fa-user-group', groupId } = {}) { const el = document.createElement('div'); el.className = `user-vignette user-vignette-group user-vignette--${size}`; el.innerHTML = `${escapeHtml(name)}`; + if (groupId) _attachMembersTooltip(el, groupId); return el; } diff --git a/static/js/components/mySharesList.js b/static/js/components/mySharesList.js index e079837f..6c374406 100644 --- a/static/js/components/mySharesList.js +++ b/static/js/components/mySharesList.js @@ -270,7 +270,8 @@ class MySharesList { } if (swimKey.startsWith('group:')) { return createGroupVignette(this._groupName(grant.subject_id), 'list', { - icon: this._groupIcon(grant.subject_id) + icon: this._groupIcon(grant.subject_id), + groupId: grant.subject_id }); } const el = document.createElement('div'); @@ -347,7 +348,8 @@ class MySharesList { } else if (grant.subject_type === 'group') { el.appendChild( createGroupVignette(this._groupName(grant.subject_id), 'xs', { - icon: this._groupIcon(grant.subject_id) + icon: this._groupIcon(grant.subject_id), + groupId: grant.subject_id }) ); } else { diff --git a/static/js/components/userVignette.js b/static/js/components/userVignette.js index a760cc9a..ba1438fc 100644 --- a/static/js/components/userVignette.js +++ b/static/js/components/userVignette.js @@ -20,6 +20,7 @@ */ import { systemUsers } from '../model/systemUsers.js'; +import { attachTooltip } from '../utils/tooltip.js'; // ── Helpers ──────────────────────────────────────────────────────────────────── @@ -167,12 +168,23 @@ export function createUserVignette(userId, size = 'sm', { showName = true, showE ]).then(([name, photo, email, isExternal]) => { if (nameEl) nameEl.textContent = name; if (emailEl) emailEl.textContent = email ?? ''; - // Tooltip: surface the email on hover when it's not already - // rendered as the visible label (showEmail mode) and isn't - // already the displayed name (the fallback case where the user - // has no given/family/username and the label IS the email). - if (email && !showEmail && email !== name) { - wrapper.title = email; + // Tooltip: surface the email on hover for every vignette that + // has one — including external users whose visible label IS + // the email already. The redundant "alice@x.com → alice@x.com" + // hover is a small price for keeping the interaction uniform: + // every user row in a list reacts to hover the same way, so + // the user doesn't learn "internal rows have tooltips, external + // rows are silent". Suppressed only in `showEmail` mode, where + // the email is already a permanent line below the name. + // + // `attachTooltip` portals the popover to `document.body` and + // applies the shared 250 ms hover-intent delay (much faster + // than the native `title` attribute's ~500–1500 ms wait). + // `aria-label` is set in parallel so screen readers still get + // the email — popover content is mouse/keyboard-hover only. + if (email && !showEmail) { + wrapper.setAttribute('aria-label', email); + attachTooltip(wrapper, email); } if (photo) { _applyPhoto(avatar, photo, name); diff --git a/static/js/core/types.js b/static/js/core/types.js index 4c16fca2..99ba6f02 100644 --- a/static/js/core/types.js +++ b/static/js/core/types.js @@ -25,6 +25,7 @@ * @property {string} owner_id * @property {string|null} parent_id the folder parent (null if is_root) * @property {string} path the full path + * @property {string} etag opaque HTTP ETag, for If-Match / If-None-Match */ //FIXME: rename into FileItem @@ -44,6 +45,8 @@ * @property {number} size * @property {string} size_formatted * @property {number} sort_date + * @property {string} etag opaque HTTP ETag, for If-Match / If-None-Match + * @property {string} content_hash raw BLAKE3 content hash, for dedup checks */ /** diff --git a/static/js/utils/tooltip.js b/static/js/utils/tooltip.js new file mode 100644 index 00000000..dc16bc78 --- /dev/null +++ b/static/js/utils/tooltip.js @@ -0,0 +1,232 @@ +// @ts-check + +/** + * Generic tooltip helper used app-wide. + * + * Two flavours: + * + * - {@link attachTooltip} — short text label, optionally populated + * from a `data-tooltip` attribute. Use + * for one-liners (the email-on-hover on + * a user vignette, the title-text on a + * chip / button, etc.). + * - {@link attachRichTooltip} — structured DOM populated lazily on + * first hover. Use when the content is + * multi-line, async-fetched, or needs + * inner styling (e.g. the member list + * in a group vignette). + * + * Both portal the popover to `document.body` and position it with + * `position: fixed` so it escapes every ancestor `overflow: hidden` + * clip in the page. This is the only reliable cross-browser way to + * keep tooltips fully visible from triggers buried inside list rows, + * scroll containers, or modal panels. + * + * The class toggle (`oxi-tooltip-popover--visible`) is JS-driven on + * `mouseenter` / `mouseleave` / `focusin` / `focusout`. The hover-intent + * delay (250 ms before fade-in, 0 ms before fade-out) lives entirely + * in the CSS transition rules — never in a JS `setTimeout`. See + * `static/css/components/tooltip.css` for the timing source of truth. + * + * Layout helpers exported for the rich variant: + * - `OxiTooltipClass.LINE` — apply to each row inside the popover + * - `OxiTooltipClass.OVERFLOW` — small "+N" badge for truncated lists + * - `OxiTooltipClass.PLACEHOLDER` — italic dimmed text for loading / + * empty states + */ + +const POPOVER_CLASS = 'oxi-tooltip-popover'; +const VISIBLE_CLASS = 'oxi-tooltip-popover--visible'; +const SIMPLE_CLASS = 'oxi-tooltip-popover--simple'; + +/** Class names exported so callers can build the popover body with the + * layout helpers without dragging in private CSS module conventions. */ +export const OxiTooltipClass = Object.freeze({ + LINE: 'oxi-tooltip-popover__line', + OVERFLOW: 'oxi-tooltip-popover__overflow', + PLACEHOLDER: 'oxi-tooltip-popover__placeholder' +}); + +/** Distance between the tooltip and the trigger edge, in pixels. */ +const GAP = 6; + +/** Inset from the viewport edges when clamping the tooltip position. */ +const MARGIN = 8; + +/** + * Position `popover` above (or below, when there isn't room above) the + * given trigger element. Uses `position: fixed` so it escapes any + * ancestor `overflow: hidden`. Clamps horizontally and vertically into + * the viewport so tooltips near the edges still read cleanly. + * + * @param {HTMLElement} popover + * @param {HTMLElement} triggerEl + */ +function _positionPopover(popover, triggerEl) { + const triggerRect = triggerEl.getBoundingClientRect(); + // Measure after content has been added so we know the final size. + const popRect = popover.getBoundingClientRect(); + const vw = window.innerWidth; + const vh = window.innerHeight; + + // Vertical: prefer above the trigger. Flip below when there's not + // enough room above. + let top = triggerRect.top - popRect.height - GAP; + if (top < MARGIN) { + top = triggerRect.bottom + GAP; + } + + // Horizontal: center on the trigger, clamp into the viewport. + let left = triggerRect.left + triggerRect.width / 2 - popRect.width / 2; + if (left < MARGIN) left = MARGIN; + if (left + popRect.width > vw - MARGIN) left = vw - popRect.width - MARGIN; + + // Final vertical clamp — covers the (very rare) case where the + // tooltip is taller than the visible viewport. + if (top + popRect.height > vh - MARGIN) top = vh - popRect.height - MARGIN; + if (top < MARGIN) top = MARGIN; + + popover.style.top = `${top}px`; + popover.style.left = `${left}px`; +} + +/** + * Internal: wire mouseenter/leave + focusin/out listeners on `triggerEl`, + * lazily create the popover element on first hover, and call `populate` + * once to fill it. Returns a cleanup function that removes the + * listeners and the popover element. + * + * @param {HTMLElement} triggerEl + * @param {(popover: HTMLElement) => void | Promise} populate + * Called exactly once when the popover is first shown. Synchronous + * populates take effect immediately; async populates show the + * placeholder span (if you created one) until the promise resolves, + * after which the popover is re-positioned to account for size + * changes. + * @param {{ simple?: boolean }} [opts] + * `simple`: add the `--simple` modifier so the popover uses the + * single-line label style (white-space: nowrap, no min-width). + * @returns {() => void} Cleanup; idempotent. + */ +function _attach(triggerEl, populate, opts = {}) { + /** @type {HTMLElement | null} */ + let popover = null; + let populated = false; + let detached = false; + + const ensurePopover = () => { + if (popover) return popover; + popover = document.createElement('div'); + popover.className = POPOVER_CLASS + (opts.simple ? ` ${SIMPLE_CLASS}` : ''); + // ARIA: behave like a tooltip for screen readers — though we + // also rely on `aria-label` / surrounding text since hover + // isn't reachable via keyboard-only assistive tech. + popover.setAttribute('role', 'tooltip'); + document.body.appendChild(popover); + return popover; + }; + + const show = () => { + if (detached) return; + const pop = ensurePopover(); + + if (!populated) { + populated = true; + // Synchronous populate paths render immediately. Async + // populates (those returning a Promise) re-position after + // resolve so the tooltip catches up to its final size — + // important when the placeholder text is much narrower + // than the eventual content. + const result = populate(pop); + if (result && typeof (/** @type {Promise} */ (result).then) === 'function') { + /** @type {Promise} */ (result).then(() => { + if (popover?.classList.contains(VISIBLE_CLASS)) { + _positionPopover(popover, triggerEl); + } + }); + } + } + + _positionPopover(pop, triggerEl); + pop.classList.add(VISIBLE_CLASS); + }; + + const hide = () => { + if (popover) popover.classList.remove(VISIBLE_CLASS); + }; + + triggerEl.addEventListener('mouseenter', show); + triggerEl.addEventListener('mouseleave', hide); + triggerEl.addEventListener('focusin', show); + triggerEl.addEventListener('focusout', hide); + + return () => { + if (detached) return; + detached = true; + triggerEl.removeEventListener('mouseenter', show); + triggerEl.removeEventListener('mouseleave', hide); + triggerEl.removeEventListener('focusin', show); + triggerEl.removeEventListener('focusout', hide); + popover?.remove(); + popover = null; + }; +} + +/** + * Attach a simple single-line tooltip to `triggerEl`. + * + * @param {HTMLElement} triggerEl + * @param {string} text The label to display. + * @returns {() => void} Cleanup function; idempotent. + * + * @example + * attachTooltip(emailBadgeEl, 'alice@example.com'); + */ +export function attachTooltip(triggerEl, text) { + return _attach( + triggerEl, + (pop) => { + pop.textContent = text; + }, + { simple: true } + ); +} + +/** + * Attach a rich tooltip with structured DOM populated lazily on first + * hover. The `populate` callback receives the popover element and can + * append whatever children it wants. Return a Promise to populate + * async — the popover re-positions on resolve. + * + * @param {HTMLElement} triggerEl + * @param {(popover: HTMLElement) => void | Promise} populate + * @returns {() => void} Cleanup function; idempotent. + * + * @example + * attachRichTooltip(groupEl, async (pop) => { + * const placeholder = document.createElement('div'); + * placeholder.className = OxiTooltipClass.PLACEHOLDER; + * placeholder.textContent = i18n.t('groups.members_loading'); + * pop.appendChild(placeholder); + * const members = await fetchMembers(groupId); + * pop.replaceChildren(); // drop the placeholder + * for (const name of members.slice(0, 8)) { + * const line = document.createElement('div'); + * line.className = OxiTooltipClass.LINE; + * line.textContent = name; + * pop.appendChild(line); + * } + * if (members.length > 8) { + * const overflow = document.createElement('div'); + * overflow.className = OxiTooltipClass.LINE; + * const badge = document.createElement('span'); + * badge.className = OxiTooltipClass.OVERFLOW; + * badge.textContent = `+${members.length - 8}`; + * overflow.append('… ', badge); + * pop.appendChild(overflow); + * } + * }); + */ +export function attachRichTooltip(triggerEl, populate) { + return _attach(triggerEl, populate); +} diff --git a/static/js/views/favorites/favoritesView.js b/static/js/views/favorites/favoritesView.js index 79d82b0a..4a81cc90 100644 --- a/static/js/views/favorites/favoritesView.js +++ b/static/js/views/favorites/favoritesView.js @@ -377,7 +377,8 @@ const favoritesView = { sort_date: toSecs(item.favorited_at), icon_class: f.icon_class, icon_special_class: f.icon_special_class ?? '', - category: 'Folder' + category: 'Folder', + etag: '' }) ); } else if (item.resource_type === 'file') { @@ -397,7 +398,9 @@ const favoritesView = { sort_date: toSecs(item.favorited_at), icon_class: f.icon_class, icon_special_class: f.icon_special_class ?? '', - category: f.category + category: f.category, + etag: '', + content_hash: '' }) ); } diff --git a/static/js/views/recent/recentView.js b/static/js/views/recent/recentView.js index 44fa0446..62fcdcb1 100644 --- a/static/js/views/recent/recentView.js +++ b/static/js/views/recent/recentView.js @@ -384,7 +384,8 @@ const recentView = { sort_date: toSecs(item.accessed_at), icon_class: f.icon_class, icon_special_class: f.icon_special_class ?? '', - category: 'Folder' + category: 'Folder', + etag: '' }) ); } else if (item.resource_type === 'file') { @@ -404,7 +405,9 @@ const recentView = { sort_date: toSecs(item.accessed_at), icon_class: f.icon_class, icon_special_class: f.icon_special_class ?? '', - category: f.category + category: f.category, + etag: '', + content_hash: '' }) ); } diff --git a/static/js/views/sharedWithMe/sharedWithMeView.js b/static/js/views/sharedWithMe/sharedWithMeView.js index 6bdacdcc..05fde699 100644 --- a/static/js/views/sharedWithMe/sharedWithMeView.js +++ b/static/js/views/sharedWithMe/sharedWithMeView.js @@ -386,7 +386,8 @@ const sharedWithMeView = { sort_date: grantedAtSecs(item.granted_at), icon_class: f.icon_class, icon_special_class: f.icon_special_class ?? '', - category: 'folder' + category: 'folder', + etag: '' }) ); } else if (item.resource_type === 'file') { @@ -406,7 +407,9 @@ const sharedWithMeView = { sort_date: grantedAtSecs(item.granted_at), icon_class: f.icon_class, icon_special_class: f.icon_special_class ?? '', - category: f.category + category: f.category, + etag: '', + content_hash: '' }) ); } diff --git a/static/js/views/trash/trashView.js b/static/js/views/trash/trashView.js index 476843b3..0a6b430d 100644 --- a/static/js/views/trash/trashView.js +++ b/static/js/views/trash/trashView.js @@ -393,7 +393,8 @@ const trashView = { deletion_date: item.deletion_date, icon_class: f.icon_class, icon_special_class: f.icon_special_class ?? '', - category: 'Folder' + category: 'Folder', + etag: '' }) ); } else if (item.resource_type === 'file') { @@ -417,7 +418,9 @@ const trashView = { deletion_date: item.deletion_date, icon_class: f.icon_class, icon_special_class: f.icon_special_class ?? '', - category: f.category + category: f.category, + etag: '', + content_hash: '' }) ); } diff --git a/static/locales/ar.json b/static/locales/ar.json index fa1cb9c3..559f0a8b 100644 --- a/static/locales/ar.json +++ b/static/locales/ar.json @@ -867,7 +867,10 @@ "member_count_other": "{count} أعضاء", "delete_confirm_label": "اكتب اسم المجموعة للتأكيد:", "delete_confirm_mismatch": "اكتب اسم المجموعة كما هو للتأكيد.", - "virtual_internal_name": "داخلي" + "virtual_internal_name": "داخلي", + "members_loading": "جارٍ تحميل الأعضاء…", + "members_empty": "لا يوجد أعضاء", + "virtual_internal_explanation": "كل مستخدم داخلي على هذا الخادم" }, "myshares": { "copyLink": "نسخ الرابط", diff --git a/static/locales/de.json b/static/locales/de.json index 984cadf2..ee333f84 100644 --- a/static/locales/de.json +++ b/static/locales/de.json @@ -867,7 +867,10 @@ "member_count_other": "{count} Mitglieder", "delete_confirm_label": "Tippe den Gruppennamen zur Bestätigung ein:", "delete_confirm_mismatch": "Tippe den Gruppennamen exakt zur Bestätigung ein.", - "virtual_internal_name": "Intern" + "virtual_internal_name": "Intern", + "members_loading": "Mitglieder werden geladen…", + "members_empty": "Keine Mitglieder", + "virtual_internal_explanation": "Jeder interne Benutzer auf diesem Server" }, "myshares": { "copyLink": "Link kopieren", diff --git a/static/locales/en.json b/static/locales/en.json index a606b0fa..7e0fa4b7 100644 --- a/static/locales/en.json +++ b/static/locales/en.json @@ -862,6 +862,8 @@ "name_placeholder": "engineering", "description_label": "Description (optional)", "members_section": "Members", + "members_loading": "Loading members…", + "members_empty": "No members", "add_member_placeholder": "Add a user or group…", "no_members": "No members yet.", "remove_member": "Remove", @@ -877,6 +879,7 @@ "member_count_other": "{count} members", "delete_confirm_label": "Type the group name to confirm:", "delete_confirm_mismatch": "Type the group name exactly to confirm.", - "virtual_internal_name": "Internal" + "virtual_internal_name": "Internal", + "virtual_internal_explanation": "Every internal user on this server" } } diff --git a/static/locales/es.json b/static/locales/es.json index 01657b03..d4baccf5 100644 --- a/static/locales/es.json +++ b/static/locales/es.json @@ -867,7 +867,10 @@ "member_count_other": "{count} miembros", "delete_confirm_label": "Escribe el nombre del grupo para confirmar:", "delete_confirm_mismatch": "Escribe el nombre del grupo exactamente para confirmar.", - "virtual_internal_name": "Interno" + "virtual_internal_name": "Interno", + "members_loading": "Cargando miembros…", + "members_empty": "Sin miembros", + "virtual_internal_explanation": "Todos los usuarios internos de este servidor" }, "myshares": { "copyLink": "Copiar enlace", diff --git a/static/locales/fa.json b/static/locales/fa.json index 61ab2a56..58b745c1 100644 --- a/static/locales/fa.json +++ b/static/locales/fa.json @@ -867,7 +867,10 @@ "member_count_other": "{count} عضو", "delete_confirm_label": "نام گروه را برای تأیید وارد کنید:", "delete_confirm_mismatch": "نام گروه را دقیقاً برای تأیید وارد کنید.", - "virtual_internal_name": "داخلی" + "virtual_internal_name": "داخلی", + "members_loading": "در حال بارگیری اعضا…", + "members_empty": "بدون عضو", + "virtual_internal_explanation": "هر کاربر داخلی روی این سرور" }, "myshares": { "copyLink": "کپی پیوند", diff --git a/static/locales/fr.json b/static/locales/fr.json index 51c4afa5..e161f541 100644 --- a/static/locales/fr.json +++ b/static/locales/fr.json @@ -867,7 +867,10 @@ "member_count_other": "{count} membres", "delete_confirm_label": "Tapez le nom du groupe pour confirmer :", "delete_confirm_mismatch": "Tapez le nom du groupe exactement pour confirmer.", - "virtual_internal_name": "Interne" + "virtual_internal_name": "Interne", + "members_loading": "Chargement des membres…", + "members_empty": "Aucun membre", + "virtual_internal_explanation": "Tous les utilisateurs internes de ce serveur" }, "myshares": { "copyLink": "Copier le lien", diff --git a/static/locales/hi.json b/static/locales/hi.json index 62b4dda1..e451ecdb 100644 --- a/static/locales/hi.json +++ b/static/locales/hi.json @@ -867,7 +867,10 @@ "member_count_other": "{count} सदस्य", "delete_confirm_label": "पुष्टि के लिए समूह का नाम लिखें:", "delete_confirm_mismatch": "पुष्टि के लिए समूह का नाम बिल्कुल वैसा ही लिखें।", - "virtual_internal_name": "आंतरिक" + "virtual_internal_name": "आंतरिक", + "members_loading": "सदस्य लोड हो रहे हैं…", + "members_empty": "कोई सदस्य नहीं", + "virtual_internal_explanation": "इस सर्वर पर हर आंतरिक उपयोगकर्ता" }, "myshares": { "copyLink": "लिंक कॉपी करें", diff --git a/static/locales/it.json b/static/locales/it.json index 0d3860d2..e16e50b1 100644 --- a/static/locales/it.json +++ b/static/locales/it.json @@ -867,7 +867,10 @@ "member_count_other": "{count} membri", "delete_confirm_label": "Digita il nome del gruppo per confermare:", "delete_confirm_mismatch": "Digita esattamente il nome del gruppo per confermare.", - "virtual_internal_name": "Interno" + "virtual_internal_name": "Interno", + "members_loading": "Caricamento membri…", + "members_empty": "Nessun membro", + "virtual_internal_explanation": "Ogni utente interno su questo server" }, "myshares": { "copyLink": "Copia link", diff --git a/static/locales/ja.json b/static/locales/ja.json index 84debb3f..25beab40 100644 --- a/static/locales/ja.json +++ b/static/locales/ja.json @@ -867,7 +867,10 @@ "member_count_other": "{count} メンバー", "delete_confirm_label": "確認のためにグループ名を入力してください:", "delete_confirm_mismatch": "確認のためにグループ名を正確に入力してください。", - "virtual_internal_name": "内部" + "virtual_internal_name": "内部", + "members_loading": "メンバーを読み込み中…", + "members_empty": "メンバーなし", + "virtual_internal_explanation": "このサーバー上のすべての内部ユーザー" }, "myshares": { "copyLink": "リンクをコピー", diff --git a/static/locales/ko.json b/static/locales/ko.json index 345cbe13..e93ba416 100644 --- a/static/locales/ko.json +++ b/static/locales/ko.json @@ -867,7 +867,10 @@ "member_count_other": "구성원 {count}명", "delete_confirm_label": "확인을 위해 그룹 이름을 입력하세요:", "delete_confirm_mismatch": "확인을 위해 그룹 이름을 정확히 입력하세요.", - "virtual_internal_name": "내부" + "virtual_internal_name": "내부", + "members_loading": "구성원 로딩 중…", + "members_empty": "구성원 없음", + "virtual_internal_explanation": "이 서버의 모든 내부 사용자" }, "myshares": { "copyLink": "링크 복사", diff --git a/static/locales/nl.json b/static/locales/nl.json index 33f93ec1..4c54be74 100644 --- a/static/locales/nl.json +++ b/static/locales/nl.json @@ -867,7 +867,10 @@ "member_count_other": "{count} leden", "delete_confirm_label": "Typ de groepsnaam ter bevestiging:", "delete_confirm_mismatch": "Typ de groepsnaam exact om te bevestigen.", - "virtual_internal_name": "Intern" + "virtual_internal_name": "Intern", + "members_loading": "Leden laden…", + "members_empty": "Geen leden", + "virtual_internal_explanation": "Iedere interne gebruiker op deze server" }, "myshares": { "copyLink": "Link kopiëren", diff --git a/static/locales/pl.json b/static/locales/pl.json index e9fff114..3d83473c 100644 --- a/static/locales/pl.json +++ b/static/locales/pl.json @@ -867,7 +867,10 @@ "member_count_other": "{count} członków", "delete_confirm_label": "Wpisz nazwę grupy, aby potwierdzić:", "delete_confirm_mismatch": "Wpisz nazwę grupy dokładnie, aby potwierdzić.", - "virtual_internal_name": "Wewnętrzni" + "virtual_internal_name": "Wewnętrzni", + "members_loading": "Ładowanie członków…", + "members_empty": "Brak członków", + "virtual_internal_explanation": "Każdy użytkownik wewnętrzny na tym serwerze" }, "myshares": { "copyLink": "Skopiuj link", diff --git a/static/locales/pt.json b/static/locales/pt.json index 358ba879..a9a2fa56 100644 --- a/static/locales/pt.json +++ b/static/locales/pt.json @@ -867,7 +867,10 @@ "member_count_other": "{count} membros", "delete_confirm_label": "Digite o nome do grupo para confirmar:", "delete_confirm_mismatch": "Digite o nome do grupo exatamente para confirmar.", - "virtual_internal_name": "Interno" + "virtual_internal_name": "Interno", + "members_loading": "A carregar membros…", + "members_empty": "Sem membros", + "virtual_internal_explanation": "Todos os utilizadores internos neste servidor" }, "myshares": { "copyLink": "Copiar link", diff --git a/static/locales/ru.json b/static/locales/ru.json index c2b1cfec..4786d4f1 100644 --- a/static/locales/ru.json +++ b/static/locales/ru.json @@ -867,7 +867,10 @@ "member_count_other": "{count} участников", "delete_confirm_label": "Введите имя группы для подтверждения:", "delete_confirm_mismatch": "Введите имя группы точно для подтверждения.", - "virtual_internal_name": "Внутренние" + "virtual_internal_name": "Внутренние", + "members_loading": "Загрузка участников…", + "members_empty": "Нет участников", + "virtual_internal_explanation": "Каждый внутренний пользователь на этом сервере" }, "myshares": { "copyLink": "Копировать ссылку", diff --git a/static/locales/zh-TW.json b/static/locales/zh-TW.json index 5d98739c..495218cd 100644 --- a/static/locales/zh-TW.json +++ b/static/locales/zh-TW.json @@ -867,7 +867,10 @@ "member_count_other": "{count} 個成員", "delete_confirm_label": "請輸入群組名稱以確認:", "delete_confirm_mismatch": "請準確輸入群組名稱以確認。", - "virtual_internal_name": "內部" + "virtual_internal_name": "內部", + "members_loading": "正在載入成員…", + "members_empty": "無成員", + "virtual_internal_explanation": "本伺服器上的所有內部使用者" }, "myshares": { "copyLink": "複製連結", diff --git a/static/locales/zh.json b/static/locales/zh.json index 428ce842..3adf619b 100644 --- a/static/locales/zh.json +++ b/static/locales/zh.json @@ -867,7 +867,10 @@ "member_count_other": "{count} 个成员", "delete_confirm_label": "请输入群组名称以确认:", "delete_confirm_mismatch": "请准确输入群组名称以确认。", - "virtual_internal_name": "内部" + "virtual_internal_name": "内部", + "members_loading": "正在加载成员…", + "members_empty": "无成员", + "virtual_internal_explanation": "本服务器上的所有内部用户" }, "myshares": { "copyLink": "复制链接", diff --git a/tests/api/dedup_blob_cleanup.hurl b/tests/api/dedup_blob_cleanup.hurl index c2563c34..63849750 100644 --- a/tests/api/dedup_blob_cleanup.hurl +++ b/tests/api/dedup_blob_cleanup.hurl @@ -90,6 +90,12 @@ file1_id: jsonpath "$.id" [Asserts] jsonpath "$.name" == "dedup-test.jpg" jsonpath "$.folder_id" == {{test_folder_id}} +# Cross-check that the server's view of the uploaded content matches +# the BLAKE3 we computed locally over fixtures/dedup-test.jpg. The +# `content_hash` field is the raw blob hash, distinct from `etag` +# (which folds in modified_at) — exposed in REST JSON by the +# etag-centralization refactor. +jsonpath "$.content_hash" == "cde1ca663a2e62e0dadb41c3194e11ecb7d971d84c7451db17063b55c09e8066" # ref_count == 1: blob has exactly one file reference after first upload @@ -118,6 +124,10 @@ file2_id: jsonpath "$.id" [Asserts] jsonpath "$.name" == "dedup-test-2.jpg" jsonpath "$.id" != "{{file1_id}}" +# Same content as fixtures/dedup-test.jpg → identical content_hash. +# This is the actual "dedup happened" assertion at the API surface, +# independent of the /api/dedup/check probe below. +jsonpath "$.content_hash" == "cde1ca663a2e62e0dadb41c3194e11ecb7d971d84c7451db17063b55c09e8066" # ref_count == 2: dedup hit — same blob now referenced by two file records diff --git a/tests/webdav/test_chunked_upload_dedup.sh b/tests/webdav/test_chunked_upload_dedup.sh index 51094cf1..c9cd10d7 100755 --- a/tests/webdav/test_chunked_upload_dedup.sh +++ b/tests/webdav/test_chunked_upload_dedup.sh @@ -146,6 +146,18 @@ MIME=$(jq -r '.mime_type' <<< "$LISTED_FILE") || fail "Expected MIME type video/mp4, got: $MIME" pass "File listed with MIME type: $MIME" +# ── Step 4b: server's content_hash matches our local BLAKE3 ────────────────── +# Cross-check the file we just uploaded: the server's view of its +# content identity (FileDto.content_hash, exposed in REST JSON since +# the etag-centralization refactor) must equal the BLAKE3 we know +# the fixture has. Catches any chunk-assembly bug that would +# silently produce a different blob than the source bytes. + +LISTED_HASH=$(jq -r '.content_hash // empty' <<< "$LISTED_FILE") +[[ "$LISTED_HASH" == "$BLOB_HASH" ]] \ + || fail "content_hash mismatch: server=$LISTED_HASH expected=$BLOB_HASH" +pass "content_hash matches local BLAKE3 ($BLOB_HASH)" + # ── Step 5: Dedup check → ref_count == 1 ───────────────────────────────────── echo " step 5: GET /api/dedup/check/$BLOB_HASH..." diff --git a/tests/webdav/test_dedup_webdav_multichunk.sh b/tests/webdav/test_dedup_webdav_multichunk.sh index 967124a8..dd742afc 100755 --- a/tests/webdav/test_dedup_webdav_multichunk.sh +++ b/tests/webdav/test_dedup_webdav_multichunk.sh @@ -141,6 +141,22 @@ FILE_B_ID=$(jq -r --arg n "$FILE_B" '.[] | select(.name == $n) | .id' <<< "$LIST || fail "File A and B share the same ID — dedup must create two distinct records" pass "Two distinct file records: A=$FILE_A_ID B=$FILE_B_ID" +# ── Step 2b: server's content_hash matches our local BLAKE3 for both ───────── +# Both files were uploaded from byte-identical bytes, so the server +# MUST report the same content_hash for both — and that hash MUST +# equal the BLAKE3 we computed locally. Without this check, a +# subtle CDC-assembly bug could produce two distinct blobs that +# happen to map to the same dedup key but differ from the source — +# the ref_count assertions below would still pass. + +FILE_A_HASH=$(jq -r --arg n "$FILE_A" '.[] | select(.name == $n) | .content_hash // empty' <<< "$LISTING") +FILE_B_HASH=$(jq -r --arg n "$FILE_B" '.[] | select(.name == $n) | .content_hash // empty' <<< "$LISTING") +[[ "$FILE_A_HASH" == "$BLOB_HASH" ]] \ + || fail "content_hash mismatch for A: server=$FILE_A_HASH expected=$BLOB_HASH" +[[ "$FILE_B_HASH" == "$BLOB_HASH" ]] \ + || fail "content_hash mismatch for B: server=$FILE_B_HASH expected=$BLOB_HASH" +pass "content_hash on both A and B matches local BLAKE3 ($BLOB_HASH)" + # ── Step 3: ref_count == 2 ──────────────────────────────────────────────────── echo " step 3: dedup/check → expect ref_count=2..." diff --git a/tests/webdav/test_dedup_webdav_ref_count.sh b/tests/webdav/test_dedup_webdav_ref_count.sh index 7a87762d..e1bf400a 100755 --- a/tests/webdav/test_dedup_webdav_ref_count.sh +++ b/tests/webdav/test_dedup_webdav_ref_count.sh @@ -147,6 +147,20 @@ FILE_B_ID=$(jq -r --arg n "$FILE_B" '.[] | select(.name == $n) | .id' <<< "$FILE || fail "File A and B share the same ID — dedup must produce two distinct records" pass "Two distinct file records: A=$FILE_A_ID B=$FILE_B_ID" +# ── Step 3b: server's content_hash matches our local BLAKE3 for both ───────── +# Both files were uploaded from byte-identical content — server's +# `content_hash` field (exposed via the etag-centralization refactor) +# must equal BLOB_HASH for both, proving the server's view of +# content identity agrees with our local computation. + +FILE_A_HASH=$(jq -r --arg n "$FILE_A" '.[] | select(.name == $n) | .content_hash // empty' <<< "$FILE_LISTING") +FILE_B_HASH=$(jq -r --arg n "$FILE_B" '.[] | select(.name == $n) | .content_hash // empty' <<< "$FILE_LISTING") +[[ "$FILE_A_HASH" == "$BLOB_HASH" ]] \ + || fail "content_hash mismatch for A: server=$FILE_A_HASH expected=$BLOB_HASH" +[[ "$FILE_B_HASH" == "$BLOB_HASH" ]] \ + || fail "content_hash mismatch for B: server=$FILE_B_HASH expected=$BLOB_HASH" +pass "content_hash on both A and B matches local BLAKE3 ($BLOB_HASH)" + # ── Step 4: Dedup check → ref_count == 2 ───────────────────────────────────── echo " step 4: GET /api/dedup/check/$BLOB_HASH..." @@ -168,6 +182,25 @@ STATUS=$(webdav_put "$FILE_B" "$FIXTURE_OTHER" "image/jpeg") [[ "$STATUS" == "204" ]] || fail "PUT $FILE_B (overwrite) expected 204, got $STATUS" pass "PUT $FILE_B overwrite → 204" +# ── Step 5b: B's content_hash flipped, A's unchanged ───────────────────────── +# After overwrite, B references a new blob (oxicloud-logo.jpg) +# whose BLAKE3 differs from BLOB_HASH; A still holds the original. +# A weaker but local-fixture-agnostic check than asserting B's new +# exact hash (avoids hardcoding a second BLAKE3) — proves that the +# COW-overwrite path swaps the blob identity rather than silently +# keeping the old one. + +REFRESHED=$(rest_get "/api/files?folder_id=$HOME_FOLDER_ID") +FILE_A_HASH_AFTER=$(jq -r --arg n "$FILE_A" '.[] | select(.name == $n) | .content_hash // empty' <<< "$REFRESHED") +FILE_B_HASH_AFTER=$(jq -r --arg n "$FILE_B" '.[] | select(.name == $n) | .content_hash // empty' <<< "$REFRESHED") +[[ "$FILE_A_HASH_AFTER" == "$BLOB_HASH" ]] \ + || fail "File A's content_hash changed unexpectedly: $FILE_A_HASH_AFTER (overwrite of B must not touch A)" +[[ "$FILE_B_HASH_AFTER" != "$BLOB_HASH" ]] \ + || fail "File B's content_hash unchanged after overwrite — COW path didn't swap the blob" +[[ -n "$FILE_B_HASH_AFTER" ]] \ + || fail "File B has empty content_hash after overwrite — server didn't compute a new blob" +pass "post-overwrite: A still on $BLOB_HASH, B flipped to $FILE_B_HASH_AFTER" + # ── Step 6: Dedup check → ref_count == 1 ───────────────────────────────────── # File B now references a different blob; file A still holds the original.