CenterForDigitalHumanities / CenterForDigitalHumanities/rerum_server_nodejs

The pagination contract is undocumented, misdocumented, and absent from the OpenAPI contract

Open
#305 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
JavaScript
Stars
3
Forks
6
Avg merge
1h 25m
Merged PRs (30d)
3

Description

## Summary

Three artifacts are supposed to tell a client how to page, and all three fail:

- **`openapi/contracts/core-provider.openapi.yaml`** — the authoritative machine-readable contract — does not mention `limit` or `skip` anywhere. `/api/query` (POST and HEAD), `/api/search`, and `/api/search/phrase` declare no pagination parameters and no response headers. A client generated from the contract cannot page at all.
- **`public/API.html`** documents a default of 10 records when the actual default is 100, publishes a `pagedQuery` example that never terminates, and warns three times about "strange behavior" instead of stating the maximums.
- **`GET /v1/api`** returns a list of endpoint descriptions and nothing else. There is no way for a client to discover the deployment's actual limits at runtime.

A well-written client that *wants* to page correctly has nowhere to look.

This issue is the sweep. Each behavioral issue in the set should carry its own documentation delta in its own PR; this one covers what nothing else does and does a final pass confirming all three artifacts match what actually shipped. **It lands last.**

## Why this matters

**The contract describes an API with no pagination.** `openapi/contracts/core-provider.openapi.yaml` is synced to the shared spec repository by `.github/workflows/sync-rerum-shared-openapi.yml` and guarded by `__tests__/openapi_sync_artifacts.test.js`. The only reusable query parameters it defines under `components/parameters` (line 692) are `ObjectId`, `ExpansionGenerator`, and `ExpansionCreator`. Whatever shape the paging fixes take, if they do not land here, the honest behavior stays undiscoverable through the artifact that is supposed to be authoritative.

**The published example is the bug.** `pagedQuery` at `public/API.html:555` stops only on an empty page and advances by `results.length`. That is precisely the non-terminating shape, and we publish it as the recommended pattern. Fixing the server without fixing the example leaves the loop in circulation.

**The documented default is wrong.** `public/API.html:506` says responses are "limited to 10 records" by default. `getPagination(req.query, 100)` is called at `controllers/crud.js:77` and `controllers/search.js:274`, so a request with no `limit` returns 100. A client sizing its pages around the documented 10 is working from a number that has never been true.

**Warnings are not a contract.** Lines 507, 612, and 730 each warn that "your application may experience strange behavior with large limits, such as ?limit=1000. It is recommended to use a limit of 100 or less." The strange behavior is the silent truncation. Nothing enforces the recommendation, and a client that sets its page size above 500 fails silently from that point on.

**Clients cannot self-configure.** No response header carries the maximums, and no endpoint publishes them. A client has to hardcode a guess about a deployment it may not control. `store.rerum.io` and `devstore.rerum.io` are not required to have the same caps.

## Affected lines

| File | Line | Current |
|------|------|---------|
| `openapi/contracts/core-provider.openapi.yaml` | 210, 233, 241, 267 | `/api/query`, `HEAD /api/query`, `/api/search`, `/api/search/phrase` declare no `limit` or `skip` |
| `openapi/contracts/core-provider.openapi.yaml` | 692 | `components/parameters` has no pagination parameters |
| `public/API.html` | 506 | Documents a default of 10; actual default is 100 |
| `public/API.html` | 507, 612, 730 | "Strange behavior" warnings instead of stated maximums |
| `public/API.html` | 555 | Published `pagedQuery` example is the non-terminating shape |
| `routes/api-routes.js` | 64-83 | `GET /api` publishes endpoint descriptions and no pagination information |

## Proposed change

### OpenAPI contract

Add a shared `PageLimit` / `PageSkip` pair under `components/parameters` and reference them from `/api/query` (POST and HEAD), `/api/search`, and `/api/search/phrase`. Include:

- The maximums and defaults in each schema.
- The `400` response for out-of-range and malformed values, once #301 lands.
- The `Link` response header, described as carrying `rel="next"`, once #302 lands.
- The applied-value response headers from #301.
- A `PageCursor` parameter for `/api/query`, once #303 lands.
- The ordering guarantees: `_id` ascending for `/query` (#300), descending relevance for `/search` (#307).
- The corrected status codes for `head: /api/query` (#304).

Check whether `__tests__/openapi_sync_artifacts.test.js` and the sync workflow need anything beyond the file edit.

### `public/API.html`

- Correct the default from 10 to 100.
- Replace all three "strange behavior" warnings with the actual maximums and what happens when they are exceeded — `limit` is clamped and reported, `skip` is rejected.
- Rewrite `pagedQuery` to follow `rel="next"` and terminate on its absence. Note that the `next` URL is followed with the same POST body, since that is not obvious from a `Link` header alone.
- Document the applied-value response headers, the ordering guarantee for each endpoint, and cursor paging with `skip` described as the bounded random-access alternative.
- Note the one case the server cannot catch: `?limit[a]=5` never reaches the server as a `limit` key under Express's `simple` query parser, so it is indistinguishable from omitting the parameter.

### `GET /v1/api`

Publish the effective pagination configuration so clients can size themselves against the deployment they are actually talking to:

```json
{
"message": "Welcome to v1 in nodeJS! Below are the available endpoints, used like /v1/api/{endpoint}",
"pagination": {
"defaultLimit": 100,
"maxLimit": 500,
"maxSkip": 100000,
"cursorSupported": ["/query"]
},
"endpoints": { "…": "…" }
}
```

Read the values from the same constants `getPagination()` uses, so this cannot drift from actual behavior.

## Notes

- **Lands last**, and should be a verification pass as much as an authoring pass: read each artifact against the shipped behavior rather than against the plan in these drafts.
- Each behavioral issue carries its own doc delta in its own PR. This issue exists so the parts nothing else touches — the wrong default, the broken example, the `/v1/api` discovery — do not fall through, and so someone checks the whole surface once at the end.
- The `/v1/api` change is the only code change here. It could reasonably move into #301, since that is where the applied values and maximums are already being surfaced — worth deciding when scheduling.
- If #309 is still open when this lands, document `/search` ordering as it actually is at that moment, including the cross-branch caveat, rather than as it is intended to be.

## Acceptance criteria

- [ ] `openapi/contracts/core-provider.openapi.yaml` declares `limit` and `skip` on `/api/query`, `HEAD /api/query`, `/api/search`, and `/api/search/phrase`, with maximums, defaults, and the `400` response
- [ ] The contract declares the `Link` response header and the applied-value headers
- [ ] The contract states the ordering guarantee for each paged endpoint
- [ ] The OpenAPI sync workflow and `__tests__/openapi_sync_artifacts.test.js` still pass
- [ ] `public/API.html` documents the real default, the real maximums, and both endpoints' ordering
- [ ] `public/API.html` publishes a paged example that terminates correctly
- [ ] The three "strange behavior" warnings are gone, replaced by the actual contract
- [ ] `GET /v1/api` publishes the effective default and maximums, read from the same constants the server enforces
- [ ] Every artifact is verified against shipped behavior, not against these drafts

Contributor guide

Open the contributing guide

Research direction

Start by reading pagination behavior in controllers/crud.js and controllers/search.js, then compare it with openapi/contracts/core-provider.openapi.yaml, public/API.html, and routes/api-routes.js. Run __tests__/openapi_sync_artifacts.test.js and inspect the sync workflow. Done means the OpenAPI contract, HTML documentation, and GET /v1/api discovery response match shipped defaults, limits, headers, ordering, and termination behavior.

Written by the indexing model from the issue text.

Assessment

Tech stack
javascript, openapi
Domain
api, backend, documentation
Issue type
Documentation
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
48/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.