diff --git a/docs/plan/drive.md b/docs/plan/drive.md index 5be76a55..1be7a3d4 100644 --- a/docs/plan/drive.md +++ b/docs/plan/drive.md @@ -176,17 +176,35 @@ layer (`DriveService` enforces the personal-drive invariants, etc.) - A shared drive can have **0 viewers** and **0 editors** — only the ≥1-owner invariant matters. -### 3. Drive entity +### 3. Drive entity — pure metadata + a 1:1 root folder + +A drive is a **metadata-only holder** (quota, kind, policies, default flag) +paired 1:1 with a *root folder* that owns the drive's visible identity +(name, path materialisation, ltree anchor). The drive itself has no +`name` column — every property the user thinks of as "the drive" +(its display name, its containing children, its location in the +ltree) lives on the root folder row. + +This is the Unix-philosophy split: the *filesystem volume* is the drive +(quota, policies, ownership metadata); the *mount point* is the root +folder (name, hierarchy, paths). Clients interact with the root folder +through the standard folder API — no special "drive root" endpoint, no +polymorphic creation surface, no "create at drive vs in folder" duality. ```sql storage.drives id uuid PRIMARY KEY DEFAULT gen_random_uuid() - name text NOT NULL -- "Personal" or user-chosen kind text NOT NULL CHECK (kind IN ('personal','shared')) default_for_user uuid NULL FK → auth.users(id) ON DELETE CASCADE quota_bytes bigint NULL -- NULL = unlimited used_bytes bigint NOT NULL DEFAULT 0 policies jsonb NOT NULL DEFAULT '{}' + -- The drive's mount-point folder. Nullable AT THE COLUMN TYPE LEVEL + -- only because the column is set mid-statement during atomic + -- creation (see "Atomic creation" below) — invariant: after any + -- successful create_personal_drive() call, this is non-NULL. Code + -- that reads drives can treat it as Uuid in Rust. + root_folder_id uuid NULL FK → storage.folders(id) ON DELETE CASCADE created_at timestamptz NOT NULL DEFAULT now() updated_at timestamptz NOT NULL DEFAULT now() -- `default_for_user` may ONLY be set on personal drives. @@ -199,6 +217,35 @@ CREATE UNIQUE INDEX drives_default_for_user_idx WHERE default_for_user IS NOT NULL; -- one DEFAULT personal drive per user ``` +#### Drive name lives on the root folder + +`storage.drives` has no `name` column. The drive's display name is +`SELECT f.name FROM storage.drives d JOIN storage.folders f +ON f.id = d.root_folder_id WHERE d.id = $drive_id`. + +Why this is the right shape: + +- **Single source of truth.** No duplication between `drives.name` and + `folders.name`, no drift risk, no decision about which side to + serve when they differ. +- **Renaming is the standard folder API.** `PATCH /api/folders/` + with a new name renames the drive. No separate + `PATCH /api/drives//name` endpoint. The lifecycle / cascade + / search-index behaviour that already exists on folder rename + applies automatically. +- **No "rename the drive but not its mount point" footgun.** They're + always in sync because they're the same column. + +Identity is still carried by `kind` + `default_for_user` — *not* by +the name. UI and NC default-drive resolution query on `kind` / +`default_for_user`, never on `name = 'Personal'`. Renaming "Personal" +→ "Ed's space" preserves identity; only the label changes. + +The migration-time default for personal drive root folder names is +`'Personal'`. Secondary personal drives (sibling roots from the M2 +backfill — see §10) carry over whatever name the original sibling +root folder had. + #### Two orthogonal properties: `kind` and `default_for_user` - **`kind`** = drive capability shape (see §2 for the rules): @@ -229,24 +276,123 @@ There is no constraint to write — externals simply have no row in `storage.drives` with `default_for_user = `, and nothing tries to create one. -#### Drive naming — `name` is a label, identity lives in `kind` + `default_for_user` +#### Atomic creation — single transaction, four writes -`name` is owner-editable for every drive (personal or shared). A -user who renames their drive from "Personal" → "Ed's space" does -**not** stop having a personal drive, and does not stop having a -default. The `kind` flag + `default_for_user` pointer carry the -identity; the name is purely a display label. +A drive and its root folder reference each other circularly: +`storage.drives.root_folder_id` points at `storage.folders.id`, and +`storage.folders.drive_id` points at `storage.drives.id`. Creating +them naively could leave inconsistent half-state on a server crash +mid-sequence: drive without folder, folder without drive, or either +without an owner role_grant. -Why this matters: -- UI and NC default-drive resolution MUST query on `kind` / - `default_for_user`, never on `name = 'Personal'`. The latter - would silently break the moment the user renames. -- The initial migration sets `name = 'Personal'` on the default - personal drive for back-compat with the label users see today; - further renames go through the normal drive-rename endpoint - and persist on the same row. Secondary personal drives carry - whatever name the sibling root folder had (e.g. `Archive`, - `2024 Projects`). +The repo's `create_personal_drive_atomic` wraps the four writes in +a single transaction so they commit together or not at all: + +1. INSERT drive (with `root_folder_id = NULL`) → returns drive id. +2. INSERT folder (with `drive_id` = the drive's id) → returns folder id. +3. UPDATE drive SET `root_folder_id` = the folder id. +4. INSERT role_grant (owner, subject = caller, resource = drive). +5. COMMIT. + +Why a transaction rather than one CTE statement: PostgreSQL's CTE +sub-statements all read the target tables from the *same snapshot* +— a later sub-statement's `UPDATE storage.drives WHERE id = …` +cannot match a row inserted by an earlier sub-statement, even if +the earlier statement returned the new id via `RETURNING`. The +documented escape hatch (`DEFERRABLE INITIALLY DEFERRED` FKs + +pre-generated UUIDs) is the alternative but adds constraint +plumbing to support a single uncommon code path. A transaction is +boring and correct. + +Crash safety: any failure between steps 1 and 4 rolls back — no +drive without folder, no folder without drive, no drive without +owner. Once step 5 commits, the invariant holds. + +For reference, the equivalent (broken) one-CTE form looks like: + +```sql +WITH new_drive AS ( + INSERT INTO storage.drives + (kind, default_for_user, quota_bytes, policies) + VALUES ('personal', $user_id, $quota, '{}'::jsonb) + RETURNING id +), +new_root AS ( + INSERT INTO storage.folders + (name, parent_id, user_id, drive_id, created_by, updated_by) + SELECT 'Personal', -- root folder name + NULL, -- parent_id (this IS the root) + $user_id, + new_drive.id, -- forward-ref to the drive's id + $user_id, $user_id + FROM new_drive + RETURNING id, drive_id +), +drive_updated AS ( + UPDATE storage.drives d + SET root_folder_id = new_root.id + FROM new_root + WHERE d.id = new_root.drive_id + RETURNING d.id +), +new_grant AS ( + INSERT INTO storage.role_grants + (subject_type, subject_id, resource_type, resource_id, role, granted_by) + SELECT 'user', $user_id, 'drive', du.id, 'owner', $user_id + FROM drive_updated du + RETURNING resource_id +) +SELECT d.id, d.root_folder_id, d.kind, d.default_for_user, + d.quota_bytes, d.used_bytes, d.policies, + d.created_at, d.updated_at + FROM storage.drives d + JOIN new_grant g ON g.resource_id = d.id; +``` + +Why the one-CTE form above does NOT work — and what we ship instead: + +The shared-snapshot rule (`postgresql.org/docs/current/queries-with.html` +§7.8.2: "they cannot 'see' one another's effects on the target tables") +breaks the `drive_updated` sub-statement. Its `UPDATE storage.drives d +… WHERE d.id = new_root.drive_id` evaluates `WHERE d.id = …` against +the snapshot, which doesn't contain the drive inserted by `new_drive`. +The UPDATE matches zero rows; `RETURNING` returns zero rows; +`new_grant` (which feeds off `drive_updated`) inserts zero role_grants; +the final SELECT joins on an empty CTE branch and returns nothing. +Symptoms in tests: drives exist with `root_folder_id IS NULL`, owners +have no `role_grants` row, `/api/drives` returns `[]`. + +The fix is the four-step transaction described above. Rust: + +```rust +let mut tx = pool.begin().await?; +let drive_id: Uuid = sqlx::query_scalar( + r#"INSERT INTO storage.drives (kind, default_for_user, quota_bytes) + VALUES ('personal', $1, $2) RETURNING id"#, +).bind(owner).bind(quota).fetch_one(&mut *tx).await?; + +let folder_id: Uuid = sqlx::query_scalar( + r#"INSERT INTO storage.folders + (name, parent_id, user_id, drive_id, created_by, updated_by) + VALUES ('Personal', NULL, $1, $2, $1, $1) RETURNING id"#, +).bind(owner).bind(drive_id).fetch_one(&mut *tx).await?; + +sqlx::query("UPDATE storage.drives SET root_folder_id = $1 WHERE id = $2") + .bind(folder_id).bind(drive_id).execute(&mut *tx).await?; + +sqlx::query( + r#"INSERT INTO storage.role_grants + (subject_type, subject_id, resource_type, resource_id, role, granted_by) + VALUES ('user', $1, 'drive', $2, 'owner', $1)"#, +).bind(owner).bind(drive_id).execute(&mut *tx).await?; + +tx.commit().await?; +``` + +Each statement sees the prior statements' writes (transaction-local +visibility, not the CTE shared snapshot). FK timing works without +`DEFERRABLE`: each FK is satisfied at the moment its row is written +because the referenced rows already exist. #### Capabilities matrix @@ -258,11 +404,12 @@ Why this matters: | Rename | allowed (by the owner) | allowed (by the owner) | allowed (by any owner) | | Delete via API | **refused** — deleting this loses all the user's files; the only path is user-delete cascade | allowed (it's just a silo) | allowed (by an owner; CASCADEs the drive's contents) | | Default-drive lookup result | this drive | never | never | -| On user-delete | `ON DELETE CASCADE` via `default_for_user` FK (free) | application-layer cleanup: enumerate via drive_members and delete | member rows referencing the user are dropped; refuse user-delete if any shared drive would lose its last owner | +| On user-delete | `ON DELETE CASCADE` via `default_for_user` FK (free) | application-layer cleanup: enumerate via `role_grants` (`subject_id= AND resource_type='drive' AND role='owner'`) and delete | role_grants rows referencing the user are dropped; refuse user-delete if any shared drive would lose its last owner | | Group ownership | no | no | yes | | Per-resource grant outward | yes (subject to drive policies) | yes | yes | | Cross-drive move | yes (subject to `forbid_cross_drive_move`) | yes | yes | | Kind conversion | no — always default-personal | yes → may be promoted to `kind='shared'` later (drops the single-user restriction, picks up members) | no | +| Change `quota_bytes` | **OxiCloud admin only** (not the drive owner — §7) | **OxiCloud admin only** | **OxiCloud admin only** | ### 4. Roles → permission bundles @@ -273,7 +420,7 @@ expansion: |---|---| | `viewer` | `Read` | | `editor` | `Read`, `Create`, `Update`, `Comment` | -| `owner` | `Read`, `Create`, `Update`, `Comment`, `Delete`, `Share`, *and* drive-level admin (rename, edit policies, manage members, change quota) | +| `owner` | `Read`, `Create`, `Update`, `Comment`, `Delete`, `Share`, *and* drive-level admin (rename, edit policies, manage members) | ### 5. Permission resolution — additive over `role_grants` @@ -305,16 +452,16 @@ against the same table. | Event | Behaviour | |---|---| -| New internal user registers | Auto-create a default personal drive (`kind='personal'`, `name='Personal'`, `default_for_user=`, `quota_bytes=`), insert the single `drive_members (drive_id, user, , owner)` row. | +| New internal user registers | Auto-create a default personal drive (`kind='personal'`, `default_for_user=`, `quota_bytes=`) + its root folder (`name='Personal'`, `parent_id=NULL`, drive_id pinned) + the Owner role_grant (`role_grants(subject_type='user', subject_id=, resource_type='drive', resource_id=, role='owner')`) — **all four writes in one CTE statement** (§3), atomic against server crash. | | External user invited (magic-link only) | **No personal drive created.** External users are grant-only recipients with no storage. | | External user converts to internal (future flow) | Default personal drive created at conversion time. | -| User deleted | **Default** personal drive cascade-deletes via `ON DELETE CASCADE` on `default_for_user`. **Secondary** personal drives (`kind='personal' AND default_for_user IS NULL` and whose sole `drive_members` row points at the user) are deleted by an application-layer pass in the same transaction. Member rows referencing the deleted user are removed from all shared drives. If a removal would leave a shared drive with zero owners, deletion is refused — admin must transfer first. | +| User deleted | **Default** personal drive cascade-deletes via `ON DELETE CASCADE` on `default_for_user`. **Secondary** personal drives (`kind='personal' AND default_for_user IS NULL` and whose sole owner `role_grants` row points at the user) are deleted by an application-layer pass in the same transaction. `role_grants` rows referencing the deleted user are removed from all shared drives. If a removal would leave a shared drive with zero owners, deletion is refused — admin must transfer first. | | Group deleted | Refuse if the group is a member of any shared drive that would lose its last owner. Admin must transfer or remove the group's role from those drives first. (Groups can't be members of personal drives.) | | Add member to personal drive | Refuse. Personal drives are single-user — collaborate via per-resource grants or by moving content into a shared drive. | | Remove sole owner of personal drive | Refuse. The only deletion path for a personal drive is user-deletion via cascade. | | Delete personal drive | Refuse from the API. Only ON DELETE CASCADE (user deletion) drops it. | -| Rename personal or shared drive | Allowed for any owner-role caller. `name` is a label only. | -| Remove last owner of shared drive | Refuse — drive must always have ≥1 owner. App-layer check on `DELETE FROM drive_members`. | +| Rename personal or shared drive | Allowed for any owner-role caller. The drive's display name lives on its root folder (§3) — rename via `PATCH /api/folders/`, not a drive-specific endpoint. | +| Remove last owner of shared drive | Refuse — drive must always have ≥1 owner. App-layer check on `DELETE FROM role_grants WHERE resource_type='drive' AND resource_id=$drive AND role='owner'`. | ### 7. Quota model @@ -336,6 +483,38 @@ After the cutover: (plus a periodic reconciliation job to fix drift, similar to the existing per-user accounting). +#### Quota mutation is OxiCloud-admin only + +Changing `drives.quota_bytes` is **not** in the drive `owner` role +bundle (§4). It requires the tenant-level OxiCloud admin role +(`auth.users.role = 'admin'`), checked at +`PATCH /api/admin/drives/{id}/quota` — the only callsite that +mutates the column. Drive owners can rename, edit policies, and +manage members; they cannot self-grant capacity. + +Why this seam matters: + +- **Resource allocation is a tenant concern, not a drive + concern.** Storage bytes are a finite system resource the + operator pays for. The drive owner is empowered over the + drive's *use*; the admin is empowered over its *budget*. Same + separation that exists today between a user and the operator + who set `OXICLOUD_DEFAULT_QUOTA_BYTES`. +- **Privilege-escalation seam closed.** Without this carve-out, + any user with a personal drive (= every internal user) could + raise their own quota by virtue of being its sole owner — + trivially defeating the quota system. +- **Shared-drive coherence.** A shared drive's quota is set by + the operator at provisioning; subsequent capacity requests go + through the admin, not the drive's group owners. Keeps the + capacity decision auditable and out of intra-team politics. + +The admin endpoint is the same surface the operator uses today to +change `auth.users.storage_quota_bytes`; D4 simply re-targets the +write at `storage.drives.quota_bytes`. Audit log emits +`drive.quota_changed` with `granted_by=` and the +old/new values, mirroring the existing user-quota change event. + **Chunk dedup vs per-drive quota.** With the CDC chunk store landed in v0.7.0 (see `delta_upload_service`, `upload_ingest`, instant upload by hash), a single chunk can be referenced by files in @@ -479,34 +658,57 @@ discriminator is the literal segment (`files` vs `drives`), never the value of ``. A user happening to have a UUID-shaped username is no longer a problem. -### 10. Storage paths — wrapper folder retired +### 10. Storage paths — wrapper folder becomes the drive's root folder Today `storage.folders.path` is e.g. `My Folder - admin/Docs`. The "My Folder - admin" wrapper is the user's home folder, created at registration via `format!("My Folder - {}", username)`. -Post-drives, **the wrapper goes away**. The drive itself is the -root; folders and files that used to live inside the wrapper sit -directly under the drive with no intermediate folder: +Post-drives, **the wrapper isn't deleted — it's *adopted* as the +drive's root folder** (§3). The drive row is created alongside it +and points at it via `drives.root_folder_id`. The wrapper's row +survives the migration; only its `name` is updated. ``` -Drive "Personal" (uuid=…, kind=personal, owner=admin) ← was the "My Folder - admin" wrapper -├── Docs/ -└── aa.pdf +Drive (uuid=…, kind=personal, default_for_user=admin) +└── root folder (parent_id=NULL, drive_id=, name="Personal") ← was "My Folder - admin" + ├── Docs/ + └── aa.pdf ``` -Same for shared drives — they already had no wrapper: +Shared drives follow the same shape — drive + root folder + content +underneath: ``` -Drive "Engineering" (uuid=…, kind=shared, owners=group:engineering) -├── Specs/ -├── Roadmap.md -└── archive/ +Drive (uuid=…, kind=shared, owners=group:engineering) +└── root folder (parent_id=NULL, drive_id=, name="Engineering") + ├── Specs/ + ├── Roadmap.md + └── archive/ ``` -The two surfaces share one rule: **drive root = `parent_id IS NULL` -within the drive's `drive_id`**. The "personal vs shared" branch -disappears from path resolution — both kinds resolve the same way. +One rule, one model: **every drive has exactly one folder where +`parent_id IS NULL` AND `drive_id = `**. + +#### Why this is a better model + +Three things converge: + +1. **No API duality.** Folder creation is always `POST /api/folders + { name, parent_id: }`. There's no polymorphic "create at + drive vs in folder" branch — the drive's root folder is just + another folder id from the client's perspective. The `parent_id` + field that exists today carries over unchanged. +2. **No path-prefix rewrite migration.** The wrapper row stays; it's + renamed to its drive's canonical name (`"Personal"` for the + default, the original sibling-root name for secondaries). The + BEFORE-UPDATE path trigger fires on the rename and the cascade + trigger automatically rewrites every descendant's `path` / + `lpath` — no per-row UPDATE in the migration. The net cost is + one UPDATE per drive plus the trigger's cascade. +3. **No "this user lost their drive" failure mode.** Migration is + safe even mid-flight — the wrapper row never disappears, just + gains a drive_id pointer above it. #### Why this is client-safe @@ -519,22 +721,48 @@ The wrapper was already invisible to WebDAV / NC clients pre-drive: - Native `/webdav/` was implicitly chrooted to the user's home by `resolve_webdav_path`. Same story. -So URL-level back-compat is preserved trivially — clients keep -asking for `/remote.php/dav/files/admin/Docs/foo.pdf`, the -resolver no longer prepends the wrapper, and the storage row's -path is now `Docs/foo.pdf` instead of `My Folder - admin/Docs/foo.pdf`. -Net effect on the wire: zero. +Post-migration the resolver chroot becomes `/` +instead of `My Folder - /`. The drive's root folder name +(e.g. `Personal`) replaces the wrapper name in the materialised +`path` column; the client's URL still doesn't carry it +because the resolver still chroots before talking to storage. Net +effect on the wire: zero. -#### Why this is a better model +#### Uniqueness constraints become drive-scoped -The original plan kept the wrapper "for back-compat" but it has -no value beyond the storage layer (clients never see it, the -filesystem mirror is happy either way). Keeping it forced path -resolution to always know whether the caller is in a personal or -shared drive and conditionally prepend a segment. Dropping it -collapses that branch and makes the personal-vs-shared distinction -purely a metadata concern (kind, quota source, member shape) — -**not** a path-shape concern. +Pre-drive, two indexes enforce "no duplicate folder names under the +same parent for the same user": + +```sql +CREATE UNIQUE INDEX idx_folders_unique_name + ON storage.folders(parent_id, name, user_id) + WHERE NOT is_trashed AND parent_id IS NOT NULL; +CREATE UNIQUE INDEX idx_folders_unique_name_root + ON storage.folders(name, user_id) + WHERE NOT is_trashed AND parent_id IS NULL; +``` + +Both move from `user_id`-scoped to `drive_id`-scoped: + +```sql +CREATE UNIQUE INDEX idx_folders_unique_name + ON storage.folders(parent_id, name, drive_id) + WHERE NOT is_trashed AND parent_id IS NOT NULL; +CREATE UNIQUE INDEX idx_folders_unique_name_root + ON storage.folders(name, drive_id) + WHERE NOT is_trashed AND parent_id IS NULL; +``` + +This is a **correctness improvement**, not just a migration concession. +The semantic users expect is "no duplicate names *within a drive*" +— a folder named "Reports" in your Personal drive shouldn't preclude +another "Reports" in a shared "Team" drive. The user-scoped +constraint forbade that. The drive-scoped constraint allows it. + +For the root variant: post-migration each drive has exactly one +`parent_id IS NULL` row (its root folder), so `(name, drive_id)` is +trivially unique. The index is still worth keeping as +defence-in-depth. #### Sibling root folders also become drives @@ -551,23 +779,26 @@ Post-migration, the model has to absorb them. Rule: - For each user, find every row with `parent_id IS NULL AND user_id = `. - The one named `My Folder - ` becomes the **default - personal drive**: `kind='personal'`, - `default_for_user=`, sole member = the user. + personal drive's root folder**: a new drive row is created with + `kind='personal'`, `default_for_user=`, the existing folder + row gets a `drive_id` pointer (and the wrapper's name is updated + to `"Personal"`), and `drives.root_folder_id` points back at it. + Sole `role_grants` row: Owner, subject=``. - Every **other** sibling becomes a fresh **secondary personal - drive**: `kind='personal'`, `default_for_user=NULL`, sole - member = the user, name carried over from the folder's `name` - column, quota initialised from the user's quota - (`auth.users.storage_quota_bytes`) — same default as the - user's primary personal drive. Membership rules from §2 apply: - the user cannot invite co-owners while the drive remains - personal. To open the silo up, the user can later **convert** - the secondary personal to `kind='shared'` (an - application-layer operation that flips the kind and lifts the - single-user restriction so the membership API can add other - users / groups). -- The folder row is deleted (the drive replaces it as the root). - Its children get their `parent_id` set to `NULL` *within their - new `drive_id`*. + drive's root folder**: a new drive row with `kind='personal'`, + `default_for_user=NULL`, the existing folder row gets its + `drive_id` set and *keeps its name* (`Archive`, `2024 Projects`, + whatever), quota initialised from the user's quota + (`auth.users.storage_quota_bytes`) — same default as the user's + primary personal drive. Membership rules from §2 apply: the user + cannot invite co-owners while the drive remains personal. To open + the silo up, the user can later **convert** the secondary + personal to `kind='shared'` (an application-layer operation that + flips the kind and lifts the single-user restriction so the + membership API can add other users / groups). +- **No folder row is deleted.** The wrapper rows survive the + migration as drive root folders; only their `drive_id` is set and + (for the default-Personal case) their `name` is updated. The chroot POC's "pick a drive at login" picker on `feat/nextcloud-drive` already produces the right shape for this: @@ -918,79 +1149,99 @@ every storage query. We phase it for safety: ### Phase A — additive (PR D0) -1. Create `storage.drives` and `storage.drive_members`. +1. Create `storage.drives` (no `name` column — see §3; has + `root_folder_id uuid NULL` populated in step 3). **No + `storage.drive_members` table** — membership lives in + `storage.role_grants` (created in D-Prep) as + `resource_type='drive'` rows. 2. Add `drive_id uuid NULL` to `storage.folders` and `storage.files`. -3. **Per-user root-folder sweep**: for each internal user, list - every `storage.folders` row where `parent_id IS NULL AND +3. **Per-user root-folder adoption sweep**: for each internal user, + list every `storage.folders` row where `parent_id IS NULL AND user_id = `. Exactly one is expected to be `My Folder - `; any extras are SQL-created siblings - (see §10). + (see §10). Each row is **adopted in place** as a drive's root + folder — no row is deleted, no descendant `parent_id` changes, + no path-prefix strip across the whole tree. - The `My Folder - ` row → becomes the **default - personal drive**: `INSERT INTO storage.drives (name='Personal', - kind='personal', default_for_user=, quota_bytes=)` - and insert one `(drive_id, user=, role='owner')` - member row. + personal drive's root folder**. In one CTE statement per + user (same shape as §3's `create_personal_drive`): + - INSERT into `storage.drives` with `kind='personal'`, + `default_for_user=`, `quota_bytes=` + (no `name` column). + - UPDATE the wrapper folder row: set `drive_id=` + and `name='Personal'` (renames the wrapper to the canonical + default name; the BEFORE-UPDATE `path` trigger fires and + cascades the new name down every descendant via the + existing AFTER-UPDATE cascade trigger — no per-row UPDATE + in the migration script). + - UPDATE `storage.drives` to set `root_folder_id=`. + - INSERT one `role_grants` row: subject=``, + `resource_type='drive'`, `resource_id=`, + `role='owner'`. - Every other sibling row → becomes a fresh **secondary - personal drive**: `kind='personal'`, `default_for_user=NULL`, - name carried over from the folder's `name`, - `quota_bytes=`, and one - `(drive_id, user=, role='owner')` member row. - Membership rules from §2 apply (single-owner, no `add_member`); - the user can later promote one to `kind='shared'` to invite - collaborators. -4. **Promote children, drop the wrapper**: for every folder/file - row that has `parent_id = `, set - `drive_id = ` and `parent_id = NULL` - (the drive itself is the new root, not a folder). Then DELETE - the root folder rows from step 3 — they no longer exist as - folders, the drive replaces them. -5. **Cascade `drive_id` down the tree** — for every remaining - folder/file row, set `drive_id` by walking the ancestry to - whichever root the row descends from. After this step every - row has the same `drive_id` as its `parent_id`'s row, which - chains up to a drive set in step 3/4. -6. **Full path-metadata reconstruction**. The `path` column on - **every** row in `storage.folders` and `storage.files` gets - rewritten. For rows that descended from `My Folder - `, - strip that prefix; for rows that descended from a sibling root, - strip that sibling's `name`. The path column now contains only - the in-drive path (e.g. `Docs/foo.pdf`, never - `My Folder - admin/Docs/foo.pdf`). - - **This is the bulk of D0's runtime cost.** Personal-drive - scope = every folder/file the user owns. A 100k-file user - gets 100k UPDATEs. Use a single `UPDATE … WHERE drive_id = - ` per drive, not a row-at-a-time loop. The ltree-path - change is what every downstream subsystem keys off, so doing - this in one transaction per drive (not per row) is also a - correctness boundary. - - **Downstream caches and indexes** — audit each for path or - path-derived keys: + personal drive's root folder**, same four-write CTE shape + except: `default_for_user=NULL`, no `name` change (the + sibling keeps its original name), and the Owner grant points + at the same user. Membership rules from §2 apply + (single-owner, no `add_member`); the user can later promote + one to `kind='shared'` to invite collaborators. +4. **Cascade `drive_id` down the tree** — for every folder/file + row, set `drive_id` by walking the ancestry up to whichever + adopted root the row descends from. After this step every row + has the same `drive_id` as its `parent_id`'s row, which chains + up to a root folder whose `drive_id` was set in step 3. + Reuse the existing ltree-aware recursive helper + (`storage.copy_folder_tree`-style descent) — single + `UPDATE … WHERE` per drive, not a row-at-a-time loop. +5. **No bulk path rewrite.** The `path` column on descendants is + untouched by this migration. The wrapper rename in step 3 + (`My Folder - admin` → `Personal`) is the only path-affecting + change; the BEFORE-UPDATE folder trigger rewrites the wrapper + row's own `path` / `lpath`, and the AFTER-UPDATE cascade + trigger propagates the new path prefix to every descendant + automatically. + - **Downstream caches and indexes** — most are unaffected + because path *content* changes only inside the renamed + wrapper segment (descendants reflect "Personal/…" instead of + "My Folder - admin/…"). Audit: - **Tantivy content index (§11)** — the index does NOT store paths (see `tantivy_content_index.rs`: indexed fields are `file_id`, `user_id`, `name` (basename only), - `content`. No `path` field, the wrapper folder name was - never a term). So the wrapper removal alone requires no - reindex. The reindex §11 calls for is driven by the - schema gaining `drive_id` and the query filter pivoting - from `user_id` to `drive_id` — NOT by the path rewrite. - Same migration window, but for a different reason. - - **Thumbnail cache** — if keyed by path rather than - file_id, invalidate; preferably switch to file_id-keyed - during this migration so the issue doesn't recur. Audit - before D0 starts. + `content`. No `path` field). Reindex IS still required — + not because of paths but because the schema gains + `drive_id` and the query filter pivots from `user_id` to + `drive_id`. Same migration window, different reason. + - **Thumbnail cache** — file_id-keyed: unaffected by the + wrapper rename. Path-keyed entries (if any) invalidate on + any path change in the wrapper; flush as a precaution and + switch to file_id keying during this migration if not + already done. - **Folder ETag queue (`async_tree_etag_queue`,** see Open - Question 8) — flush or recompute; ETags derived from old - paths are stale. + Question 8) — recompute. The wrapper rename touches the + wrapper's own ETag at minimum; ancestors-of-ancestors + below the wrapper are structurally unchanged. - **Recent-items / favorites** — referenced by file_id, not - path; probably fine. Verify. + path; unaffected. - **On-disk storage mirror** — see Open Question 10. If the - filesystem layout is path-mirrored, every file moves on - disk too; if content-addressable, the FS is untouched. - Audit before D0 starts. -7. Verify: every row has `drive_id IS NOT NULL`, no row has - `parent_id` pointing at a non-existent folder, no `path` value - contains the legacy `My Folder - ` prefix. -8. Add `NOT NULL` constraint on `drive_id`. + filesystem layout mirrors `path`, the wrapper directory + itself is renamed (one `mv`) and the descendant directories + don't move; the rename is atomic on the filesystem. If + content-addressable, the FS is untouched. +6. Verify: every row has `drive_id IS NOT NULL`; every drive has + `root_folder_id IS NOT NULL` and pointing at a real folder row + whose `parent_id IS NULL` and whose `drive_id` matches the + drive's id (the 1:1 invariant from §3); no row has `parent_id` + pointing at a non-existent folder. +7. Add `NOT NULL` constraints: `drive_id` on `storage.folders` + and `storage.files`. `root_folder_id` on `storage.drives` + stays NULLable at the column level (§3 explains why — the + atomic CTE writes NULL on the drive INSERT and populates the + column with an UPDATE later in the same statement; a column- + level `NOT NULL` would refuse the initial INSERT). The + invariant "every drive has a root folder" is enforced by the + CTE being the only creation path, not by a constraint. + Verification step 6 checks the invariant on the populated + dataset; ongoing enforcement is application-layer. **Keep `user_id`** on resources alongside `drive_id` for the entire Phase A release cycle. Code is updated to read `drive_id` everywhere; @@ -1129,18 +1380,26 @@ place on `storage.files` / `storage.folders`, which already carry not. 10. **On-disk storage mirror — does the file path under - `OXICLOUD_STORAGE_PATH` change too?** Phase A step 6 strips - the `My Folder - /` prefix from `storage.folders.path` - / `storage.files.path` columns. If the on-disk layout mirrors - these paths (`//My Folder - admin/Docs/foo.pdf`), - the migration also has to `mv` every file on disk. If on-disk - is content-addressable (BLAKE3-keyed), the columns can be - rewritten without touching the filesystem. **Audit the - storage adapter before starting D0** and decide whether the - migration script: - - just renames the path columns (CAS layout — cheap), or - - renames the path columns AND issues a `mv` per file - (path-mirrored layout — expensive on big instances). + `OXICLOUD_STORAGE_PATH` change too?** Phase A step 3 renames + the wrapper folder row (`My Folder - admin` → `Personal`) for + each default personal drive; the AFTER-UPDATE trigger + rewrites descendant `storage.folders.path` / + `storage.files.path` values automatically (no bulk UPDATE in + the migration script). If the on-disk layout mirrors these + paths (`//My Folder - admin/Docs/foo.pdf`), + the migration ALSO has to rename the wrapper directory on + disk — **but only the wrapper directory itself**, one `mv` + per drive, atomic on the filesystem; no descendant `mv` + needed. If on-disk is content-addressable (BLAKE3-keyed), + the columns can be rewritten without touching the filesystem + at all. **Audit the storage adapter before starting D0** and + decide whether the migration script: + - just lets the trigger rewrite the path columns (CAS layout + — cheap), or + - rewrites the path columns AND issues a single `mv` per + drive on disk (path-mirrored layout — still cheap; only + the wrapper directory moves, the subtree comes along for + free). The blob store is content-addressable as of v0.7.0 so most file content lives under `.blobs//` and is already wrapper-agnostic; the concern is only the @@ -1186,11 +1445,13 @@ place on `storage.files` / `storage.folders`, which already carry check uses this to resolve group-owner subjects. - **`folder_service::create_home_folder`** at `src/application/services/folder_service.rs:644` is where the - per-user wrapper folder is created today. Post-migration this - function **goes away** — there is no wrapper folder anymore. The - user-create lifecycle hook now creates a Drive row directly and - inserts the owner-role member row. The lifecycle path is the same; - the work it does shrinks. + per-user wrapper folder is created today. Post-migration the + function is **replaced** by a single `create_personal_drive` + call against `DriveRepository` that runs the §3 atomic CTE: + drive + root folder (named "Personal", `parent_id=NULL`, + `drive_id` pinned) + Owner `role_grants` row, all in one SQL + statement. The lifecycle path is the same; the work moves to + the drive repository. - **NC path resolver `nc_to_internal_path`** at `src/interfaces/nextcloud/webdav_handler.rs:51` and the native resolver `resolve_webdav_path` at @@ -1198,11 +1459,12 @@ place on `storage.files` / `storage.folders`, which already carry callsites that learn about drives. Both gain a "drive context" parameter resolved from the URL prefix (`/files//` or `{user}~{uuid}` for NC; `/webdav/` or `/webdav/drives//` - for native). **Neither resolver prepends `My Folder - /` - anymore** — the storage path IS the in-drive path. Both - functions also get simpler, not more complex, despite gaining - the drive parameter (the personal-vs-shared branch is now a - metadata lookup, not a path-shape decision). + for native). Each resolves to the drive's root folder via + `drives.root_folder_id` and prepends that folder's `path` + (after the migration this is `Personal/…` for default personal + drives, the original sibling-root name for secondaries, the + shared-drive root name for shared drives). The personal-vs-shared + branch is now a single metadata lookup, not a path-shape decision. - **`MagicLinkInviteService`** and the share-notification pipeline (`RecipientNotificationService`) need the new policy checks (`forbid_external_sharing`, `forbid_sharing`) wired in at their @@ -1258,12 +1520,19 @@ test`), **(c)** `cargo fmt && cargo clippy --all-features cleanly with the expected `DomainError`. - **Migration round-trip**: roll forward against a populated DB → every existing folder/file row has `drive_id` set (no NULLs); - every user has exactly one drive with `default_for_user` set; - sibling root folders became secondary `kind='personal'` drives; - every `storage.folders.path` and `storage.files.path` value has - the `My Folder - /` prefix stripped → roll back via - `sqlx migrate revert` → `drive_id` column gone, `user_id` intact - thanks to dual-write, original paths recovered. + every drive has `root_folder_id IS NOT NULL` and the row it + points at has `parent_id IS NULL AND drive_id = ` (the + 1:1 invariant from §3); every user has exactly one drive with + `default_for_user` set; sibling root folders became secondary + `kind='personal'` drives whose root folders kept their original + names; default-personal wrapper folders were renamed from + `My Folder - ` to `Personal` and the AFTER-UPDATE + trigger cascaded the rename down the descendant `path` values + → roll back via `sqlx migrate revert` → `drive_id` / + `root_folder_id` columns gone, `user_id` intact thanks to + dual-write, wrapper folder names restored to + `My Folder - ` (and the trigger cascade restores + descendant paths). - **Storage check**: post-migration `bash tests/api/storage_cleanup_check.sh` still reports a clean tree (no orphans). - **Tantivy reindex**: every indexed doc carries a `drive_id`; @@ -1498,6 +1767,13 @@ static/css/components/driveSwitcher.css ← D1 the `auth.app_passwords.drive_id` binding — see §9). - **Wrapper folder** — historical name for `My Folder - `, the folder created at registration - via `format!("My Folder - {}", username)`. **Retired** in the - Drive migration: drive root replaces it. Every reference to + via `format!("My Folder - {}", username)`. **Adopted** in the + Drive migration: the same folder row is renamed to `Personal` + and reused as the default personal drive's root folder + (`drives.root_folder_id`). No row is deleted; the wrapper IS + the root folder under the new model. Every reference to "wrapper" in older comments / docs is by definition pre-Drive. +- **Drive's root folder** — the folder row pointed at by + `storage.drives.root_folder_id`. `parent_id IS NULL`, + `drive_id` = the drive. Every drive has exactly one (§3); the + drive's display name lives on this folder's `name` column. diff --git a/migrations/20260802000000_drives_schema_additive.sql b/migrations/20260802000000_drives_schema_additive.sql index d0b2b657..18744d69 100644 --- a/migrations/20260802000000_drives_schema_additive.sql +++ b/migrations/20260802000000_drives_schema_additive.sql @@ -20,7 +20,6 @@ CREATE TABLE IF NOT EXISTS storage.drives ( id UUID PRIMARY KEY DEFAULT gen_random_uuid(), - name TEXT NOT NULL, -- Discriminant. Two kinds today; extending the set is a DROP + ADD -- CHECK constraint pair (no separate lookup table). @@ -36,10 +35,24 @@ CREATE TABLE IF NOT EXISTS storage.drives ( default_for_user UUID REFERENCES auth.users(id) ON DELETE CASCADE, + -- The drive's mount-point folder. The display name lives here (drives + -- have no `name` column — see docs/plan/drive.md §3). NULL at the + -- column type level so the atomic creation CTE can INSERT the drive + -- row before the root folder exists, then UPDATE this column from + -- a later CTE branch within the same statement (a column-level + -- NOT NULL would refuse that initial INSERT). The invariant + -- "every drive has a root folder" is enforced by the CTE being + -- the only creation path, plus the M2 backfill populating this + -- column for migrated drives. Code reading this column may treat + -- it as Uuid (not Option); a NULL here is a bug. + root_folder_id UUID + REFERENCES storage.folders(id) ON DELETE CASCADE, + -- Storage quota in bytes. NULL = no quota (admin override / system -- drives). Initial value on personal-drive creation is taken from -- the owner's `auth.users.storage_quota_bytes` at the application - -- layer. + -- layer. **Mutation is OxiCloud-admin only** (docs/plan/drive.md §7) — + -- not in the drive `owner` role bundle. quota_bytes BIGINT, -- Running total of bytes consumed. Maintained by D4's incremental @@ -59,13 +72,19 @@ CREATE TABLE IF NOT EXISTS storage.drives ( ); COMMENT ON TABLE storage.drives IS - 'Drive entity. Top-level container that owns a tree of folders/files; ' - 'membership lives in storage.role_grants with resource_type=''drive''. ' - 'Replaced the per-user My Folder wrapper at D0 (see docs/plan/drive.md).'; + 'Drive entity — pure metadata. The display name and mount point live ' + 'on the root folder (root_folder_id). Membership lives in ' + 'storage.role_grants with resource_type=''drive''. Replaces the ' + 'per-user My Folder wrapper at D0 (see docs/plan/drive.md §3).'; COMMENT ON COLUMN storage.drives.kind IS 'personal = single-owner (no add_member); shared = multi-member with full role roster.'; COMMENT ON COLUMN storage.drives.default_for_user IS 'Set iff this is the user''s default personal drive. NULL on secondaries and shared drives.'; +COMMENT ON COLUMN storage.drives.root_folder_id IS + 'Drive''s root folder. NULLable at the column level only so the ' + 'atomic creation CTE can write it mid-statement; populated invariant ' + 'enforced by application. Display name = SELECT name FROM ' + 'storage.folders WHERE id = root_folder_id.'; COMMENT ON COLUMN storage.drives.policies IS 'JSONB capability-flag bag; see docs/plan/drive.md §8 §15 for known keys.'; diff --git a/migrations/20260802000001_drives_backfill.sql b/migrations/20260802000001_drives_backfill.sql index 058ef921..24811b5c 100644 --- a/migrations/20260802000001_drives_backfill.sql +++ b/migrations/20260802000001_drives_backfill.sql @@ -1,25 +1,29 @@ -- ════════════════════════════════════════════════════════════════════════════ --- D0 / M2 — Drive backfill: create drives + stamp drive_id + provenance +-- D0 / M2 — Drive backfill: adopt wrappers + stamp drive_id + provenance -- ════════════════════════════════════════════════════════════════════════════ --- Second of the D0 migration trio. Half (1) of the §A backfill — the safe, --- focused half: +-- Second of the D0 migration trio. Implements §A of the migration plan in +-- docs/plan/drive.md — the "rename-and-adopt" model: -- --- * For every internal user with a root folder, create a Personal drive. --- * The folder literally named `My Folder - ` becomes the --- user's default Personal drive (`default_for_user = `). --- * Any sibling root folders become secondary Personal drives --- (`default_for_user = NULL`, name carried over verbatim). +-- * For every internal user with a root folder, create a Personal drive +-- (metadata only — no `name` column; the display name lives on the +-- root folder). +-- * The folder literally named `My Folder - ` is **adopted +-- in place** as the user's default Personal drive's root folder: +-- `drives.root_folder_id` points at it, its `drive_id` is stamped, +-- and it is renamed to `Personal`. The wrapper row is NOT deleted; +-- descendants are NOT promoted. The AFTER-UPDATE folder cascade +-- trigger rewrites descendant `path`/`lpath` automatically when the +-- wrapper rename fires — no bulk path UPDATE in this migration. +-- * Any sibling root folders become secondary Personal drives' +-- root folders (`default_for_user = NULL`, original folder name +-- preserved). Same adoption pattern: drive_id stamped, drives.root_folder_id +-- wired, no rename. -- * One owner role_grants row per new drive. -- * Every existing folder/file row gets a `drive_id` (cascaded down the -- ltree from the wrapper). -- * Every existing folder/file row gets `created_by` and `updated_by` -- backfilled from the existing `user_id` column. -- --- The aggressive half (drop the wrapper folder, rewrite path columns, --- strip the `My Folder - /` prefix from every path/lpath value) --- lands in M2b — kept separate so the tree-shape rewrite can be reviewed --- in isolation. --- -- External users (`auth.users.is_external = TRUE`) are intentionally -- skipped — they have no root folder of their own, only role_grants -- against other users' resources. @@ -56,6 +60,49 @@ BEGIN END $BODY$; +-- ── Pre-flight 1b: refuse on rename collision with sibling root 'Personal' ─ +-- The default-wrapper rename in step 4 changes `My Folder - ` → +-- `Personal`. The pre-M3 folder unique index is user_id-scoped +-- (`(name, user_id) WHERE parent_id IS NULL`), so a user who already has +-- a SQL-created sibling root literally named `Personal` would trip the +-- index when M2 tries to rename the wrapper. Surface the collision now — +-- operator renames the offending sibling before retrying, then it gets +-- adopted as a secondary drive with whatever new name it carries. + +DO $BODY$ +DECLARE + collisions BIGINT; +BEGIN + SELECT count(*) INTO collisions + FROM auth.users u + JOIN storage.folders wrapper + ON wrapper.user_id = u.id + AND wrapper.parent_id IS NULL + AND NOT wrapper.is_trashed + AND wrapper.name = 'My Folder - ' || u.username + JOIN storage.folders sibling + ON sibling.user_id = u.id + AND sibling.parent_id IS NULL + AND NOT sibling.is_trashed + AND sibling.id != wrapper.id + AND sibling.name = 'Personal' + WHERE NOT u.is_external; + + IF collisions > 0 THEN + RAISE EXCEPTION + 'D0 backfill refused: % user(s) have both a `My Folder - ` ' + 'wrapper AND a sibling root named ''Personal''. The wrapper rename ' + 'step would collide on the user_id-scoped folder unique index. ' + 'Rename the offending sibling first. Query to inspect: SELECT u.id, ' + 'u.username FROM auth.users u JOIN storage.folders w ON w.user_id=u.id ' + 'AND w.parent_id IS NULL AND w.name=''My Folder - ''||u.username ' + 'JOIN storage.folders s ON s.user_id=u.id AND s.parent_id IS NULL ' + 'AND s.id!=w.id AND s.name=''Personal'' WHERE NOT u.is_external;', + collisions; + END IF; +END $BODY$; + + -- ── Pre-flight 2: report sibling-root distribution (informational) ───────── -- Most users have exactly one root (`My Folder - `). Some may -- have SQL-added siblings — those become secondary drives. Surface the @@ -173,16 +220,15 @@ BEGIN END $BODY$; --- ── 2. Insert the drive rows ─────────────────────────────────────────────── --- Default drives carry the i18n-neutral name 'Personal' (renameable --- later via the drive settings panel). Secondary drives carry their --- original folder name verbatim. +-- ── 2. Insert the drive rows (metadata only — no `name` column) ─────────── +-- Drives are pure metadata under the new design (docs/plan/drive.md §3). +-- The display name lives on the root folder; the wrapper is renamed in +-- step 4b for default drives and kept as-is for secondaries. INSERT INTO storage.drives - (id, name, kind, default_for_user, quota_bytes) + (id, kind, default_for_user, quota_bytes) SELECT p.new_drive_id, - CASE WHEN p.is_default THEN 'Personal' ELSE p.wrapper_name END, 'personal', CASE WHEN p.is_default THEN p.user_id ELSE NULL END, p.quota @@ -199,16 +245,29 @@ SELECT 'user', p.user_id, 'drive', p.new_drive_id, 'owner', p.user_id FROM _drive_plan p; --- ── 4. Stamp drive_id on each wrapper folder ────────────────────────────── --- The wrapper still exists as a folder during M2 (the wrapper-drop lives --- in M2b). Setting drive_id on the wrapper lets the cascade in §5 walk --- the ltree subtree without needing a separate index. +-- ── 4. Adopt the wrapper as the drive's root folder ─────────────────────── +-- 4a. Stamp drive_id on each wrapper so the cascade in §5 can walk the +-- ltree subtree without a separate index. +-- 4b. Rename the default-drive wrapper from `My Folder - ` to +-- `Personal` (the canonical default name; renameable via the folder +-- API later). The BEFORE-UPDATE folder path trigger fires on the +-- rename and the AFTER-UPDATE cascade trigger rewrites every +-- descendant `path` / `lpath` automatically — no per-row UPDATE here. +-- 4c. Wire drives.root_folder_id to the wrapper. This is the adoption +-- step: the wrapper row IS the drive's root folder after M2 (no +-- wrapper-deletion, no descendant promotion). UPDATE storage.folders f - SET drive_id = p.new_drive_id + SET drive_id = p.new_drive_id, + name = CASE WHEN p.is_default THEN 'Personal' ELSE f.name END FROM _drive_plan p WHERE f.id = p.wrapper_id; +UPDATE storage.drives d + SET root_folder_id = p.wrapper_id + FROM _drive_plan p + WHERE d.id = p.new_drive_id; + -- ── 5. Cascade drive_id down the folder tree ────────────────────────────── -- For every folder descended from a wrapper, set drive_id to that @@ -284,6 +343,7 @@ DECLARE grantless_drives BIGINT; null_folder_drive_id BIGINT; null_file_drive_id BIGINT; + rootless_drives BIGINT; BEGIN SELECT count(*) INTO missing_default FROM auth.users u @@ -320,6 +380,29 @@ BEGIN grantless_drives; END IF; + -- Root-folder adoption invariant (docs/plan/drive.md §3): every + -- drive must point at a real folder row whose drive_id closes the + -- cycle. The column is NULLable at the type level so the atomic + -- CTE can write it mid-statement; this check enforces the data + -- invariant after the migration. + SELECT count(*) INTO rootless_drives + FROM storage.drives d + WHERE d.root_folder_id IS NULL + OR NOT EXISTS ( + SELECT 1 FROM storage.folders f + WHERE f.id = d.root_folder_id + AND f.drive_id = d.id + AND f.parent_id IS NULL + ); + IF rootless_drives > 0 THEN + RAISE EXCEPTION + 'D0 backfill consistency check failed: % drive(s) have no ' + 'valid root_folder_id (NULL, or pointing at a folder that ' + 'isn''t a root in this drive). Investigate before declaring ' + 'the migration successful.', + rootless_drives; + END IF; + SELECT count(*) INTO null_folder_drive_id FROM storage.folders f WHERE f.drive_id IS NULL diff --git a/migrations/20260802000002_drives_not_null.sql b/migrations/20260802000002_drives_not_null.sql index b7987a03..2cc3ce3b 100644 --- a/migrations/20260802000002_drives_not_null.sql +++ b/migrations/20260802000002_drives_not_null.sql @@ -56,6 +56,28 @@ CREATE INDEX IF NOT EXISTS idx_folders_drive_id ON storage.folders (drive_id); CREATE INDEX IF NOT EXISTS idx_files_drive_id ON storage.files (drive_id); +-- ── 3b. Drive-scoped folder uniqueness indexes ───────────────────────────── +-- Pre-D0 the "no duplicate folder name under the same parent for the same +-- user" constraint was user_id-scoped (docs/plan/drive.md §10). The +-- semantics users actually want is "no duplicate names *within a drive*" +-- — a folder named "Reports" in your Personal drive shouldn't preclude +-- another "Reports" in a shared "Team" drive. Flip the scope here, now +-- that every row has a drive_id. +-- +-- Same partial predicate as the originals (NOT is_trashed, plus the +-- root-vs-non-root split via parent_id IS NULL). + +DROP INDEX IF EXISTS storage.idx_folders_unique_name; +DROP INDEX IF EXISTS storage.idx_folders_unique_name_root; + +CREATE UNIQUE INDEX IF NOT EXISTS idx_folders_unique_name + ON storage.folders(parent_id, name, drive_id) + WHERE NOT is_trashed AND parent_id IS NOT NULL; +CREATE UNIQUE INDEX IF NOT EXISTS idx_folders_unique_name_root + ON storage.folders(name, drive_id) + WHERE NOT is_trashed AND parent_id IS NULL; + + -- ── 4. Post-flight: confirm constraints landed ──────────────────────────── -- Belt-and-suspenders verification that the NOT NULL + FK actually -- exist after the ALTERs above. Any failure here means PostgreSQL diff --git a/src/application/dtos/drive_dto.rs b/src/application/dtos/drive_dto.rs index 98e4fe3f..42fbfd01 100644 --- a/src/application/dtos/drive_dto.rs +++ b/src/application/dtos/drive_dto.rs @@ -8,7 +8,8 @@ use serde::{Deserialize, Serialize}; use utoipa::ToSchema; use uuid::Uuid; -use crate::domain::entities::drive::{Drive, DriveKind}; +use crate::domain::entities::drive::DriveKind; +use crate::domain::repositories::drive_repository::DriveWithRootName; #[derive(Debug, Clone, Copy, Serialize, Deserialize, ToSchema, PartialEq, Eq)] #[serde(rename_all = "lowercase")] @@ -34,12 +35,22 @@ impl From for DriveKindDto { #[derive(Debug, Clone, Serialize, ToSchema)] pub struct DriveDto { pub id: Uuid, + /// Display name. Sourced from `storage.folders.name` of the row + /// pointed at by `root_folder_id` (drives have no `name` column — + /// see docs/plan/drive.md §3). The wire shape is unchanged from + /// the client's perspective. pub name: String, pub kind: DriveKindDto, #[serde(skip_serializing_if = "Option::is_none")] pub default_for_user: Option, + /// The drive's mount-point folder. Folder API calls + /// (`POST /api/folders { parent_id: }`, + /// `PATCH /api/folders/` to rename) use this id — + /// no polymorphic "create at drive root" surface needed. + pub root_folder_id: Uuid, /// Storage cap in bytes. `None` means "no quota" (admin override / - /// future system drives). + /// future system drives). Mutation is OxiCloud-admin only — drive + /// owners cannot self-grant capacity. #[serde(skip_serializing_if = "Option::is_none")] pub quota_bytes: Option, /// Running total of bytes consumed. Maintained incrementally in D4; @@ -53,18 +64,19 @@ pub struct DriveDto { pub updated_at: chrono::DateTime, } -impl From for DriveDto { - fn from(d: Drive) -> Self { +impl From for DriveDto { + fn from(d: DriveWithRootName) -> Self { Self { - id: d.id, - name: d.name, - kind: d.kind.into(), - default_for_user: d.default_for_user, - quota_bytes: d.quota_bytes, - used_bytes: d.used_bytes, - policies: d.policies, - created_at: d.created_at, - updated_at: d.updated_at, + id: d.drive.id, + name: d.root_folder_name, + kind: d.drive.kind.into(), + default_for_user: d.drive.default_for_user, + root_folder_id: d.drive.root_folder_id, + quota_bytes: d.drive.quota_bytes, + used_bytes: d.drive.used_bytes, + policies: d.drive.policies, + created_at: d.drive.created_at, + updated_at: d.drive.updated_at, } } } diff --git a/src/application/ports/folder_ports.rs b/src/application/ports/folder_ports.rs index 165e11f0..dfae242e 100644 --- a/src/application/ports/folder_ports.rs +++ b/src/application/ports/folder_ports.rs @@ -36,8 +36,19 @@ pub trait FolderUseCase: Send + Sync + 'static { caller_id: Uuid, ) -> Result; - /// Gets a folder by its path - async fn get_folder_by_path(&self, path: &str) -> Result; + /// Gets a folder by its path within the caller's tree. + /// + /// Scoped by `user_id` because `storage.folders.path` is unique + /// only within a single user's drive after D0 — multiple users + /// share names like `"Personal"` for their default-drive root + /// folder (docs/plan/drive.md §10). Pre-D0 the wrapper name + /// embedded the username and made the path globally unique; + /// post-D0 the caller_id filter is required. + async fn get_folder_by_path( + &self, + path: &str, + user_id: Uuid, + ) -> Result; /// Lists folders within a parent folder async fn list_folders(&self, parent_id: Option<&str>) -> Result, DomainError>; diff --git a/src/application/services/folder_service.rs b/src/application/services/folder_service.rs index dbab952f..f00d7583 100644 --- a/src/application/services/folder_service.rs +++ b/src/application/services/folder_service.rs @@ -82,7 +82,11 @@ impl FolderService { Ok(FolderDto::empty()) } - async fn get_folder_by_path(&self, _path: &str) -> Result { + async fn get_folder_by_path( + &self, + _path: &str, + _user_id: Uuid, + ) -> Result { Ok(FolderDto::empty()) } @@ -293,14 +297,17 @@ impl FolderUseCase for FolderService { self.get_folder(id).await } - /// Gets a folder by its path - async fn get_folder_by_path(&self, path: &str) -> Result { - // Convert the string path to StoragePath + /// Gets a folder by its path, scoped to the caller's tree. + async fn get_folder_by_path( + &self, + path: &str, + user_id: Uuid, + ) -> Result { let storage_path = StoragePath::from_string(path); let folder = self .folder_storage - .get_folder_by_path(&storage_path) + .get_folder_by_path(&storage_path, user_id) .await .map_err(|e| { DomainError::internal_error( @@ -766,23 +773,22 @@ use crate::domain::entities::user::User; /// when the Owner row is already present. pub struct PersonalDriveLifecycleHook { drive_repo: Arc, - folder_service: Arc, // The `AuthorizationEngine` trait isn't `dyn`-compatible (native // async-fn-in-trait methods are not object-safe), so we hold the // concrete engine. This matches the convention already used by - // `AppState.authorization`. + // `AppState.authorization`. Only the idempotent-rerun path uses it + // now; the create path goes through the repo's atomic CTE which + // writes the role_grant inline. authorization: Arc, } impl PersonalDriveLifecycleHook { pub fn new( drive_repo: Arc, - folder_service: Arc, authorization: Arc, ) -> Self { Self { drive_repo, - folder_service, authorization, } } @@ -792,9 +798,7 @@ impl PersonalDriveLifecycleHook { /// trait docstring — they have no resources of their own, only /// grants on other users' resources. async fn provision_if_needed(&self, user: &User) -> Result<(), DomainError> { - use crate::domain::repositories::drive_repository::{ - CreatePersonalDriveInput, DriveRepositoryError, - }; + use crate::domain::repositories::drive_repository::DriveRepositoryError; use crate::domain::services::authorization::{Resource, Role, Subject}; if user.is_external() { @@ -802,20 +806,19 @@ impl PersonalDriveLifecycleHook { } // Idempotent shortcut: if the user already has a default drive, - // nothing to do. Covers re-runs from `on_user_login` plus the - // case where `on_user_created` ran successfully but logged in - // before reaching the role_grant step (next-login retry lands - // here and finds the drive, completing the role_grant if missing). + // the atomic CTE already ran on a prior turn. The CTE writes + // the Owner role_grant inline, so there's nothing to repair — + // but we still re-emit the grant via `set_role` (UPSERT-safe) + // to cover the historical case where a pre-CTE provisioning + // path partially completed (drive created, grant missing). match self.drive_repo.find_default_for_user(user.id()).await { - Ok(drive) => { - // Drive exists; ensure the Owner role_grant is in - // place too. `set_role` is an UPSERT — safe to re-run. + Ok(drive_with_name) => { self.authorization .set_role( user.id(), Subject::User(user.id()), Role::Owner, - Resource::Drive(drive.id), + Resource::Drive(drive_with_name.drive.id), None, ) .await @@ -831,49 +834,28 @@ impl PersonalDriveLifecycleHook { } } - // Create the drive. - let drive = self + // One atomic CTE — drive row + root folder ("Personal", + // parent_id=NULL, drive_id pinned) + drives.root_folder_id + // wire-up + Owner role_grant. Single SQL statement, atomic + // against server crash mid-sequence (docs/plan/drive.md §3). + let drive_with_name = self .drive_repo - .create_personal(CreatePersonalDriveInput { - name: "Personal".to_owned(), - owner_id: user.id(), - is_default: true, - quota_bytes: Some(user.storage_quota_bytes()), - }) + .create_personal_drive_atomic(user.id(), Some(user.storage_quota_bytes())) .await .map_err(|e| { - DomainError::internal_error("PersonalDriveHook", format!("create_personal: {e}")) + DomainError::internal_error( + "PersonalDriveHook", + format!("create_personal_drive_atomic: {e}"), + ) })?; - // Stamp the Owner role_grant. - self.authorization - .set_role( - user.id(), - Subject::User(user.id()), - Role::Owner, - Resource::Drive(drive.id), - None, - ) - .await - .map(|_grant| ())?; - - // Provision the wrapper `My Folder - ` folder under - // the new drive. The wrapper is retained through the D0 dual- - // write window (M2b retires it later); without it, existing API - // surfaces that assume `GET /api/folders` returns a root folder - // (the UI listing, the WebDAV resolver, the Hurl baselines) all - // break for newly-provisioned users. - self.folder_service - .ensure_home_folder(user.id(), drive.id, user.username()) - .await - .map(|_created| ())?; - tracing::info!( target: "user_lifecycle", hook = "personal_drive", user_id = %user.id(), - drive_id = %drive.id, - "Default personal drive + wrapper folder provisioned" + drive_id = %drive_with_name.drive.id, + root_folder_id = %drive_with_name.drive.root_folder_id, + "Default personal drive + root folder + owner grant provisioned (atomic CTE)" ); Ok(()) } diff --git a/src/application/services/search_service.rs b/src/application/services/search_service.rs index 379be7fe..e0ed4602 100644 --- a/src/application/services/search_service.rs +++ b/src/application/services/search_service.rs @@ -306,7 +306,7 @@ impl SearchService { .list_for_subjects(&subject_types, &subject_ids) .await { - Ok(drives) => drives.into_iter().map(|d| d.id).collect(), + Ok(drives) => drives.into_iter().map(|d| d.drive.id).collect(), Err(e) => { tracing::warn!("Content-index: drive lookup failed — degrading to empty: {e}"); return Vec::new(); diff --git a/src/application/services/share_service.rs b/src/application/services/share_service.rs index 1a6bbc86..d2366d6f 100644 --- a/src/application/services/share_service.rs +++ b/src/application/services/share_service.rs @@ -925,6 +925,7 @@ mod tests { async fn get_folder_by_path( &self, _storage_path: &crate::domain::services::path_service::StoragePath, + _user_id: uuid::Uuid, ) -> Result { unimplemented!() } diff --git a/src/application/services/trash_service_test.rs b/src/application/services/trash_service_test.rs index 8bb2bb8e..c6582284 100644 --- a/src/application/services/trash_service_test.rs +++ b/src/application/services/trash_service_test.rs @@ -709,6 +709,7 @@ impl FolderRepository for MockFolderRepository { async fn get_folder_by_path( &self, _storage_path: &StoragePath, + _user_id: Uuid, ) -> std::result::Result { unimplemented!() } diff --git a/src/common/di.rs b/src/common/di.rs index 48481918..160c3c2d 100644 --- a/src/common/di.rs +++ b/src/common/di.rs @@ -1202,7 +1202,6 @@ impl AppServiceFactory { .with_hook(Arc::new( crate::application::services::folder_service::PersonalDriveLifecycleHook::new( drive_repo.clone(), - apps.folder_service_concrete.clone(), authorization.clone(), ), )) diff --git a/src/common/stubs.rs b/src/common/stubs.rs index 48d793f7..1ef4a0fc 100644 --- a/src/common/stubs.rs +++ b/src/common/stubs.rs @@ -242,7 +242,11 @@ impl FolderRepository for StubFolderStoragePort { Ok(Folder::default()) } - async fn get_folder_by_path(&self, _storage_path: &StoragePath) -> Result { + async fn get_folder_by_path( + &self, + _storage_path: &StoragePath, + _user_id: Uuid, + ) -> Result { Ok(Folder::default()) } @@ -398,7 +402,11 @@ impl FolderUseCase for StubFolderUseCase { Ok(FolderDto::default()) } - async fn get_folder_by_path(&self, _path: &str) -> Result { + async fn get_folder_by_path( + &self, + _path: &str, + _user_id: Uuid, + ) -> Result { Ok(FolderDto::default()) } diff --git a/src/domain/entities/drive.rs b/src/domain/entities/drive.rs index c991f6a6..fcedb54d 100644 --- a/src/domain/entities/drive.rs +++ b/src/domain/entities/drive.rs @@ -76,29 +76,39 @@ impl DriveKind { /// Domain entity for a row in `storage.drives`. /// +/// Drives are pure metadata under the D0 design (docs/plan/drive.md §3): +/// no `name` column — the display name lives on the root folder pointed +/// at by `root_folder_id`. Code that needs the name pairs this struct +/// with a JOIN through `storage.folders`; see the repository's +/// `DriveWithRootName` view-model. +/// /// Field-level constraints are enforced at the SQL layer (CHECK on /// `kind`, partial UNIQUE on `default_for_user`). The struct mirrors /// the column set 1:1; behaviour beyond field access lives in -/// `DriveRepository` (D0-5) and `DriveService` (post-D0). +/// `DriveRepository` and `DriveService` (post-D0). #[derive(Debug, Clone, PartialEq, Eq)] pub struct Drive { /// Stable identifier. Generated server-side at creation. pub id: Uuid, - /// Display name. Renameable by owners; defaults to "Personal" for - /// the user's default personal drive, or the original sibling-root - /// folder name for secondaries promoted by the D0 backfill. - pub name: String, /// Discriminant — see [`DriveKind`]. pub kind: DriveKind, /// Set iff this is the user's default personal drive (UNIQUE in SQL /// via a partial index `WHERE default_for_user IS NOT NULL`). NULL /// on shared drives and on secondary personal drives. pub default_for_user: Option, + /// The drive's mount-point folder. The column is NULLable in SQL + /// only because the atomic creation CTE writes it mid-statement + /// (a column-level `NOT NULL` would refuse the initial drive INSERT + /// — see docs/plan/drive.md §3). After any successful creation path, + /// this is populated; code reading `Drive` may treat it as `Uuid`, + /// not `Option`. A NULL at read time is a data-invariant bug. + pub root_folder_id: Uuid, /// Soft cap on this drive's storage usage, in bytes. `None` means /// "no quota" (rare; reserved for admin overrides). The default /// initial quota for a fresh personal drive is taken from the - /// owner's `auth.users.storage_quota_bytes` at creation time (see - /// Open Question 2 in `docs/plan/drive.md`). + /// owner's `auth.users.storage_quota_bytes` at creation time. + /// **Mutation is OxiCloud-admin only** (docs/plan/drive.md §7) — + /// not in the drive `owner` role bundle. pub quota_bytes: Option, /// Running total of bytes consumed. Maintained incrementally by /// upload/delete paths in D4; on D0 still reflects the pre-Drive diff --git a/src/domain/repositories/drive_repository.rs b/src/domain/repositories/drive_repository.rs index e800e2ac..a384c9d9 100644 --- a/src/domain/repositories/drive_repository.rs +++ b/src/domain/repositories/drive_repository.rs @@ -37,52 +37,56 @@ pub enum DriveRepositoryError { StorageError(String), } -/// Input parameters for creating a new personal drive. +/// A drive paired with the display name from its root folder. /// -/// Shared drives land in D3 with their own creation surface -/// (`create_shared_drive`). For now D0 only mints personal drives — -/// either as the default for a fresh user (via the lifecycle hook) or -/// as a secondary promoted by the M2 backfill. -#[derive(Debug, Clone)] -pub struct CreatePersonalDriveInput { - /// Display name. The lifecycle hook passes `"Personal"`; the M2 - /// backfill carries over the original sibling-root folder name for - /// secondaries. - pub name: String, - /// The owner. For personal drives the owner is exactly one user. - pub owner_id: Uuid, - /// `true` when this is the user's default drive (sets the partial- - /// unique `default_for_user` column). `false` for secondaries. - pub is_default: bool, - /// Initial storage quota in bytes. `None` defers to admin policy - /// (typically copied from `auth.users.storage_quota_bytes` at the - /// call site). - pub quota_bytes: Option, +/// `storage.drives` has no `name` column under the D0 design +/// (docs/plan/drive.md §3) — the display name lives on +/// `storage.folders.name` of the row pointed at by `drive.root_folder_id`. +/// Read paths join the two tables and hand callers this view-model so the +/// API surface can continue to expose a single "drive with name" shape +/// without a follow-up query per drive. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct DriveWithRootName { + pub drive: Drive, + /// The drive's display name. Sourced from `storage.folders.name` + /// of the root folder via JOIN at read time. + pub root_folder_name: String, } #[async_trait::async_trait] pub trait DriveRepository: Send + Sync + 'static { - /// Insert a personal drive row. The caller is responsible for - /// inserting the matching owner row in `storage.role_grants` in the - /// same transaction (the lifecycle hook handles this; M2's backfill - /// did it directly in SQL). + /// Atomically create a personal drive together with its root folder + /// and the owner role_grant — all four DB writes in a single SQL + /// statement (docs/plan/drive.md §3 "Atomic creation"). The + /// statement runs as its own implicit transaction in autocommit mode + /// so a server crash mid-statement leaves no half-row state. /// - /// Returns `DefaultDriveAlreadyExists` when `is_default=true` and the - /// owner already has a default drive — relies on the partial UNIQUE - /// index on `default_for_user`. - async fn create_personal( + /// The root folder is created with name `"Personal"` (the canonical + /// default) and `parent_id IS NULL`. The drive's `root_folder_id` + /// is wired to point at it before the statement commits. + /// + /// Returns `DefaultDriveAlreadyExists` when the owner already has a + /// default drive — relies on the partial UNIQUE index on + /// `default_for_user`. + async fn create_personal_drive_atomic( &self, - input: CreatePersonalDriveInput, - ) -> Result; + owner_id: Uuid, + quota_bytes: Option, + ) -> Result; - /// Fetch a drive by id. `NotFound` when no row matches. - async fn get_by_id(&self, id: Uuid) -> Result; + /// Fetch a drive by id together with its display name. `NotFound` + /// when no row matches. + async fn get_by_id(&self, id: Uuid) -> Result; - /// Return the caller's default personal drive, or `NotFound` if they - /// don't have one (e.g. external users; users created before the - /// lifecycle hook fired). Drives the Photos timeline scope, the - /// `/api/recent/*` scope, and D1's redirect-from-`/`. - async fn find_default_for_user(&self, user_id: Uuid) -> Result; + /// Return the caller's default personal drive paired with its + /// display name, or `NotFound` if they don't have one (e.g. + /// external users; users created before the lifecycle hook fired). + /// Drives the Photos timeline scope, the `/api/recent/*` scope, and + /// D1's redirect-from-`/`. + async fn find_default_for_user( + &self, + user_id: Uuid, + ) -> Result; /// List drives the caller can read, resolved via `role_grants` for /// `resource_type='drive'`. The caller's group memberships are @@ -90,13 +94,13 @@ pub trait DriveRepository: Send + Sync + 'static { /// is what this method's `subject_ids` argument carries. /// /// Returns rows in a stable order: default drive first (if any), - /// then by name. The `/api/drives` handler relies on that order for - /// the picker UI without a follow-up sort. + /// then by display name. The `/api/drives` handler relies on that + /// order for the picker UI without a follow-up sort. async fn list_for_subjects( &self, subject_types: &[&str], subject_ids: &[Uuid], - ) -> Result, DriveRepositoryError>; + ) -> Result, DriveRepositoryError>; } /// Convenience: convert the canonical kind discriminator from its SQL diff --git a/src/domain/repositories/folder_repository.rs b/src/domain/repositories/folder_repository.rs index 135d6545..642a83e4 100644 --- a/src/domain/repositories/folder_repository.rs +++ b/src/domain/repositories/folder_repository.rs @@ -28,8 +28,18 @@ pub trait FolderRepository: Send + Sync + 'static { /// Gets a folder by its ID async fn get_folder(&self, id: &str) -> Result; - /// Gets a folder by its storage path - async fn get_folder_by_path(&self, storage_path: &StoragePath) -> Result; + /// Gets a folder by its storage path within the caller's tree. + /// + /// Post-D0, `storage.folders.path` is no longer globally unique — + /// multiple users share root-folder names like `"Personal"`. The + /// `user_id` filter scopes the lookup to the caller's own folders + /// (the equivalent of the pre-D0 implicit user-namespacing that + /// came from `My Folder - ` paths). + async fn get_folder_by_path( + &self, + storage_path: &StoragePath, + user_id: Uuid, + ) -> Result; /// Lists folders within a parent folder async fn list_folders(&self, parent_id: Option<&str>) -> Result, DomainError>; diff --git a/src/infrastructure/repositories/pg/drive_pg_repository.rs b/src/infrastructure/repositories/pg/drive_pg_repository.rs index 06a2ad3b..12a60424 100644 --- a/src/infrastructure/repositories/pg/drive_pg_repository.rs +++ b/src/infrastructure/repositories/pg/drive_pg_repository.rs @@ -15,7 +15,7 @@ use sqlx::{PgPool, Row, types::Uuid}; use crate::domain::entities::drive::{Drive, DriveKind}; use crate::domain::repositories::drive_repository::{ - CreatePersonalDriveInput, DriveRepository, DriveRepositoryError, + DriveRepository, DriveRepositoryError, DriveWithRootName, }; pub struct DrivePgRepository { @@ -35,67 +35,159 @@ impl DrivePgRepository { // unique_violation. With drives, the only relevant unique is // the partial index `idx_drives_default_for_user_unique` — // surface the typed variant so the lifecycle hook can detect - // idempotent re-runs (D0-9 calls create_personal during - // user provisioning). + // idempotent re-runs (D0-9 calls create_personal_drive_atomic + // during user provisioning). return DriveRepositoryError::DefaultDriveAlreadyExists(dberr.to_string()); } DriveRepositoryError::StorageError(format!("{context}: {e}")) } - fn row_to_drive(row: &sqlx::postgres::PgRow) -> Result { + /// Map a row carrying both the drive's columns AND a `root_folder_name` + /// column (sourced via JOIN with `storage.folders`) into the view-model. + fn row_to_drive_with_name( + row: &sqlx::postgres::PgRow, + ) -> Result { let kind_str: String = row.get("kind"); let kind = DriveKind::from_sql(&kind_str)?; - Ok(Drive { + let drive = Drive { id: row.get("id"), - name: row.get("name"), kind, default_for_user: row.get("default_for_user"), + root_folder_id: row.get("root_folder_id"), quota_bytes: row.get("quota_bytes"), used_bytes: row.get("used_bytes"), policies: row.get("policies"), created_at: row.get("created_at"), updated_at: row.get("updated_at"), + }; + Ok(DriveWithRootName { + drive, + root_folder_name: row.get("root_folder_name"), }) } } #[async_trait::async_trait] impl DriveRepository for DrivePgRepository { - async fn create_personal( + async fn create_personal_drive_atomic( &self, - input: CreatePersonalDriveInput, - ) -> Result { - let default_for_user = if input.is_default { - Some(input.owner_id) - } else { - None - }; - let row = sqlx::query( + owner_id: Uuid, + quota_bytes: Option, + ) -> Result { + // Four writes wrapped in a single transaction so either all + // commit or none does (docs/plan/drive.md §3). A single CTE + // statement would be cleaner on paper but doesn't work in + // PostgreSQL: CTE sub-statements share an MVCC snapshot, so + // `UPDATE storage.drives WHERE id = …` cannot match a row + // inserted by an earlier CTE branch. We use plain sequential + // statements inside `pool.begin()` instead — each statement + // sees the prior ones' writes (transaction-local visibility), + // and FK constraints are satisfied at insert time because the + // referenced rows already exist. + // + // Rollback semantics: any error before `tx.commit()` (FK + // violation, unique_violation on `default_for_user`, server + // crash) discards every partial write. No orphan drive, no + // folder without a drive, no drive without an owner. + let mut tx = self + .pool + .begin() + .await + .map_err(|e| Self::map_sqlx_err("create_personal_drive_atomic.begin", e))?; + + // 1. Drive row (root_folder_id NULL — populated in step 3). + let drive_id: Uuid = sqlx::query_scalar( r#" INSERT INTO storage.drives - (name, kind, default_for_user, quota_bytes, policies) - VALUES ($1, 'personal', $2, $3, '{}'::jsonb) - RETURNING id, name, kind, default_for_user, quota_bytes, - used_bytes, policies, created_at, updated_at + (kind, default_for_user, quota_bytes, policies) + VALUES ('personal', $1, $2, '{}'::jsonb) + RETURNING id "#, ) - .bind(&input.name) - .bind(default_for_user) - .bind(input.quota_bytes) - .fetch_one(self.pool.as_ref()) + .bind(owner_id) + .bind(quota_bytes) + .fetch_one(&mut *tx) .await - .map_err(|e| Self::map_sqlx_err("create_personal", e))?; + .map_err(|e| Self::map_sqlx_err("create_personal_drive_atomic.drive", e))?; - Self::row_to_drive(&row) - } + // 2. Root folder. `parent_id IS NULL` makes it a root in the + // drive; `drive_id` closes the FK in this direction. + let folder_id: Uuid = sqlx::query_scalar( + r#" + INSERT INTO storage.folders + (name, parent_id, user_id, drive_id, created_by, updated_by) + VALUES ('Personal', NULL, $1, $2, $1, $1) + RETURNING id + "#, + ) + .bind(owner_id) + .bind(drive_id) + .fetch_one(&mut *tx) + .await + .map_err(|e| Self::map_sqlx_err("create_personal_drive_atomic.folder", e))?; - async fn get_by_id(&self, id: Uuid) -> Result { + // 3. Close the other side of the circular reference. + sqlx::query( + r#"UPDATE storage.drives SET root_folder_id = $1 WHERE id = $2"#, + ) + .bind(folder_id) + .bind(drive_id) + .execute(&mut *tx) + .await + .map_err(|e| Self::map_sqlx_err("create_personal_drive_atomic.wire", e))?; + + // 4. Owner role_grant — the caller becomes the drive's sole + // owner (single-user invariant on personal drives, §2). + sqlx::query( + r#" + INSERT INTO storage.role_grants + (subject_type, subject_id, resource_type, resource_id, + role, granted_by) + VALUES ('user', $1, 'drive', $2, 'owner', $1) + "#, + ) + .bind(owner_id) + .bind(drive_id) + .execute(&mut *tx) + .await + .map_err(|e| Self::map_sqlx_err("create_personal_drive_atomic.grant", e))?; + + // Fetch the row in its final state so the caller gets a + // consistent view (including DB-computed defaults like + // `created_at`, `used_bytes`). let row = sqlx::query( r#" - SELECT id, name, kind, default_for_user, quota_bytes, - used_bytes, policies, created_at, updated_at - FROM storage.drives - WHERE id = $1 + SELECT d.id, d.kind, d.default_for_user, d.root_folder_id, + d.quota_bytes, d.used_bytes, d.policies, + d.created_at, d.updated_at, + f.name AS root_folder_name + FROM storage.drives d + JOIN storage.folders f ON f.id = d.root_folder_id + WHERE d.id = $1 + "#, + ) + .bind(drive_id) + .fetch_one(&mut *tx) + .await + .map_err(|e| Self::map_sqlx_err("create_personal_drive_atomic.read", e))?; + + tx.commit() + .await + .map_err(|e| Self::map_sqlx_err("create_personal_drive_atomic.commit", e))?; + + Self::row_to_drive_with_name(&row) + } + + async fn get_by_id(&self, id: Uuid) -> Result { + let row = sqlx::query( + r#" + SELECT d.id, d.kind, d.default_for_user, d.root_folder_id, + d.quota_bytes, d.used_bytes, d.policies, + d.created_at, d.updated_at, + f.name AS root_folder_name + FROM storage.drives d + JOIN storage.folders f ON f.id = d.root_folder_id + WHERE d.id = $1 "#, ) .bind(id) @@ -104,16 +196,22 @@ impl DriveRepository for DrivePgRepository { .map_err(|e| Self::map_sqlx_err("get_by_id", e))? .ok_or_else(|| DriveRepositoryError::NotFound(id.to_string()))?; - Self::row_to_drive(&row) + Self::row_to_drive_with_name(&row) } - async fn find_default_for_user(&self, user_id: Uuid) -> Result { + async fn find_default_for_user( + &self, + user_id: Uuid, + ) -> Result { let row = sqlx::query( r#" - SELECT id, name, kind, default_for_user, quota_bytes, - used_bytes, policies, created_at, updated_at - FROM storage.drives - WHERE default_for_user = $1 + SELECT d.id, d.kind, d.default_for_user, d.root_folder_id, + d.quota_bytes, d.used_bytes, d.policies, + d.created_at, d.updated_at, + f.name AS root_folder_name + FROM storage.drives d + JOIN storage.folders f ON f.id = d.root_folder_id + WHERE d.default_for_user = $1 "#, ) .bind(user_id) @@ -122,39 +220,41 @@ impl DriveRepository for DrivePgRepository { .map_err(|e| Self::map_sqlx_err("find_default_for_user", e))? .ok_or_else(|| DriveRepositoryError::NotFound(user_id.to_string()))?; - Self::row_to_drive(&row) + Self::row_to_drive_with_name(&row) } async fn list_for_subjects( &self, subject_types: &[&str], subject_ids: &[Uuid], - ) -> Result, DriveRepositoryError> { - // Joining `role_grants` → `storage.drives` returns every drive - // the expanded subject set can read. ORDER BY puts default - // drives first (so the picker UI doesn't need a follow-up - // sort), then alphabetical by name. DISTINCT collapses the - // case where a caller has multiple role_grants on the same - // drive (e.g. direct + group-mediated); a GROUP BY on the - // drive id sidesteps PostgreSQL's "ORDER BY expression must - // appear in select list" rule that `SELECT DISTINCT` imposes. + ) -> Result, DriveRepositoryError> { + // Joining role_grants → drives → folders returns every drive the + // expanded subject set can read, paired with its display name. + // ORDER BY puts default drives first (so the picker UI doesn't + // need a follow-up sort), then alphabetical by name. GROUP BY + // collapses duplicate role_grants on the same drive (direct + + // group-mediated) and sidesteps PostgreSQL's "ORDER BY + // expression must appear in select list" rule that SELECT + // DISTINCT imposes. let rows = sqlx::query( r#" - SELECT d.id, d.name, d.kind, d.default_for_user, + SELECT d.id, d.kind, d.default_for_user, d.root_folder_id, d.quota_bytes, d.used_bytes, d.policies, - d.created_at, d.updated_at + d.created_at, d.updated_at, + f.name AS root_folder_name FROM storage.drives d + JOIN storage.folders f ON f.id = d.root_folder_id JOIN storage.role_grants g ON g.resource_type = 'drive' AND g.resource_id = d.id WHERE g.subject_type = ANY($1) AND g.subject_id = ANY($2) AND (g.expires_at IS NULL OR g.expires_at > NOW()) - GROUP BY d.id, d.name, d.kind, d.default_for_user, + GROUP BY d.id, d.kind, d.default_for_user, d.root_folder_id, d.quota_bytes, d.used_bytes, d.policies, - d.created_at, d.updated_at + d.created_at, d.updated_at, f.name ORDER BY (d.default_for_user IS NULL) ASC, - LOWER(d.name) ASC + LOWER(f.name) ASC "#, ) .bind( @@ -168,6 +268,6 @@ impl DriveRepository for DrivePgRepository { .await .map_err(|e| Self::map_sqlx_err("list_for_subjects", e))?; - rows.iter().map(Self::row_to_drive).collect() + rows.iter().map(Self::row_to_drive_with_name).collect() } } diff --git a/src/infrastructure/repositories/pg/folder_db_repository.rs b/src/infrastructure/repositories/pg/folder_db_repository.rs index bc15009a..9f27980a 100644 --- a/src/infrastructure/repositories/pg/folder_db_repository.rs +++ b/src/infrastructure/repositories/pg/folder_db_repository.rs @@ -236,7 +236,11 @@ impl FolderRepository for FolderDbRepository { Self::row_to_folder(row.0, row.1, row.2, row.3, Some(row.4), row.5, row.6, row.7) } - async fn get_folder_by_path(&self, storage_path: &StoragePath) -> Result { + async fn get_folder_by_path( + &self, + storage_path: &StoragePath, + user_id: Uuid, + ) -> Result { let path_str = storage_path.to_string(); // Strip leading '/' if present — DB stores "Home - user/Docs", not "/Home - user/Docs" let lookup = path_str.strip_prefix('/').unwrap_or(&path_str); @@ -245,6 +249,13 @@ impl FolderRepository for FolderDbRepository { return Err(DomainError::not_found("Folder", "empty path")); } + // Scoped by user_id: post-D0 the wrapper folder is named + // "Personal" for every user, so `path = 'Personal'` matches + // every user's root folder. Without the user_id filter, this + // returns a non-deterministic row (whichever the planner emits + // first) — which broke owner-short-circuit checks for the + // caller whose folder wasn't returned. See bug-fix on rewind + // commit. let row = sqlx::query_as::<_, FolderRow>( r#" SELECT id::text, name, path, parent_id::text, user_id, @@ -252,10 +263,11 @@ impl FolderRepository for FolderDbRepository { EXTRACT(EPOCH FROM updated_at)::bigint, EXTRACT(EPOCH FROM tree_modified_at)::bigint FROM storage.folders - WHERE path = $1 AND NOT is_trashed + WHERE path = $1 AND user_id = $2 AND NOT is_trashed "#, ) .bind(lookup) + .bind(user_id) .fetch_optional(self.pool()) .await .map_err(|e| DomainError::internal_error("FolderDb", format!("path lookup: {e}")))? diff --git a/src/interfaces/api/handlers/webdav_handler.rs b/src/interfaces/api/handlers/webdav_handler.rs index 83987e28..ea9f8343 100644 --- a/src/interfaces/api/handlers/webdav_handler.rs +++ b/src/interfaces/api/handlers/webdav_handler.rs @@ -469,7 +469,7 @@ async fn handle_propfind( } } else { // Fallback: legacy double-query path when PathResolver is unavailable - if let Ok(folder) = folder_service.get_folder_by_path(&path).await { + if let Ok(folder) = folder_service.get_folder_by_path(&path, user.id).await { assert_owner(folder.owner_id.as_deref(), &user.id.to_string(), &path)?; let folder_id = folder.id.clone(); return build_streaming_propfind_response( @@ -656,7 +656,7 @@ async fn handle_proppatch( req: Request, path: String, ) -> Result, AppError> { - let _user = extract_user(&req)?; + let user = extract_user(&req)?; // Active-lock guard (RFC 4918 §9.10.4): PROPPATCH writes properties, // so a lock on the target must release them via `If:`. Captured @@ -691,7 +691,7 @@ async fn handle_proppatch( state .applications .folder_service - .get_folder_by_path(&path) + .get_folder_by_path(&path, user.id) .await .is_ok() }; @@ -877,7 +877,7 @@ async fn handle_head( } // Fallback: legacy double-query path (with ownership check) - if let Ok(folder) = folder_service.get_folder_by_path(&path).await { + if let Ok(folder) = folder_service.get_folder_by_path(&path, user.id).await { assert_owner(folder.owner_id.as_deref(), &user.id.to_string(), &path)?; return Ok(Response::builder() .status(StatusCode::OK) @@ -941,7 +941,7 @@ async fn resolve_or_legacy( let user_id_str = user_id.to_string(); let folder_service = &state.applications.folder_service; - if let Ok(folder) = folder_service.get_folder_by_path(path).await + if let Ok(folder) = folder_service.get_folder_by_path(path, user_id).await && folder.owner_id.as_deref() == Some(&user_id_str) { return Some(ResolvedResource::Folder(folder)); @@ -1208,7 +1208,7 @@ async fn handle_mkcol( } accumulated_path.push_str(segment); - match folder_service.get_folder_by_path(&accumulated_path).await { + match folder_service.get_folder_by_path(&accumulated_path, user.id).await { Ok(existing) => { parent_id = Some(existing.id); } @@ -1401,7 +1401,7 @@ async fn handle_move( .unwrap_or(false) } else { folder_service - .get_folder_by_path(&destination_path) + .get_folder_by_path(&destination_path, user.id) .await .is_ok() || file_retrieval_service @@ -1443,7 +1443,7 @@ async fn handle_move( let move_dto = crate::application::dtos::folder_dto::MoveFolderDto { parent_id: if dest_parent_path.is_empty() { None - } else if let Ok(parent) = folder_service.get_folder_by_path(dest_parent_path).await + } else if let Ok(parent) = folder_service.get_folder_by_path(dest_parent_path, user.id).await { assert_owner( parent.owner_id.as_deref(), @@ -1483,7 +1483,7 @@ async fn handle_move( None } else { let parent = folder_service - .get_folder_by_path(dest_parent_path) + .get_folder_by_path(dest_parent_path, user.id) .await .map_err(|_| { AppError::not_found(format!( @@ -1610,7 +1610,7 @@ async fn handle_copy( .unwrap_or(false) } else { folder_service - .get_folder_by_path(&destination_path) + .get_folder_by_path(&destination_path, user.id) .await .is_ok() || file_retrieval_service @@ -1644,7 +1644,7 @@ async fn handle_copy( let target_parent_id = if dest_parent_path.is_empty() { None - } else if let Ok(parent) = folder_service.get_folder_by_path(dest_parent_path).await { + } else if let Ok(parent) = folder_service.get_folder_by_path(dest_parent_path, user.id).await { assert_owner( parent.owner_id.as_deref(), &user.id.to_string(), @@ -1743,7 +1743,7 @@ async fn handle_lock( state .applications .folder_service - .get_folder_by_path(&path) + .get_folder_by_path(&path, user.id) .await .is_ok() }; diff --git a/src/interfaces/nextcloud/ocs_handler.rs b/src/interfaces/nextcloud/ocs_handler.rs index 6c86fb8d..f9d0a4a8 100644 --- a/src/interfaces/nextcloud/ocs_handler.rs +++ b/src/interfaces/nextcloud/ocs_handler.rs @@ -399,10 +399,14 @@ pub async fn handle_search( let mut entries: Vec = Vec::new(); // Map file results + // TODO(D1): drop the hardcoded "Personal/" prefix and read the + // caller's default-drive root folder name from `drives.root_folder_id` + // instead. Correct for D0-provisioned default drives; secondary + // drives keep their original root name. for file in &results.files { let display_path = file .path - .strip_prefix(&format!("My Folder - {}/", user.username)) + .strip_prefix("Personal/") .unwrap_or(&file.path); let display_path = format!("/{}", display_path); @@ -427,11 +431,11 @@ pub async fn handle_search( })); } - // Map folder results + // Map folder results — same TODO(D1) as above. for folder in &results.folders { let display_path = folder .path - .strip_prefix(&format!("My Folder - {}/", user.username)) + .strip_prefix("Personal/") .unwrap_or(&folder.path); let display_path = format!("/{}", display_path); diff --git a/src/interfaces/nextcloud/report_handler.rs b/src/interfaces/nextcloud/report_handler.rs index 9e056d0b..a66cd63a 100644 --- a/src/interfaces/nextcloud/report_handler.rs +++ b/src/interfaces/nextcloud/report_handler.rs @@ -83,7 +83,11 @@ async fn handle_filter_files( // All items in this response are favorites. let favorite_ids: HashSet = favorites.iter().map(|f| f.item_id.clone()).collect(); - let home_prefix = format!("My Folder - {}/", user.username); + // TODO(D1): replace the hardcoded "Personal/" prefix with the + // caller's default-drive root folder name read from + // `drives.root_folder_id`. Correct for D0-provisioned default + // drives; secondary drives keep their original root name. + let home_prefix = "Personal/"; // Pass 1: resolve the favorited DTOs in two batch queries (was one // get_* per favorite — up to N serial round-trips on a sync client's @@ -146,7 +150,7 @@ async fn handle_filter_files( write_multistatus_start(&mut xml)?; for file in &files { - let subpath = strip_home_prefix(&file.path, &home_prefix); + let subpath = strip_home_prefix(&file.path, home_prefix); let href = nc_href(&user.username, subpath); let fid = file_id_map.get(&file.id).copied(); let oc_id = fid.map(|id| format_oc_id(id, file_id_svc)); @@ -163,7 +167,7 @@ async fn handle_filter_files( } for folder in &folders { - let subpath = strip_home_prefix(&folder.path, &home_prefix); + let subpath = strip_home_prefix(&folder.path, home_prefix); let href = format!("{}/", nc_href(&user.username, subpath)); let fid = folder_id_map.get(&folder.id).copied(); let oc_id = fid.map(|id| format_oc_id(id, file_id_svc)); @@ -210,7 +214,7 @@ async fn handle_search( let nresults = parse_nresults(body).unwrap_or(100); // Resolve folder scope from inside . - let folder_id = resolve_scope_folder(&state, body, &user.username).await; + let folder_id = resolve_scope_folder(&state, body, &user.username, user.id).await; let criteria = SearchCriteriaDto { name_contains: Some(term), @@ -227,7 +231,10 @@ async fn handle_search( let nc = state.nextcloud.as_ref(); let file_id_svc = nc.map(|n| &n.file_ids); - let home_prefix = format!("My Folder - {}/", user.username); + // TODO(D1): same as the favorites pass above — replace the + // hardcoded "Personal/" with the caller's actual default-drive + // root folder name from `drives.root_folder_id`. + let home_prefix = "Personal/"; // No favorite checking for search results -- pass an empty set. let favorite_ids: HashSet = HashSet::new(); @@ -249,7 +256,7 @@ async fn handle_search( // Files. for file in &files { - let subpath = strip_home_prefix(&file.path, &home_prefix); + let subpath = strip_home_prefix(&file.path, home_prefix); let href = nc_href(&user.username, subpath); let fid = file_id_map.get(&file.id).copied(); let oc_id = fid.map(|id| format_oc_id(id, file_id_svc)); @@ -267,7 +274,7 @@ async fn handle_search( // Folders. for folder in &folders { - let subpath = strip_home_prefix(&folder.path, &home_prefix); + let subpath = strip_home_prefix(&folder.path, home_prefix); let href = format!("{}/", nc_href(&user.username, subpath)); let fid = folder_id_map.get(&folder.id).copied(); let oc_id = fid.map(|id| format_oc_id(id, file_id_svc)); @@ -461,7 +468,12 @@ fn xml_extract_text(body: &str, local_name: &[u8]) -> Option { } /// Resolve a scope href (e.g. `/files/username/Documents`) to a folder ID. -async fn resolve_scope_folder(state: &AppState, body: &str, username: &str) -> Option { +async fn resolve_scope_folder( + state: &AppState, + body: &str, + username: &str, + user_id: uuid::Uuid, +) -> Option { let href = parse_scope_href(body)?; // The href is typically `/files/{user}/subpath` or `/remote.php/dav/files/{user}/subpath`. @@ -477,7 +489,7 @@ async fn resolve_scope_folder(state: &AppState, body: &str, username: &str) -> O let folder_service = &state.applications.folder_service; folder_service - .get_folder_by_path(&internal_path) + .get_folder_by_path(&internal_path, user_id) .await .ok() .map(|f| f.id) diff --git a/src/interfaces/nextcloud/trashbin_handler.rs b/src/interfaces/nextcloud/trashbin_handler.rs index 8799354b..f40c71c3 100644 --- a/src/interfaces/nextcloud/trashbin_handler.rs +++ b/src/interfaces/nextcloud/trashbin_handler.rs @@ -133,7 +133,7 @@ async fn handle_restore( let file_service = &state.applications.file_retrieval_service; let dest_taken = file_service.get_file_by_path(&dest_internal).await.is_ok() || folder_service - .get_folder_by_path(&dest_internal) + .get_folder_by_path(&dest_internal, user.id) .await .is_ok(); if dest_taken { @@ -248,11 +248,18 @@ fn mime_from_name(name: &str) -> String { .to_string() } -/// Strip the "My Folder - {username}/" prefix from an original path to produce -/// the Nextcloud-relative original location. -fn strip_home_prefix<'a>(original_path: &'a str, username: &str) -> &'a str { - let prefix = format!("My Folder - {}/", username); - original_path.strip_prefix(&prefix).unwrap_or(original_path) +/// Strip the home-folder prefix from an original path to produce the +/// Nextcloud-relative original location. +/// +/// TODO(D1): replace the hardcoded "Personal/" with the caller's actual +/// default-drive root folder name read from `drives.root_folder_id`. +/// Correct for D0-provisioned default drives; secondary drives keep +/// their original root name. The `_username` arg stays for now so the +/// upcoming dynamic lookup has a way to identify the caller. +fn strip_home_prefix<'a>(original_path: &'a str, _username: &str) -> &'a str { + original_path + .strip_prefix("Personal/") + .unwrap_or(original_path) } // ────────────── Trashbin PROPFIND XML Generation ────────────── diff --git a/src/interfaces/nextcloud/uploads_handler.rs b/src/interfaces/nextcloud/uploads_handler.rs index 16dd5b07..193d2e55 100644 --- a/src/interfaces/nextcloud/uploads_handler.rs +++ b/src/interfaces/nextcloud/uploads_handler.rs @@ -256,11 +256,12 @@ async fn handle_assemble( let file_service = &state.applications.file_retrieval_service; let folder_service = &state.applications.folder_service; - let internal_path = format!( - "My Folder - {}/{}", - user.username, - dest_subpath.trim_matches('/') - ); + // TODO(D1): read the caller's default-drive root folder name from + // `drives.root_folder_id` instead of hardcoding "Personal". The + // constant is correct for every default personal drive provisioned + // by the D0 lifecycle hook, but secondary drives (M2 backfill from + // SQL-created sibling root folders) keep their original name. + let internal_path = format!("Personal/{}", dest_subpath.trim_matches('/')); let filename = filename_from_path(&dest_subpath).to_string(); let ingested = ingest_stream_to_cas( @@ -291,15 +292,11 @@ async fn handle_assemble( Some((p, n)) => (p, n), None => ("", dest_subpath.as_str()), }; - let parent_internal = format!( - "My Folder - {}/{}", - user.username, - parent_sub.trim_matches('/') - ); + let parent_internal = format!("Personal/{}", parent_sub.trim_matches('/')); let parent_internal = parent_internal.trim_end_matches('/'); use crate::application::ports::folder_ports::FolderUseCase; - let parent_folder = match folder_service.get_folder_by_path(parent_internal).await { + let parent_folder = match folder_service.get_folder_by_path(parent_internal, user.id).await { Ok(folder) => folder, Err(e) => { discard_ingested(&state.core.dedup_service, &ingested).await; diff --git a/src/interfaces/nextcloud/webdav_handler.rs b/src/interfaces/nextcloud/webdav_handler.rs index 135872cb..b0aa33e7 100644 --- a/src/interfaces/nextcloud/webdav_handler.rs +++ b/src/interfaces/nextcloud/webdav_handler.rs @@ -53,8 +53,15 @@ const HEADER_DAV: HeaderName = HeaderName::from_static("dav"); /// Internal: My Folder - {username}/{subpath} /// /// An empty subpath maps to the user's home folder root. -pub fn nc_to_internal_path(username: &str, subpath: &str) -> Result { - let home = format!("My Folder - {}", username); +pub fn nc_to_internal_path(_username: &str, subpath: &str) -> Result { + // D0: every default personal drive's root folder is named "Personal" + // (docs/plan/drive.md §3 — the canonical post-D0 default). The NC + // dispatcher chroots into the caller's default drive, so the leading + // segment of the internal path is always the drive's root folder + // name. Hardcoded for now; a follow-up will read it from + // `drives.root_folder_id`'s name to support secondary drives with + // custom root-folder names. + let home = "Personal".to_string(); let subpath = subpath.trim_matches('/'); if subpath.is_empty() { return Ok(home); @@ -203,7 +210,7 @@ async fn handle_propfind( let file_service = &state.applications.file_retrieval_service; // Try to resolve as folder first. - let folder_result = folder_service.get_folder_by_path(&internal_path).await; + let folder_result = folder_service.get_folder_by_path(&internal_path, user.id).await; if let Ok(folder) = folder_result { // It's a folder — stream the multistatus: children are fetched in @@ -281,7 +288,7 @@ async fn handle_get( // Check if path is a folder first (NC clients use GET as existence check) if folder_service - .get_folder_by_path(&internal_path) + .get_folder_by_path(&internal_path, user.id) .await .is_ok() { @@ -358,7 +365,7 @@ async fn handle_head( // Check if path is a folder (NC clients use HEAD as existence check) if folder_service - .get_folder_by_path(&internal_path) + .get_folder_by_path(&internal_path, user.id) .await .is_ok() { @@ -433,7 +440,7 @@ async fn handle_proppatch( let folder_service = &state.applications.folder_service; let resource = if let Ok(file) = file_service.get_file_by_path(&internal_path).await { Some((file.id, "file")) - } else if let Ok(folder) = folder_service.get_folder_by_path(&internal_path).await { + } else if let Ok(folder) = folder_service.get_folder_by_path(&internal_path, user.id).await { Some((folder.id, "folder")) } else { None @@ -733,7 +740,7 @@ async fn handle_mkcol( // auto-create doesn't break real clients. if folder_service - .get_folder_by_path(&internal_path) + .get_folder_by_path(&internal_path, user.id) .await .is_ok() { @@ -758,7 +765,7 @@ async fn handle_mkcol( format!("{}/{}", user_root, parent_segments.join("/")) }; - let parent_folder = match folder_service.get_folder_by_path(&parent_path).await { + let parent_folder = match folder_service.get_folder_by_path(&parent_path, user.id).await { Ok(folder) => folder, Err(_) => { return Ok(Response::builder() @@ -797,7 +804,7 @@ async fn handle_delete( // Prefer soft-delete (move to trash) when trash service is available. // This is what Nextcloud clients expect — items appear in the trashbin. if let Some(trash_svc) = state.trash_service.as_ref() { - if let Ok(folder) = folder_service.get_folder_by_path(&internal_path).await { + if let Ok(folder) = folder_service.get_folder_by_path(&internal_path, user.id).await { trash_svc .move_to_trash(&folder.id, "folder", user.id) .await @@ -823,7 +830,7 @@ async fn handle_delete( // Fallback: hard delete when trash service is not available. let file_mgmt = &state.applications.file_management_service; - if let Ok(folder) = folder_service.get_folder_by_path(&internal_path).await { + if let Ok(folder) = folder_service.get_folder_by_path(&internal_path, user.id).await { folder_service .delete_folder_with_perms(&folder.id, user.id) .await @@ -897,7 +904,7 @@ async fn handle_move( .await .ok(); let dest_existing_folder = folder_service - .get_folder_by_path(&dest_internal_precheck) + .get_folder_by_path(&dest_internal_precheck, user.id) .await .ok(); let dest_existed_before = dest_existing_file.is_some() || dest_existing_folder.is_some(); @@ -962,7 +969,7 @@ async fn handle_move( } else { // Different parent → move. let dest_parent = folder_service - .get_folder_by_path(&dest_parent_internal) + .get_folder_by_path(&dest_parent_internal, user.id) .await .map_err(|_| AppError::not_found("Destination folder not found"))?; @@ -997,7 +1004,7 @@ async fn handle_move( } // Try as folder. - if let Ok(folder) = folder_service.get_folder_by_path(&src_internal).await { + if let Ok(folder) = folder_service.get_folder_by_path(&src_internal, user.id).await { let (dest_parent_sub, dest_name) = match dest_subpath.rsplit_once('/') { Some((parent, name)) => (parent, name), None => ("", dest_subpath.as_str()), @@ -1025,7 +1032,7 @@ async fn handle_move( } else { // Different parent → move. let dest_parent = folder_service - .get_folder_by_path(&dest_parent_internal) + .get_folder_by_path(&dest_parent_internal, user.id) .await .map_err(|_| AppError::not_found("Destination parent not found"))?; diff --git a/tests/api/drives_foundation.hurl b/tests/api/drives_foundation.hurl index fe5641f9..55202224 100644 --- a/tests/api/drives_foundation.hurl +++ b/tests/api/drives_foundation.hurl @@ -48,8 +48,14 @@ jsonpath "$" count >= 1 jsonpath "$[0].kind" == "personal" jsonpath "$[0].default_for_user" == "{{admin_user_id}}" jsonpath "$[0].name" == "Personal" +# root_folder_id surfaces the drive's mount-point folder. Sourced via +# JOIN from storage.folders.name — drives have no `name` column under +# the D0 design (docs/plan/drive.md §3). Folder API operations +# (create-in-drive, rename-drive) all key off this id. +jsonpath "$[0].root_folder_id" exists [Captures] admin_drive_id: jsonpath "$[0].id" +admin_root_folder_id: jsonpath "$[0].root_folder_id" # ───────────────────────────────────────────────────────────── @@ -110,6 +116,8 @@ HTTP 200 jsonpath "$" count == 1 jsonpath "$[0].kind" == "personal" jsonpath "$[0].default_for_user" == "{{alice_user_id}}" +jsonpath "$[0].name" == "Personal" +jsonpath "$[0].root_folder_id" exists [Captures] alice_drive_id: jsonpath "$[0].id" @@ -122,6 +130,8 @@ HTTP 200 jsonpath "$" count == 1 jsonpath "$[0].kind" == "personal" jsonpath "$[0].default_for_user" == "{{bob_user_id}}" +jsonpath "$[0].name" == "Personal" +jsonpath "$[0].root_folder_id" exists [Captures] bob_drive_id: jsonpath "$[0].id" diff --git a/tests/api/permissions.hurl b/tests/api/permissions.hurl index 2c1a8052..220489c4 100644 --- a/tests/api/permissions.hurl +++ b/tests/api/permissions.hurl @@ -281,12 +281,16 @@ jsonpath "$.items[*].resource.name" not contains "bob-attack-2" # WebDAV MKCOL — namespace isolation # ═════════════════════════════════════════════════════════════ # WebDAV requests are isolated per-user by `resolve_webdav_path` -# (webdav_handler.rs:189). If the requested path doesn't begin -# with the caller's home folder name ("My Folder - "), -# the handler silently prefixes the caller's home folder path -# onto the front. Effect: any WebDAV path a client sends is -# always resolved INSIDE the caller's own tree, regardless of -# what they wrote. +# (webdav_handler.rs:235). If the requested path doesn't begin +# with the caller's home folder name (the drive's root folder +# name — "Personal" by default post-D0), the handler silently +# prefixes the caller's home folder path onto the front. Effect: +# any WebDAV path a client sends is always resolved INSIDE the +# caller's own tree, regardless of what they wrote. +# The test URLs below use "My Folder - " as a path +# segment that's GUARANTEED not to match any caller's home name +# (all home folders are "Personal" post-D0), so the resolver's +# prepend branch always fires. # # These tests assert the isolation works (regression guard) and # that the service-level verify_owner still acts as @@ -307,8 +311,13 @@ HTTP 201 # ───────────────────────────────────────────────────────────── # Step 17 – Positive control: bob MKCOL inside his own home. +# Uses "Personal" — bob's home folder name post-D0 +# (docs/plan/drive.md §3, the canonical default). The resolver +# detects the URL already starts with the caller's home name and +# does NOT prepend again, so the new folder lands directly in +# bob's home rather than in a fresh intermediate. # ───────────────────────────────────────────────────────────── -MKCOL {{base_url}}/webdav/My%20Folder%20-%20bob/bob-webdav-own +MKCOL {{base_url}}/webdav/Personal/bob-webdav-own Authorization: Bearer {{bob_token}} HTTP 201 diff --git a/tests/common/init-test-schema.sh b/tests/common/init-test-schema.sh index f696bc75..fb36972d 100644 --- a/tests/common/init-test-schema.sh +++ b/tests/common/init-test-schema.sh @@ -43,39 +43,50 @@ psql -v ON_ERROR_STOP=1 -c " " >/dev/null # The OxiCloud server normally provisions a default Personal drive + -# Owner role_grant on user creation via PersonalDriveLifecycleHook -# (D0). This script bypasses that pipeline — it INSERTs directly into -# auth.users — so we mirror the hook's behaviour here. Without it, -# integration test fixtures that hand-roll INSERTs into storage.files -# fail with "drive_id not-null violation" (M3 made the column -# mandatory), and helpers that JOIN auth.users with storage.drives -# return RowNotFound. +# its root folder + Owner role_grant on user creation via +# PersonalDriveLifecycleHook (D0). This script bypasses that pipeline +# — it INSERTs directly into auth.users — so we mirror the hook's +# behaviour here. Without it, integration test fixtures that hand-roll +# INSERTs into storage.files fail with "drive_id not-null violation" +# (M3 made the column mandatory), and helpers that JOIN auth.users +# with storage.drives return RowNotFound. +# +# Four sequential writes inside one transaction (docs/plan/drive.md §3): +# drive + root folder + drives.root_folder_id wire-up + Owner role_grant. +# A single CTE would be more compact but doesn't work — PG's CTE +# sub-statements share an MVCC snapshot, so a later branch's UPDATE +# can't match a row inserted by an earlier branch. The transaction +# form is the production path's shape (DrivePgRepository::create_personal_drive_atomic). +# Idempotency: skipped on retry by the `default_for_user` precondition. echo "[init-schema] provisioning ci-admin's default Personal drive (idempotent)" psql -v ON_ERROR_STOP=1 <<'SQL' >/dev/null -WITH admin AS ( - SELECT id FROM auth.users WHERE username = 'ci-admin' -), -ins_drive AS ( - INSERT INTO storage.drives (name, kind, default_for_user, quota_bytes) - SELECT 'Personal', 'personal', admin.id, NULL - FROM admin - WHERE NOT EXISTS ( - SELECT 1 FROM storage.drives d WHERE d.default_for_user = admin.id - ) - RETURNING id, default_for_user -) -INSERT INTO storage.role_grants - (subject_type, subject_id, resource_type, resource_id, role, granted_by) -SELECT 'user', ins_drive.default_for_user, 'drive', ins_drive.id, 'owner', - ins_drive.default_for_user - FROM ins_drive - WHERE NOT EXISTS ( - SELECT 1 FROM storage.role_grants g - WHERE g.subject_type = 'user' - AND g.subject_id = ins_drive.default_for_user - AND g.resource_type = 'drive' - AND g.resource_id = ins_drive.id - ); +DO $$ +DECLARE + admin_id uuid; + drive_id uuid; + folder_id uuid; +BEGIN + SELECT id INTO admin_id FROM auth.users WHERE username = 'ci-admin'; + IF EXISTS (SELECT 1 FROM storage.drives WHERE default_for_user = admin_id) THEN + RETURN; -- already provisioned, idempotent no-op + END IF; + + INSERT INTO storage.drives (kind, default_for_user, quota_bytes) + VALUES ('personal', admin_id, NULL) + RETURNING id INTO drive_id; + + INSERT INTO storage.folders + (name, parent_id, user_id, drive_id, created_by, updated_by) + VALUES ('Personal', NULL, admin_id, drive_id, admin_id, admin_id) + RETURNING id INTO folder_id; + + UPDATE storage.drives SET root_folder_id = folder_id WHERE id = drive_id; + + INSERT INTO storage.role_grants + (subject_type, subject_id, resource_type, resource_id, role, granted_by) + VALUES ('user', admin_id, 'drive', drive_id, 'owner', admin_id); +END +$$; SQL echo "[init-schema] done"