Merge pull request #539 from EdouardVanbelle/feat/metadata-binded-to-resource-id
This commit is contained in:
@@ -0,0 +1,364 @@
|
||||
# Plan — Expose dead properties as a REST metadata API
|
||||
|
||||
## Context
|
||||
|
||||
Migration `20260830000001` rekeyed `storage.webdav_dead_properties` from
|
||||
`(resource_path, user_id)` to a polymorphic resource id
|
||||
(`folder_id` XOR `file_id`) with `ON DELETE CASCADE`. The table is now a
|
||||
clean per-resource key-value store: `(resource id, namespace, local_name) → value`,
|
||||
shaped exactly like a generic metadata layer.
|
||||
|
||||
Today only WebDAV (PROPPATCH / PROPFIND) reads and writes it. NextCloud DAV
|
||||
sees the rows (id-keyed, no path coupling) but isn't yet wired up to
|
||||
emit/consume them. No REST surface exists.
|
||||
|
||||
We discussed the question "any interest in exposing this as a REST
|
||||
metadata API?" on **2026-06-30** during the rekey landing and agreed it's
|
||||
worth a follow-up plan but not part of the rekey itself. This document
|
||||
captures the design we sketched so we can pick it up without
|
||||
re-litigating.
|
||||
|
||||
## Why this is worth doing now (and not before the rekey)
|
||||
|
||||
| Pre-rekey schema | Post-rekey schema |
|
||||
|---|---|
|
||||
| `(resource_path, user_id)` key | `(folder_id XOR file_id, namespace, local_name)` key |
|
||||
| Path-keyed → invalidated on rename / move | Id-keyed → stable across rename / move (DB invariant) |
|
||||
| User-siloed → wrong for shared drives | Resource-state — correct under D1+ shared-drive semantics |
|
||||
| Service-layer deletes leak tombstones | FK `ON DELETE CASCADE` reaps on every delete code path |
|
||||
|
||||
Pre-rekey, exposing the store via REST would have been wrong: REST clients
|
||||
operate on resource ids, but the store keyed on paths; cross-protocol
|
||||
parity would have been a mess. Post-rekey, the store IS already shaped
|
||||
like the API we'd want — a thin REST layer matches it 1:1.
|
||||
|
||||
## Use cases
|
||||
|
||||
| Use case | What it looks like | Why dead-props help |
|
||||
|---|---|---|
|
||||
| Photo annotations | captions, ratings (1-5), notes per photo | already keyed by `file_id`; round-trips via WebDAV without re-implementing |
|
||||
| Web-UI tags / labels | `oxi:user:tag/project=alpha`, color flags, "archived" markers | per-resource user metadata without new tables |
|
||||
| Folder UI preferences | default sort, default view mode, "favourite" flag | persistent per-folder, shared across users on shared drives |
|
||||
| Cross-protocol bridge | Thunderbird sets `oxi:lastsync=...` via PROPPATCH → web UI reads it via REST | one store, two surfaces — visibility goes both ways |
|
||||
| Workflow / approval state | `reviewed_by=alice`, `due=2026-09-15` | ad-hoc state per resource without schema sprawl |
|
||||
| Third-party integrations | external apps store scratch space per resource | lower barrier than implementing WebDAV |
|
||||
|
||||
Each use case is the same store; only the values differ. That's why
|
||||
exposing it as a generic API is more leverage than adding ad-hoc columns
|
||||
for any one of them.
|
||||
|
||||
## API shape (decided)
|
||||
|
||||
### Per-resource CRUD — nested under the resource
|
||||
|
||||
```
|
||||
GET /api/files/{id}/metadata → list all keys
|
||||
GET /api/files/{id}/metadata/{namespace}/{name} → fetch one value
|
||||
PUT /api/files/{id}/metadata/{namespace}/{name} → upsert (body = value)
|
||||
DELETE /api/files/{id}/metadata/{namespace}/{name} → remove one key
|
||||
|
||||
GET /api/folders/{id}/metadata → list all keys
|
||||
GET /api/folders/{id}/metadata/{namespace}/{name} → fetch one value
|
||||
PUT /api/folders/{id}/metadata/{namespace}/{name} → upsert
|
||||
DELETE /api/folders/{id}/metadata/{namespace}/{name} → remove one key
|
||||
```
|
||||
|
||||
The `{kind}` is encoded in the URL prefix, so we don't carry a
|
||||
discriminator field. `{namespace}` and `{name}` are passed verbatim to
|
||||
the store; URL-encode the colon-containing namespaces
|
||||
(`oxi:user:tag` → `oxi%3Auser%3Atag`).
|
||||
|
||||
This shape matches the rest of the API — `/api/files/{id}/thumbnail`,
|
||||
`/api/files/{id}/preview`, `/api/folders/{id}/contents` — and stays
|
||||
discoverable as a sub-resource of the file/folder.
|
||||
|
||||
AuthZ goes through the `_with_perms` service path:
|
||||
- `Read` on the resource → GET allowed.
|
||||
- `Update` on the resource → PUT / DELETE allowed.
|
||||
- 404 on no-Read (anti-enumeration), 403 on Read-but-no-Update.
|
||||
|
||||
GET (list) response shape:
|
||||
|
||||
```json
|
||||
{
|
||||
"properties": [
|
||||
{
|
||||
"namespace": "oxi:user:tag",
|
||||
"name": "project",
|
||||
"value": "alpha",
|
||||
"updated_by": "<user-uuid-or-null>",
|
||||
"updated_at": "2026-06-30T20:33:38Z"
|
||||
},
|
||||
...
|
||||
]
|
||||
}
|
||||
```
|
||||
|
||||
The resource itself is identified by the URL — no need to echo
|
||||
`{ "kind": ..., "id": ... }` in the body.
|
||||
|
||||
### Cross-resource lookup — separate search endpoint (deferred to phase 3)
|
||||
|
||||
Cross-resource search ("which files have `oxi:user:tag/project=alpha`?")
|
||||
is a fundamentally different operation from CRUD — it's a SEARCH, not a
|
||||
fetch. Nesting it under a single resource URL would be wrong, and
|
||||
overloading CRUD with `?filter=...` would muddy the shape. It lives at
|
||||
its own endpoint:
|
||||
|
||||
```
|
||||
GET /api/search/metadata?namespace=...&name=...&value=...&kind=file
|
||||
```
|
||||
|
||||
This separation has three concrete payoffs:
|
||||
|
||||
- **CRUD path stays simple**: per-resource fetch/upsert/remove with no
|
||||
query-string filter logic.
|
||||
- **AuthZ shape differs**: per-resource CRUD enforces permissions on
|
||||
ONE resource; search must enumerate every resource the caller can
|
||||
Read, then filter. That's expensive enough to need its own
|
||||
rate-limit / pagination story. Isolating it keeps the CRUD path
|
||||
cheap.
|
||||
- **Search can grow** more filter syntax (multiple keys, value
|
||||
patterns, `>` / `<` comparisons) without touching the CRUD shape.
|
||||
|
||||
Search is **phase 3** — it's not required for the read-only or
|
||||
read-write cases (phases 1 and 2). Don't build it until a UI feature
|
||||
asks for it.
|
||||
|
||||
Note this is the LOW-VOLUME lookup option. Genuine tag-based faceted
|
||||
browse at scale needs a first-class tags table with indexes — not a
|
||||
metadata-table scan. The search endpoint exists for debugging, small
|
||||
instances, and occasional one-off queries. See "Out of scope" below.
|
||||
|
||||
## Schema additions needed
|
||||
|
||||
```sql
|
||||
ALTER TABLE storage.webdav_dead_properties
|
||||
ADD COLUMN updated_by UUID NULL REFERENCES auth.users(id) ON DELETE SET NULL;
|
||||
```
|
||||
|
||||
The `updated_at` column already exists. `updated_by` is the new bit —
|
||||
load-bearing if both WebDAV and REST are writing. Without it, "why did
|
||||
this caption change overnight?" is blind.
|
||||
|
||||
Set on every `set()` / `remove()` (the latter currently has no provenance
|
||||
concept, but the audit value would be "who reaped it" — same column).
|
||||
`ON DELETE SET NULL` so a user delete doesn't lose the property itself,
|
||||
only the authorship — symmetric with how other audit columns in the
|
||||
schema behave (`created_by` on folders/files is `ON DELETE SET NULL` for
|
||||
the same reason).
|
||||
|
||||
## Decisions to lock in before implementation
|
||||
|
||||
### 1. Namespace policy — denylist or allowlist?
|
||||
|
||||
Server-managed namespaces (`DAV:`, anything we want to use internally for
|
||||
sync state, locks, etc.) should be REST-write-rejected so REST can't
|
||||
poison live WebDAV behaviour.
|
||||
|
||||
**Recommendation: denylist.** More permissive, less surprising, matches
|
||||
the WebDAV side (which lets clients write any namespace they please).
|
||||
Initial denylist:
|
||||
|
||||
- `DAV:` — RFC 4918 live properties; server-managed.
|
||||
- `oxi:internal:*` — reserved for future server-managed properties.
|
||||
|
||||
REST read is unrestricted; only write is filtered.
|
||||
|
||||
### 2. Size limits
|
||||
|
||||
Today no cap. WebDAV is bounded by `MAX_XML_BODY` (1 MB) on the request,
|
||||
but per-row there's no limit and no per-resource key-count limit. A
|
||||
REST API in the wild needs both:
|
||||
|
||||
- per-value: **64 KB** (enough for any human-authored caption, JSON blob,
|
||||
or sync token; rejects "use the metadata table as a file store"
|
||||
abuse).
|
||||
- per-resource: **100 keys** (enough for any reasonable application;
|
||||
rejects "use it as a directory listing").
|
||||
|
||||
Both as 413 Payload Too Large on the offending endpoint.
|
||||
|
||||
WebDAV PROPPATCH should adopt the same per-key limit (currently bounded
|
||||
only by the 1 MB body); per-resource count limit applies on the
|
||||
incremental write.
|
||||
|
||||
### 3. Value content type
|
||||
|
||||
Stored as `TEXT` today. If REST PUTs JSON, WebDAV clients reading it
|
||||
back via PROPFIND wrap it in their XML envelope and see `"{...}"` as a
|
||||
literal string. Defensible (it's "just a string"); document the
|
||||
convention.
|
||||
|
||||
If we add a `content_type` column (RFC 4918 §15.5 `getcontenttype` on
|
||||
properties is murky), REST can return the original `Content-Type` to
|
||||
REST callers and WebDAV continues to see the literal value. Probably
|
||||
**not worth it** until a real use case needs it — adds a column + a
|
||||
write path branch for zero functional benefit today.
|
||||
|
||||
### 4. Listing semantics
|
||||
|
||||
Inlining child metadata into `GET /api/folders/{id}/contents` is
|
||||
tempting — fewer round-trips for the UI — but PROPFIND already pays this
|
||||
O(N) cost and it's expensive on big folders.
|
||||
|
||||
**Recommendation: dedicated endpoint only.** No inlining. UIs that need
|
||||
per-child metadata can batch via `GET /api/files/{id}/metadata` calls
|
||||
in parallel (HTTP/2 multiplexing makes that cheap) until measured
|
||||
demand justifies a bulk endpoint.
|
||||
|
||||
### 5. WebDAV-write hygiene
|
||||
|
||||
REST writes go through the same `DeadPropertyStore::set` as PROPPATCH —
|
||||
no special branch. The denylist (above) gates which namespaces REST may
|
||||
write; WebDAV stays unrestricted.
|
||||
|
||||
## Scope: phase 1 / 2 / 3
|
||||
|
||||
### Phase 1 (read-only)
|
||||
|
||||
GET endpoints only, nested under `/api/{files,folders}/{id}/metadata`:
|
||||
|
||||
- `GET /api/files/{id}/metadata` → list
|
||||
- `GET /api/files/{id}/metadata/{namespace}/{name}` → single
|
||||
- `GET /api/folders/{id}/metadata` → list
|
||||
- `GET /api/folders/{id}/metadata/{namespace}/{name}` → single
|
||||
|
||||
Plus the `updated_by` schema migration (so phase 2 doesn't break wire
|
||||
contracts).
|
||||
|
||||
Use case unlocked: the SvelteKit UI can READ properties Thunderbird /
|
||||
DAVx5 / Cyberduck have written. Cross-protocol visibility, one
|
||||
direction.
|
||||
|
||||
Cost: ~150 LOC handler + ~20 LOC migration. No new authz primitives —
|
||||
`Read` permission already exists.
|
||||
|
||||
### Phase 2 (write)
|
||||
|
||||
PUT + DELETE. Namespace denylist. Size limits.
|
||||
|
||||
Decide first what the primary REST writer is:
|
||||
|
||||
- **Photo captions**: probably wants a dedicated `/api/photos/{id}/caption`
|
||||
endpoint that stores under a fixed `oxi:photo:caption` key. Generic
|
||||
API still useful but not the obvious surface.
|
||||
- **Tags**: deserves a structured tags table (queryable, faceted search)
|
||||
rather than k/v. Generic API is the wrong shape.
|
||||
- **Workflow state**: generic API IS the right shape — exactly what k/v
|
||||
was designed for.
|
||||
- **Third-party integrations**: generic API is the right shape.
|
||||
|
||||
If the primary writer turns out to be one of the structured-data cases,
|
||||
phase 2 may never ship — the read API plus a dedicated write endpoint
|
||||
per feature is the better factoring. The decision should be driven by
|
||||
real demand, not speculative design.
|
||||
|
||||
### Phase 3 (cross-resource search)
|
||||
|
||||
`GET /api/search/metadata?namespace=...&name=...&value=...&kind=file`
|
||||
with pagination. AuthZ enumerates resources the caller has Read on
|
||||
and filters in-engine.
|
||||
|
||||
Scope guidance:
|
||||
|
||||
- Low-volume **only**. Implemented as a sequential scan with
|
||||
permission filter. No new indexes (the (`namespace`, `local_name`,
|
||||
`value`) shape doesn't index cheaply, and adding `value` to a
|
||||
composite index changes the table's write pattern).
|
||||
- Filter syntax stays minimal until a UI feature drives it. Start
|
||||
with `namespace=` and `name=`; add `value=` literal match next; add
|
||||
`value~=` pattern match only when needed.
|
||||
- Hard rate-limit per caller — search is expensive enough that an
|
||||
unbounded REST client could starve the database.
|
||||
|
||||
If demand for tag-based browse at scale ever materialises, do NOT
|
||||
extend this endpoint — build a dedicated tags table with an inverted
|
||||
index. Search-on-metadata is the debug/scratch tool, not the
|
||||
production tag system.
|
||||
|
||||
## Out of scope
|
||||
|
||||
- **Tags / faceted search** — deserves a first-class table with foreign
|
||||
keys and a search index. The metadata API can hold tags as values,
|
||||
but querying "all files tagged `alpha`" via the metadata table is a
|
||||
full scan. Don't build features that need indexed tag queries on top
|
||||
of this.
|
||||
- **Live properties** (RFC 4918 §15) — `getcontentlength`,
|
||||
`creationdate`, `getetag`, etc. are server-computed. The REST API
|
||||
exposes only dead properties; live properties are derived from the
|
||||
resource and surface through their existing endpoints.
|
||||
- **Bulk write** — phase 2 could add a batch endpoint if needed, but
|
||||
initial design ships one-at-a-time. Bulk read (list) is already
|
||||
there.
|
||||
- **Versioning / history** — `updated_at` + `updated_by` give an audit
|
||||
timestamp but not history. If someone wants "show me the previous
|
||||
caption", that's a separate append-only journal table.
|
||||
- **WebDAV PROPPATCH size enforcement** mentioned above as "should
|
||||
adopt the same limit" — actually applying the limit to PROPPATCH is
|
||||
a separate small change, deferable.
|
||||
|
||||
## Open questions left for implementation
|
||||
|
||||
1. **Sub-resource name** — `/metadata` matches REST conventions and
|
||||
reads naturally to API consumers; `/properties` would be more
|
||||
WebDAV-faithful but users don't know what "dead properties" means.
|
||||
Locked in: `/metadata`.
|
||||
2. **Permission for empty list** — GET on a resource with zero metadata
|
||||
returns `{ properties: [] }` and 200, OR 404? RFC 4918 §9.1 says
|
||||
PROPFIND on a resource that exists but has no requested properties
|
||||
is 207 with empty `<D:prop>`. REST should mirror: 200 with empty
|
||||
array. 404 only when the underlying resource doesn't exist (or
|
||||
anti-enum 404 on no-Read).
|
||||
3. **Listing order** — alphabetical by `(namespace, name)`, or by
|
||||
`updated_at DESC`? Probably the former (stable, deterministic).
|
||||
4. **Caching** — `ETag` on the list response? Cheap if the underlying
|
||||
resource already emits one; we could derive a sub-ETag from
|
||||
`MAX(updated_at)` across the metadata rows. Defer until UI asks.
|
||||
5. **NC DAV wiring** (see memory note
|
||||
`project-nc-webdav-dead-props-unwired.md`) — the read API would
|
||||
work over the unwired NC surface trivially since REST and NC DAV
|
||||
read the same store; the unwired bit is just PROPPATCH/PROPFIND on
|
||||
the NC URL prefix. Worth doing alongside phase 1 read so the cross-
|
||||
protocol story is complete on day one.
|
||||
6. **Search endpoint shape** (phase 3, not phase 1) — the cross-
|
||||
resource lookup `/api/search/metadata?...` will need pagination,
|
||||
sort, and resource-kind filter; design when the first concrete
|
||||
use case lands. Don't speculatively build it.
|
||||
|
||||
## Verification (phase 1)
|
||||
|
||||
1. Migration adds `updated_by`; downgrade leaves data intact (no
|
||||
`DROP COLUMN` on rollback — just leave it; harmless).
|
||||
2. `cargo check` green; `cargo clippy --all-features --all-targets -D
|
||||
warnings` green.
|
||||
3. New Hurl scenario `tests/api/metadata_api.hurl`:
|
||||
- PUT a file via WebDAV, PROPPATCH a marker → 207.
|
||||
- GET `/api/files/{id}/metadata/oxi%3Atest/marker` → 200 with the
|
||||
value.
|
||||
- GET `/api/files/{id}/metadata` → 200 with list containing the
|
||||
marker.
|
||||
- Asserts `updated_by` field is the authenticated user id.
|
||||
- GET against a foreign user's file → 404 (anti-enum).
|
||||
- GET `/api/folders/{id}/metadata` round-trip on a folder PROPPATCH.
|
||||
4. The existing `webdav_dead_properties.hurl` continues to pass —
|
||||
read-only REST shouldn't perturb any existing write path.
|
||||
|
||||
## Verification (phase 2)
|
||||
|
||||
To be expanded when phase 2 starts. At minimum:
|
||||
|
||||
- PUT writes round-trip via PROPFIND.
|
||||
- PUT to denylisted namespace → 403.
|
||||
- PUT exceeding per-value cap → 413.
|
||||
- PUT exceeding per-resource cap → 413.
|
||||
- DELETE removes via PROPFIND verification.
|
||||
- PUT/DELETE without Update permission → 403.
|
||||
|
||||
## Memory notes to write when this lands
|
||||
|
||||
- `project_metadata_api.md` — phase shipped, what's still open.
|
||||
- Update `project_webdav_dead_properties_drive_rekey.md` to mention
|
||||
this API as the consumer that justified the id-rekey effort.
|
||||
- If the namespace denylist is contentious, capture it as
|
||||
`feedback_metadata_namespace_denylist.md`.
|
||||
@@ -172,9 +172,17 @@ front-design:
|
||||
# .github/workflows/ci.yml; keep the order in sync so a local pass means
|
||||
# CI passes.
|
||||
api-test:
|
||||
bash tests/api/run.sh
|
||||
bash tests/webdav/run.sh
|
||||
bash tests/oidc/run.sh
|
||||
#!/usr/bin/env bash
|
||||
set -euo pipefail
|
||||
./tests/api/run.sh
|
||||
./tests/webdav/run.sh
|
||||
./tests/oidc/run.sh
|
||||
if which litmus >/dev/null 2>/dev/null
|
||||
then
|
||||
./tests/webdav/run-litmus.sh
|
||||
else
|
||||
echo "XXX litmus webdav not found, ignore test"
|
||||
fi
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# SvelteKit frontend (frontend/) — the only frontend. These `fe-*` recipes
|
||||
|
||||
@@ -0,0 +1,112 @@
|
||||
-- ════════════════════════════════════════════════════════════════════════════
|
||||
-- WebDAV dead properties: rekey from (resource_path, user_id) to resource id
|
||||
-- ════════════════════════════════════════════════════════════════════════════
|
||||
-- The original schema (20260825000000) keyed dead properties on
|
||||
-- `(resource_path, user_id, namespace, local_name)`. That model was wrong on
|
||||
-- two counts:
|
||||
--
|
||||
-- 1. Dead properties are RESOURCE state per RFC 4918 §4.2 — not user
|
||||
-- state. Two users on a shared drive PROPFIND'ing the same resource
|
||||
-- must see the same dead-properties. The user_id key siloed them.
|
||||
-- 2. Every non-WebDAV delete path (REST `DELETE /api/files/{id}`, bulk
|
||||
-- delete, trash empty, folder cascade) operates on a resource id —
|
||||
-- not a path. None of those code paths could cheaply call
|
||||
-- `remove_resource(path, user_id)`, so they leaked dead-property
|
||||
-- tombstones. WebDAV DELETE itself had a workaround explicit-cleanup
|
||||
-- call, but the REST surface (which the SvelteKit web UI uses) is the
|
||||
-- dominant delete path in practice.
|
||||
--
|
||||
-- This migration switches the key to a polymorphic resource reference:
|
||||
-- exactly one of `folder_id` / `file_id` is set, each with `ON DELETE
|
||||
-- CASCADE` to its owning table. After this lands every existing
|
||||
-- delete code path — REST, WebDAV, NextCloud DAV, trash, folder
|
||||
-- cascade — automatically reaps dead-property rows when the underlying
|
||||
-- file or folder is removed, with no service-layer changes.
|
||||
--
|
||||
-- MOVE / RENAME also become no-ops at the dead-properties layer: a
|
||||
-- folder's id is stable across renames, so its dead properties move
|
||||
-- with it for free. The `rename_resource()` method on the store is
|
||||
-- removed in the matching Rust change.
|
||||
--
|
||||
-- ── Migration shape ─────────────────────────────────────────────────────────
|
||||
-- 1. ADD COLUMN folder_id / file_id (NULL-able for now).
|
||||
-- 2. Backfill folder_id from any row whose resource_path matches a
|
||||
-- folder row's `path` + `user_id`.
|
||||
-- 3. Backfill file_id for the rest by joining through the parent folder
|
||||
-- and matching `parent.path || '/' || fi.name`.
|
||||
-- 4. Reap rows that didn't resolve — they're tombstones from before
|
||||
-- the FK-cascade fix, and there's no resource left to attach them to.
|
||||
-- 5. Add the CHECK constraint that exactly one column is set.
|
||||
-- 6. Add two partial unique indexes (one per kind).
|
||||
-- 7. DROP the old columns; PG drops the inline UNIQUE constraint and
|
||||
-- the explicit path/user index along with them.
|
||||
--
|
||||
-- The migration runs in a single sqlx transaction. If any step fails
|
||||
-- the schema rolls back to (20260825000000) intact.
|
||||
|
||||
ALTER TABLE storage.webdav_dead_properties
|
||||
ADD COLUMN folder_id UUID NULL REFERENCES storage.folders(id) ON DELETE CASCADE,
|
||||
ADD COLUMN file_id UUID NULL REFERENCES storage.files(id) ON DELETE CASCADE;
|
||||
|
||||
-- Backfill: every row whose resource_path matches an existing folder
|
||||
-- row's `path` + `user_id` gets its folder_id stamped. `NOT is_trashed`
|
||||
-- mirrors what the handler does at lookup time — trashed rows can't be
|
||||
-- the live target of a PROPPATCH anyway, so any old row pointing at a
|
||||
-- trashed folder is a tombstone (handled in step 4).
|
||||
UPDATE storage.webdav_dead_properties d
|
||||
SET folder_id = fo.id
|
||||
FROM storage.folders fo
|
||||
WHERE fo.path = d.resource_path
|
||||
AND fo.user_id = d.user_id
|
||||
AND NOT fo.is_trashed;
|
||||
|
||||
-- Backfill: any remaining row must be a file's properties. Match the
|
||||
-- same path-computation the resolver uses for files —
|
||||
-- `parent.path || '/' || fi.name` — so the rewrite mirrors the
|
||||
-- handler's runtime behaviour exactly.
|
||||
UPDATE storage.webdav_dead_properties d
|
||||
SET file_id = fi.id
|
||||
FROM storage.files fi
|
||||
JOIN storage.folders parent ON parent.id = fi.folder_id
|
||||
WHERE d.folder_id IS NULL
|
||||
AND fi.user_id = d.user_id
|
||||
AND NOT fi.is_trashed
|
||||
AND parent.path || '/' || fi.name = d.resource_path;
|
||||
|
||||
-- Reap orphans. A row that didn't resolve to a folder or file is a
|
||||
-- tombstone left by some pre-fix delete path: the resource is long
|
||||
-- gone but the dead-property row was never reaped because the old
|
||||
-- `(path, user_id)` key kept it disconnected from the resource's
|
||||
-- lifecycle. The FK-cascade era makes this category structurally
|
||||
-- impossible, so dropping them on migration is the right cleanup.
|
||||
DELETE FROM storage.webdav_dead_properties
|
||||
WHERE folder_id IS NULL AND file_id IS NULL;
|
||||
|
||||
-- Exactly-one-is-set: defends against future code accidentally
|
||||
-- writing both columns or neither. `<>` between two boolean
|
||||
-- IS NULL probes is the idiomatic PG shape for XOR.
|
||||
ALTER TABLE storage.webdav_dead_properties
|
||||
ADD CONSTRAINT webdav_dead_properties_one_resource_chk
|
||||
CHECK ((folder_id IS NULL) <> (file_id IS NULL));
|
||||
|
||||
-- Partial unique indexes — one per resource kind. PG's ON CONFLICT
|
||||
-- can infer either via `(folder_id, namespace, local_name)
|
||||
-- WHERE folder_id IS NOT NULL`, matching the partial index, so
|
||||
-- upsert continues to work without quirky ON CONSTRAINT plumbing.
|
||||
CREATE UNIQUE INDEX IF NOT EXISTS idx_webdav_dead_props_folder_unique
|
||||
ON storage.webdav_dead_properties (folder_id, namespace, local_name)
|
||||
WHERE folder_id IS NOT NULL;
|
||||
|
||||
CREATE UNIQUE INDEX IF NOT EXISTS idx_webdav_dead_props_file_unique
|
||||
ON storage.webdav_dead_properties (file_id, namespace, local_name)
|
||||
WHERE file_id IS NOT NULL;
|
||||
|
||||
-- Drop the old key columns. PG cascades the auto-named inline UNIQUE
|
||||
-- constraint and the explicit `(resource_path, user_id)` lookup index
|
||||
-- along with the columns (idx is on resource_path which is going away,
|
||||
-- so CASCADE is required).
|
||||
DROP INDEX IF EXISTS storage.idx_webdav_dead_properties_path_user;
|
||||
|
||||
ALTER TABLE storage.webdav_dead_properties
|
||||
DROP COLUMN resource_path CASCADE,
|
||||
DROP COLUMN user_id CASCADE;
|
||||
@@ -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;
|
||||
@@ -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)
|
||||
|
||||
@@ -4,13 +4,35 @@
|
||||
//! server without interpreting their value. Properties are persisted to
|
||||
//! `storage.webdav_dead_properties` and survive server restarts.
|
||||
//!
|
||||
//! Queries here use `sqlx::query()` (runtime-bound) rather than the
|
||||
//! compile-time-checked `sqlx::query!()` macro. The macro would require either
|
||||
//! a live DB at compile time OR committed `.sqlx/` offline metadata; the rest
|
||||
//! of this codebase consistently uses the runtime variant (see
|
||||
//! `user_pg_repository.rs` for the canonical style), so a fresh checkout
|
||||
//! compiles without any DB connection. Trading the macro's compile-time column
|
||||
//! check for that bootstrap-friendliness is the project's standing convention.
|
||||
//! Keying contract (after migration 20260830000001): the row is keyed by
|
||||
//! the underlying resource id — exactly one of `folder_id` / `file_id` is
|
||||
//! set — not by the resource's current path. Three consequences:
|
||||
//!
|
||||
//! * Every delete code path (REST, WebDAV, NextCloud DAV, trash empty,
|
||||
//! folder cascade) reaps dead-property rows for free via FK
|
||||
//! `ON DELETE CASCADE`. The store has no `remove_resource()` method
|
||||
//! because it isn't needed: deleting the file/folder row reaps the
|
||||
//! attached dead properties as a database invariant.
|
||||
//! * MOVE / RENAME never changes the resource id, so dead properties
|
||||
//! follow the resource without any store-side bookkeeping. The store
|
||||
//! has no `rename_resource()` method for the same reason.
|
||||
//! * Dead properties are RESOURCE state (RFC 4918 §4.2), not user
|
||||
//! state. Two users on a shared drive PROPFIND'ing the same resource
|
||||
//! see the same dead properties. The `user_id` scope key from the
|
||||
//! pre-rekey schema is gone; user-delete cleanup happens
|
||||
//! transitively through `auth.users` → `storage.{folders,files}` →
|
||||
//! this table.
|
||||
//!
|
||||
//! Queries use `sqlx::query()` (runtime-bound) rather than `sqlx::query!()`
|
||||
//! to keep fresh checkouts compilable without a DB connection — the
|
||||
//! codebase's standing convention.
|
||||
//!
|
||||
//! COPY semantics (RFC 4918 §8.8 — dead properties MUST be duplicated)
|
||||
//! are NOT handled here. The COPY handler is responsible for explicitly
|
||||
//! reading the source's dead properties via `get_all()` and writing them
|
||||
//! against the new resource id via `set()`. Not done in this migration —
|
||||
//! it was not handled by the path-based store either, so this is a
|
||||
//! parity decision, not a regression.
|
||||
|
||||
use std::sync::Arc;
|
||||
|
||||
@@ -20,6 +42,19 @@ use uuid::Uuid;
|
||||
use crate::application::adapters::webdav_adapter::QualifiedName;
|
||||
use crate::domain::errors::DomainError;
|
||||
|
||||
/// Polymorphic reference to the resource a dead property hangs off.
|
||||
///
|
||||
/// Exactly one variant — folder or file — is ever stored in a single
|
||||
/// row. The CHECK constraint
|
||||
/// `(folder_id IS NULL) <> (file_id IS NULL)` enforces this at the
|
||||
/// database level so the application layer cannot accidentally write a
|
||||
/// row that's both or neither.
|
||||
#[derive(Clone, Copy, Debug)]
|
||||
pub enum ResourceRef {
|
||||
Folder(Uuid),
|
||||
File(Uuid),
|
||||
}
|
||||
|
||||
pub struct DeadPropertyStore {
|
||||
pool: Arc<PgPool>,
|
||||
}
|
||||
@@ -30,47 +65,78 @@ impl DeadPropertyStore {
|
||||
}
|
||||
|
||||
/// Upsert a dead property. `value = None` means an empty XML element.
|
||||
///
|
||||
/// The two SQL branches are deliberately kept separate so each
|
||||
/// ON CONFLICT clause can target the matching partial unique
|
||||
/// index (`idx_webdav_dead_props_folder_unique` /
|
||||
/// `idx_webdav_dead_props_file_unique`). A combined upsert would
|
||||
/// require a non-partial unique index that treats NULL as
|
||||
/// distinct, which doesn't match the (folder XOR file) shape.
|
||||
pub async fn set(
|
||||
&self,
|
||||
path: &str,
|
||||
user_id: Uuid,
|
||||
r: ResourceRef,
|
||||
name: QualifiedName,
|
||||
value: Option<String>,
|
||||
) -> Result<(), DomainError> {
|
||||
sqlx::query(
|
||||
r#"
|
||||
INSERT INTO storage.webdav_dead_properties
|
||||
(resource_path, user_id, namespace, local_name, value)
|
||||
VALUES ($1, $2, $3, $4, $5)
|
||||
ON CONFLICT (resource_path, user_id, namespace, local_name)
|
||||
DO UPDATE SET value = EXCLUDED.value, updated_at = CURRENT_TIMESTAMP
|
||||
"#,
|
||||
)
|
||||
.bind(path)
|
||||
.bind(user_id)
|
||||
.bind(&name.namespace)
|
||||
.bind(&name.name)
|
||||
.bind(&value)
|
||||
.execute(&*self.pool)
|
||||
.await
|
||||
.map_err(|e| DomainError::internal_error("DeadPropertyStore", format!("set: {e}")))?;
|
||||
match r {
|
||||
ResourceRef::Folder(folder_id) => {
|
||||
sqlx::query(
|
||||
r#"
|
||||
INSERT INTO storage.webdav_dead_properties
|
||||
(folder_id, namespace, local_name, value)
|
||||
VALUES ($1, $2, $3, $4)
|
||||
ON CONFLICT (folder_id, namespace, local_name)
|
||||
WHERE folder_id IS NOT NULL
|
||||
DO UPDATE SET value = EXCLUDED.value, updated_at = CURRENT_TIMESTAMP
|
||||
"#,
|
||||
)
|
||||
.bind(folder_id)
|
||||
.bind(&name.namespace)
|
||||
.bind(&name.name)
|
||||
.bind(&value)
|
||||
.execute(&*self.pool)
|
||||
.await
|
||||
.map_err(|e| {
|
||||
DomainError::internal_error("DeadPropertyStore", format!("set folder: {e}"))
|
||||
})?;
|
||||
}
|
||||
ResourceRef::File(file_id) => {
|
||||
sqlx::query(
|
||||
r#"
|
||||
INSERT INTO storage.webdav_dead_properties
|
||||
(file_id, namespace, local_name, value)
|
||||
VALUES ($1, $2, $3, $4)
|
||||
ON CONFLICT (file_id, namespace, local_name)
|
||||
WHERE file_id IS NOT NULL
|
||||
DO UPDATE SET value = EXCLUDED.value, updated_at = CURRENT_TIMESTAMP
|
||||
"#,
|
||||
)
|
||||
.bind(file_id)
|
||||
.bind(&name.namespace)
|
||||
.bind(&name.name)
|
||||
.bind(&value)
|
||||
.execute(&*self.pool)
|
||||
.await
|
||||
.map_err(|e| {
|
||||
DomainError::internal_error("DeadPropertyStore", format!("set file: {e}"))
|
||||
})?;
|
||||
}
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Delete a specific dead property. No-op if not present.
|
||||
pub async fn remove(
|
||||
&self,
|
||||
path: &str,
|
||||
user_id: Uuid,
|
||||
name: &QualifiedName,
|
||||
) -> Result<(), DomainError> {
|
||||
pub async fn remove(&self, r: ResourceRef, name: &QualifiedName) -> Result<(), DomainError> {
|
||||
let (folder_id, file_id) = split_ref(r);
|
||||
sqlx::query(
|
||||
"DELETE FROM storage.webdav_dead_properties
|
||||
WHERE resource_path = $1 AND user_id = $2
|
||||
AND namespace = $3 AND local_name = $4",
|
||||
WHERE folder_id IS NOT DISTINCT FROM $1
|
||||
AND file_id IS NOT DISTINCT FROM $2
|
||||
AND namespace = $3
|
||||
AND local_name = $4",
|
||||
)
|
||||
.bind(path)
|
||||
.bind(user_id)
|
||||
.bind(folder_id)
|
||||
.bind(file_id)
|
||||
.bind(&name.namespace)
|
||||
.bind(&name.name)
|
||||
.execute(&*self.pool)
|
||||
@@ -79,19 +145,20 @@ impl DeadPropertyStore {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Return all dead properties for `path`.
|
||||
/// Return all dead properties for the given resource.
|
||||
pub async fn get_all(
|
||||
&self,
|
||||
path: &str,
|
||||
user_id: Uuid,
|
||||
r: ResourceRef,
|
||||
) -> Result<Vec<(QualifiedName, Option<String>)>, DomainError> {
|
||||
let (folder_id, file_id) = split_ref(r);
|
||||
let rows = sqlx::query(
|
||||
"SELECT namespace, local_name, value
|
||||
FROM storage.webdav_dead_properties
|
||||
WHERE resource_path = $1 AND user_id = $2",
|
||||
FROM storage.webdav_dead_properties
|
||||
WHERE folder_id IS NOT DISTINCT FROM $1
|
||||
AND file_id IS NOT DISTINCT FROM $2",
|
||||
)
|
||||
.bind(path)
|
||||
.bind(user_id)
|
||||
.bind(folder_id)
|
||||
.bind(file_id)
|
||||
.fetch_all(&*self.pool)
|
||||
.await
|
||||
.map_err(|e| DomainError::internal_error("DeadPropertyStore", format!("get_all: {e}")))?;
|
||||
@@ -111,17 +178,19 @@ impl DeadPropertyStore {
|
||||
/// Returns `Some(None)` when the property exists with an empty value.
|
||||
pub async fn get(
|
||||
&self,
|
||||
path: &str,
|
||||
user_id: Uuid,
|
||||
r: ResourceRef,
|
||||
name: &QualifiedName,
|
||||
) -> Result<Option<Option<String>>, DomainError> {
|
||||
let (folder_id, file_id) = split_ref(r);
|
||||
let row = sqlx::query(
|
||||
"SELECT value FROM storage.webdav_dead_properties
|
||||
WHERE resource_path = $1 AND user_id = $2
|
||||
AND namespace = $3 AND local_name = $4",
|
||||
WHERE folder_id IS NOT DISTINCT FROM $1
|
||||
AND file_id IS NOT DISTINCT FROM $2
|
||||
AND namespace = $3
|
||||
AND local_name = $4",
|
||||
)
|
||||
.bind(path)
|
||||
.bind(user_id)
|
||||
.bind(folder_id)
|
||||
.bind(file_id)
|
||||
.bind(&name.namespace)
|
||||
.bind(&name.name)
|
||||
.fetch_optional(&*self.pool)
|
||||
@@ -130,65 +199,15 @@ impl DeadPropertyStore {
|
||||
|
||||
Ok(row.map(|r| r.get::<Option<String>, _>("value")))
|
||||
}
|
||||
}
|
||||
|
||||
/// Delete all dead properties for `path` (called on DELETE).
|
||||
pub async fn remove_resource(&self, path: &str, user_id: Uuid) -> Result<(), DomainError> {
|
||||
sqlx::query(
|
||||
"DELETE FROM storage.webdav_dead_properties
|
||||
WHERE resource_path = $1 AND user_id = $2",
|
||||
)
|
||||
.bind(path)
|
||||
.bind(user_id)
|
||||
.execute(&*self.pool)
|
||||
.await
|
||||
.map_err(|e| {
|
||||
DomainError::internal_error("DeadPropertyStore", format!("remove_resource: {e}"))
|
||||
})?;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Move dead properties from `old_path` to `new_path` (called on MOVE).
|
||||
/// Clears any stale properties at `new_path` first.
|
||||
pub async fn rename_resource(
|
||||
&self,
|
||||
old_path: &str,
|
||||
user_id: Uuid,
|
||||
new_path: &str,
|
||||
) -> Result<(), DomainError> {
|
||||
let mut tx = self.pool.begin().await.map_err(|e| {
|
||||
DomainError::internal_error("DeadPropertyStore", format!("rename_resource tx: {e}"))
|
||||
})?;
|
||||
|
||||
sqlx::query(
|
||||
"DELETE FROM storage.webdav_dead_properties
|
||||
WHERE resource_path = $1 AND user_id = $2",
|
||||
)
|
||||
.bind(new_path)
|
||||
.bind(user_id)
|
||||
.execute(&mut *tx)
|
||||
.await
|
||||
.map_err(|e| {
|
||||
DomainError::internal_error("DeadPropertyStore", format!("rename_resource delete: {e}"))
|
||||
})?;
|
||||
|
||||
sqlx::query(
|
||||
"UPDATE storage.webdav_dead_properties
|
||||
SET resource_path = $2
|
||||
WHERE resource_path = $1 AND user_id = $3",
|
||||
)
|
||||
.bind(old_path)
|
||||
.bind(new_path)
|
||||
.bind(user_id)
|
||||
.execute(&mut *tx)
|
||||
.await
|
||||
.map_err(|e| {
|
||||
DomainError::internal_error("DeadPropertyStore", format!("rename_resource update: {e}"))
|
||||
})?;
|
||||
|
||||
tx.commit().await.map_err(|e| {
|
||||
DomainError::internal_error("DeadPropertyStore", format!("rename_resource commit: {e}"))
|
||||
})?;
|
||||
Ok(())
|
||||
/// Splits a `ResourceRef` into `(folder_id, file_id)` Option pairs for
|
||||
/// binding into SQL. The unused slot is `None` so `IS NOT DISTINCT FROM`
|
||||
/// matches the NULL stored in the unused column.
|
||||
fn split_ref(r: ResourceRef) -> (Option<Uuid>, Option<Uuid>) {
|
||||
match r {
|
||||
ResourceRef::Folder(id) => (Some(id), None),
|
||||
ResourceRef::File(id) => (None, Some(id)),
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -31,6 +31,7 @@ use crate::application::services::folder_service::FolderService;
|
||||
use crate::common::di::AppState;
|
||||
use crate::domain::repositories::drive_repository::DriveRepository;
|
||||
use crate::infrastructure::services::path_resolver_service::ResolvedResource;
|
||||
use crate::infrastructure::services::webdav_dead_property_store::{DeadPropertyStore, ResourceRef};
|
||||
use crate::interfaces::errors::AppError;
|
||||
use crate::interfaces::middleware::auth::{AuthUser, CurrentUser};
|
||||
use crate::interfaces::range_requests::{not_modified_response, range_response};
|
||||
@@ -462,7 +463,6 @@ async fn handle_propfind(
|
||||
file_retrieval_service,
|
||||
user.id,
|
||||
state.webdav_dead_props.clone(),
|
||||
path.clone(),
|
||||
)
|
||||
.await;
|
||||
}
|
||||
@@ -482,16 +482,11 @@ async fn handle_propfind(
|
||||
file_retrieval_service,
|
||||
user.id,
|
||||
state.webdav_dead_props.clone(),
|
||||
path.clone(),
|
||||
)
|
||||
.await;
|
||||
}
|
||||
Ok(ResolvedResource::File(file)) => {
|
||||
let dead_props = state
|
||||
.webdav_dead_props
|
||||
.get_all(&path, user.id)
|
||||
.await
|
||||
.unwrap_or_default();
|
||||
let dead_props = file_dead_props(&state, &file).await;
|
||||
let file_href = webdav_href(&client_path);
|
||||
let mut buf = Vec::with_capacity(1024);
|
||||
{
|
||||
@@ -535,7 +530,6 @@ async fn handle_propfind(
|
||||
file_retrieval_service,
|
||||
user.id,
|
||||
state.webdav_dead_props.clone(),
|
||||
path.clone(),
|
||||
)
|
||||
.await;
|
||||
}
|
||||
@@ -544,11 +538,7 @@ async fn handle_propfind(
|
||||
.await
|
||||
{
|
||||
assert_owner(file.owner_id.as_deref(), &user.id.to_string(), &path)?;
|
||||
let dead_props = state
|
||||
.webdav_dead_props
|
||||
.get_all(&path, user.id)
|
||||
.await
|
||||
.unwrap_or_default();
|
||||
let dead_props = file_dead_props(&state, &file).await;
|
||||
let file_href = webdav_href(&client_path);
|
||||
let mut buf = Vec::with_capacity(1024);
|
||||
{
|
||||
@@ -593,10 +583,7 @@ async fn build_streaming_propfind_response(
|
||||
folder_service: std::sync::Arc<FolderService>,
|
||||
file_retrieval_service: std::sync::Arc<FileRetrievalService>,
|
||||
user_id: Uuid,
|
||||
dead_props_store: Arc<
|
||||
crate::infrastructure::services::webdav_dead_property_store::DeadPropertyStore,
|
||||
>,
|
||||
folder_internal_path: String,
|
||||
dead_props_store: Arc<DeadPropertyStore>,
|
||||
) -> Result<Response<Body>, AppError> {
|
||||
let depth = depth.to_string();
|
||||
let base_href = base_href.to_string();
|
||||
@@ -604,11 +591,17 @@ async fn build_streaming_propfind_response(
|
||||
|
||||
let stream = async_stream::try_stream! {
|
||||
// ── XML header + <D:multistatus> + folder entry ──────────
|
||||
//
|
||||
// Dead-property lookups key on the resource's stable id, so we
|
||||
// pass each FolderDto / FileDto to a small helper that parses
|
||||
// its `id` field into a `ResourceRef` and queries the store.
|
||||
// The synthetic root folder (id = "root") fails to parse and
|
||||
// the helper returns an empty list — correct, since the root
|
||||
// has no DB row to anchor properties on.
|
||||
let folder_dead = folder_dead_props(&dead_props_store, &folder).await;
|
||||
let mut buf = Vec::with_capacity(4096);
|
||||
{
|
||||
let mut w = Writer::new(&mut buf);
|
||||
let folder_dead = dead_props_store.get_all(&folder_internal_path, user_id).await
|
||||
.map_err(|e| std::io::Error::other(e.to_string()))?;
|
||||
WebDavAdapter::write_multistatus_start(&mut w)
|
||||
.map_err(|e| std::io::Error::other(e.to_string()))?;
|
||||
WebDavAdapter::write_folder_entry_with_dead_props(&mut w, &folder, &propfind_request, &base_href, &folder_dead)
|
||||
@@ -640,15 +633,21 @@ async fn build_streaming_propfind_response(
|
||||
break;
|
||||
}
|
||||
|
||||
// Materialise dead-props for the whole page before
|
||||
// we start writing — keeps the borrow checker happy
|
||||
// (the writer borrows the FolderDto and the dead-props
|
||||
// vec for the duration of write_folder_entry_*).
|
||||
let mut subfolder_deads = Vec::with_capacity(result.items.len());
|
||||
for subfolder in &result.items {
|
||||
subfolder_deads.push(folder_dead_props(&dead_props_store, subfolder).await);
|
||||
}
|
||||
|
||||
let mut chunk = Vec::with_capacity(result.items.len() * 800);
|
||||
{
|
||||
let mut w = Writer::new(&mut chunk);
|
||||
for subfolder in &result.items {
|
||||
for (subfolder, child_dead) in result.items.iter().zip(subfolder_deads.iter()) {
|
||||
let href = format!("{}{}/", base_href, encode_path_segment(&subfolder.name));
|
||||
let child_path = format!("{}/{}", folder_internal_path, subfolder.name);
|
||||
let child_dead = dead_props_store.get_all(&child_path, user_id).await
|
||||
.map_err(|e| std::io::Error::other(e.to_string()))?;
|
||||
WebDavAdapter::write_folder_entry_with_dead_props(&mut w, subfolder, &propfind_request, &href, &child_dead)
|
||||
WebDavAdapter::write_folder_entry_with_dead_props(&mut w, subfolder, &propfind_request, &href, child_dead)
|
||||
.map_err(|e| std::io::Error::other(e.to_string()))?;
|
||||
}
|
||||
}
|
||||
@@ -674,15 +673,17 @@ async fn build_streaming_propfind_response(
|
||||
}
|
||||
|
||||
let batch_len = batch.len();
|
||||
let mut file_deads = Vec::with_capacity(batch_len);
|
||||
for file in &batch {
|
||||
file_deads.push(streamed_file_dead_props(&dead_props_store, file).await);
|
||||
}
|
||||
|
||||
let mut chunk = Vec::with_capacity(batch_len * 800);
|
||||
{
|
||||
let mut w = Writer::new(&mut chunk);
|
||||
for file in &batch {
|
||||
for (file, child_dead) in batch.iter().zip(file_deads.iter()) {
|
||||
let href = format!("{}{}", base_href, encode_path_segment(&file.name));
|
||||
let child_path = format!("{}/{}", folder_internal_path, file.name);
|
||||
let child_dead = dead_props_store.get_all(&child_path, user_id).await
|
||||
.map_err(|e| std::io::Error::other(e.to_string()))?;
|
||||
WebDavAdapter::write_file_entry_with_dead_props(&mut w, file, &propfind_request, &href, &child_dead)
|
||||
WebDavAdapter::write_file_entry_with_dead_props(&mut w, file, &propfind_request, &href, child_dead)
|
||||
.map_err(|e| std::io::Error::other(e.to_string()))?;
|
||||
}
|
||||
}
|
||||
@@ -752,29 +753,45 @@ async fn handle_proppatch(
|
||||
return Ok(resp);
|
||||
}
|
||||
|
||||
// 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
|
||||
// Resolve the target resource BEFORE consuming the body. We need
|
||||
// the resolved kind for two reasons:
|
||||
//
|
||||
// 1. The store key is the resource id (folder_id XOR file_id)
|
||||
// after migration 20260830000001; we need to know which one
|
||||
// to bind into `ResourceRef`.
|
||||
// 2. The href shape in the multi-status response differs for
|
||||
// collections vs leaves — RFC 4918 §5.2 + strict WebDAV-
|
||||
// client parser rules require a trailing `/` for collection
|
||||
// hrefs, and emitting `/webdav/foo` for a folder breaks
|
||||
// NC-desktop / Cyberduck / other multi-status consumers.
|
||||
//
|
||||
// PROPPATCH on a non-existent resource returns 404. This is a
|
||||
// tighter contract than the pre-rekey code, which silently
|
||||
// wrote a dead-prop row keyed by the ghost path — that was a
|
||||
// foot-gun, not a feature.
|
||||
let (resource_ref, is_collection) = if path.is_empty() || path == "/" {
|
||||
// The synthetic root has no DB row to anchor properties on.
|
||||
// Treat it as a collection for href shaping; reject the
|
||||
// PROPPATCH itself below so we don't fabricate a target.
|
||||
(None, true)
|
||||
} else {
|
||||
let drive_id = resolve_drive_id_for_native_webdav(&state, user.id).await?;
|
||||
state
|
||||
.applications
|
||||
.folder_service
|
||||
.get_folder_by_path(&path, drive_id)
|
||||
.await
|
||||
.is_ok()
|
||||
match resolve_or_legacy(&state, &path, user.id).await {
|
||||
Some(ResolvedResource::Folder(folder)) => {
|
||||
let id = Uuid::parse_str(&folder.id).map_err(|e| {
|
||||
AppError::internal_error(format!("Folder id is not a UUID: {e}"))
|
||||
})?;
|
||||
(Some(ResourceRef::Folder(id)), true)
|
||||
}
|
||||
Some(ResolvedResource::File(file)) => {
|
||||
let id = Uuid::parse_str(&file.id)
|
||||
.map_err(|e| AppError::internal_error(format!("File id is not a UUID: {e}")))?;
|
||||
(Some(ResourceRef::File(id)), false)
|
||||
}
|
||||
None => return Err(AppError::not_found(format!("Resource not found: {}", path))),
|
||||
}
|
||||
};
|
||||
let resource_ref = resource_ref
|
||||
.ok_or_else(|| AppError::forbidden("PROPPATCH on the WebDAV root is not supported"))?;
|
||||
|
||||
// Read request body (XML — bounded to 1 MB)
|
||||
let body_bytes = body::to_bytes(req.into_body(), MAX_XML_BODY)
|
||||
@@ -792,7 +809,7 @@ async fn handle_proppatch(
|
||||
match op {
|
||||
PropPatchOp::Set(pv) => {
|
||||
dead_props
|
||||
.set(&path, user.id, pv.name.clone(), pv.value.clone())
|
||||
.set(resource_ref, pv.name.clone(), pv.value.clone())
|
||||
.await
|
||||
.map_err(|e| {
|
||||
AppError::internal_error(format!("Failed to store dead property: {e}"))
|
||||
@@ -800,7 +817,7 @@ async fn handle_proppatch(
|
||||
results.push((&pv.name, true));
|
||||
}
|
||||
PropPatchOp::Remove(name) => {
|
||||
dead_props.remove(&path, user.id, name).await.map_err(|e| {
|
||||
dead_props.remove(resource_ref, name).await.map_err(|e| {
|
||||
AppError::internal_error(format!("Failed to remove dead property: {e}"))
|
||||
})?;
|
||||
results.push((name, true));
|
||||
@@ -1068,6 +1085,58 @@ async fn resolve_or_legacy(
|
||||
None
|
||||
}
|
||||
|
||||
/// Fetch a file's dead properties for a PROPFIND response.
|
||||
///
|
||||
/// Lenient on every failure mode: malformed id, DB error → empty list.
|
||||
/// PROPFIND must still emit the resource's live properties even when
|
||||
/// the dead-prop lookup is broken; surfacing a 500 here would mask the
|
||||
/// resource entirely from sync clients. The legacy path-keyed lookup
|
||||
/// behaved the same way (`.unwrap_or_default()`); we preserve it.
|
||||
async fn file_dead_props(
|
||||
state: &Arc<AppState>,
|
||||
file: &FileDto,
|
||||
) -> Vec<(QualifiedName, Option<String>)> {
|
||||
let Ok(file_id) = Uuid::parse_str(&file.id) else {
|
||||
return Vec::new();
|
||||
};
|
||||
state
|
||||
.webdav_dead_props
|
||||
.get_all(ResourceRef::File(file_id))
|
||||
.await
|
||||
.unwrap_or_default()
|
||||
}
|
||||
|
||||
/// Same shape as `file_dead_props` but for folder rows. Used by the
|
||||
/// streaming PROPFIND walker.
|
||||
async fn folder_dead_props(
|
||||
store: &DeadPropertyStore,
|
||||
folder: &FolderDto,
|
||||
) -> Vec<(QualifiedName, Option<String>)> {
|
||||
let Ok(folder_id) = Uuid::parse_str(&folder.id) else {
|
||||
return Vec::new();
|
||||
};
|
||||
store
|
||||
.get_all(ResourceRef::Folder(folder_id))
|
||||
.await
|
||||
.unwrap_or_default()
|
||||
}
|
||||
|
||||
/// File-leaf variant for the streaming walker (takes a `&DeadPropertyStore`
|
||||
/// rather than the full `&Arc<AppState>` so it can be called from inside
|
||||
/// the async-stream future without cloning state).
|
||||
async fn streamed_file_dead_props(
|
||||
store: &DeadPropertyStore,
|
||||
file: &FileDto,
|
||||
) -> Vec<(QualifiedName, Option<String>)> {
|
||||
let Ok(file_id) = Uuid::parse_str(&file.id) else {
|
||||
return Vec::new();
|
||||
};
|
||||
store
|
||||
.get_all(ResourceRef::File(file_id))
|
||||
.await
|
||||
.unwrap_or_default()
|
||||
}
|
||||
|
||||
/// Extract every `<...>` token from a WebDAV `If:` header value.
|
||||
///
|
||||
/// RFC 4918 §10.4 defines a richer grammar (tagged-list / no-tag-list of
|
||||
@@ -1587,22 +1656,12 @@ async fn handle_delete(
|
||||
None => return Err(AppError::not_found(format!("Resource not found: {}", path))),
|
||||
}
|
||||
|
||||
// Reap dead properties so a future resource at the same path
|
||||
// doesn't inherit tombstone metadata from the deleted one. Best-
|
||||
// effort: a failure to clear leaves orphan rows but the user-
|
||||
// facing DELETE has succeeded, so we don't propagate the error.
|
||||
// Caught by tests/api/webdav_dead_properties.hurl Step 10.
|
||||
if let Err(e) = state
|
||||
.webdav_dead_props
|
||||
.remove_resource(&path, user.id)
|
||||
.await
|
||||
{
|
||||
tracing::warn!(
|
||||
user_id = %user.id,
|
||||
path = %path,
|
||||
"dead-property cleanup on DELETE failed: {e}"
|
||||
);
|
||||
}
|
||||
// Dead-property rows attached to the deleted file/folder are reaped
|
||||
// automatically by `storage.webdav_dead_properties.{folder,file}_id`
|
||||
// ON DELETE CASCADE (migration 20260830000001). Same guarantee
|
||||
// applies to every other delete code path — REST `DELETE
|
||||
// /api/files/{id}`, bulk delete, trash empty, folder cascade —
|
||||
// without any service-layer call. No explicit cleanup needed here.
|
||||
|
||||
Ok(Response::builder()
|
||||
.status(StatusCode::NO_CONTENT)
|
||||
@@ -1860,12 +1919,13 @@ async fn handle_move(
|
||||
}
|
||||
}
|
||||
|
||||
// Migrate dead properties to the new path (RFC 4918 §9.9 — MOVE preserves properties).
|
||||
state
|
||||
.webdav_dead_props
|
||||
.rename_resource(&source_path, user.id, &destination_path)
|
||||
.await
|
||||
.map_err(|e| AppError::internal_error(format!("Failed to migrate dead properties: {e}")))?;
|
||||
// Dead properties follow the resource automatically across MOVE
|
||||
// and RENAME: the rows in `storage.webdav_dead_properties` key on
|
||||
// the underlying folder/file id, which is stable across both
|
||||
// operations (BEFORE trigger rewrites path on the row, AFTER
|
||||
// cascade rewrites descendants' path/lpath — but no id ever
|
||||
// changes). RFC 4918 §9.9 "MOVE preserves properties" satisfied
|
||||
// by the database invariant, no store call needed.
|
||||
|
||||
// RFC 4918 §9.9.5: 201 Created when destination is new, 204 when overwritten.
|
||||
let status = if dest_existed {
|
||||
|
||||
@@ -14,15 +14,40 @@
|
||||
# server is broken; OxiCloud sees nothing wrong in its logs).
|
||||
#
|
||||
# Coverage:
|
||||
# 1. Setup admin, capture JWT, PUT a probe file.
|
||||
# 2. PROPPATCH set → 207
|
||||
# 3. PROPFIND get → value round-trips verbatim
|
||||
# 4. PROPPATCH upsert (set same name → new value) → 207
|
||||
# 5. PROPFIND get → new value (upsert worked)
|
||||
# 6. PROPPATCH remove → 207
|
||||
# 7. PROPFIND get → property absent
|
||||
# 8. MOVE file → properties follow the path (rename_resource)
|
||||
# 9. DELETE file → properties cleaned up (no orphan rows)
|
||||
# 1. Setup admin, capture JWT, PUT a probe file.
|
||||
# 2. PROPPATCH set → 207
|
||||
# 3. PROPFIND get → value round-trips verbatim
|
||||
# 4. PROPPATCH upsert (set same name → new value) → 207
|
||||
# 5. PROPFIND get → new value (upsert worked)
|
||||
# 6. PROPPATCH remove → 207
|
||||
# 7. PROPFIND get → property absent
|
||||
# 8. MOVE file → properties follow the resource id automatically
|
||||
# (no rename_resource() call; the row's file_id is stable
|
||||
# across MOVE so dead-props travel with the resource).
|
||||
# 9. PROPFIND on moved path returns the property.
|
||||
# 10. DELETE via WebDAV → FK CASCADE reaps dead-prop rows.
|
||||
# 11. PROPPATCH + REST DELETE `/api/files/{id}` → FK CASCADE
|
||||
# reaps via the REST-side delete path too. This is the
|
||||
# new coverage unlocked by migration 20260830000001 — the
|
||||
# old path-keyed store had no way to clean up here, so
|
||||
# the SvelteKit web UI (which deletes via REST) was
|
||||
# silently leaking tombstones every time a user deleted
|
||||
# a file that had ever carried dead properties.
|
||||
# 12. Folder MOVE preserves dead properties (id-stable
|
||||
# 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 —
|
||||
@@ -208,12 +233,15 @@ xpath "count(//*[local-name()='testlabel'])" == 0
|
||||
|
||||
|
||||
# ─────────────────────────────────────────────────────────────
|
||||
# Step 9 — Re-set a property, then MOVE the file. The
|
||||
# rename_resource path in DeadPropertyStore must
|
||||
# re-key the row to the new path so the property
|
||||
# follows the file (a regression that leaves the row
|
||||
# at the old path would silently break every client
|
||||
# that does a MOVE then a PROPFIND).
|
||||
# Step 9 — Re-set a property, then MOVE the file. Post-rekey
|
||||
# (migration 20260830000001) the dead-property row
|
||||
# keys on `file_id`, which never changes across MOVE
|
||||
# or RENAME — so properties follow the resource by a
|
||||
# database invariant, without any store-side call.
|
||||
# A regression that broke this would be a regression
|
||||
# on the id-stability guarantee in the move SQL itself
|
||||
# (i.e. it would surface elsewhere too); this assertion
|
||||
# locks it in for sync clients that do MOVE → PROPFIND.
|
||||
# ─────────────────────────────────────────────────────────────
|
||||
PROPPATCH {{base_url}}/webdav/dead-props-probe.txt
|
||||
Authorization: Bearer {{token}}
|
||||
@@ -260,12 +288,14 @@ xpath "string(//*[local-name()='testlabel'])" == "survives-move"
|
||||
|
||||
|
||||
# ─────────────────────────────────────────────────────────────
|
||||
# Step 10 — DELETE the file; remove_resource() must reap the
|
||||
# dead-property rows so they don't accumulate as
|
||||
# tombstones the next time a file is created at the
|
||||
# same path. We verify by recreating the same path
|
||||
# and PROPFIND'ing — a leak would resurface the old
|
||||
# "survives-move" value.
|
||||
# Step 10 — DELETE the file via WebDAV; the FK
|
||||
# `webdav_dead_properties.file_id → storage.files.id
|
||||
# ON DELETE CASCADE` (migration 20260830000001) must
|
||||
# reap the dead-property rows automatically, so they
|
||||
# don't accumulate as tombstones the next time a file
|
||||
# is created at the same path. We verify by recreating
|
||||
# the same path and PROPFIND'ing — a leak would
|
||||
# resurface the old "survives-move" value.
|
||||
# ─────────────────────────────────────────────────────────────
|
||||
DELETE {{base_url}}/webdav/dead-props-moved.txt
|
||||
Authorization: Bearer {{token}}
|
||||
@@ -302,6 +332,129 @@ HTTP 207
|
||||
xpath "count(//*[local-name()='testlabel'])" == 0
|
||||
|
||||
|
||||
# ─────────────────────────────────────────────────────────────
|
||||
# Step 11 — Same FK-cascade property test but via the REST API
|
||||
# delete path. The path-based store would have leaked
|
||||
# here forever (REST DELETE receives a file_id, not a
|
||||
# path; the old store had no efficient way to clean
|
||||
# up). The id-keyed schema reaps the dead-property
|
||||
# row through the same FK CASCADE on `storage.files`,
|
||||
# so this proves the new coverage end-to-end.
|
||||
#
|
||||
# Sequence:
|
||||
# a. PROPPATCH a marker dead property on the file.
|
||||
# b. PROPFIND — confirm it's stored.
|
||||
# c. Resolve the file's id via REST listing of the
|
||||
# home folder.
|
||||
# d. DELETE via `/api/files/{id}` — pure REST,
|
||||
# never touches the WebDAV surface.
|
||||
# e. PUT a fresh file at the same WebDAV path.
|
||||
# f. PROPFIND — must not see the marker.
|
||||
# ─────────────────────────────────────────────────────────────
|
||||
|
||||
# Step 11a — set a new marker dead property on the just-PUT file
|
||||
PROPPATCH {{base_url}}/webdav/dead-props-moved.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:restmarker xmlns:X="oxi:test">rest-delete-coverage</X:restmarker>
|
||||
</D:prop>
|
||||
</D:set>
|
||||
</D:propertyupdate>
|
||||
```
|
||||
|
||||
HTTP 207
|
||||
|
||||
|
||||
# Step 11b — confirm the marker is stored
|
||||
PROPFIND {{base_url}}/webdav/dead-props-moved.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()='restmarker'])" == "rest-delete-coverage"
|
||||
|
||||
|
||||
# Step 11c — resolve the file id from the home folder listing.
|
||||
# The home folder is whatever `GET /api/folders` returns as the
|
||||
# first root-level entry for the admin user (Personal drive root,
|
||||
# post drive-no-wrapper).
|
||||
GET {{base_url}}/api/folders
|
||||
Authorization: Bearer {{token}}
|
||||
|
||||
HTTP 200
|
||||
[Captures]
|
||||
home_folder_id: jsonpath "$[0].id"
|
||||
|
||||
|
||||
GET {{base_url}}/api/files?folder_id={{home_folder_id}}
|
||||
Authorization: Bearer {{token}}
|
||||
|
||||
HTTP 200
|
||||
[Captures]
|
||||
# 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 —
|
||||
# the cleanup must happen via the FK CASCADE on storage.files.
|
||||
DELETE {{base_url}}/api/files/{{rest_file_id}}
|
||||
Authorization: Bearer {{token}}
|
||||
|
||||
# The REST delete handler returns 204 No Content on success.
|
||||
HTTP 204
|
||||
|
||||
|
||||
# Step 11e — recreate the file at the same WebDAV path
|
||||
PUT {{base_url}}/webdav/dead-props-moved.txt
|
||||
Authorization: Bearer {{token}}
|
||||
Content-Type: text/plain
|
||||
```
|
||||
fresh file post REST DELETE
|
||||
```
|
||||
|
||||
HTTP 201
|
||||
|
||||
|
||||
# Step 11f — PROPFIND must not surface the old marker
|
||||
PROPFIND {{base_url}}/webdav/dead-props-moved.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]
|
||||
# If REST DELETE failed to cascade, the marker would still be
|
||||
# attached to the (recreated) path under the old `(path, user_id)`
|
||||
# key — but the new schema keys by file_id, and the REST DELETE
|
||||
# took the storage.files row with it. Asserting absence proves
|
||||
# the cascade fired.
|
||||
xpath "count(//*[local-name()='restmarker'])" == 0
|
||||
|
||||
|
||||
# ─────────────────────────────────────────────────────────────
|
||||
# Cleanup
|
||||
# ─────────────────────────────────────────────────────────────
|
||||
@@ -309,3 +462,332 @@ DELETE {{base_url}}/webdav/dead-props-moved.txt
|
||||
Authorization: Bearer {{token}}
|
||||
|
||||
HTTP 204
|
||||
|
||||
|
||||
# ─────────────────────────────────────────────────────────────
|
||||
# Step 12 — Folder MOVE dead-property preservation.
|
||||
# Same invariant as step 9 (id-stability under MOVE)
|
||||
# but for folders. The Hurl suite had no folder-side
|
||||
# coverage of this until now, so a regression that
|
||||
# broke folder dead-property preservation could
|
||||
# silently land — calendar / contacts / NextCloud
|
||||
# clients that PROPPATCH per-folder sync state would
|
||||
# lose it on every rename.
|
||||
#
|
||||
# Sequence:
|
||||
# a. MKCOL a fresh test folder.
|
||||
# b. PROPPATCH a dead property on it.
|
||||
# c. MOVE / rename the folder.
|
||||
# d. PROPFIND the new collection path; assert
|
||||
# the property survived.
|
||||
# e. Cleanup: DELETE the renamed folder.
|
||||
# ─────────────────────────────────────────────────────────────
|
||||
|
||||
# Step 12a — fresh collection (no prior state at this path)
|
||||
MKCOL {{base_url}}/webdav/dead-props-folder/
|
||||
Authorization: Bearer {{token}}
|
||||
|
||||
HTTP 201
|
||||
|
||||
|
||||
# Step 12b — attach a marker dead property to the FOLDER row
|
||||
PROPPATCH {{base_url}}/webdav/dead-props-folder/
|
||||
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:foldermark xmlns:X="oxi:test">folder-keeps-this</X:foldermark>
|
||||
</D:prop>
|
||||
</D:set>
|
||||
</D:propertyupdate>
|
||||
```
|
||||
|
||||
HTTP 207
|
||||
|
||||
|
||||
# Step 12c — rename the folder via MOVE. Same-parent rename
|
||||
# (intra-collection name change) — the most common shape clients
|
||||
# issue and the one that previously needed `rename_resource()`
|
||||
# to keep dead properties attached.
|
||||
MOVE {{base_url}}/webdav/dead-props-folder/
|
||||
Authorization: Bearer {{token}}
|
||||
Destination: {{base_url}}/webdav/dead-props-folder-renamed/
|
||||
|
||||
HTTP 201
|
||||
|
||||
|
||||
# Step 12d — PROPFIND the new collection path; the dead property
|
||||
# must still be attached. If the folder row's id had changed
|
||||
# under MOVE (it doesn't), or if anything had reaped the
|
||||
# webdav_dead_properties row, the property would be gone.
|
||||
PROPFIND {{base_url}}/webdav/dead-props-folder-renamed/
|
||||
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()='foldermark'])" == "folder-keeps-this"
|
||||
|
||||
|
||||
# Step 12e — cleanup. The DELETE cascades the foldermark row
|
||||
# away via FK ON DELETE CASCADE, leaving the schema clean for
|
||||
# any subsequent test that touches this path.
|
||||
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
|
||||
|
||||
Reference in New Issue
Block a user