fix: pagination for null-datetime items and empty fields object

CHANGELOG.md

@@ -9,6 +9,23 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 
 ### Fixed
 
+- Pagination no longer breaks for items without a `datetime` (e.g. when only
+  `start_datetime`/`end_datetime` are set). The default sort now assigns missing
+  datetimes a concrete value instead of the Long sentinel that could not be
+  reused as a `search_after` value, and the `next` token is base64url-encoded so
+  sort values containing commas or nulls survive the round-trip (a plain
+  comma-join corrupted them).
+  ([608](https://github.com/stac-utils/stac-server/issues/608),
+  [1082](https://github.com/stac-utils/stac-server/issues/1082))
+- Pagination no longer skips items when sorting by a `sortby` field that some
+  items lack. A custom sort replaced the default sort's unique tiebreaker, so
+  items sharing a sort value could not be disambiguated across pages; custom
+  sorts now append the `id` tiebreaker the default sort already used.
+  ([608](https://github.com/stac-utils/stac-server/issues/608),
+  [1082](https://github.com/stac-utils/stac-server/issues/1082))
+- GET `/search` and items pagination no longer drop the response body fields on
+  the second page: an empty `fields` object is no longer serialized into the
+  `next` link as `fields={}` (which was re-parsed as a field named `{}`).
 - Link hrefs no longer start with `undefined://` when neither `STAC_API_URL` nor a
   `stac-endpoint`/`X-Forwarded-*` header is set (e.g. behind API Gateway, which does
   not set `X-Forwarded-Proto`/`-Host` by default). The endpoint now falls back to the

src/lib/api.ts

@@ -347,8 +347,17 @@ const buildPaginationLinks = function (
         method: httpMethod,
         type: 'application/geo+json'
       }
+      // Drop empty values, including empty objects/arrays — a bare `fields: {}`
+      // would otherwise serialize into the GET link as `fields={}` and be
+      // re-parsed as a field named "{}", stripping the response _source (#1082).
+      const isPresent = (value: unknown): boolean => {
+        if (value === undefined || value === null || value === '') return false
+        if (typeof value === 'object') return Object.keys(value as object).length > 0
+        return true
+      }
       const nextParams = pickBy(
-        assign(parameters, { bbox, intersects, limit, next: lastItemSort, collections, filter })
+        assign(parameters, { bbox, intersects, limit, next: lastItemSort, collections, filter }),
+        isPresent
       )
       if (httpMethod === 'GET') {
         const nextQueryParameters = dictToURI(nextParams)

src/lib/database.ts

@@ -601,32 +601,57 @@ function buildIdQuery(id: string): OpenSearchBody {
 }
 
 const DEFAULT_SORTING: SortParameters = [
-  { 'properties.datetime': { order: 'desc' } },
+  // `missing: 0` (epoch) gives items without a `datetime` a concrete sort value
+  // instead of OpenSearch's Long sentinel, which loses precision in JS and can't
+  // be reused as a `search_after` value — breaking pagination (#608 / #1082).
+  { 'properties.datetime': { order: 'desc', missing: 0 } },
   { id: { order: 'desc' } },
   { collection: { order: 'desc' } }
 ]
 
 function buildSort(parameters: QueryParameters): SortParameters {
   const { sortby } = parameters
   if (sortby && sortby.length) {
-    return sortby.map((sortRule) => {
+    const rules: SortParameters = sortby.map((sortRule) => {
       const { field, direction } = sortRule
-      return {
-        [field]: {
-          order: direction
-        }
-      }
+      return { [field]: { order: direction } }
     })
+    // `search_after` pagination needs the sort to end in a unique key, or items
+    // that share the user's sort value get skipped or duplicated across pages
+    // (worst with fields that are absent on some items — they all collapse to
+    // the same sentinel value). Append the `id` tiebreaker the default sort
+    // already guarantees, unless the caller is already sorting by it. (#608 /
+    // #1082)
+    if (!rules.some((rule) => 'id' in rule)) {
+      rules.push({ id: { order: 'desc' } })
+    }
+    return rules
   }
   return DEFAULT_SORTING
 }
 
-function buildSearchAfter(parameters: QueryParameters): string[] | undefined {
+function buildSearchAfter(
+  parameters: QueryParameters
+): Array<string | number | null> | undefined {
   const { next } = parameters
-  if (next) {
-    return next.split(',')
+  if (!next) return undefined
+
+  // Current format: base64url-encoded JSON of the OpenSearch sort values. Unlike
+  // a plain comma-join/split, this round-trips values that contain commas (e.g.
+  // an item id or collection id) and preserves nulls instead of coercing them to
+  // empty strings. (It does not rescue the Long sentinel OpenSearch emits for a
+  // missing sort field — that value is already lossy once parsed into a JS
+  // number, before it is ever encoded here. That is handled at the sort level
+  // instead: `missing: 0` on the default `datetime` sort, and a unique `id`
+  // tiebreaker on custom sorts. See buildSort / DEFAULT_SORTING and #608 / #1082.)
+  try {
+    const decoded: unknown = JSON.parse(Buffer.from(next, 'base64url').toString('utf8'))
+    if (Array.isArray(decoded)) return decoded
+  } catch {
+    // Not a base64url-JSON token; fall back to the legacy comma-joined format
+    // so pagination links issued by older versions keep working.
   }
-  return undefined
+  return next.split(',')
 }
 
 /**
@@ -957,9 +982,12 @@ async function search(
   // @ts-ignore -- OpenSearch response body is of unknown shape
   const results = hits.map((r) => (r._source))
   const lastItem = hits.at(-1)
-  let lastItemSort = null
+  let lastItemSort: string | null = null
   if (lastItem && lastItem.sort) {
-    lastItemSort = lastItem.sort.join(',')
+    // base64url-encoded JSON so sort values survive the `next` pagination token
+    // even when they contain commas or nulls (which a comma-join corrupts). See
+    // buildSearchAfter.
+    lastItemSort = Buffer.from(JSON.stringify(lastItem.sort)).toString('base64url')
   }
 
   const response = {

src/lib/types.ts

@@ -226,13 +226,13 @@ export interface DateQuery {
 export interface OpenSearchBody {
   query: OpenSearchFilterQuery
   sort?: SortParameters
-  search_after?: string[] | undefined
+  search_after?: Array<string | number | null> | undefined
   size?: number
   aggs?: Record<string, unknown>
 }
 
 export interface SortRule {
-  [field: string]: { order: string }
+  [field: string]: { order: string, missing?: string | number }
 }
 
 export type SortParameters = SortRule[]

tests/system/test-api-pagination.ts

@@ -0,0 +1,167 @@
+import anyTest, { type TestFn } from 'ava'
+import { deleteAllIndices, refreshIndices } from '../helpers/database.js'
+import { ingestItem } from '../helpers/ingest.js'
+import { randomId, loadFixture } from '../helpers/utils.js'
+import { setup } from '../helpers/system-tests.js'
+import type { StandUpResult } from '../helpers/system-tests.js'
+import type { Link } from '../../src/lib/types.js'
+
+type TestContext = StandUpResult & { collectionId: string }
+const test = anyTest as TestFn<TestContext>
+
+/* eslint-disable @typescript-eslint/no-explicit-any, no-await-in-loop */
+
+const ingest = async (t: any, item: unknown) => ingestItem({
+  ingestQueueUrl: t.context.ingestQueueUrl,
+  ingestTopicArn: t.context.ingestTopicArn,
+  item
+})
+
+// follow a server-issued absolute link href against the local test client
+const followNext = async (t: any, links: Link[]) => {
+  const next = links.find((l) => l.rel === 'next')
+  if (!next) return undefined
+  const path = next.href.replace(/^https?:\/\/[^/]+\//, '')
+  // POST searches return POST-style pagination links (the token is in the body,
+  // not the href), so follow them with the link's method.
+  if (next.method === 'POST') {
+    return t.context.api.client.post(path, {
+      resolveBodyOnly: false,
+      throwHttpErrors: false,
+      json: next.body
+    })
+  }
+  return t.context.api.client.get(path, { resolveBodyOnly: false, throwHttpErrors: false })
+}
+
+test.before(async (t) => {
+  await deleteAllIndices()
+  t.context = (await setup()) as TestContext
+  t.context.collectionId = randomId('collection')
+  await ingest(t, await loadFixture('landsat-8-l1-collection.json', { id: t.context.collectionId }))
+  await refreshIndices()
+})
+
+test.after.always(async (t) => { if (t.context.api) await t.context.api.close() })
+
+// Regression for #608 / #1082: items with a null `datetime` (start/end set
+// instead) broke pagination — OpenSearch emits a Long sentinel sort value that
+// the old comma-joined `next` token corrupted, 400ing the follow-up page.
+test.serial('paginates through items with null datetime', async (t) => {
+  const { collectionId } = t.context
+  const ids: string[] = []
+  for (let i = 0; i < 3; i += 1) {
+    const id = `nodt-${i}`
+    ids.push(id)
+    await ingest(t, await loadFixture('stac/LC80100102015050LGN00.json', {
+      id,
+      collection: collectionId,
+      properties: {
+        datetime: null,
+        start_datetime: `2015-01-0${i + 1}T00:00:00Z`,
+        end_datetime: `2015-01-0${i + 1}T01:00:00Z`
+      }
+    }))
+  }
+  await refreshIndices()
+
+  const seen = new Set<string>()
+  let resp = await t.context.api.client.get(
+    `collections/${collectionId}/items?limit=1`,
+    { resolveBodyOnly: false, throwHttpErrors: false }
+  )
+  for (let page = 0; page < 5; page += 1) {
+    t.is(resp.statusCode, 200, `page ${page} status`)
+    for (const f of resp.body.features) seen.add(f.id)
+    const next = await followNext(t, resp.body.links)
+    if (!next || resp.body.features.length === 0) break
+    resp = next
+  }
+
+  for (const id of ids) t.true(seen.has(id), `paginated to ${id}`)
+})
+
+// Regression for the custom-`sortby` variant of #608 / #1082: a custom sort
+// replaces the default sort's unique `id`/`collection` tiebreakers. When some
+// items lack the sort field they all collapse to the same missing-value sort
+// key, and with no tiebreaker `search_after` can't disambiguate them — the
+// follow-up page skips those items entirely. buildSort must append the `id`
+// tiebreaker to custom sorts, as the default sort already guarantees.
+test.serial('paginates a custom sortby when some items lack the field', async (t) => {
+  const { collectionId } = t.context
+  const ids: string[] = []
+  for (let i = 0; i < 4; i += 1) {
+    const id = `csort-${i}`
+    ids.push(id)
+    // Half the items have `eo:cloud_cover`; half omit it (fixture properties are
+    // shallow-replaced by the override, so omitting the key removes the field).
+    const properties: Record<string, unknown> = {
+      datetime: `2017-01-0${i + 1}T00:00:00Z`
+    }
+    if (i % 2 === 0) properties['eo:cloud_cover'] = i * 10
+    await ingest(t, await loadFixture('stac/LC80100102015050LGN00.json', {
+      id,
+      collection: collectionId,
+      properties
+    }))
+  }
+  await refreshIndices()
+
+  const seen = new Set<string>()
+  let resp = await t.context.api.client.post('search', {
+    resolveBodyOnly: false,
+    throwHttpErrors: false,
+    json: {
+      collections: [collectionId],
+      limit: 1,
+      sortby: [{ field: 'properties.eo:cloud_cover', direction: 'desc' }]
+    }
+  })
+  for (let page = 0; page < 8; page += 1) {
+    t.is(resp.statusCode, 200, `page ${page} status`)
+    for (const f of resp.body.features) seen.add(f.id)
+    const next = await followNext(t, resp.body.links)
+    if (!next || resp.body.features.length === 0) break
+    resp = next
+  }
+
+  for (const id of ids) t.true(seen.has(id), `paginated to ${id}`)
+})
+
+// Regression for #823: a `next` link must be returned even when the `sortby`
+// field is excluded via the fields extension (pagination uses OpenSearch's sort
+// metadata, not the item body).
+test.serial('returns next link when the sortby field is excluded', async (t) => {
+  const { collectionId } = t.context
+  for (let i = 0; i < 3; i += 1) {
+    await ingest(t, await loadFixture('stac/LC80100102015050LGN00.json', {
+      id: `srt-${i}`,
+      collection: collectionId,
+      properties: {
+        datetime: `2016-01-0${i + 1}T00:00:00Z`,
+        updated: `2016-02-0${i + 1}T00:00:00Z`
+      }
+    }))
+  }
+  await refreshIndices()
+
+  const resp = await t.context.api.client.post('search', {
+    resolveBodyOnly: false,
+    json: {
+      collections: [collectionId],
+      limit: 1,
+      sortby: [{ field: 'properties.updated', direction: 'desc' }],
+      fields: { exclude: ['properties.updated'], include: [] }
+    }
+  })
+  t.is(resp.statusCode, 200)
+  t.is(resp.body.features.length, 1)
+  const next = resp.body.links.find((l: Link) => l.rel === 'next')
+  t.truthy(next, 'next link present despite excluded sortby field')
+
+  // Following the (POST) next link must return the *next* item, not repeat page 1.
+  const page2 = await followNext(t, resp.body.links)
+  t.is(page2?.statusCode, 200, 'second page fetches successfully')
+  t.is(page2?.body.features.length, 1)
+  t.not(page2?.body.features[0].id, resp.body.features[0].id, 'page 2 is a different item')
+})

tests/system/test-api-search-get.ts

@@ -76,12 +76,15 @@ test('/search preserve bbox in next links', async (t) => {
   const nextUrl = new URL(nextLink.href)
   t.deepEqual(nextUrl.searchParams.get('bbox'), bbox)
 
-  t.deepEqual(nextUrl.searchParams.get('next'),
-    [
-      new Date(response.features[1].properties.datetime).getTime(),
-      response.features[1].id,
-      response.features[1].collection
-    ].join(','))
+  // the `next` token is base64url-encoded JSON of the OpenSearch sort values
+  const decodedNext = JSON.parse(
+    Buffer.from(nextUrl.searchParams.get('next') ?? '', 'base64url').toString('utf8')
+  )
+  t.deepEqual(decodedNext, [
+    new Date(response.features[1].properties.datetime).getTime(),
+    response.features[1].id,
+    response.features[1].collection
+  ])
 
   const nextResponse = await got.get(nextUrl).json() as SearchBody
   t.is(nextResponse.features.length, 0)
@@ -114,12 +117,14 @@ test('/search preserve bbox and datetime in next links', async (t) => {
 
   const nextLink = response.links.find((x: Link) => x.rel === 'next')
   const nextUrl = new URL(nextLink.href)
-  t.deepEqual(nextUrl.searchParams.get('next'),
-    [
-      new Date(response.features[0].properties.datetime).getTime(),
-      response.features[0].id,
-      response.features[0].collection
-    ].join(','))
+  const decodedNext = JSON.parse(
+    Buffer.from(nextUrl.searchParams.get('next') ?? '', 'base64url').toString('utf8')
+  )
+  t.deepEqual(decodedNext, [
+    new Date(response.features[0].properties.datetime).getTime(),
+    response.features[0].id,
+    response.features[0].collection
+  ])
   t.deepEqual(nextUrl.searchParams.get('bbox'), bbox)
   t.deepEqual(nextUrl.searchParams.get('datetime'), datetime)
 })