Support hierarchical folders and reliable drive browsing

core/migrations/00008_folder_hierarchy.sql

@@ -0,0 +1,19 @@
+-- Existing folders remain roots; names are now unique among siblings.
+ALTER TABLE folders
+    ADD COLUMN parent_id UUID NULL,
+    DROP CONSTRAINT folders_owner_type_owner_id_name_key,
+    ADD CONSTRAINT folders_owner_identity_key UNIQUE (id, owner_type, owner_id),
+    ADD CONSTRAINT folders_parent_owner_fkey
+        FOREIGN KEY (parent_id, owner_type, owner_id)
+        REFERENCES folders (id, owner_type, owner_id) ON DELETE RESTRICT,
+    ADD CONSTRAINT folders_not_own_parent CHECK (parent_id IS DISTINCT FROM id);
+
+CREATE UNIQUE INDEX folders_root_name_key
+    ON folders (owner_type, owner_id, name) WHERE parent_id IS NULL;
+CREATE UNIQUE INDEX folders_sibling_name_key
+    ON folders (owner_type, owner_id, parent_id, name) WHERE parent_id IS NOT NULL;
+CREATE INDEX folders_parent_idx ON folders (parent_id);
+
+-- Support root/direct-child pagination without changing legacy all-files queries.
+CREATE INDEX files_owner_folder_created_idx
+    ON files (owner_type, owner_id, folder_id, created_at DESC, id DESC);

core/src/drive/file_listing_tests.rs

@@ -0,0 +1,224 @@
+use super::*;
+use crate::drive::test_support::{context, setup};
+
+async fn list(
+    state: &AppState,
+    owner: &Owner,
+    query: ListQuery,
+) -> Result<Vec<FileResponse>, AppError> {
+    list_files(context(owner), State(state.clone()), Query(query))
+        .await
+        .map(|response| response.0.items)
+}
+
+#[tokio::test]
+#[ignore = "requires PostgreSQL (BOOSKIFF_DATABASE_URL)"]
+async fn pg_file_root_and_child_filters_paginate_without_dropping_equal_timestamp_rows() {
+    let (state, owner) = setup("root-files").await;
+    let folder: Uuid = sqlx::query_scalar(
+        "INSERT INTO folders (owner_type, owner_id, name) VALUES ($1, $2, 'folder') RETURNING id",
+    )
+    .bind(&owner.owner_type)
+    .bind(&owner.owner_id)
+    .fetch_one(&state.pool)
+    .await
+    .unwrap();
+    let child: Uuid = sqlx::query_scalar("INSERT INTO folders (owner_type, owner_id, name, parent_id) VALUES ($1, $2, 'child', $3) RETURNING id")
+        .bind(&owner.owner_type).bind(&owner.owner_id).bind(folder).fetch_one(&state.pool).await.unwrap();
+    // One timestamp for every row exercises the id tie-breaker across both pages.
+    let timestamp = time::OffsetDateTime::now_utc();
+    let mut root_ids = Vec::new();
+    for i in 0..205 {
+        let id: Uuid = sqlx::query_scalar("INSERT INTO files (owner_type, owner_id, name, mime_type, size_bytes, created_at) VALUES ($1, $2, $3, 'text/plain', 1, $4) RETURNING id")
+            .bind(&owner.owner_type).bind(&owner.owner_id).bind(format!("root-{i}"))
+            .bind(timestamp).fetch_one(&state.pool).await.unwrap();
+        root_ids.push(id);
+    }
+    for folder_id in [folder, child] {
+        sqlx::query("INSERT INTO files (owner_type, owner_id, folder_id, name, mime_type, size_bytes, created_at) VALUES ($1, $2, $3, 'nested', 'text/plain', 1, $4)")
+            .bind(&owner.owner_type).bind(&owner.owner_id).bind(folder_id).bind(timestamp)
+            .execute(&state.pool).await.unwrap();
+    }
+    let first = list(
+        &state,
+        &owner,
+        ListQuery {
+            root: Some(true),
+            limit: Some(200),
+            ..Default::default()
+        },
+    )
+    .await
+    .unwrap();
+    let second = list(
+        &state,
+        &owner,
+        ListQuery {
+            root: Some(true),
+            limit: Some(200),
+            offset: Some(200),
+            ..Default::default()
+        },
+    )
+    .await
+    .unwrap();
+    assert_eq!(first.len(), 200);
+    assert_eq!(second.len(), 5);
+    assert!(first.iter().chain(&second).all(|f| f.folder_id.is_none()));
+    root_ids.sort_by(|a, b| b.cmp(a));
+    assert_eq!(
+        first
+            .iter()
+            .chain(&second)
+            .map(|f| f.id)
+            .collect::<Vec<_>>(),
+        root_ids
+    );
+    let direct = list(
+        &state,
+        &owner,
+        ListQuery {
+            folder_id: Some(folder),
+            ..Default::default()
+        },
+    )
+    .await
+    .unwrap();
+    assert_eq!(direct.len(), 1);
+    assert_eq!(direct[0].folder_id, Some(folder));
+    let legacy = list(&state, &owner, ListQuery::default()).await.unwrap();
+    assert_eq!(legacy.len(), 50);
+    let total: i64 = sqlx::query_scalar("SELECT count(*) FROM files WHERE owner_id = $1")
+        .bind(&owner.owner_id)
+        .fetch_one(&state.pool)
+        .await
+        .unwrap();
+    assert_eq!(total, 207);
+    let legacy_tail = list(
+        &state,
+        &owner,
+        ListQuery {
+            offset: Some(200),
+            limit: Some(200),
+            ..Default::default()
+        },
+    )
+    .await
+    .unwrap();
+    assert_eq!(legacy_tail.len(), 7);
+    let foreign = Owner::new(&owner.owner_type, Uuid::now_v7().to_string());
+    assert!(
+        list(
+            &state,
+            &foreign,
+            ListQuery {
+                root: Some(true),
+                ..Default::default()
+            }
+        )
+        .await
+        .unwrap()
+        .is_empty()
+    );
+    for id in [folder, Uuid::now_v7()] {
+        assert!(matches!(
+            list(
+                &state,
+                &foreign,
+                ListQuery {
+                    folder_id: Some(id),
+                    ..Default::default()
+                }
+            )
+            .await,
+            Err(AppError::NotFound(_))
+        ));
+    }
+    for query in [
+        ListQuery {
+            root: Some(true),
+            folder_id: Some(folder),
+            ..Default::default()
+        },
+        ListQuery {
+            limit: Some(0),
+            ..Default::default()
+        },
+        ListQuery {
+            limit: Some(201),
+            ..Default::default()
+        },
+        ListQuery {
+            offset: Some(-1),
+            ..Default::default()
+        },
+    ] {
+        assert!(matches!(
+            list(&state, &owner, query).await,
+            Err(AppError::Validation(_))
+        ));
+    }
+    let last = first.last().unwrap();
+    // A deletion in an already consumed page shifts legacy offsets, but the
+    // immutable (created_at, id) boundary must still return every remaining file.
+    sqlx::query("DELETE FROM files WHERE id = $1")
+        .bind(first[0].id)
+        .execute(&state.pool)
+        .await
+        .unwrap();
+    let after_delete = list(
+        &state,
+        &owner,
+        ListQuery {
+            root: Some(true),
+            limit: Some(200),
+            before_created_at: Some(last.created_at.clone()),
+            before_id: Some(last.id),
+            ..Default::default()
+        },
+    )
+    .await
+    .unwrap();
+    assert_eq!(
+        after_delete.iter().map(|f| f.id).collect::<Vec<_>>(),
+        second.iter().map(|f| f.id).collect::<Vec<_>>()
+    );
+    for query in [
+        ListQuery {
+            before_created_at: Some(last.created_at.clone()),
+            ..Default::default()
+        },
+        ListQuery {
+            before_id: Some(last.id),
+            ..Default::default()
+        },
+        ListQuery {
+            before_created_at: Some("invalid".into()),
+            before_id: Some(last.id),
+            ..Default::default()
+        },
+        ListQuery {
+            before_created_at: Some(last.created_at.clone()),
+            before_id: Some(last.id),
+            offset: Some(1),
+            ..Default::default()
+        },
+    ] {
+        assert!(matches!(
+            list(&state, &owner, query).await,
+            Err(AppError::Validation(_))
+        ));
+    }
+    sqlx::query("DELETE FROM files WHERE owner_id = $1")
+        .bind(&owner.owner_id)
+        .execute(&state.pool)
+        .await
+        .unwrap();
+    for id in [child, folder] {
+        sqlx::query("DELETE FROM folders WHERE id = $1")
+            .bind(id)
+            .execute(&state.pool)
+            .await
+            .unwrap();
+    }
+}

core/src/drive/files.rs

@@ -57,11 +57,19 @@ pub struct UploadQuery {
     folder_id: Option<Uuid>,
 }
 
-#[derive(Debug, Deserialize)]
+#[derive(Debug, Default, Deserialize, utoipa::IntoParams)]
+#[into_params(parameter_in = Query)]
 pub struct ListQuery {
     limit: Option<i64>,
     offset: Option<i64>,
+    /// List immediate children of this folder.
     folder_id: Option<Uuid>,
+    /// List only root files. Omitted filters preserve legacy all-files listing.
+    root: Option<bool>,
+    /// RFC 3339 created_at of the previous page's last file. Requires before_id.
+    before_created_at: Option<String>,
+    /// ID of the previous page's last file. Requires before_created_at; incompatible with nonzero offset.
+    before_id: Option<Uuid>,
 }
 
 #[derive(Debug, FromRow)]
@@ -208,12 +216,18 @@ async fn upload_file(
     response.map(|file| (StatusCode::CREATED, Json(file)))
 }
 
-#[utoipa::path(get, path = "/v1/files", tag = "files", security(("bearer_auth" = [])), responses((status = 200, body = FileListResponse)))]
+#[utoipa::path(get, path = "/v1/files", tag = "files", security(("bearer_auth" = [])), params(ListQuery), responses((status = 200, body = FileListResponse), (status = 400), (status = 404)))]
 async fn list_files(
     ctx: AccountContext,
     State(state): State<AppState>,
     Query(query): Query<ListQuery>,
 ) -> Result<Json<FileListResponse>, AppError> {
+    let root = query.root.unwrap_or(false);
+    if root && query.folder_id.is_some() {
+        return Err(AppError::Validation(
+            "root and folder_id cannot be combined".into(),
+        ));
+    }
     let limit = query.limit.unwrap_or(DEFAULT_LIST_LIMIT);
     let offset = query.offset.unwrap_or(0);
     if !(1..=MAX_LIST_LIMIT).contains(&limit) {
@@ -224,17 +238,43 @@ async fn list_files(
     if offset < 0 {
         return Err(AppError::Validation("offset must be non-negative".into()));
     }
+    let cursor = match (query.before_created_at.as_deref(), query.before_id) {
+        (None, None) => None,
+        (Some(created_at), Some(id)) => {
+            if offset != 0 {
+                return Err(AppError::Validation(
+                    "cursor and nonzero offset cannot be combined".into(),
+                ));
+            }
+            let created_at = time::OffsetDateTime::parse(created_at, &Rfc3339)
+                .map_err(|_| AppError::Validation("before_created_at must be RFC 3339".into()))?;
+            Some((created_at, id))
+        }
+        _ => {
+            return Err(AppError::Validation(
+                "before_created_at and before_id must be supplied together".into(),
+            ));
+        }
+    };
+    if let Some(folder_id) = query.folder_id {
+        ensure_owned_folder(&state.pool, &ctx.owner, folder_id).await?;
+    }
     let rows = sqlx::query_as::<_, FileRow>(
         "SELECT id, owner_type, owner_id, name, mime_type, size_bytes, folder_id, is_public, created_at \
          FROM files WHERE owner_type = $1 AND owner_id = $2 \
            AND ($3::uuid IS NULL OR folder_id = $3) \
-         ORDER BY created_at DESC LIMIT $4 OFFSET $5",
+           AND (NOT $6 OR folder_id IS NULL) \
+           AND ($7::timestamptz IS NULL OR (created_at, id) < ($7, $8::uuid)) \
+         ORDER BY created_at DESC, id DESC LIMIT $4 OFFSET $5",
     )
     .bind(&ctx.owner.owner_type)
     .bind(&ctx.owner.owner_id)
     .bind(query.folder_id)
     .bind(limit)
     .bind(offset)
+    .bind(root)
+    .bind(cursor.map(|(created_at, _)| created_at))
+    .bind(cursor.map(|(_, id)| id))
     .fetch_all(&state.pool)
     .await
     .map_err(|err| AppError::Internal(format!("list files: {err}")))?;
@@ -302,7 +342,7 @@ async fn download_url(
     State(state): State<AppState>,
     Path(id): Path<Uuid>,
 ) -> Result<Json<UrlResponse>, AppError> {
-    load_owned_file(&state.pool, &ctx.owner, id).await?;
+    let file = load_owned_file(&state.pool, &ctx.owner, id).await?;
     let key: Option<String> = sqlx::query_scalar(
         "SELECT storage_key FROM file_objects WHERE file_id = $1 AND object_kind = $2",
     )
@@ -313,7 +353,7 @@ async fn download_url(
     .map_err(|err| AppError::Internal(format!("load original object: {err}")))?;
     let key = key.ok_or_else(|| AppError::NotFound(format!("file object {id}")))?;
     let ttl = Duration::from_secs(state.config.presigned_get_ttl_secs);
-    let url = state.s3.presign_get(&key, ttl).await?;
+    let url = state.s3.presign_download(&key, ttl, &file.name).await?;
     Ok(Json(UrlResponse { url }))
 }
 
@@ -818,3 +858,7 @@ mod tests {
         pool
     }
 }
+
+#[cfg(test)]
+#[path = "file_listing_tests.rs"]
+mod listing_tests;