Mirrors/etherpad-lite
1
0
Fork 0
mirror of https://github.com/ether/etherpad-lite.git synced 2026-07-17 16:47:05 +00:00
Code Issues Projects Releases Wiki Activity

feat(api): clean up published OpenAPI spec for downstream codegens (#7714)

Browse source 
* feat(api): clean up published OpenAPI spec for downstream codegens

The /api/openapi.json doc had four issues that broke generated tooling
(printingpress.dev, openapi-generator, Postman): empty top-level tags
array, every operation duplicated as GET+POST, 14 operations missing
from the resources map (drift since API 1.2.8), and empty summaries on
several tracked ops.

This PR splits the runtime spec from the published spec via a {public}
flag on generateDefinitionForVersion: the runtime definition fed to
openapi-backend keeps both verbs (existing third-party clients that
call GET /api/x.x.x/foo?apikey=... continue to work), while the spec
served at /api/openapi.json, /rest/openapi.json, and per-version paths
advertises only POST.

Other changes:
  - top-level tags array declares pad/author/session/group/chat/server
  - per-op tags override added to SwaggerUIResource type so chat ops
    (still nested under pad for routing) and checkToken can be tagged
    without changing existing REST URLs
  - 14 missing ops (getAttributePool, getRevisionChangeset, copyPad,
    movePad, getPadID, getSavedRevisionsCount, listSavedRevisions,
    saveRevision, restoreRevision, appendText, copyPadWithoutHistory,
    compactPad, anonymizeAuthor, getStats) backfilled with summaries
  - empty summaries on listSessionsOfGroup, listAllGroups,
    createDiffHTML, createPad filled in
  - new backend tests assert the public spec shape (tags, summaries,
    POST-only) and that runtime routing still resolves both verbs

Driven by integrating Etherpad with printingpress.dev: pointing the
generator at the previous spec produced a 96-command CLI with no
resource grouping and many empty descriptions. Design notes in
docs/superpowers/specs/2026-05-10-openapi-cleanup-design.md.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(api): keep checkToken at /rest/<ver>/pad/checkToken (Qodo #2)

Moving checkToken to a new `server` resource broke REST-style
backward compat: existing callers of /rest/<ver>/pad/checkToken
would have hit `code: 3` (no such function). The whole point of
per-op tag overrides is to preserve REST URLs while still grouping
correctly in OpenAPI tags — checkToken should follow the same
pattern as the chat ops.

Keep checkToken in `resources.pad`, give it `tags: ['server']`,
and add a regression test asserting /rest/<ver>/pad/checkToken
still resolves. The `server` resource still exists for `getStats`
(genuinely new server-level op with no prior REST URL).

Updates the design doc accordingly.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
John McLear 2026-05-10 20:02:05 +08:00 • committed by GitHub
parent df0ecec834
commit cbf71285a2
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 369 additions and 25 deletions
Download patch file Download diff file Expand all files Collapse all files

137
docs/superpowers/specs/2026-05-10-openapi-cleanup-design.md Normal file
View file

@ -0,0 +1,137 @@
# OpenAPI cleanup for downstream tooling — Design
**Date:** 2026-05-10
**Owner:** John McLear
**Scope:** `src/node/hooks/express/openapi.ts` + type tweak + tests
**Driven by:** integrating Etherpad with [printingpress.dev](https://printingpress.dev). Generating a Go CLI / Claude Code skill from `/api/openapi.json` revealed structural problems in the served spec that hurt every downstream consumer (printing-press, Postman, Swagger UI, openapi-generator, etc.). This PR fixes Etherpad's spec; generating and publishing a CLI is a follow-up that depends on this landing.
## Problems in the current spec
A live capture of `/api/openapi.json` (Etherpad 1.3.0, 48 paths) showed:
1. **Top-level `tags` array is empty/null.** Per-operation `tags: ["pad"]`, `["group"]`, etc. are populated for ops in the `resources` map, but consumers that group by tag (printing-press, Swagger UI sidebar, openapi-generator's resource modules) need the top-level array to discover and order them.
2. **Every operation duplicated as GET and POST.** Lines 562–573 of `openapi.ts` deliberately emit `paths[path] = { get: {...UsingGET}, post: {...UsingPOST} }`. The original comment ("It may be confusing that every operation can be called with both GET and POST") acknowledges this. A 48-path API generates a 96-operation CLI with `check-token using-get` + `check-token using-post`, etc.
3. **14 operations missing from the `resources` map.** As new API versions were added (1.2.8 → 1.3.1), `APIHandler.ts` got new functions but the `resources` map in `openapi.ts` was never updated. Affected ops have no `tags`, no `summary`, no `description`:
- `getAttributePool`, `getRevisionChangeset`, `copyPad`, `movePad`, `getPadID`, `getSavedRevisionsCount`, `listSavedRevisions`, `saveRevision`, `restoreRevision`, `appendText`, `getStats`, `copyPadWithoutHistory`, `compactPad`, `anonymizeAuthor`
4. **Empty summaries on tracked ops.** `listSessionsOfGroup`, `listAllGroups`, `createDiffHTML` had `summary: ''`. `createPad` had no summary at all.
## Non-goals
- **Deprecating GET routes at runtime.** Existing third-party clients use `GET /api/1.x.x/foo?apikey=...`. Removing GET would be a breaking change for them — out of scope. This PR only changes what the spec *advertises*.
- **Fixing printing-press's operationId derivation bug** (`get-html_get-htmlusing-get`). Generator-side issue.
- **Admin API spec** (`/admin/openapi.json`). Different surface, separate cleanup if needed later.
## Design
### Per-op tag overrides
The simplest fix for Problem 3-and-friends is to allow per-operation `tags` overrides in the `resources` map. Existing chat ops (`getChatHistory`, `getChatHead`, `appendChatMessage`) and `checkToken` are nested under `pad` for routing reasons; tagging them as `chat` / `server` without restructuring the map preserves all REST URLs.
The operations builder destructures `tags` from each spec entry and falls back to `[resource]` when absent:
```ts
const {operationId, responseSchema, tags: customTags, ...operation} = spec as any;
// ...
operations[operationId] = {
operationId,
...operation,
responses,
tags: customTags || [resource],
_restPath: `/${resource}/${action}`,
};
```
The `SwaggerUIResource` type gains an optional `tags?: string[]` field.
### Top-level tags array
Added inside `generateDefinitionForVersion`'s returned `definition` object:
```ts
tags: [
{name: 'pad', description: 'Pad lifecycle, content, revisions, attributes'},
{name: 'author', description: 'Authors and authorship'},
{name: 'session', description: 'Group sessions'},
{name: 'group', description: 'Groups (multi-tenant pads)'},
{name: 'chat', description: 'In-pad chat history'},
{name: 'server', description: 'Server-level operations (stats, token check)'},
],
```
### Backfill missing entries
14 operations added to `resources` with proper summaries. Most go under `pad`; `anonymizeAuthor` under `author`; `getStats` under a new `server` resource group.
| Tag | Operation | Summary |
|--------|----------------------------|---------------------------------------------------------------|
| pad | `getAttributePool` | returns the attribute pool of a pad |
| pad | `getRevisionChangeset` | returns the changeset at a given revision of a pad |
| pad | `copyPad` | copies a pad with full history and chat |
| pad | `movePad` | moves a pad — copy then delete the original |
| pad | `getPadID` | returns the read-write pad ID for a given read-only pad ID |
| pad | `getSavedRevisionsCount` | returns the number of saved revisions of a pad |
| pad | `listSavedRevisions` | returns the list of saved revisions of a pad |
| pad | `saveRevision` | saves a revision of a pad |
| pad | `restoreRevision` | restores a pad to a specific revision |
| pad | `appendText` | appends text to a pad |
| pad | `copyPadWithoutHistory` | copies a pad without history or chat |
| pad | `compactPad` | compacts a pad's revision history, keeping recent ones |
| author | `anonymizeAuthor` | anonymizes an author across all their edits |
| server | `getStats` | returns server-wide statistics |
`checkToken` stays under `pad` in the resources map (preserves `/rest/<ver>/pad/checkToken`) but gains an explicit `tags: ['server']` override so it groups correctly in OpenAPI without changing its REST URL.
### Runtime vs published spec split
`generateDefinitionForVersion` gains a `{public}` flag:
```ts
const generateDefinitionForVersion = (
version: string,
style: string = APIPathStyle.FLAT,
{public: isPublic = false}: {public?: boolean} = {},
) => { ... }
```
When `isPublic`, paths emit only `post:`. Otherwise both `get:` and `post:` (current behavior).
- The `definition` passed to `new OpenAPIBackend({...})` stays as-is (no flag) → both verbs routed at runtime → backward compat preserved.
- The handlers serving `/api/openapi.json`, `/rest/openapi.json`, `/api/{version}/openapi.json` call with `{public: true}` → clients see clean POST-only API.
operationIds in the public spec are unchanged (`${name}UsingPOST`), so any tooling already generated from the previous spec still finds its operations — strict subset, not rename.
## Test plan
Two new describe blocks in `src/tests/backend/specs/api/api.ts` (existing home for `/api/openapi.json` tests):
1. **public OpenAPI spec shape** — fetches `/api/openapi.json` once, asserts:
- Top-level `tags` array contains `{pad, author, session, group, chat, server}`
- Every operation has `tags: [...]` with ≥1 non-empty entry
- Every operation has a non-empty `summary` (≥3 chars)
- Every path advertises only `post:`
2. **runtime backward compatibility** — drives the live API:
- `GET /api/{v}/checkToken?apikey=...` returns code 0
- `POST /api/{v}/checkToken` returns code 0
These assert both halves of the design: the published spec is clean, and the runtime hasn't lost backward-compat routing.
## Blast radius
- **Runtime callers** (third-party scripts, ep_ai_mcp's HTTP fallback paths if any, dashboards, CI hooks): zero impact. Both GET and POST routes still resolve.
- **Tooling regenerators** (Postman collections, Swagger UI, openapi-generator clients): strict improvement. Smaller, better-named, properly-grouped surface. operationIds stable.
- **REST-style URLs** (`/rest/...`): unchanged for every existing op. No restructuring of `resources` was needed because per-op tag overrides do the work. New backfilled ops (`getAttributePool` etc.) gain a `/rest/X/pad/getAttributePool` path; their previous fallback `/rest/X/getAttributePool` is no longer the canonical REST route, but FLAT (`/api/...`) is unchanged.
## Out-of-band note
The spec already serves at three URLs (`/api/openapi.json`, `/rest/openapi.json`, `/api/{version}/openapi.json`); the cleanup applies to all three because the same builder backs them.
A separate admin spec exists at `/admin/openapi.json` (added in #7693/#7705) — out of scope here, worth a similar audit later.
## Follow-up phases (not part of this PR)
- **Phase B:** point printing-press at the cleaned spec, generate `etherpad` Go CLI + Claude Code skill, push to a new `ether/etherpad-cli` repo, submit to printingpress.dev community library.
- **Phase C:** submit `ep_ai_mcp` as the canonical MCP entry in printingpress.dev's library — generated MCP from OpenAPI would be strictly worse (no changeset/authorship reach-through).

147
src/node/hooks/express/openapi.ts
View file

@ -86,14 +86,14 @@ const resources:SwaggerUIResource = {
},
listSessions: {
operationId: 'listSessionsOfGroup',
summary: '',
summary: 'returns all sessions of a group',
responseSchema: {
sessions: {type: 'array', items: {$ref: '#/components/schemas/SessionInfo'}},
},
},
list: {
operationId: 'listAllGroups',
summary: '',
summary: 'returns the IDs of all groups on this server',
responseSchema: {groupIDs: {type: 'array', items: {type: 'string'}}},
},
},
@ -128,6 +128,10 @@ const resources:SwaggerUIResource = {
summary: 'Returns the Author Name of the author',
responseSchema: {info: {$ref: '#/components/schemas/UserInfo'}},
},
anonymize: {
operationId: 'anonymizeAuthor',
summary: 'anonymizes an author across all their edits',
},
},
// Session
@ -158,11 +162,12 @@ const resources:SwaggerUIResource = {
},
createDiffHTML: {
operationId: 'createDiffHTML',
summary: '',
summary: 'returns an HTML diff between two revisions of a pad',
responseSchema: {},
},
create: {
operationId: 'createPad',
summary: 'creates a new (non-group) pad',
description:
'creates a new (non-group) pad. Note that if you need to create a group Pad, ' +
'you should call createGroupPad',
@ -234,22 +239,82 @@ const resources:SwaggerUIResource = {
},
checkToken: {
operationId: 'checkToken',
summary: 'returns ok when the current api token is valid',
summary: 'returns ok when the current API token is valid',
tags: ['server'],
},
getChatHistory: {
operationId: 'getChatHistory',
summary: 'returns the chat history',
tags: ['chat'],
responseSchema: {messages: {type: 'array', items: {$ref: '#/components/schemas/Message'}}},
},
// We need an operation that returns a Message so it can be picked up by the codegen :(
getChatHead: {
operationId: 'getChatHead',
summary: 'returns the chatHead (chat-message) of the pad',
tags: ['chat'],
responseSchema: {chatHead: {$ref: '#/components/schemas/Message'}},
},
appendChatMessage: {
operationId: 'appendChatMessage',
summary: 'appends a chat message',
tags: ['chat'],
},
getAttributePool: {
operationId: 'getAttributePool',
summary: 'returns the attribute pool of a pad',
},
getRevisionChangeset: {
operationId: 'getRevisionChangeset',
summary: 'returns the changeset at a given revision of a pad',
},
copyPad: {
operationId: 'copyPad',
summary: 'copies a pad with full history and chat',
},
movePad: {
operationId: 'movePad',
summary: 'moves a pad — copy then delete the original',
},
getPadID: {
operationId: 'getPadID',
summary: 'returns the read-write pad ID for a given read-only pad ID',
},
getSavedRevisionsCount: {
operationId: 'getSavedRevisionsCount',
summary: 'returns the number of saved revisions of a pad',
},
listSavedRevisions: {
operationId: 'listSavedRevisions',
summary: 'returns the list of saved revisions of a pad',
},
saveRevision: {
operationId: 'saveRevision',
summary: 'saves a revision of a pad',
},
restoreRevision: {
operationId: 'restoreRevision',
summary: 'restores a pad to a specific revision',
},
appendText: {
operationId: 'appendText',
summary: 'appends text to a pad',
},
copyPadWithoutHistory: {
operationId: 'copyPadWithoutHistory',
summary: 'copies a pad without history or chat',
},
compactPad: {
operationId: 'compactPad',
summary: 'compacts a pad\'s revision history, keeping recent revisions only',
},
},
// Server
server: {
getStats: {
operationId: 'getStats',
summary: 'returns server-wide statistics',
},
},
};
@ -396,7 +461,7 @@ const defaultResponseRefs:OpenAPISuccessResponse = {
const operations: OpenAPIOperations = {};
for (const [resource, actions] of Object.entries(resources)) {
for (const [action, spec] of Object.entries(actions)) {
const {operationId,responseSchema, ...operation} = spec;
const {operationId, responseSchema, tags: customTags, ...operation} = spec as any;
// add response objects
const responses:OpenAPISuccessResponse = {...defaultResponseRefs};
@ -409,20 +474,43 @@ for (const [resource, actions] of Object.entries(resources)) {
}
// add final operation object to dictionary
// tags default to [resource] but can be overridden per-op via spec.tags
// (e.g. chat ops nested under pad use tags: ['chat']).
operations[operationId] = {
operationId,
...operation,
responses,
tags: [resource],
tags: customTags || [resource],
_restPath: `/${resource}/${action}`,
};
}
}
const generateDefinitionForVersion = (version:string, style = APIPathStyle.FLAT) => {
const definition = {
/**
* Generate the OpenAPI definition for a given API version + path style.
*
* The `public` flag controls whether the spec is the *runtime* definition (used
* to dispatch requests via openapi-backend, which still routes both GET and
* POST for backward compatibility with older clients) or the *published* spec
* (served at /api/openapi.json etc., advertising only POST so generated tooling
* — printingpress.dev, openapi-generator, Postman — sees a clean surface).
*/
const generateDefinitionForVersion = (
version: string,
style: string = APIPathStyle.FLAT,
{public: isPublic = false}: {public?: boolean} = {},
) => {
const definition: any = {
openapi: OPENAPI_VERSION,
info,
tags: [
{name: 'pad', description: 'Pad lifecycle, content, revisions, attributes'},
{name: 'author', description: 'Authors and authorship'},
{name: 'session', description: 'Group sessions'},
{name: 'group', description: 'Groups (multi-tenant pads)'},
{name: 'chat', description: 'In-pad chat history'},
{name: 'server', description: 'Server-level operations (stats, token check)'},
],
paths: {},
components: {
parameters: {},
@ -559,18 +647,29 @@ const generateDefinitionForVersion = (version:string, style = APIPathStyle.FLAT)
delete operation._restPath;
// add to definition
// NOTE: It may be confusing that every operation can be called with both GET and POST
// @ts-ignore
definition.paths[path] = {
get: {
...operation,
operationId: `${operation.operationId}UsingGET`,
},
post: {
...operation,
operationId: `${operation.operationId}UsingPOST`,
},
};
// The runtime spec advertises both GET and POST so existing clients (some of
// which still pass apikey/params via query string) keep working. The public
// spec served at /api/openapi.json advertises only POST — that is the
// recommended call style and what downstream codegens should target.
if (isPublic) {
definition.paths[path] = {
post: {
...operation,
operationId: `${operation.operationId}UsingPOST`,
},
};
} else {
definition.paths[path] = {
get: {
...operation,
operationId: `${operation.operationId}UsingGET`,
},
post: {
...operation,
operationId: `${operation.operationId}UsingPOST`,
},
};
}
}
return definition;
};
@ -588,11 +687,13 @@ exports.expressPreSession = async (hookName:string, {app}:any) => {
const definition = generateDefinitionForVersion(version, style);
// serve version specific openapi definition; regenerate per request so runtime
// settings (e.g. authenticationMethod) are reflected
// settings (e.g. authenticationMethod) are reflected. The served spec uses
// {public: true} to advertise only POST per path (the runtime backend below
// still routes both GET and POST for backward compatibility).
app.get(`${apiRoot}/openapi.json`, (req:any, res:any) => {
// For openapi definitions, wide CORS is probably fine
res.header('Access-Control-Allow-Origin', '*');
const liveDefinition = generateDefinitionForVersion(version, style);
const liveDefinition = generateDefinitionForVersion(version, style, {public: true});
res.json({...liveDefinition, servers: [generateServerForApiVersion(apiRoot, req)]});
});
@ -601,7 +702,7 @@ exports.expressPreSession = async (hookName:string, {app}:any) => {
if (isLatestAPIVersion) {
app.get(`/${style}/openapi.json`, (req:any, res:any) => {
res.header('Access-Control-Allow-Origin', '*');
const liveDefinition = generateDefinitionForVersion(version, style);
const liveDefinition = generateDefinitionForVersion(version, style, {public: true});
res.json({...liveDefinition, servers: [generateServerForApiVersion(apiRoot, req)]});
});
}

5
src/node/types/SwaggerUIResource.ts
View file

@ -3,8 +3,9 @@ export type SwaggerUIResource = {
[secondKey: string]: {
operationId: string,
summary?: string,
description?:string
responseSchema?: object
description?: string,
responseSchema?: object,
tags?: string[],
}
}
}

105
src/tests/backend/specs/api/api.ts
View file

@ -112,4 +112,109 @@ describe(__filename, function () {
}
});
});
describe('public OpenAPI spec shape (for downstream codegens)', function () {
let spec: any;
before(async function () {
this.timeout(15000);
spec = (await agent.get('/api/openapi.json').expect(200)).body;
});
it('declares a top-level tags array with all expected resource groups', function () {
if (!Array.isArray(spec.tags)) {
throw new Error(`Expected top-level tags to be an array, got ${typeof spec.tags}`);
}
const names = spec.tags.map((t: any) => t.name);
const expected = ['pad', 'author', 'session', 'group', 'chat', 'server'];
const missing = expected.filter((n) => !names.includes(n));
if (missing.length) {
throw new Error(`Top-level tags missing entries: ${missing.join(', ')}; got: ${names}`);
}
});
it('tags every operation with at least one non-empty tag', function () {
const untagged: string[] = [];
for (const [path, methods] of Object.entries(spec.paths)) {
for (const [method, op] of Object.entries(methods as any)) {
const tags = (op as any).tags;
if (!Array.isArray(tags) || tags.length === 0 || tags.some((t) => !t)) {
untagged.push(`${method.toUpperCase()} ${path}`);
}
}
}
if (untagged.length) {
throw new Error(`${untagged.length} operations are untagged: ${untagged.join(', ')}`);
}
});
it('summarizes every operation', function () {
const unsummarized: string[] = [];
for (const [path, methods] of Object.entries(spec.paths)) {
for (const [method, op] of Object.entries(methods as any)) {
const summary = (op as any).summary;
if (typeof summary !== 'string' || summary.trim().length < 3) {
unsummarized.push(
`${method.toUpperCase()} ${path} (summary=${JSON.stringify(summary)})`);
}
}
}
if (unsummarized.length) {
throw new Error(
`${unsummarized.length} operations have empty/missing summaries: ` +
unsummarized.join(', '));
}
});
it('advertises only POST per path (downstream tooling cleanliness)', function () {
const offenders: string[] = [];
for (const [path, methods] of Object.entries(spec.paths)) {
const verbs = Object.keys(methods as any);
if (verbs.length !== 1 || verbs[0] !== 'post') {
offenders.push(`${path} has methods: ${verbs.join(', ')}`);
}
}
if (offenders.length) {
throw new Error(
`Public spec must advertise only POST per path; offenders:\n ${
offenders.join('\n ')}`);
}
});
});
describe('runtime backward compatibility (GET + POST still routed)', function () {
// The runtime spec used by openapi-backend keeps both verbs even though the
// public /api/openapi.json advertises POST only. The point of these tests
// is to prove openapi-backend still resolves both verbs to the handler
// — not to exercise auth. A 401 (or any non-`code 3` body) proves the
// request reached the handler. `code: 3` is Etherpad's "no such function"
// response, returned by openapi-backend's notFound when a method is not
// declared in the runtime spec.
const assertResolved = (path: string, body: any) => {
if (body && body.code === 3) {
throw new Error(
`${path} got 'no such function' (code 3) — runtime spec dropped the ` +
`verb. Response body: ${JSON.stringify(body)}`);
}
};
it('GET requests still reach the API handler', async function () {
const r = await agent.get(endPoint('checkToken'));
assertResolved('GET checkToken', r.body);
});
it('POST requests still reach the API handler', async function () {
const r = await agent.post(endPoint('checkToken'));
assertResolved('POST checkToken', r.body);
});
// Regression for the REST-style routes — checkToken's _restPath is
// derived from its position in the resources map (pad/checkToken).
// Tagging it as 'server' must not move it to /rest/X/server/checkToken.
it('REST-style /rest/<ver>/pad/checkToken still resolves', async function () {
const r = await agent.get(`/rest/${apiVersion}/pad/checkToken`);
assertResolved('GET /rest pad/checkToken', r.body);
});
});
});