feat(drive): stop writing in {file,folder}.user_id
plan:
```
What step 5 is actually doing
Goal: stop writing to storage.files.user_id / storage.folders.user_id on new INSERTs. This is a prerequisite for step 6 (dropping the column entirely). The column has existed since the pre-D0 single-owner era; every read that used to key on it has already been migrated to drive-membership grants over the last N days.
What has to change together for this to be safe:
┌──────────────┬─────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┬───────────────────────────────────────────────────────────────────────────────────────────────────────┐
│ Piece │ What changes │ Why │
├──────────────┼─────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┼───────────────────────────────────────────────────────────────────────────────────────────────────────┤
│ Migration │ Swap storage.files uniqueness indexes from (folder_id, name, user_id) to (drive_id, folder_id, name) │ Otherwise, two new rows with user_id = NULL would both be allowed (PG treats NULLs as distinct) — │
│ (a) │ │ uniqueness silently breaks │
├──────────────┼─────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┼───────────────────────────────────────────────────────────────────────────────────────────────────────┤
│ Migration │ ALTER user_id DROP NOT NULL on both tables │ Otherwise, dropping the INSERT bind violates NOT NULL and every write 500s │
│ (b) │ │ │
├──────────────┼─────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┼───────────────────────────────────────────────────────────────────────────────────────────────────────┤
│ Migration │ Drop dead user_id-leading indexes │ Cheap cleanup — nothing scans them anymore │
│ (c) │ │ │
├──────────────┼─────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┼───────────────────────────────────────────────────────────────────────────────────────────────────────┤
│ PL/pgSQL (d) │ Rewrite storage.copy_folder_tree without user_id in the INSERT column list │ Cross-drive copy runs entirely in SQL, needs the same treatment │
├──────────────┼─────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┼───────────────────────────────────────────────────────────────────────────────────────────────────────┤
│ Rust (e) │ ~12 INSERT column-list drops across folder_db_repository, file_blob_write_repository, drive_pg_repository, dedup_service, │ Actual write path │
│ │ folder_service, load-seed.rs │ │
├──────────────┼─────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┼───────────────────────────────────────────────────────────────────────────────────────────────────────┤
│ Rust (f) │ Simplify folder_db_repository::create_folder's parent lookup to only fetch drive_id (was fetching (user_id, drive_id)) │ It's fetching a value it no longer needs │
└──────────────┴─────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┴───────────────────────────────────────────────────────────────────────────────────────────────────────┘
```
This commit is contained in:
Edouard Vanbelle
@@ -123,26 +123,64 @@ impl TrashRepository for TrashDbRepository {
|
||||
}
|
||||
|
||||
async fn get_trash_items(&self, user_id: &Uuid) -> Result<Vec<TrashedItem>> {
|
||||
let rows =
|
||||
sqlx::query_as::<_, (Uuid, String, String, Uuid, Option<DateTime<Utc>>, String)>(
|
||||
r#"
|
||||
// Post-D7: the `WHERE t.user_id = $1` filter no longer works —
|
||||
// new rows land with `user_id = NULL`, so the trash view's
|
||||
// `user_id` projection is nullable and can't be the scope
|
||||
// axis any more. Filter by drive-membership instead: any
|
||||
// trashed item in a drive the caller has any role_grant on.
|
||||
// Group memberships expand inline via
|
||||
// `storage.caller_group_ids`. Same predicate shape as
|
||||
// `list_root_folders_for_caller` / the file listings.
|
||||
//
|
||||
// Legacy method — the paginated `list_resources_paged` is
|
||||
// the modern shape and takes explicit drive_ids from the
|
||||
// service layer.
|
||||
let rows = sqlx::query_as::<
|
||||
_,
|
||||
(
|
||||
Uuid,
|
||||
String,
|
||||
String,
|
||||
Option<Uuid>,
|
||||
Option<DateTime<Utc>>,
|
||||
String,
|
||||
),
|
||||
>(
|
||||
r#"
|
||||
SELECT t.id, t.name, t.item_type, t.user_id, t.trashed_at,
|
||||
COALESCE(p.path || '/' || t.name, t.name) AS original_path
|
||||
FROM storage.trash_items t
|
||||
LEFT JOIN storage.folders p ON p.id = t.original_parent_id
|
||||
WHERE t.user_id = $1
|
||||
WHERE EXISTS (
|
||||
SELECT 1 FROM storage.role_grants g
|
||||
WHERE g.resource_type = 'drive'
|
||||
AND g.resource_id = t.drive_id
|
||||
AND (g.expires_at IS NULL OR g.expires_at > NOW())
|
||||
AND (
|
||||
(g.subject_type = 'user' AND g.subject_id = $1)
|
||||
OR (g.subject_type = 'group' AND g.subject_id IN
|
||||
(SELECT storage.caller_group_ids($1)))
|
||||
)
|
||||
)
|
||||
ORDER BY t.trashed_at DESC
|
||||
"#,
|
||||
)
|
||||
.bind(user_id)
|
||||
.fetch_all(self.pool.as_ref())
|
||||
.await
|
||||
.map_err(|e| DomainError::internal_error("TrashDb", format!("list: {e}")))?;
|
||||
)
|
||||
.bind(user_id)
|
||||
.fetch_all(self.pool.as_ref())
|
||||
.await
|
||||
.map_err(|e| DomainError::internal_error("TrashDb", format!("list: {e}")))?;
|
||||
|
||||
Ok(rows
|
||||
.into_iter()
|
||||
.map(|(id, name, item_type, uid, trashed_at, path)| {
|
||||
self.row_to_trashed_item(id, name, item_type, uid, trashed_at, path)
|
||||
self.row_to_trashed_item(
|
||||
id,
|
||||
name,
|
||||
item_type,
|
||||
uid.unwrap_or(*user_id),
|
||||
trashed_at,
|
||||
path,
|
||||
)
|
||||
})
|
||||
.collect())
|
||||
}
|
||||
@@ -153,7 +191,23 @@ impl TrashRepository for TrashDbRepository {
|
||||
// …)` in the service callers (`restore_item`, `delete_permanently`).
|
||||
// The drive precheck in `pg_acl_engine` then resolves Owner-on-drive
|
||||
// → Delete-permission for items in shared drives.
|
||||
let row = sqlx::query_as::<_, (Uuid, String, String, Uuid, Option<DateTime<Utc>>, String)>(
|
||||
//
|
||||
// Post-D7: `t.user_id` is nullable (new rows land NULL). The
|
||||
// entity's `user_id` field is still non-optional; fall back to
|
||||
// `Uuid::nil()` when the view row is NULL. AuthZ decisions
|
||||
// don't consult this field — they've already resolved the
|
||||
// caller's role on the target's drive.
|
||||
let row = sqlx::query_as::<
|
||||
_,
|
||||
(
|
||||
Uuid,
|
||||
String,
|
||||
String,
|
||||
Option<Uuid>,
|
||||
Option<DateTime<Utc>>,
|
||||
String,
|
||||
),
|
||||
>(
|
||||
r#"
|
||||
SELECT t.id, t.name, t.item_type, t.user_id, t.trashed_at,
|
||||
COALESCE(p.path || '/' || t.name, t.name) AS original_path
|
||||
@@ -168,7 +222,14 @@ impl TrashRepository for TrashDbRepository {
|
||||
.map_err(|e| DomainError::internal_error("TrashDb", format!("get: {e}")))?;
|
||||
|
||||
Ok(row.map(|(id, name, item_type, uid, trashed_at, path)| {
|
||||
self.row_to_trashed_item(id, name, item_type, uid, trashed_at, path)
|
||||
self.row_to_trashed_item(
|
||||
id,
|
||||
name,
|
||||
item_type,
|
||||
uid.unwrap_or_else(Uuid::nil),
|
||||
trashed_at,
|
||||
path,
|
||||
)
|
||||
}))
|
||||
}
|
||||
|
||||
@@ -529,7 +590,7 @@ LIMIT $6"
|
||||
size,
|
||||
resource_created_at: row.get("resource_created_at"),
|
||||
modified_at: row.get("modified_at"),
|
||||
owner_id: row.get("owner_id"),
|
||||
owner_id: row.try_get("owner_id").ok(),
|
||||
drive_id: row.get("drive_id"),
|
||||
blob_hash: row.try_get("blob_hash").ok(),
|
||||
trashed_at,
|
||||
|
||||
Reference in New Issue
Block a user