cjw/Oxicloud
1
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

feat(dead-props): ensure replication on copy

Browse Source
This commit is contained in:
Edouard Vanbelle
2026-06-30 23:44:42 +02:00
parent cb7b653a15
commit d8adae3572
3 changed files with 509 additions and 5 deletions
Download Patch File Download Diff File Expand all files Collapse all files
migrations/20260830000002_copy_dead_properties_on_folder_tree.sql
+223
View File
@@ -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;
src/infrastructure/repositories/pg/file_blob_write_repository.rs
+23 -4
View File
@@ -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)
tests/api/webdav_dead_properties.hurl
+263 -1
View File
@@ -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
```
<?xml version="1.0" encoding="utf-8"?>
<D:propertyupdate xmlns:D="DAV:">
<D:set>
<D:prop>
<X:copymark xmlns:X="oxi:test">survives-copy</X:copymark>
</D:prop>
</D:set>
</D:propertyupdate>
```
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
```
<?xml version="1.0" encoding="utf-8"?>
<D:propfind xmlns:D="DAV:">
<D:allprop/>
</D:propfind>
```
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
```
<?xml version="1.0" encoding="utf-8"?>
<D:propfind xmlns:D="DAV:">
<D:allprop/>
</D:propfind>
```
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
```
<?xml version="1.0" encoding="utf-8"?>
<D:propertyupdate xmlns:D="DAV:">
<D:set>
<D:prop>
<X:innermark xmlns:X="oxi:test">inner-folder-mark</X:innermark>
</D:prop>
</D:set>
</D:propertyupdate>
```
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
```
<?xml version="1.0" encoding="utf-8"?>
<D:propertyupdate xmlns:D="DAV:">
<D:set>
<D:prop>
<X:leafmark xmlns:X="oxi:test">leaf-file-mark</X:leafmark>
</D:prop>
</D:set>
</D:propertyupdate>
```
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
```
<?xml version="1.0" encoding="utf-8"?>
<D:propfind xmlns:D="DAV:">
<D:allprop/>
</D:propfind>
```
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
```
<?xml version="1.0" encoding="utf-8"?>
<D:propfind xmlns:D="DAV:">
<D:allprop/>
</D:propfind>
```
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