fix(manager-core): F-P-012 keyset cursor on app_user_repo::list (created_at, id) tiebreaker
ORDER BY created_at DESC, id DESC but cursor was `WHERE created_at <
$2` — when two users were created at the same instant, pagination
could skip the second row at a page boundary or return it twice.
- Add ListCursor { created_at, id } with `<rfc3339>_<uuid>` encode /
decode helpers.
- Change WHERE predicate to `(created_at, id) < ($2, $3)` matching the
ORDER BY.
- Surface the opaque cursor string through UsersListOpts /
UsersListPage / users_admin_api ListUsersResponse instead of the raw
DateTime — keeps the wire format stable while the id half stops the
boundary bug.
Same shape as F-P-005 (execution_logs).
AUDIT.md anchor: F-P-012.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -190,10 +190,7 @@ fn bind_list(module: &mut Module, svc: &Arc<dyn UsersService>, cx: &Arc<SdkCallC
|
||||
let cursor = match opts.get("cursor") {
|
||||
None => 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<chrono::DateTime<chrono::Utc>, Box<EvalAltResult>> {
|
||||
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<AppUserId, Box<EvalAltResult>> {
|
||||
uuid::Uuid::parse_str(s)
|
||||
.map(AppUserId::from)
|
||||
|
||||
@@ -117,16 +117,48 @@ pub trait AppUserRepository: Send + Sync {
|
||||
async fn delete(&self, app_id: AppId, id: AppUserId) -> Result<bool, AppUserRepositoryError>;
|
||||
}
|
||||
|
||||
/// 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<Utc>,
|
||||
pub id: picloud_shared::AppUserId,
|
||||
}
|
||||
|
||||
impl ListCursor {
|
||||
/// Encode as `<rfc3339>_<uuid>` — 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<Self> {
|
||||
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<DateTime<Utc>>,
|
||||
pub cursor: Option<ListCursor>,
|
||||
pub limit: i64,
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone)]
|
||||
pub struct ListPage<T> {
|
||||
pub items: Vec<T>,
|
||||
pub next_cursor: Option<DateTime<Utc>>,
|
||||
pub next_cursor: Option<ListCursor>,
|
||||
}
|
||||
|
||||
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<AppUserRow> = 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
|
||||
};
|
||||
|
||||
@@ -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<AppUser>,
|
||||
pub next_cursor: Option<DateTime<Utc>>,
|
||||
/// F-P-012: opaque `<rfc3339>_<uuid>` cursor with id tiebreaker.
|
||||
pub next_cursor: Option<String>,
|
||||
}
|
||||
|
||||
#[derive(Debug, Deserialize)]
|
||||
pub struct ListUsersQuery {
|
||||
pub cursor: Option<DateTime<Utc>>,
|
||||
/// F-P-012: opaque `<rfc3339>_<uuid>` cursor with id tiebreaker.
|
||||
pub cursor: Option<String>,
|
||||
pub limit: Option<i64>,
|
||||
}
|
||||
|
||||
|
||||
@@ -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()),
|
||||
})
|
||||
}
|
||||
|
||||
|
||||
@@ -69,18 +69,20 @@ pub struct UpdateUserInput {
|
||||
pub display_name: Option<Option<String>>,
|
||||
}
|
||||
|
||||
/// 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
|
||||
/// `<rfc3339>_<uuid>` 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<DateTime<Utc>>,
|
||||
pub cursor: Option<String>,
|
||||
pub limit: Option<i64>,
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone)]
|
||||
pub struct UsersListPage {
|
||||
pub items: Vec<AppUser>,
|
||||
pub next_cursor: Option<DateTime<Utc>>,
|
||||
pub next_cursor: Option<String>,
|
||||
}
|
||||
|
||||
/// Newly-minted session — returned exactly once by `users::login` and
|
||||
|
||||
Reference in New Issue
Block a user