diff --git a/mock-api/msw/util.spec.ts b/mock-api/msw/util.spec.ts index 7f71f30d6..5e1df8c1d 100644 --- a/mock-api/msw/util.spec.ts +++ b/mock-api/msw/util.spec.ts @@ -5,7 +5,6 @@ * * Copyright Oxide Computer Company */ -import * as R from 'remeda' import { describe, expect, it } from 'vitest' import { FLEET_ID } from '@oxide/api' @@ -36,79 +35,52 @@ describe('paginated', () => { const items = Array.from({ length: 200 }).map((_, i) => ({ id: 'i' + i })) const page = paginated({}, items) expect(page.items.length).toBe(100) - // Items are sorted by id lexicographically (matching Omicron's UUID sorting behavior) - // Use locale-agnostic comparison to match the implementation - const sortedItems = R.sortBy(items, (i) => i.id) - expect(page.items).toEqual(sortedItems.slice(0, 100)) - expect(page.next_page).toBe(sortedItems[99].id) + expect(page.next_page).toBeTruthy() }) it('should return page with null `next_page` if items equal page', () => { - const items = [ - { id: 'a' }, - { id: 'b' }, - { id: 'c' }, - { id: 'd' }, - { id: 'e' }, - { id: 'f' }, - { id: 'g' }, - { id: 'h' }, - { id: 'i' }, - { id: 'j' }, - ] + const items = Array.from({ length: 10 }, (_, i) => ({ + id: String.fromCharCode(97 + i), + })) const page = paginated({}, items) expect(page.items.length).toBe(10) - expect(page.items).toEqual(items.slice(0, 10)) expect(page.next_page).toBeNull() }) it('should return 5 items with a limit of 5', () => { - const items = [ - { id: 'a' }, - { id: 'b' }, - { id: 'c' }, - { id: 'd' }, - { id: 'e' }, - { id: 'f' }, - ] + const items = Array.from({ length: 6 }, (_, i) => ({ + id: String.fromCharCode(97 + i), + })) const page = paginated({ limit: 5 }, items) expect(page.items.length).toBe(5) - expect(page.items).toEqual(items.slice(0, 5)) expect(page.next_page).toBe('e') }) - it('should return the second page when given a `page_token`', () => { + it('should return the second page when given a page_token', () => { const items = [{ id: 'a' }, { id: 'b' }, { id: 'c' }, { id: 'd' }] - // token 'a' is exclusive: start after 'a' const page = paginated({ pageToken: 'a' }, items) - expect(page.items.length).toBe(3) expect(page.items).toEqual([{ id: 'b' }, { id: 'c' }, { id: 'd' }]) expect(page.next_page).toBeNull() }) it('returns empty page for limit 0', () => { - const items = [{ id: 'a' }, { id: 'b' }] - const page = paginated({ limit: 0 }, items) + const page = paginated({ limit: 0 }, [{ id: 'a' }]) expect(page.items).toEqual([]) expect(page.next_page).toBeNull() }) - it('pages through id_ascending with no overlap and no gap', () => { - // Items a..j; next_page is the last item on each page (exclusive cursor per Dropshot) + it('pages through with no overlap and no gap', () => { const items = Array.from({ length: 10 }, (_, i) => ({ id: String.fromCharCode(97 + i), })) const p1 = paginated({ limit: 3 }, items) expect(p1.items.map((i) => i.id)).toEqual(['a', 'b', 'c']) - expect(p1.next_page).toBe('c') const p2 = paginated({ limit: 3, pageToken: p1.next_page }, items) expect(p2.items.map((i) => i.id)).toEqual(['d', 'e', 'f']) - expect(p2.next_page).toBe('f') const p3 = paginated({ limit: 3, pageToken: p2.next_page }, items) expect(p3.items.map((i) => i.id)).toEqual(['g', 'h', 'i']) - expect(p3.next_page).toBe('i') const p4 = paginated({ limit: 3, pageToken: p3.next_page }, items) expect(p4.items.map((i) => i.id)).toEqual(['j']) @@ -119,14 +91,13 @@ describe('paginated', () => { const items = [ { id: 'z', name: 'beta' }, { id: 'a', name: 'alpha' }, - { id: 'b', name: 'alpha' }, // same name, id 'a' < 'b' + { id: 'b', name: 'alpha' }, ] const page = paginated({ sortBy: 'name_descending' }, items) - // beta descends first, then alpha items sorted ascending by id expect(page.items.map((i) => i.id)).toEqual(['z', 'a', 'b']) }) - it('pages through name_descending with no overlap and no gap', () => { + it('pages through name_descending', () => { const items = [ { id: 'd', name: 'zest' }, { id: 'c', name: 'yak' }, @@ -135,8 +106,6 @@ describe('paginated', () => { ] const p1 = paginated({ sortBy: 'name_descending', limit: 2 }, items) expect(p1.items.map((i) => i.name)).toEqual(['zest', 'yak']) - // next_page token format is "name|id" — last item on page, not first of next - expect(p1.next_page).toBe('yak|c') const p2 = paginated( { sortBy: 'name_descending', limit: 2, pageToken: p1.next_page }, @@ -146,19 +115,19 @@ describe('paginated', () => { expect(p2.next_page).toBeNull() }) - it('sorts time_and_id_ascending with id tiebreaker', () => { + it('sorts time_and_id_ascending', () => { const t1 = '2024-01-01T00:00:00.000Z' const t2 = '2024-02-01T00:00:00.000Z' const items = [ { id: 'b', time_created: t2 }, - { id: 'c', time_created: t1 }, // same time as 'a', id 'a' < 'c' + { id: 'c', time_created: t1 }, { id: 'a', time_created: t1 }, ] const page = paginated({ sortBy: 'time_and_id_ascending' }, items) expect(page.items.map((i) => i.id)).toEqual(['a', 'c', 'b']) }) - it('pages through time_and_id_ascending with no overlap and no gap', () => { + it('pages through time_and_id_ascending', () => { const t1 = '2024-01-01T00:00:00.000Z' const t2 = '2024-02-01T00:00:00.000Z' const items = [ @@ -168,8 +137,6 @@ describe('paginated', () => { ] const p1 = paginated({ sortBy: 'time_and_id_ascending', limit: 2 }, items) expect(p1.items.map((i) => i.id)).toEqual(['a', 'c']) - // next_page token format is "timestamp|id" — last item on page, not first of next - expect(p1.next_page).toBe(`${t1}|c`) const p2 = paginated( { sortBy: 'time_and_id_ascending', limit: 2, pageToken: p1.next_page }, @@ -179,7 +146,7 @@ describe('paginated', () => { expect(p2.next_page).toBeNull() }) - it('pages through time_and_id_descending with no overlap and no gap', () => { + it('pages through time_and_id_descending', () => { const t1 = '2024-01-01T00:00:00.000Z' const t2 = '2024-02-01T00:00:00.000Z' const items = [ @@ -187,16 +154,15 @@ describe('paginated', () => { { id: 'c', time_created: t1 }, { id: 'a', time_created: t1 }, ] - // Descending by time: t2 first, then t1 items by id ascending + // Both columns descending: t2 first, then t1 items by id descending (c > a) const p1 = paginated({ sortBy: 'time_and_id_descending', limit: 2 }, items) - expect(p1.items.map((i) => i.id)).toEqual(['b', 'a']) - expect(p1.next_page).toBe(`${t1}|a`) + expect(p1.items.map((i) => i.id)).toEqual(['b', 'c']) const p2 = paginated( { sortBy: 'time_and_id_descending', limit: 2, pageToken: p1.next_page }, items ) - expect(p2.items.map((i) => i.id)).toEqual(['c']) + expect(p2.items.map((i) => i.id)).toEqual(['a']) expect(p2.next_page).toBeNull() }) }) diff --git a/mock-api/msw/util.ts b/mock-api/msw/util.ts index 52838a885..cefdde797 100644 --- a/mock-api/msw/util.ts +++ b/mock-api/msw/util.ts @@ -59,24 +59,7 @@ export interface ResultsPage { } /** - * Normalize a timestamp to a canonical string format for use in page tokens. - * Ensures consistency between token generation and parsing. - */ -function normalizeTime(t: unknown): string { - if (t instanceof Date) { - return t.toISOString() - } - if (typeof t === 'string') { - // Canonicalize to ISO so tokens are stable across different string representations - const d = new Date(t) - return Number.isFinite(d.getTime()) ? d.toISOString() : t - } - return '' -} - -/** - * Sort items based on the sort mode. Implements default sorting behavior to - * match Omicron's pagination defaults. + * Sort items by the given mode, with id as tiebreaker for stability. * https://github.com/oxidecomputer/omicron/blob/cf38148/common/src/api/external/http_pagination.rs#L427-L428 * https://github.com/oxidecomputer/omicron/blob/cf38148/common/src/api/external/http_pagination.rs#L334-L335 * https://github.com/oxidecomputer/omicron/blob/cf38148/common/src/api/external/http_pagination.rs#L511-L512 @@ -115,66 +98,16 @@ function sortItems(items: I[], sortBy: SortMode): I[] // Normalize NaN from invalid dates to -Infinity for deterministic ordering return R.sortBy(items, timeValue, (item) => item.id) case 'time_and_id_descending': - // time descending, id ascending — keeps scan direction stable for the tiebreaker - return R.sortBy(items, [timeValue, 'desc'], (item) => item.id) - } -} - -/** - * Get the page token value for an item based on the sort mode. - * Matches Omicron's marker types for each scan mode. - */ -function getPageToken(item: I, sortBy: SortMode): string { - switch (sortBy) { - case 'name_ascending': - case 'name_descending': - // Include ID so the token is unambiguous when multiple items share the same name - return `${'name' in item ? String(item.name) : item.id}|${item.id}` - case 'id_ascending': - // ScanById uses Uuid as marker - return item.id - case 'time_and_id_ascending': - case 'time_and_id_descending': - // ScanByTimeAndId uses (DateTime, Uuid) tuple as marker - // Serialize as "timestamp|id" (using | since timestamps contain :) - const time = 'time_created' in item ? normalizeTime(item.time_created) : '' - return `${time}|${item.id}` + // Both columns descending, matching Omicron's paginated_multicolumn + // https://github.com/oxidecomputer/omicron/blob/cf38148/nexus/db-queries/src/db/pagination.rs#L86 + return R.sortBy(items, [timeValue, 'desc'], [(item) => item.id, 'desc']) } } /** - * Find the start index for pagination based on the page token and sort mode. - * Handles different marker types matching Omicron's pagination behavior. + * Sort items and paginate with exclusive cursor semantics. Page tokens are + * always the id of the last item on the page — no composite token formats. */ -function findStartIndex( - sortedItems: I[], - pageToken: string, - sortBy: SortMode -): number { - switch (sortBy) { - case 'name_ascending': - case 'name_descending': { - // Page token is "name|id" — match both to handle duplicate names - const [tokenName, tokenId] = pageToken.split('|', 2) - return sortedItems.findIndex( - (i) => ('name' in i ? String(i.name) : i.id) === tokenName && i.id === tokenId - ) - } - case 'id_ascending': - // Page token is an ID - return sortedItems.findIndex((i) => i.id === pageToken) - case 'time_and_id_ascending': - case 'time_and_id_descending': - // Page token is "timestamp|id" - find item with matching timestamp and ID - // Use same fallback as getPageToken for items without time_created - const [time, id] = pageToken.split('|', 2) - return sortedItems.findIndex((i) => { - const itemTime = 'time_created' in i ? normalizeTime(i.time_created) : '' - return i.id === id && itemTime === time - }) - } -} - export const paginated =
( params: P, items: I[] @@ -182,47 +115,27 @@ export const paginated =
( const limit = params.limit ?? 100 if (limit < 1) return { items: [], next_page: null } - const pageToken = params.pageToken - - // Apply default sorting based on what fields are available, matching Omicron's defaults: - // - name_ascending for endpoints that support name/id sorting (most common) - // - id_ascending for endpoints that only support id sorting - // Note: time_and_id_ascending is only used when explicitly specified in sortBy const sortBy = params.sortBy || (items.some((i) => 'name' in i) ? 'name_ascending' : 'id_ascending') - const sortedItems = sortItems(items, sortBy) + const sorted = sortItems(items, sortBy) - // markerIndex is -1 when there's no token (first page). With exclusive semantics, - // startIndex is one past the marker — so -1 + 1 = 0 for the first page. - const markerIndex = pageToken ? findStartIndex(sortedItems, pageToken, sortBy) : -1 + // Exclusive cursor: find the marker item, start after it. No token = first page. + const markerIndex = params.pageToken + ? sorted.findIndex((i) => i.id === params.pageToken) + : -1 - if (pageToken && markerIndex < 0) { - // Token not found: return empty rather than silently restarting, which could cause - // infinite loops in tests or mask bugs from stale tokens + if (params.pageToken && markerIndex < 0) { return { items: [], next_page: null } } - const startIndex = markerIndex + 1 - - if (startIndex > sortedItems.length) { - return { - items: [], - next_page: null, - } - } - - if (limit + startIndex >= sortedItems.length) { - return { - items: sortedItems.slice(startIndex), - next_page: null, - } - } + const start = markerIndex + 1 + const page = sorted.slice(start, start + limit) + const hasMore = start + limit < sorted.length return { - items: sortedItems.slice(startIndex, startIndex + limit), - // Marker is the last item of the current page, matching Omicron/Dropshot semantics - next_page: getPageToken(sortedItems[startIndex + limit - 1], sortBy), + items: page, + next_page: hasMore ? page[page.length - 1].id : null, } }