diff --git a/crates/executor-core/src/sdk/users.rs b/crates/executor-core/src/sdk/users.rs index 3b50d19..dc22466 100644 --- a/crates/executor-core/src/sdk/users.rs +++ b/crates/executor-core/src/sdk/users.rs @@ -190,10 +190,7 @@ fn bind_list(module: &mut Module, svc: &Arc, cx: &Arc None, Some(d) if d.is_unit() => None, - Some(d) if d.is_string() => { - let s = d.clone().into_string().unwrap_or_default(); - Some(parse_cursor(&s)?) - } + Some(d) if d.is_string() => Some(d.clone().into_string().unwrap_or_default()), Some(_) => return Err(runtime_err("users::list: cursor must be a string or ()")), }; let svc = svc.clone(); @@ -508,17 +505,12 @@ fn list_page_to_map(page: &UsersListPage) -> Map { m.insert( "next_cursor".into(), page.next_cursor - .map_or(Dynamic::UNIT, |d| Dynamic::from(d.to_rfc3339())), + .as_ref() + .map_or(Dynamic::UNIT, |s| Dynamic::from(s.clone())), ); m } -fn parse_cursor(s: &str) -> Result, Box> { - chrono::DateTime::parse_from_rfc3339(s) - .map(|d| d.with_timezone(&chrono::Utc)) - .map_err(|e| runtime_err(&format!("users::list: invalid cursor: {e}"))) -} - fn parse_user_id(s: &str, ctx: &str) -> Result> { uuid::Uuid::parse_str(s) .map(AppUserId::from) diff --git a/crates/manager-core/src/app_user_repo.rs b/crates/manager-core/src/app_user_repo.rs index 23d8c97..5be393a 100644 --- a/crates/manager-core/src/app_user_repo.rs +++ b/crates/manager-core/src/app_user_repo.rs @@ -117,16 +117,48 @@ pub trait AppUserRepository: Send + Sync { async fn delete(&self, app_id: AppId, id: AppUserId) -> Result; } +/// F-P-012: cursor carries `(created_at, id)` for keyset pagination. +/// Without the `id` tiebreaker, two users created at the same instant +/// could be skipped or duplicated at a page boundary. +#[derive(Debug, Clone, Copy)] +pub struct ListCursor { + pub created_at: DateTime, + pub id: picloud_shared::AppUserId, +} + +impl ListCursor { + /// Encode as `_` — what we surface to script-land + /// via `users::list`'s `next_cursor` field. + #[must_use] + pub fn encode(&self) -> String { + format!("{}_{}", self.created_at.to_rfc3339(), self.id.into_inner()) + } + + /// Decode the wire format. Returns `None` on any parse error — the + /// caller treats that as "start of page" rather than 400'ing on a + /// stale cursor from a previous schema. + #[must_use] + pub fn decode(s: &str) -> Option { + let (ts, id) = s.rsplit_once('_')?; + let created_at = DateTime::parse_from_rfc3339(ts).ok()?.to_utc(); + let id = uuid::Uuid::parse_str(id).ok()?; + Some(Self { + created_at, + id: picloud_shared::AppUserId::from(id), + }) + } +} + #[derive(Debug, Clone, Default)] pub struct ListOpts { - pub cursor: Option>, + pub cursor: Option, pub limit: i64, } #[derive(Debug, Clone)] pub struct ListPage { pub items: Vec, - pub next_cursor: Option>, + pub next_cursor: Option, } pub struct PostgresAppUserRepository { @@ -203,11 +235,13 @@ impl AppUserRepository for PostgresAppUserRepository { sqlx::query_as::<_, AppUserRecord>( "SELECT id, app_id, email, display_name, email_verified_at, \ last_login_at, created_at, updated_at \ - FROM app_users WHERE app_id = $1 AND created_at < $2 \ - ORDER BY created_at DESC, id DESC LIMIT $3", + FROM app_users WHERE app_id = $1 \ + AND (created_at, id) < ($2, $3) \ + ORDER BY created_at DESC, id DESC LIMIT $4", ) .bind(app_id.into_inner()) - .bind(cursor) + .bind(cursor.created_at) + .bind(cursor.id.into_inner()) .bind(limit + 1) .fetch_all(&self.pool) .await? @@ -227,7 +261,10 @@ impl AppUserRepository for PostgresAppUserRepository { let mut items: Vec = rows.into_iter().map(Into::into).collect(); let next_cursor = if i64::try_from(items.len()).unwrap_or(i64::MAX) > limit { let cursor_row = items.pop().expect("len > limit so there is a row"); - Some(cursor_row.created_at) + Some(ListCursor { + created_at: cursor_row.created_at, + id: cursor_row.id, + }) } else { None }; diff --git a/crates/manager-core/src/users_admin_api.rs b/crates/manager-core/src/users_admin_api.rs index ea9fd16..e04d780 100644 --- a/crates/manager-core/src/users_admin_api.rs +++ b/crates/manager-core/src/users_admin_api.rs @@ -19,7 +19,6 @@ use axum::http::StatusCode; use axum::response::{IntoResponse, Json, Response}; use axum::routing::{delete, get, post}; use axum::{Extension, Router}; -use chrono::{DateTime, Utc}; use picloud_shared::{ AppId, AppUser, AppUserId, CreateUserInput, EmailTemplateOpts, Invitation, InvitationId, InviteOpts, Principal, UpdateUserInput, UsersError, UsersListOpts, UsersService, @@ -75,12 +74,14 @@ pub fn app_users_router(state: AppUsersState) -> Router { #[derive(Debug, Serialize)] pub struct ListUsersResponse { pub users: Vec, - pub next_cursor: Option>, + /// F-P-012: opaque `_` cursor with id tiebreaker. + pub next_cursor: Option, } #[derive(Debug, Deserialize)] pub struct ListUsersQuery { - pub cursor: Option>, + /// F-P-012: opaque `_` cursor with id tiebreaker. + pub cursor: Option, pub limit: Option, } diff --git a/crates/manager-core/src/users_service.rs b/crates/manager-core/src/users_service.rs index d8755e7..62e37bc 100644 --- a/crates/manager-core/src/users_service.rs +++ b/crates/manager-core/src/users_service.rs @@ -560,7 +560,10 @@ impl UsersService for UsersServiceImpl { .list( cx.app_id, RepoListOpts { - cursor: opts.cursor, + cursor: opts + .cursor + .as_deref() + .and_then(crate::app_user_repo::ListCursor::decode), limit, }, ) @@ -584,7 +587,7 @@ impl UsersService for UsersServiceImpl { .collect(); Ok(UsersListPage { items, - next_cursor: page.next_cursor, + next_cursor: page.next_cursor.as_ref().map(|c| c.encode()), }) } diff --git a/crates/shared/src/users.rs b/crates/shared/src/users.rs index 30e3ef8..8f528df 100644 --- a/crates/shared/src/users.rs +++ b/crates/shared/src/users.rs @@ -69,18 +69,20 @@ pub struct UpdateUserInput { pub display_name: Option>, } -/// Pagination knobs for `users::list`. The cursor is the `created_at` -/// of the last row from the previous page; `None` is "first page". +/// Pagination knobs for `users::list`. The cursor is an opaque +/// `_` string from a previous page's `next_cursor`; +/// `None` is "first page". F-P-012: the id half is the tiebreaker that +/// prevents skip/duplicate at a created_at-equal page boundary. #[derive(Debug, Clone, Default)] pub struct UsersListOpts { - pub cursor: Option>, + pub cursor: Option, pub limit: Option, } #[derive(Debug, Clone)] pub struct UsersListPage { pub items: Vec, - pub next_cursor: Option>, + pub next_cursor: Option, } /// Newly-minted session — returned exactly once by `users::login` and