CenterForDigitalHumanities / CenterForDigitalHumanities/rerum_server_nodejs
The pagination contract is undocumented, misdocumented, and absent from the OpenAPI contract
- 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
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