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

fix(trash): cascade soft-delete and restore across folder subtree

Browse Source 
Closes G9.

DELETE on a folder now flips is_trashed on every descendant folder
and file under it in a single CTE pipeline (lpath <@ root.lpath
covers the whole subtree via the GiST index). Previously only the
root row was flipped — descendants stayed live, directly addressable
via their full path, and confused desktop-sync tree walks that
expected the parent-collection 404 to imply the children were gone
too.

restore_from_trash mirrors the cascade: descendants where the
original_*_parent_id column is NULL are the ones we cascade-trashed,
so they get cascade-restored too. Descendants that were independently
trashed before the parent went to trash have original_*_parent_id
set, so they stay in trash and remain visible as top-level entries
in storage.trash_items.

No schema migration needed — both original_parent_id (folders) and
original_folder_id (files) were already nullable and already encoded
'where this came from when it was independently trashed'; using NULL
as the cascade-marker reuses that existing distinction cleanly.

Test G9 flipped from KNOWN BUG to assert every descendant 404s after
the parent DELETE; new G9b proves the inverse cascade by restoring
the trashed root and verifying every descendant comes back at its
original path.
This commit is contained in:
Edouard Vanbelle
2026-06-16 23:35:23 +02:00
parent 5e1c99e227
commit e2702ac673
2 changed files with 114 additions and 29 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/infrastructure/repositories/pg/folder_db_repository.rs
+72 -14
View File
@@ -581,23 +581,55 @@ impl FolderRepository for FolderDbRepository {
// ── Trash operations ──
async fn move_to_trash(&self, folder_id: &str) -> Result<(), DomainError> {
// Only mark the folder itself as trashed.
// Child files and sub-folders are implicitly hidden because their
// ancestor is trashed — list queries already filter NOT is_trashed,
// and folder navigation won't reach a trashed folder's children.
// Soft-delete the whole subtree in one statement: the root flips
// `is_trashed` and records `original_parent_id` so restore knows
// where to put it back; every descendant (folder or file) that
// wasn't already in trash flips `is_trashed` too but leaves the
// `original_*` column NULL. That NULL is the marker the restore
// path uses to tell "cascade-trashed with the root" from
// "independently trashed earlier" — the latter must stay in
// trash even when the root is restored.
//
// Without this cascade, descendants used to remain `is_trashed = false`
// and stay directly addressable by their full path (PROPFIND on
// `/g9-tree/file.txt` still resolved 207 even though the parent
// collection was gone) — a class of data-integrity drift that
// confused desktop-sync tree walks.
let result = retry_on_deadlock("folders.trash", || {
sqlx::query_scalar::<_, i64>(
r#"
WITH trash_folder AS (
WITH trash_root AS (
UPDATE storage.folders
SET is_trashed = TRUE,
trashed_at = NOW(),
original_parent_id = parent_id,
updated_at = NOW()
WHERE id = $1::uuid AND NOT is_trashed
RETURNING id, lpath
),
trash_descendant_folders AS (
UPDATE storage.folders f
SET is_trashed = TRUE,
trashed_at = NOW(),
updated_at = NOW()
FROM trash_root tr
WHERE f.lpath <@ tr.lpath
AND f.id != tr.id
AND NOT f.is_trashed
RETURNING 1
),
trash_descendant_files AS (
UPDATE storage.files fi
SET is_trashed = TRUE,
trashed_at = NOW(),
updated_at = NOW()
FROM trash_root tr
JOIN storage.folders f ON f.lpath <@ tr.lpath
WHERE fi.folder_id = f.id
AND NOT fi.is_trashed
RETURNING 1
)
SELECT COUNT(*) FROM trash_folder
SELECT COUNT(*) FROM trash_root
"#,
)
.bind(folder_id)
@@ -618,16 +650,18 @@ impl FolderRepository for FolderDbRepository {
folder_id: &str,
_original_path: &str,
) -> Result<(), DomainError> {
// Only restore the folder itself.
// Child files were never marked as trashed — they become visible
// again automatically once their parent folder is un-trashed.
// The BEFORE UPDATE trigger recomputes path/lpath when
// original_parent_id is restored; the cascade trigger
// batch-updates all descendants via the GiST lpath index.
// Inverse of the cascade in `move_to_trash`: restore the root
// (BEFORE UPDATE trigger recomputes path/lpath via the parent_id
// change), then un-trash every descendant whose `original_*`
// column is NULL — those are the rows we cascade-trashed
// ourselves. Descendants that were independently trashed
// *before* this folder went to trash have `original_*` set, so
// they correctly stay in trash and continue to show up as
// top-level trash entries via `storage.trash_items`.
let result = retry_on_deadlock("folders.restore", || {
sqlx::query_scalar::<_, i64>(
r#"
WITH restore_folder AS (
WITH restore_root AS (
UPDATE storage.folders
SET is_trashed = FALSE,
trashed_at = NULL,
@@ -635,9 +669,33 @@ impl FolderRepository for FolderDbRepository {
original_parent_id = NULL,
updated_at = NOW()
WHERE id = $1::uuid AND is_trashed
RETURNING id, lpath
),
restore_descendant_folders AS (
UPDATE storage.folders f
SET is_trashed = FALSE,
trashed_at = NULL,
updated_at = NOW()
FROM restore_root rr
WHERE f.lpath <@ rr.lpath
AND f.id != rr.id
AND f.is_trashed
AND f.original_parent_id IS NULL
RETURNING 1
),
restore_descendant_files AS (
UPDATE storage.files fi
SET is_trashed = FALSE,
trashed_at = NULL,
updated_at = NOW()
FROM restore_root rr
JOIN storage.folders f ON f.lpath <@ rr.lpath
WHERE fi.folder_id = f.id
AND fi.is_trashed
AND fi.original_folder_id IS NULL
RETURNING 1
)
SELECT COUNT(*) FROM restore_folder
SELECT COUNT(*) FROM restore_root
"#,
)
.bind(folder_id)
tests/webdav/test_nc_move_copy_delete_trash.sh
+42 -15
View File
@@ -256,7 +256,7 @@ pass "G8: DELETE → 204 + GET 404"
# descendant assertions below will trip and you can flip them
# to strict 404.
# ─────────────────────────────────────────────────────────────
echo " G9: DELETE folder (pinned: descendants currently orphan — KNOWN BUG)"
echo " G9: DELETE folder cascades soft-delete to descendants"
nc_curl -o /dev/null -X MKCOL "$NC_FILES_BASE/g9-tree/" > /dev/null
nc_curl -o /dev/null -X MKCOL "$NC_FILES_BASE/g9-tree/inner/" > /dev/null
put_nc_file "g9-tree/file.txt" "G9 file"
@@ -265,22 +265,49 @@ STATUS=$(nc_curl -o /dev/null -w "%{http_code}" -X DELETE "$NC_FILES_BASE/g9-tre
[[ "$STATUS" == "204" ]] \
|| fail "G9: folder DELETE expected 204, got $STATUS"
# Folder itself: correctly 404.
# Folder itself: 404.
[[ "$(nc_status_propfind_depth0 "$NC_FILES_BASE/g9-tree/")" == "404" ]] \
|| fail "G9: folder still present after DELETE — that part should always be 404"
|| fail "G9: folder still resolvable after DELETE"
# Descendants: pin the current (buggy) "still alive" status.
# Either current 207 (bug) or future 404 (fix) is acceptable;
# anything else means something has drifted unexpectedly.
CHILD_STATUS=$(nc_status_propfind_depth0 "$NC_FILES_BASE/g9-tree/file.txt")
DEEP_STATUS=$(nc_status_propfind_depth0 "$NC_FILES_BASE/g9-tree/inner/deep.txt")
if [[ "$CHILD_STATUS" == "207" && "$DEEP_STATUS" == "207" ]]; then
pass "G9: descendants still reachable (file=207, deep=207) — KNOWN BUG pinned: move_to_trash isn't recursive at the row level"
elif [[ "$CHILD_STATUS" == "404" && "$DEEP_STATUS" == "404" ]]; then
fail "G9: descendants now correctly 404 (file=$CHILD_STATUS, deep=$DEEP_STATUS) — bug is fixed, flip this case to strict 404 assertions."
else
fail "G9: mixed/unexpected descendant statuses (file=$CHILD_STATUS, deep=$DEEP_STATUS) — pin needs review"
fi
# Descendants must now also be 404 (cascade soft-delete reaches the
# whole subtree). Previous behaviour left them reachable at their
# full path while the parent was gone — a data-integrity drift that
# confused desktop-sync tree walks.
[[ "$(nc_status_propfind_depth0 "$NC_FILES_BASE/g9-tree/file.txt")" == "404" ]] \
|| fail "G9: direct-child file still resolvable after parent DELETE — cascade not working"
[[ "$(nc_status_propfind_depth0 "$NC_FILES_BASE/g9-tree/inner/")" == "404" ]] \
|| fail "G9: descendant folder still resolvable after parent DELETE — cascade not working"
[[ "$(nc_status_propfind_depth0 "$NC_FILES_BASE/g9-tree/inner/deep.txt")" == "404" ]] \
|| fail "G9: descendant file still resolvable after parent DELETE — cascade not working"
pass "G9: DELETE folder → 204, descendants all 404 (cascade reaches the whole subtree)"
# G9b — restore the trashed root and verify cascade-restore brings
# every descendant back with the same paths. Cascade-trashed
# descendants (original_parent_id IS NULL) get un-trashed; rows that
# were independently trashed before the folder went to trash stay
# trashed.
echo " G9b: restore the trashed g9-tree → cascade-restore reaches descendants"
BODY=$(nc_curl -X PROPFIND -H "Depth: 1" "$NC_TRASH_BASE/")
G9_TRASHED_HREF=$(extract_response_href_containing "$BODY" "g9-tree")
G9_TRASHED_ID=$(basename "$G9_TRASHED_HREF")
[[ -n "$G9_TRASHED_ID" ]] || fail "G9b: trashed g9-tree not found via PROPFIND"
STATUS=$(nc_curl -o /dev/null -w "%{http_code}" -X MOVE \
-H "Destination: $NC_FILES_BASE/g9-tree-restored/" \
"$NC_TRASH_BASE/$G9_TRASHED_ID")
[[ "$STATUS" == "201" || "$STATUS" == "204" ]] \
|| fail "G9b: restore expected 201/204, got $STATUS"
# The folder and ALL its descendants are reachable again at their
# original paths (restore goes to original location, not the
# Destination header).
[[ "$(nc_status_propfind_depth0 "$NC_FILES_BASE/g9-tree/")" == "207" ]] \
|| fail "G9b: root folder not back after restore"
[[ "$(nc_status_propfind_depth0 "$NC_FILES_BASE/g9-tree/file.txt")" == "207" ]] \
|| fail "G9b: direct-child file not restored alongside parent"
[[ "$(nc_status_propfind_depth0 "$NC_FILES_BASE/g9-tree/inner/")" == "207" ]] \
|| fail "G9b: descendant folder not restored alongside parent"
[[ "$(nc_status_propfind_depth0 "$NC_FILES_BASE/g9-tree/inner/deep.txt")" == "207" ]] \
|| fail "G9b: descendant file not restored alongside parent"
pass "G9b: restored g9-tree carries the whole subtree back"
# ═════════════════════════════════════════════════════════════
# Group K — Trashbin DAV (depends on G8's deletion above)