Skip to content

Commit c02dcda

Browse files
benthecarmanclaude
andcommitted
Paginate by creation time instead of key order
Clients often need to fetch recent payments or entries without iterating over the entire keyspace. Ordering by created_at lets callers retrieve the newest records first and stop early, which is not possible with lexicographic key ordering. The page token now encodes (created_at, key) so the cursor remains unique even when multiple rows share the same timestamp. A composite index on (user_token, store_id, created_at, key) keeps the query efficient, and a migration back-fills any NULL created_at values and adds the NOT NULL constraint. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 53d1fad commit c02dcda

5 files changed

Lines changed: 239 additions & 50 deletions

File tree

proto/vss.proto

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -210,20 +210,16 @@ message ListKeyVersionsRequest {
210210
// Server response for `ListKeyVersions` API.
211211
message ListKeyVersionsResponse {
212212

213-
// Fetched keys and versions.
213+
// Fetched keys and versions, ordered by creation time (newest first) ties broken by key (alphabetically).
214214
// Even though this API reuses the `KeyValue` struct, the `value` sub-field will not be set by the server.
215215
repeated KeyValue key_versions = 1;
216216

217217
// `next_page_token` is a pagination token, used to retrieve the next page of results.
218218
// Use this value to query for next-page of paginated `ListKeyVersions` operation, by specifying
219219
// this value as the `page_token` in the next request.
220220
//
221-
// If `next_page_token` is empty (""), then the "last page" of results has been processed and
222-
// there is no more data to be retrieved.
223-
//
224-
// If `next_page_token` is not empty, it does not necessarily mean that there is more data in the
225-
// result set. The only way to know when you have reached the end of the result set is when
226-
// `next_page_token` is empty.
221+
// If `next_page_token` is not set, then all results have been returned and there is no more data
222+
// to be retrieved.
227223
//
228224
// Caution: Clients must not assume a specific number of key_versions to be present in a page for
229225
// paginated response.

rust/api/src/kv_store_tests.rs

Lines changed: 107 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,10 @@ macro_rules! define_kv_store_tests {
5353
create_test!(list_should_honour_page_size_and_key_prefix_if_provided);
5454
create_test!(list_should_return_zero_global_version_when_global_versioning_not_enabled);
5555
create_test!(list_should_limit_max_page_size);
56+
create_test!(list_should_return_results_ordered_by_creation_time);
57+
create_test!(list_should_paginate_by_creation_time_with_prefix);
58+
create_test!(list_should_return_none_page_token_when_exact_fit);
59+
create_test!(list_should_return_none_page_token_on_last_page);
5660
};
5761
}
5862

@@ -393,12 +397,11 @@ pub trait KvStoreTestSuite {
393397
},
394398
};
395399

396-
if current_page.key_versions.is_empty() {
397-
break;
398-
}
399-
400400
all_key_versions.extend(current_page.key_versions);
401-
next_page_token = current_page.next_page_token;
401+
match current_page.next_page_token {
402+
Some(token) if !token.is_empty() => next_page_token = Some(token),
403+
_ => break,
404+
}
402405
}
403406

404407
if let Some(k1_response) = all_key_versions.iter().find(|kv| kv.key == "k1") {
@@ -444,13 +447,12 @@ pub trait KvStoreTestSuite {
444447
},
445448
};
446449

447-
if current_page.key_versions.is_empty() {
448-
break;
449-
}
450-
451450
assert!(current_page.key_versions.len() <= page_size as usize);
452451
all_key_versions.extend(current_page.key_versions);
453-
next_page_token = current_page.next_page_token;
452+
match current_page.next_page_token {
453+
Some(token) if !token.is_empty() => next_page_token = Some(token),
454+
_ => break,
455+
}
454456
}
455457

456458
let unique_keys: std::collections::HashSet<String> =
@@ -490,12 +492,11 @@ pub trait KvStoreTestSuite {
490492
Some(next_page_token) => ctx.list(Some(next_page_token), None, None).await?,
491493
};
492494

493-
if current_page.key_versions.is_empty() {
494-
break;
495-
}
496-
497495
all_key_versions.extend(current_page.key_versions);
498-
next_page_token = current_page.next_page_token;
496+
match current_page.next_page_token {
497+
Some(token) if !token.is_empty() => next_page_token = Some(token),
498+
_ => break,
499+
}
499500
}
500501

501502
let unique_keys: std::collections::HashSet<String> =
@@ -506,6 +507,93 @@ pub trait KvStoreTestSuite {
506507
Ok(())
507508
}
508509

510+
async fn list_should_return_results_ordered_by_creation_time() -> Result<(), VssError> {
511+
let kv_store = Self::create_store().await;
512+
let ctx = TestContext::new(&kv_store);
513+
514+
ctx.put_objects(None, vec![kv("z_first", "v1", 0)]).await?;
515+
ctx.put_objects(None, vec![kv("a_third", "v1", 0)]).await?;
516+
ctx.put_objects(None, vec![kv("m_second", "v1", 0)]).await?;
517+
518+
let page = ctx.list(None, None, None).await?;
519+
let keys: Vec<&str> = page.key_versions.iter().map(|kv| kv.key.as_str()).collect();
520+
521+
// Results should be in reverse creation order (newest first), not alphabetical.
522+
assert_eq!(keys, vec!["m_second", "a_third", "z_first"]);
523+
524+
Ok(())
525+
}
526+
527+
async fn list_should_paginate_by_creation_time_with_prefix() -> Result<(), VssError> {
528+
let kv_store = Self::create_store().await;
529+
let ctx = TestContext::new(&kv_store);
530+
531+
// Insert prefixed keys in reverse-alphabetical order with a page_size of 1
532+
// to force multiple pages and verify cross-page ordering.
533+
ctx.put_objects(None, vec![kv("pfx_z", "v1", 0)]).await?;
534+
ctx.put_objects(None, vec![kv("pfx_a", "v1", 0)]).await?;
535+
ctx.put_objects(None, vec![kv("other", "v1", 0)]).await?;
536+
ctx.put_objects(None, vec![kv("pfx_m", "v1", 0)]).await?;
537+
538+
let mut next_page_token: Option<String> = None;
539+
let mut all_keys: Vec<String> = Vec::new();
540+
541+
loop {
542+
let current_page = match next_page_token.take() {
543+
None => ctx.list(None, Some(1), Some("pfx_".to_string())).await?,
544+
Some(token) => ctx.list(Some(token), Some(1), Some("pfx_".to_string())).await?,
545+
};
546+
547+
all_keys.extend(current_page.key_versions.into_iter().map(|kv| kv.key));
548+
match current_page.next_page_token {
549+
Some(token) if !token.is_empty() => next_page_token = Some(token),
550+
_ => break,
551+
}
552+
}
553+
554+
// Should get prefixed keys in reverse creation order (newest first), excluding "other".
555+
assert_eq!(all_keys, vec!["pfx_m", "pfx_a", "pfx_z"]);
556+
557+
Ok(())
558+
}
559+
560+
async fn list_should_return_none_page_token_when_exact_fit() -> Result<(), VssError> {
561+
let kv_store = Self::create_store().await;
562+
let ctx = TestContext::new(&kv_store);
563+
564+
let page_size = 5;
565+
for i in 0..page_size {
566+
ctx.put_objects(None, vec![kv(&format!("k{}", i), "v1", 0)]).await?;
567+
}
568+
569+
let page = ctx.list(None, Some(page_size), None).await?;
570+
assert_eq!(page.key_versions.len(), page_size as usize);
571+
assert!(page.next_page_token.is_none(), "expected None when results fit exactly in one page");
572+
573+
Ok(())
574+
}
575+
576+
async fn list_should_return_none_page_token_on_last_page() -> Result<(), VssError> {
577+
let kv_store = Self::create_store().await;
578+
let ctx = TestContext::new(&kv_store);
579+
580+
let page_size = 5;
581+
for i in 0..page_size + 1 {
582+
ctx.put_objects(None, vec![kv(&format!("k{}", i), "v1", 0)]).await?;
583+
}
584+
585+
let first_page = ctx.list(None, Some(page_size), None).await?;
586+
assert_eq!(first_page.key_versions.len(), page_size as usize);
587+
assert!(first_page.next_page_token.is_some(), "expected a page token when more items exist");
588+
589+
let second_page =
590+
ctx.list(first_page.next_page_token, Some(page_size), None).await?;
591+
assert_eq!(second_page.key_versions.len(), 1);
592+
assert!(second_page.next_page_token.is_none(), "expected None on the last page");
593+
594+
Ok(())
595+
}
596+
509597
async fn list_should_limit_max_page_size() -> Result<(), VssError> {
510598
let kv_store = Self::create_store().await;
511599
let ctx = TestContext::new(&kv_store);
@@ -524,16 +612,15 @@ pub trait KvStoreTestSuite {
524612
None => ctx.list(None, None, None).await?,
525613
Some(next_page_token) => ctx.list(Some(next_page_token), None, None).await?,
526614
};
527-
if current_page.key_versions.is_empty() {
528-
break;
529-
}
530-
531615
assert!(
532616
current_page.key_versions.len() < vss_arbitrary_page_size_max as usize,
533617
"Page size exceeds the maximum allowed size"
534618
);
535619
all_key_versions.extend(current_page.key_versions);
536-
next_page_token = current_page.next_page_token;
620+
match current_page.next_page_token {
621+
Some(token) if !token.is_empty() => next_page_token = Some(token),
622+
_ => break,
623+
}
537624
}
538625

539626
assert_eq!(all_key_versions.len(), total_kv_objects as usize);

rust/api/src/types.rs

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -220,12 +220,8 @@ pub struct ListKeyVersionsResponse {
220220
/// Use this value to query for next-page of paginated `ListKeyVersions` operation, by specifying
221221
/// this value as the `page_token` in the next request.
222222
///
223-
/// If `next_page_token` is empty (""), then the "last page" of results has been processed and
224-
/// there is no more data to be retrieved.
225-
///
226-
/// If `next_page_token` is not empty, it does not necessarily mean that there is more data in the
227-
/// result set. The only way to know when you have reached the end of the result set is when
228-
/// `next_page_token` is empty.
223+
/// If `next_page_token` is not set, then all results have been returned and there is no more data
224+
/// to be retrieved.
229225
///
230226
/// Caution: Clients must not assume a specific number of key_versions to be present in a page for
231227
/// paginated response.

rust/impls/src/migrations.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,9 @@ pub(crate) const MIGRATIONS: &[&str] = &[
3535
PRIMARY KEY (user_token, store_id, key)
3636
);",
3737
"ALTER TABLE vss_db DROP CONSTRAINT IF EXISTS vss_db_store_id_check;",
38+
"UPDATE vss_db SET created_at = COALESCE(last_updated_at, NOW()) WHERE created_at IS NULL;",
39+
"ALTER TABLE vss_db ALTER COLUMN created_at SET NOT NULL;",
40+
"CREATE INDEX idx_vss_db_created_at ON vss_db (user_token, store_id, created_at, key);",
3841
];
3942
#[cfg(test)]
4043
pub(crate) const DUMMY_MIGRATION: &str = "SELECT 1 WHERE FALSE;";

0 commit comments

Comments
 (0)