feat(api): richer OpenAPI metadata for typed endpoints

.changeset/loud-otters-document.md

@@ -0,0 +1,6 @@
+---
+'@rocket.chat/rest-typings': patch
+'@rocket.chat/meteor': patch
+---
+
+Improves the generated OpenAPI document: path parameters are now templated and described, query parameters are documented one by one instead of as a single opaque object, responses always carry a description, and endpoints can declare a summary, a description, examples and a deprecation notice

.changeset/plain-lions-agree.md

@@ -0,0 +1,6 @@
+---
+'@rocket.chat/core-typings': patch
+'@rocket.chat/meteor': patch
+---
+
+Serves the OpenAPI document as 3.1, the version whose schemas are the JSON Schema dialect the endpoints already validate against

apps/meteor/ee/server/apps/communication/endpoints/appGeneralLogsHandler.ts

@@ -33,7 +33,7 @@ export const registerAppGeneralLogsHandler = ({ api, _orch }: AppsRestApi) =>
 					properties: {
 						offset: { type: 'number' },
 						// TODO: reference a schema for the array items
-						logs: { type: 'array' },
+						logs: { type: 'array', items: {} },
 						count: { type: 'number' },
 						total: { type: 'number' },
 						success: { type: 'boolean' },

apps/meteor/ee/server/apps/communication/endpoints/appLogsHandler.ts

@@ -31,7 +31,7 @@ export const registerAppLogsHandler = ({ api, _manager, _orch }: AppsRestApi) =>
 					type: 'object',
 					properties: {
 						offset: { type: 'number' },
-						logs: { type: 'array' },
+						logs: { type: 'array', items: {} },
 						count: { type: 'number' },
 						total: { type: 'number' },
 						success: { type: 'boolean' },

apps/meteor/server/api/ApiClass.ts

@@ -1,11 +1,10 @@
 import type { IMethodConnection, IUser } from '@rocket.chat/core-typings';
-import type { Route, Router } from '@rocket.chat/http-router';
+import type { Router } from '@rocket.chat/http-router';
 import { License } from '@rocket.chat/license';
 import { Logger } from '@rocket.chat/logger';
 import { Users } from '@rocket.chat/models';
 import { Random } from '@rocket.chat/random';
 import type { JoinPathPattern, Method } from '@rocket.chat/rest-typings';
-import { ajv } from '@rocket.chat/rest-typings';
 import { wrapExceptions } from '@rocket.chat/tools';
 import type { ValidateFunction } from 'ajv';
 import { Accounts } from 'meteor/accounts-base';
@@ -161,8 +160,6 @@ export const generateConnection = (
 });
 
 export class APIClass<TBasePath extends string = '', TOperations extends Record<string, unknown> = Record<string, never>> {
-	public typedRoutes: Record<string, Record<string, Route>> = {};
-
 	protected apiPath?: string;
 
 	readonly version?: string;
@@ -548,104 +545,6 @@ export class APIClass<TBasePath extends string = '', TOperations extends Record<
 		return routeActions.map((action) => this.getFullRouteName(route, action));
 	}
 
-	private registerTypedRoutesLegacy<TSubPathPattern extends string, TOptions extends Options>(
-		method: Method,
-		subpath: TSubPathPattern,
-		options: TOptions,
-	): void {
-		const { authRequired, validateParams } = options;
-
-		const opt = {
-			authRequired,
-			...(validateParams &&
-				method.toLowerCase() === 'get' &&
-				('GET' in validateParams
-					? { query: validateParams.GET }
-					: {
-							query: validateParams as ValidateFunction<any>,
-						})),
-
-			...(validateParams &&
-				method.toLowerCase() === 'post' &&
-				('POST' in validateParams ? { query: validateParams.POST } : { body: validateParams as ValidateFunction<any> })),
-
-			...(validateParams &&
-				method.toLowerCase() === 'put' &&
-				('PUT' in validateParams ? { query: validateParams.PUT } : { body: validateParams as ValidateFunction<any> })),
-			...(validateParams &&
-				method.toLowerCase() === 'delete' &&
-				('DELETE' in validateParams ? { query: validateParams.DELETE } : { body: validateParams as ValidateFunction<any> })),
-
-			tags: ['Missing Documentation'],
-			response: {
-				200: ajv.compile({
-					type: 'object',
-					properties: {
-						success: { type: 'boolean' },
-						error: { type: 'string' },
-					},
-					required: ['success'],
-				}),
-			},
-		};
-
-		this.registerTypedRoutes(method, subpath, opt);
-	}
-
-	private registerTypedRoutes<
-		TSubPathPattern extends string,
-		TOptions extends TypedOptions,
-		TPathPattern extends `${TBasePath}/${TSubPathPattern}`,
-	>(method: MinimalRoute['method'], subpath: TSubPathPattern, options: TOptions): void {
-		const path = `/${this.apiPath}/${subpath}`.replaceAll('//', '/') as TPathPattern;
-		this.typedRoutes = this.typedRoutes || {};
-		this.typedRoutes[path] = this.typedRoutes[path] || {};
-		const { query, authRequired, response, body, tags, ...rest } = options;
-		this.typedRoutes[path][method.toLowerCase()] = {
-			...(response && {
-				responses: Object.fromEntries(
-					Object.entries(response).map(([status, schema]) => [
-						status,
-						{
-							description: '',
-							content: {
-								'application/json': 'schema' in schema ? { schema: schema.schema } : schema,
-							},
-						},
-					]),
-				),
-			}),
-			...(query && {
-				parameters: [
-					{
-						schema: query.schema,
-						in: 'query',
-						name: 'query',
-						required: true,
-					},
-				],
-			}),
-			...(body && {
-				requestBody: {
-					required: true,
-					content: {
-						'application/json': { schema: body.schema },
-					},
-				},
-			}),
-			...(authRequired && {
-				...rest,
-				security: [
-					{
-						userId: [],
-						authToken: [],
-					},
-				],
-			}),
-			tags,
-		};
-	}
-
 	private method<TSubPathPattern extends string, TOptions extends TypedOptions, TPathPattern extends `${TBasePath}/${TSubPathPattern}`>(
 		method: MinimalRoute['method'],
 		subpath: TSubPathPattern,
@@ -662,7 +561,6 @@ export class APIClass<TBasePath extends string = '', TOperations extends Record<
 		  >
 	> {
 		this.addRoute([subpath], { tags: [], ...options, typed: true }, { [method.toLowerCase()]: { action } } as any);
-		this.registerTypedRoutes(method, subpath, options);
 		return this;
 	}
 
@@ -940,11 +838,6 @@ export class APIClass<TBasePath extends string = '', TOperations extends Record<
 					options: _options,
 					endpoints: operations[method as keyof Operations<TPathPattern, TOptions>] as unknown as Record<string, string>,
 				});
-
-				this.registerTypedRoutesLegacy(method as Method, route, {
-					...options,
-					...operations[method as keyof Operations<TPathPattern, TOptions>],
-				});
 			});
 		});
 	}

apps/meteor/server/api/default/openApi.ts

@@ -1,12 +1,14 @@
 import { schemas } from '@rocket.chat/core-typings';
 import type { Route } from '@rocket.chat/http-router';
+import { getSharedSchemas, openAPIErrorComponents, withOperationIds } from '@rocket.chat/http-router';
 import { ajv, isOpenAPIJSONEndpoint } from '@rocket.chat/rest-typings';
 import express from 'express';
 import { WebApp } from 'meteor/webapp';
 import swaggerUi from 'swagger-ui-express';
 
 import { settings } from '../../settings';
 import { API } from '../api';
+import type { SuccessResult } from '../definition';
 import { getTrimmedServerVersion } from '../lib/getTrimmedServerVersion';
 
 const app = express();
@@ -41,18 +43,42 @@ const getTypedRoutes = (
 	);
 };
 
+const TAG_DESCRIPTIONS: Record<string, string> = {
+	'Missing Documentation': 'Endpoints that are not typed yet; their request and response shapes are not described.',
+};
+
+const getTags = (paths: Record<string, Record<string, Route>>) => {
+	const names = new Set(
+		Object.values(paths)
+			.flatMap((methods) => Object.values(methods))
+			.flatMap((route) => route.tags ?? []),
+	);
+
+	return [...names].sort().map((name) => ({
+		name,
+		...(TAG_DESCRIPTIONS[name] && { description: TAG_DESCRIPTIONS[name] }),
+	}));
+};
+
+const siteUrl = () => settings.get<string>('Site_Url')?.replace(/\/$/, '');
+
 const makeOpenAPIResponse = (paths: Record<string, Record<string, Route>>) => ({
-	openapi: '3.0.3',
+	openapi: '3.1.0',
 	info: {
 		title: 'Rocket.Chat API',
-		description: 'Rocket.Chat API',
+		description:
+			'REST API of this Rocket.Chat workspace. Authenticate by sending the `X-User-Id` and `X-Auth-Token` headers obtained from `/api/v1/login`.',
 		version: getTrimmedServerVersion(),
 	},
-	servers: [
-		{
-			url: settings.get('Site_Url'),
-		},
-	],
+	externalDocs: {
+		url: 'https://developer.rocket.chat/apidocs',
+		description: 'Rocket.Chat developer documentation',
+	},
+	// trailing slash would make every path in the document resolve with a double one, and a Server
+	// Object without a url is invalid, so an unset `Site_Url` means no `servers` at all
+	...(siteUrl() && { servers: [{ url: siteUrl() }] }),
+	tags: getTags(paths),
+	paths: withOperationIds(paths),
 	components: {
 		securitySchemes: {
 			userId: {
@@ -66,24 +92,26 @@ const makeOpenAPIResponse = (paths: Record<string, Record<string, Route>>) => ({
 				name: 'X-Auth-Token',
 			},
 		},
-		schemas: schemas.components.schemas,
+		schemas: {
+			...schemas.components.schemas,
+			...openAPIErrorComponents,
+			...getSharedSchemas(),
+		},
 	},
-	schemas: schemas.components.schemas,
-	paths,
 });
 
 const openApiResponseSchema = ajv.compile<Record<string, unknown>>({
 	type: 'object',
 	properties: {
 		openapi: { type: 'string' },
 		info: { type: 'object' },
-		servers: { type: 'array' },
+		externalDocs: { type: 'object' },
+		servers: { type: 'array', items: {} },
+		tags: { type: 'array', items: {} },
 		components: { type: 'object' },
 		paths: { type: 'object' },
-		schemas: { type: 'object' },
-		success: { type: 'boolean', enum: [true] },
 	},
-	required: ['openapi', 'info', 'paths', 'success'],
+	required: ['openapi', 'info', 'paths'],
 	additionalProperties: false,
 });
 
@@ -99,7 +127,13 @@ API.default.get(
 	function action() {
 		const { withUndocumented = false } = this.queryParams;
 
-		return API.default.success(makeOpenAPIResponse(getTypedRoutes(API.api.typedRoutes, { withUndocumented })));
+		// The document is served as it is: `API.default.success` would add a `success` key to its root,
+		// which is not an OpenAPI field and fails validation. The cast is the price of saying so - every
+		// 2xx body is typed as `{ success: true } & T`, and this one is the exception.
+		return {
+			statusCode: 200 as const,
+			body: makeOpenAPIResponse(getTypedRoutes(API.api.typedRoutes, { withUndocumented })),
+		} as unknown as SuccessResult<Record<string, unknown>>;
 	},
 );
 
@@ -108,7 +142,10 @@ app.use(
 	swaggerUi.serve,
 	swaggerUi.setup(null, {
 		swaggerOptions: {
-			url: `${settings.get('Site_Url')}/api/docs/json`,
+			// Relative to `/api-docs`, not to the root: this runs at import time, before settings are
+			// loaded, so `Site_Url` would render as "undefined", and a leading slash would drop the
+			// deployment prefix of a workspace hosted under ROOT_URL_PATH_PREFIX.
+			url: '../api/docs/json',
 		},
 	}),
 );

apps/meteor/server/api/definition.ts

@@ -1,6 +1,7 @@
 import type { IncomingMessage } from 'node:http';
 
 import type { IUser, LicenseModule, RequiredField } from '@rocket.chat/core-typings';
+import type { OpenAPIDocumentation } from '@rocket.chat/http-router';
 import type { Logger } from '@rocket.chat/logger';
 import type { Method, MethodOf, OperationParams, OperationResult, PathPattern, UrlParams } from '@rocket.chat/rest-typings';
 import type { ValidateFunction } from 'ajv';
@@ -294,7 +295,8 @@ export type TypedOptions = {
 	tags?: string[];
 	typed?: boolean;
 	license?: LicenseModule[];
-} & SharedOptions<'GET' | 'POST' | 'PUT' | 'DELETE' | 'PATCH'>;
+} & OpenAPIDocumentation &
+	SharedOptions<'GET' | 'POST' | 'PUT' | 'DELETE' | 'PATCH'>;
 
 export type TypedThis<TOptions extends TypedOptions, TPath extends string = ''> = {
 	readonly logger: Logger;