diff --git a/migrations/20260830000002_copy_dead_properties_on_folder_tree.sql b/migrations/20260830000002_copy_dead_properties_on_folder_tree.sql new file mode 100644 index 00000000..e33fa77f --- /dev/null +++ b/migrations/20260830000002_copy_dead_properties_on_folder_tree.sql @@ -0,0 +1,223 @@ +-- ════════════════════════════════════════════════════════════════════════════ +-- COPY: duplicate dead properties along with files and folders +-- ════════════════════════════════════════════════════════════════════════════ +-- RFC 4918 §8.8 — "If a property cannot be copied live, then its value +-- MUST be duplicated, exactly as it would be for a PROPPATCH SET +-- operation, in the copy." Dead properties are by definition not live +-- (the server stores them verbatim with no interpretation), so every +-- COPY MUST duplicate the source's dead properties onto the new +-- resource. +-- +-- The pre-rekey path-based store handled this by accident in some +-- cases and missed it in others; the id-keyed store (migration +-- 20260830000001) makes the requirement explicit — dead properties +-- key on `folder_id` / `file_id`, so a copy that doesn't insert new +-- rows for the destination's ids loses the properties entirely. +-- +-- This migration replaces `storage.copy_folder_tree` with a version +-- that: +-- +-- 1. Pre-allocates destination file ids in a new temp table +-- `_copy_file_map(old_id, new_id)` — analogous to the +-- pre-existing `_copy_map` that already does this for folders. +-- Previously, file ids were generated by the `gen_random_uuid()` +-- DEFAULT during the batch INSERT, leaving no way to relate src +-- and dst files afterward. +-- 2. Switches the batch file INSERT to use the explicit +-- pre-allocated id, so src→dst is bidirectionally known by +-- `_copy_file_map`. +-- 3. Adds two `INSERT INTO storage.webdav_dead_properties` SELECTs +-- at the end that duplicate dead-property rows for every copied +-- folder (via `_copy_map`) and every copied file (via +-- `_copy_file_map`). Each duplicated row carries the same +-- `(namespace, local_name, value)` triple as the source — the +-- definition of "duplicate" in RFC 4918 §8.8. +-- +-- Idempotent via CREATE OR REPLACE FUNCTION. No callers change (the +-- function signature and return shape are unchanged). +-- +-- COPY semantics out of scope for this migration: +-- * Cross-user permission handling on the copied resources is the +-- caller's responsibility (the `_with_perms` service variant +-- already enforces this on the source side). Dead properties +-- hitch a ride on the resource's ACL; nothing additional needed. +-- * Trash: trashed source rows are excluded by the existing +-- `NOT is_trashed` filter; dead-props on trashed rows live on +-- until the resource itself is hard-deleted, at which point +-- CASCADE handles them. Same model holds in the new COPY path. + +CREATE OR REPLACE FUNCTION storage.copy_folder_tree( + p_source_id UUID, + p_target_parent_id UUID, -- NULL = copy to root (keeps source drive) + p_dest_name TEXT DEFAULT NULL -- NULL = keep source folder name +) RETURNS TABLE(new_root_id TEXT, folders_copied BIGINT, files_copied BIGINT) AS $$ +DECLARE + v_root_lpath ltree; + v_root_depth INT; + v_max_depth INT; + v_level INT; + v_folders BIGINT := 0; + v_files BIGINT := 0; + v_inserted BIGINT; + v_new_root UUID; + v_dest_drive_id UUID; +BEGIN + -- Validate source exists + SELECT fo.lpath, nlevel(fo.lpath) + INTO v_root_lpath, v_root_depth + FROM storage.folders fo + WHERE fo.id = p_source_id AND NOT fo.is_trashed; + + IF v_root_lpath IS NULL THEN + RAISE EXCEPTION 'Source folder not found: %', p_source_id + USING ERRCODE = 'P0002'; -- no_data_found + END IF; + + -- Resolve the destination drive_id ONCE up front. The whole copied + -- subtree lands in this drive; pulling it per-row from `fo.drive_id` + -- (the previous body) was the cross-drive bug. + -- + -- When p_target_parent_id is NULL the caller asked for "copy to + -- root" — there is no global root in the multi-drive world, so we + -- preserve the source's drive_id (legacy behaviour, defensive). + -- Real API call sites always pass a concrete target folder. + IF p_target_parent_id IS NULL THEN + SELECT fo.drive_id INTO v_dest_drive_id + FROM storage.folders fo + WHERE fo.id = p_source_id; + ELSE + SELECT fo.drive_id INTO v_dest_drive_id + FROM storage.folders fo + WHERE fo.id = p_target_parent_id AND NOT fo.is_trashed; + IF v_dest_drive_id IS NULL THEN + RAISE EXCEPTION 'Target parent folder not found: %', p_target_parent_id + USING ERRCODE = 'P0002'; -- no_data_found + END IF; + END IF; + + -- Temp mapping: every folder in the subtree → new UUID + CREATE TEMP TABLE IF NOT EXISTS _copy_map( + old_id UUID PRIMARY KEY, + new_id UUID NOT NULL DEFAULT gen_random_uuid() + ) ON COMMIT DROP; + TRUNCATE _copy_map; + + INSERT INTO _copy_map(old_id) + SELECT fo.id + FROM storage.folders fo + WHERE NOT fo.is_trashed + AND fo.lpath <@ v_root_lpath; + + -- Remember new root ID + SELECT cm.new_id INTO v_new_root + FROM _copy_map cm WHERE cm.old_id = p_source_id; + + -- Max depth for level iteration + SELECT MAX(nlevel(fo.lpath)) + INTO v_max_depth + FROM storage.folders fo + JOIN _copy_map cm ON fo.id = cm.old_id; + + -- ── Insert folders level by level ── + -- Each level is a separate INSERT so the BEFORE INSERT trigger + -- (trg_folders_path) can resolve the parent's path/lpath from rows + -- inserted in the previous level. drive_id is the destination's + -- (resolved once above); user_id + provenance preserved from source. + FOR v_level IN v_root_depth .. v_max_depth LOOP + INSERT INTO storage.folders( + id, name, parent_id, user_id, + drive_id, created_by, updated_by + ) + SELECT cm.new_id, + CASE WHEN fo.id = p_source_id AND p_dest_name IS NOT NULL + THEN p_dest_name ELSE fo.name END, + CASE WHEN fo.id = p_source_id THEN p_target_parent_id + ELSE pm.new_id END, + fo.user_id, + v_dest_drive_id, + fo.created_by, + fo.updated_by + FROM storage.folders fo + JOIN _copy_map cm ON fo.id = cm.old_id + LEFT JOIN _copy_map pm ON fo.parent_id = pm.old_id + WHERE NOT fo.is_trashed + AND nlevel(fo.lpath) = v_level; + + GET DIAGNOSTICS v_inserted = ROW_COUNT; + v_folders := v_folders + v_inserted; + END LOOP; + + -- ── NEW: temp mapping for files src→dst ─────────────────────────── + -- Pre-allocate destination ids so we can: + -- (a) reference each dst file by id in the dead-property INSERT + -- below — a batched INSERT...RETURNING couldn't tell us which + -- new id corresponded to which source id, so the mapping + -- has to be stamped at planning time, not after the fact; + -- (b) batch the file INSERT with explicit ids exactly the same + -- way folders are batched. + CREATE TEMP TABLE IF NOT EXISTS _copy_file_map( + old_id UUID PRIMARY KEY, + new_id UUID NOT NULL DEFAULT gen_random_uuid() + ) ON COMMIT DROP; + TRUNCATE _copy_file_map; + + INSERT INTO _copy_file_map(old_id) + SELECT f.id + FROM storage.files f + JOIN _copy_map cm ON f.folder_id = cm.old_id + WHERE NOT f.is_trashed; + + -- ── Batch copy all files (zero-copy: same blob_hash) ── + -- drive_id from destination; everything else (user_id, created_by, + -- updated_by) preserved from source so authorship survives the copy. + -- `id` is the pre-allocated dst id from _copy_file_map. + INSERT INTO storage.files( + id, name, folder_id, user_id, blob_hash, size, mime_type, + media_sort_date, drive_id, created_by, updated_by + ) + SELECT fm.new_id, f.name, cm.new_id, f.user_id, f.blob_hash, f.size, + f.mime_type, f.media_sort_date, v_dest_drive_id, f.created_by, + f.updated_by + FROM storage.files f + JOIN _copy_map cm ON f.folder_id = cm.old_id + JOIN _copy_file_map fm ON fm.old_id = f.id + WHERE NOT f.is_trashed; + + GET DIAGNOSTICS v_files = ROW_COUNT; + + -- ── Batch increment blob ref_counts ── + IF v_files > 0 THEN + UPDATE storage.blobs b + SET ref_count = ref_count + hc.cnt + FROM ( + SELECT f.blob_hash, COUNT(*)::int AS cnt + FROM storage.files f + JOIN _copy_map cm ON f.folder_id = cm.new_id + WHERE NOT f.is_trashed + GROUP BY f.blob_hash + ) hc + WHERE b.hash = hc.blob_hash; + END IF; + + -- ── NEW: duplicate dead properties for every copied folder ──────── + -- RFC 4918 §8.8 — dead properties MUST be duplicated. The id-keyed + -- store (migration 20260830000001) keys on `folder_id`, so we + -- emit a new row per source dead-property pointing at the + -- destination folder id. `(namespace, local_name, value)` is + -- preserved verbatim — that's the "duplicate" definition. + INSERT INTO storage.webdav_dead_properties + (folder_id, namespace, local_name, value) + SELECT cm.new_id, dp.namespace, dp.local_name, dp.value + FROM storage.webdav_dead_properties dp + JOIN _copy_map cm ON dp.folder_id = cm.old_id; + + -- ── NEW: duplicate dead properties for every copied file ────────── + INSERT INTO storage.webdav_dead_properties + (file_id, namespace, local_name, value) + SELECT fm.new_id, dp.namespace, dp.local_name, dp.value + FROM storage.webdav_dead_properties dp + JOIN _copy_file_map fm ON dp.file_id = fm.old_id; + + RETURN QUERY SELECT v_new_root::text, v_folders, v_files; +END; +$$ LANGUAGE plpgsql; diff --git a/src/infrastructure/repositories/pg/file_blob_write_repository.rs b/src/infrastructure/repositories/pg/file_blob_write_repository.rs index cdb4aa8f..43063851 100644 --- a/src/infrastructure/repositories/pg/file_blob_write_repository.rs +++ b/src/infrastructure/repositories/pg/file_blob_write_repository.rs @@ -642,14 +642,33 @@ impl FileWritePort for FileBlobWriteRepository { $4, $4 FROM src, dest_folder - RETURNING id::text, name, folder_id::text, size, mime_type, - EXTRACT(EPOCH FROM created_at)::bigint, - EXTRACT(EPOCH FROM updated_at)::bigint, + RETURNING id, + id::text AS id_text, + name, folder_id::text, size, mime_type, + EXTRACT(EPOCH FROM created_at)::bigint AS created_at, + EXTRACT(EPOCH FROM updated_at)::bigint AS updated_at, blob_hash, created_by, updated_by + ), + -- RFC 4918 §8.8 — dead properties MUST be duplicated on + -- COPY. With the id-keyed store (migration + -- 20260830000001) this is a single batch INSERT keyed on + -- the new file's id. Runs in the same query as the file + -- INSERT so either both land or neither does — atomic + -- by virtue of being one statement. + dead_prop_copy AS ( + INSERT INTO storage.webdav_dead_properties + (file_id, namespace, local_name, value) + SELECT (SELECT id FROM new_file), + dp.namespace, dp.local_name, dp.value + FROM storage.webdav_dead_properties dp + WHERE dp.file_id = $1::uuid ) - SELECT * FROM new_file + SELECT id_text, name, folder_id, size, mime_type, + created_at, updated_at, + blob_hash, created_by, updated_by + FROM new_file "#, ) .bind(file_id) diff --git a/tests/api/webdav_dead_properties.hurl b/tests/api/webdav_dead_properties.hurl index b78556a0..a1bba249 100644 --- a/tests/api/webdav_dead_properties.hurl +++ b/tests/api/webdav_dead_properties.hurl @@ -37,6 +37,17 @@ # guarantee under rename). The Hurl suite had no folder- # side coverage of this until 20260830000001; only the # file MOVE case (step 9) was guarded. +# 13. Single-file COPY duplicates dead properties (RFC 4918 +# §8.8). Destination carries a copy of the source's +# marker; source retains its copy (COPY ≠ MOVE). +# Implementation: `dead_prop_copy` CTE branch in +# `copy_file` (migration 20260830000002). +# 14. Folder COPY (Depth: infinity) duplicates dead +# properties for every descendant — both folder and file +# dead-props. Implementation: the two INSERT...SELECT +# branches in `storage.copy_folder_tree` (migration +# 20260830000002) that walk `_copy_map` and the new +# `_copy_file_map` respectively. # # XPath assertions deliberately use `local-name()` so the test # is robust against the server's choice of namespace prefix — @@ -393,7 +404,13 @@ Authorization: Bearer {{token}} HTTP 200 [Captures] -rest_file_id: jsonpath "$[?(@.name=='dead-props-moved.txt')].id" nth 0 +# Hurl quirk: `$[?(...)]` collapses to a scalar (not a list) when the +# filter matches exactly one element, so `nth 0` fails with "invalid +# filter input type". The bare filter capture returns that scalar +# directly. Filename uniqueness across the home folder makes the +# single-match assumption safe — `dead-props-moved.txt` is created +# only by this test (no other Hurl test ever PUTs that name). +rest_file_id: jsonpath "$[?(@.name=='dead-props-moved.txt')].id" # Step 11d — REST DELETE. No webdav, no dead-prop API call — @@ -529,3 +546,248 @@ DELETE {{base_url}}/webdav/dead-props-folder-renamed/ Authorization: Bearer {{token}} HTTP 204 + + +# ───────────────────────────────────────────────────────────── +# Step 13 — Single-file COPY duplicates dead properties. +# RFC 4918 §8.8: dead properties MUST be duplicated. +# Implementation is the `dead_prop_copy` CTE branch in +# `file_blob_write_repository::copy_file` (inserts a +# new dead-prop row per source row, keyed on the new +# file's id). +# +# Sequence: +# a. PUT a source file. +# b. PROPPATCH a marker dead property. +# c. COPY (WebDAV) to a new path. +# d. PROPFIND the new path; marker must be present. +# e. PROPFIND the source path; marker still present +# on source too (COPY duplicates — it doesn't +# move). +# f. Cleanup both files. +# ───────────────────────────────────────────────────────────── + +# Step 13a — source file +PUT {{base_url}}/webdav/dead-props-copy-src.txt +Authorization: Bearer {{token}} +Content-Type: text/plain +``` +copy source +``` + +HTTP 201 + + +# Step 13b — set the marker dead property on the source +PROPPATCH {{base_url}}/webdav/dead-props-copy-src.txt +Authorization: Bearer {{token}} +Content-Type: application/xml; charset=utf-8 +``` + + + + + survives-copy + + + +``` + +HTTP 207 + + +# Step 13c — COPY the file. Destination is fresh → 201 Created. +# §9.8.5: 201 when destination is new, 204 when overwriting. +COPY {{base_url}}/webdav/dead-props-copy-src.txt +Authorization: Bearer {{token}} +Destination: {{base_url}}/webdav/dead-props-copy-dst.txt + +HTTP 201 + + +# Step 13d — destination must carry the property (RFC 4918 §8.8) +PROPFIND {{base_url}}/webdav/dead-props-copy-dst.txt +Authorization: Bearer {{token}} +Depth: 0 +Content-Type: application/xml; charset=utf-8 +``` + + + + +``` + +HTTP 207 +[Asserts] +xpath "string(//*[local-name()='copymark'])" == "survives-copy" + + +# Step 13e — source still has it too (COPY, not MOVE) +PROPFIND {{base_url}}/webdav/dead-props-copy-src.txt +Authorization: Bearer {{token}} +Depth: 0 +Content-Type: application/xml; charset=utf-8 +``` + + + + +``` + +HTTP 207 +[Asserts] +xpath "string(//*[local-name()='copymark'])" == "survives-copy" + + +# Step 13f — cleanup both +DELETE {{base_url}}/webdav/dead-props-copy-src.txt +Authorization: Bearer {{token}} + +HTTP 204 + + +DELETE {{base_url}}/webdav/dead-props-copy-dst.txt +Authorization: Bearer {{token}} + +HTTP 204 + + +# ───────────────────────────────────────────────────────────── +# Step 14 — Folder COPY duplicates dead properties on every +# descendant. RFC 4918 §8.8 + §9.8.3 (Depth: infinity +# for collections). Implementation is the two +# INSERT...SELECT branches added to +# `storage.copy_folder_tree` in migration +# 20260830000002: +# - folders mapped via `_copy_map` +# - files mapped via the new `_copy_file_map` +# +# Test shape: +# a. MKCOL outer collection. +# b. MKCOL inner collection (descendant). +# c. PUT a leaf file inside inner. +# d. PROPPATCH a marker on the descendant FOLDER. +# e. PROPPATCH a different marker on the leaf FILE. +# f. COPY outer/ → outer-copy/ (Depth: infinity). +# g. PROPFIND descendant in copy; marker present. +# h. PROPFIND leaf in copy; marker present. +# i. Cleanup both trees. +# ───────────────────────────────────────────────────────────── + +# Step 14a/b/c — build the source subtree +MKCOL {{base_url}}/webdav/dead-props-copy-tree/ +Authorization: Bearer {{token}} + +HTTP 201 + + +MKCOL {{base_url}}/webdav/dead-props-copy-tree/inner/ +Authorization: Bearer {{token}} + +HTTP 201 + + +PUT {{base_url}}/webdav/dead-props-copy-tree/inner/leaf.txt +Authorization: Bearer {{token}} +Content-Type: text/plain +``` +leaf inside the copy tree +``` + +HTTP 201 + + +# Step 14d — marker on the descendant FOLDER +PROPPATCH {{base_url}}/webdav/dead-props-copy-tree/inner/ +Authorization: Bearer {{token}} +Content-Type: application/xml; charset=utf-8 +``` + + + + + inner-folder-mark + + + +``` + +HTTP 207 + + +# Step 14e — marker on the leaf FILE +PROPPATCH {{base_url}}/webdav/dead-props-copy-tree/inner/leaf.txt +Authorization: Bearer {{token}} +Content-Type: application/xml; charset=utf-8 +``` + + + + + leaf-file-mark + + + +``` + +HTTP 207 + + +# Step 14f — recursive COPY (Depth: infinity is the default for +# collections per RFC 4918 §9.8.3). Destination is fresh → 201. +COPY {{base_url}}/webdav/dead-props-copy-tree/ +Authorization: Bearer {{token}} +Destination: {{base_url}}/webdav/dead-props-copy-tree-clone/ + +HTTP 201 + + +# Step 14g — descendant folder in the COPY carries the folder marker. +# The path resolves only if `storage.copy_folder_tree` correctly +# duplicated the descendant folder AND its dead-prop row. +PROPFIND {{base_url}}/webdav/dead-props-copy-tree-clone/inner/ +Authorization: Bearer {{token}} +Depth: 0 +Content-Type: application/xml; charset=utf-8 +``` + + + + +``` + +HTTP 207 +[Asserts] +xpath "string(//*[local-name()='innermark'])" == "inner-folder-mark" + + +# Step 14h — leaf file in the COPY carries the file marker +PROPFIND {{base_url}}/webdav/dead-props-copy-tree-clone/inner/leaf.txt +Authorization: Bearer {{token}} +Depth: 0 +Content-Type: application/xml; charset=utf-8 +``` + + + + +``` + +HTTP 207 +[Asserts] +xpath "string(//*[local-name()='leafmark'])" == "leaf-file-mark" + + +# Step 14i — cleanup both trees. Recursive DELETE cascades each +# subtree's folder + file rows, and the FK ON DELETE CASCADE on +# webdav_dead_properties takes the dead-prop rows with them. +DELETE {{base_url}}/webdav/dead-props-copy-tree/ +Authorization: Bearer {{token}} + +HTTP 204 + + +DELETE {{base_url}}/webdav/dead-props-copy-tree-clone/ +Authorization: Bearer {{token}} + +HTTP 204