feat: add operationId to HTTP methods on paths (#6108)

* feat: add operationId to HTTP methods on paths

* refactor(core): strictly handle routes for building operation id

* chore: add changeset

* refactor: reorg code

* refactor: use get as verb for singular items

---------

Co-authored-by: Gao Sun <gao@silverhand.io>

.changeset/brown-cobras-know.md

@@ -0,0 +1,21 @@
+---
+"@logto/core": patch
+---
+
+build `operationId` for Management API in OpenAPI response (credit to @mostafa)
+
+As per [the specification](https://swagger.io/docs/specification/paths-and-operations/):
+
+> `operationId` is an optional unique string used to identify an operation. If provided, these IDs must be unique among all operations described in your API.
+
+This greatly simplifies the creation of client SDKs in different languages, because it generates more meaningful function names instead of auto-generated ones, like the following examples:
+
+```diff
+- org, _, err := s.Client.OrganizationsAPI.ApiOrganizationsIdGet(ctx, req.GetId()).Execute()
++ org, _, err := s.Client.OrganizationsAPI.GetOrganization(ctx, req.GetId()).Execute()
+```
+
+```diff
+- users, _, err := s.Client.OrganizationsAPI.ApiOrganizationsIdUsersGet(ctx, req.GetId()).Execute()
++ users, _, err := s.Client.OrganizationsAPI.ListOrganizationUsers(ctx, req.GetId()).Execute()
+```

packages/core/src/routes/swagger/index.test.ts

@@ -92,7 +92,7 @@ describe('GET /swagger.json', () => {
   it('should parse the path parameters', async () => {
     const queryParametersRouter = new Router();
     queryParametersRouter.get(
-      '/mock/:id/:field',
+      '/mock/:id/fields/:field',
       koaGuard({
         params: object({
           id: number(),
@@ -103,7 +103,7 @@ describe('GET /swagger.json', () => {
     );
     // Test plural
     queryParametersRouter.get(
-      '/mocks/:id/:field',
+      '/mocks/:id/fields/:field',
       koaGuard({
         params: object({
           id: number(),
@@ -116,7 +116,7 @@ describe('GET /swagger.json', () => {
 
     const response = await swaggerRequest.get('/swagger.json');
     expect(response.body.paths).toMatchObject({
-      '/api/mock/{id}/{field}': {
+      '/api/mock/{id}/fields/{field}': {
         get: {
           parameters: [
             {
@@ -131,7 +131,7 @@ describe('GET /swagger.json', () => {
           ],
         },
       },
-      '/api/mocks/{id}/{field}': {
+      '/api/mocks/{id}/fields/{field}': {
         get: {
           parameters: [
             {

packages/core/src/routes/swagger/index.ts

@@ -28,9 +28,11 @@ import {
   findSupplementFiles,
   normalizePath,
   removeUnnecessaryOperations,
+  shouldThrow,
   validateSupplement,
   validateSwaggerDocument,
 } from './utils/general.js';
+import { buildOperationId, customRoutes, throwByDifference } from './utils/operation-id.js';
 import {
   buildParameters,
   paginationParameters,
@@ -54,6 +56,7 @@ type RouteObject = {
 };
 
 const buildOperation = (
+  method: OpenAPIV3.HttpMethods,
   stack: IMiddleware[],
   path: string,
   isAuthGuarded: boolean
@@ -111,6 +114,7 @@ const buildOperation = (
   const [firstSegment] = path.split('/').slice(1);
 
   return {
+    operationId: buildOperationId(method, path),
     tags: [buildTag(path)],
     parameters: [...pathParameters, ...queryParameters],
     requestBody,
@@ -170,6 +174,12 @@ export default function swaggerRoutes<T extends AnonymousRouter, R extends Route
   allRouters: R[]
 ) {
   router.get('/swagger.json', async (ctx, next) => {
+    /**
+     * A set to store all custom routes that have been built.
+     * @see {@link customRoutes}
+     */
+    const builtCustomRoutes = new Set<string>();
+
     const routes = allRouters.flatMap<RouteObject>((router) => {
       const isAuthGuarded = isManagementApiRouter(router);
 
@@ -184,7 +194,11 @@ export default function swaggerRoutes<T extends AnonymousRouter, R extends Route
               .filter((method): method is OpenAPIV3.HttpMethods => method !== 'head')
               .map((httpMethod) => {
                 const path = normalizePath(routerPath);
-                const operation = buildOperation(stack, routerPath, isAuthGuarded);
+                const operation = buildOperation(httpMethod, stack, routerPath, isAuthGuarded);
+
+                if (customRoutes[`${httpMethod} ${routerPath}`]) {
+                  builtCustomRoutes.add(`${httpMethod} ${routerPath}`);
+                }
 
                 return {
                   path,
@@ -196,6 +210,9 @@ export default function swaggerRoutes<T extends AnonymousRouter, R extends Route
       );
     });
 
+    // Ensure all custom routes are built.
+    throwByDifference(builtCustomRoutes);
+
     const pathMap = new Map<string, OpenAPIV3.PathItemObject>();
     const tags = new Set<string>();
 
@@ -286,7 +303,7 @@ export default function swaggerRoutes<T extends AnonymousRouter, R extends Route
       getConsoleLogFromContext(ctx).warn('Skip validating swagger document in unit test.');
     }
     // Don't throw for integrity check in production as it has no benefit.
-    else if (!EnvSet.values.isProduction || EnvSet.values.isIntegrationTest) {
+    else if (shouldThrow()) {
       for (const document of supplementDocuments) {
         validateSupplement(baseDocument, document);
       }

packages/core/src/routes/swagger/utils/general.ts

@@ -262,3 +262,5 @@ export const removeUnnecessaryOperations = (
 
   return document;
 };
+
+export const shouldThrow = () => !EnvSet.values.isProduction || EnvSet.values.isIntegrationTest;

packages/core/src/routes/swagger/utils/operation-id.test.ts

@@ -0,0 +1,53 @@
+import { OpenAPIV3 } from 'openapi-types';
+
+import { buildOperationId, customRoutes } from './operation-id.js';
+
+describe('buildOperationId', () => {
+  it('should return the custom operation id if it exists', () => {
+    for (const [path, operationId] of Object.entries(customRoutes)) {
+      const [method, pathSegments] = path.split(' ');
+      expect(buildOperationId(method! as OpenAPIV3.HttpMethods, pathSegments!)).toBe(operationId);
+    }
+  });
+
+  it('should skip interactions APIs', () => {
+    expect(buildOperationId(OpenAPIV3.HttpMethods.GET, '/interaction/footballs')).toBeUndefined();
+  });
+
+  it('should handle JIT APIs', () => {
+    expect(buildOperationId(OpenAPIV3.HttpMethods.GET, '/footballs/:footballId/jit/bars')).toBe(
+      'ListFootballJitBars'
+    );
+  });
+
+  it('should throw if the path is invalid', () => {
+    expect(() =>
+      buildOperationId(OpenAPIV3.HttpMethods.GET, '/footballs/:footballId/bar/baz')
+    ).toThrow();
+    expect(() => buildOperationId(OpenAPIV3.HttpMethods.GET, '/')).toThrow();
+  });
+
+  it('should singularize the item for POST requests', () => {
+    expect(buildOperationId(OpenAPIV3.HttpMethods.POST, '/footballs/:footballId/bars')).toBe(
+      'CreateFootballBar'
+    );
+  });
+
+  it('should singularize for single item requests', () => {
+    expect(buildOperationId(OpenAPIV3.HttpMethods.DELETE, '/footballs/:footballId')).toBe(
+      'DeleteFootball'
+    );
+  });
+
+  it('should use "Get" if the last item is singular', () => {
+    expect(buildOperationId(OpenAPIV3.HttpMethods.GET, '/footballs/:footballId/bar')).toBe(
+      'GetFootballBar'
+    );
+  });
+
+  it('should use "List" if the last item is plural', () => {
+    expect(buildOperationId(OpenAPIV3.HttpMethods.GET, '/footballs/:footballId/bars')).toBe(
+      'ListFootballBars'
+    );
+  });
+});

packages/core/src/routes/swagger/utils/operation-id.ts

@@ -0,0 +1,211 @@
+import camelcase from 'camelcase';
+import { OpenAPIV3 } from 'openapi-types';
+import pluralize from 'pluralize';
+
+import { EnvSet } from '#src/env-set/index.js';
+
+import { shouldThrow } from './general.js';
+
+const chunk = <T>(array: T[], chunkSize: number): T[][] =>
+  Array.from({ length: Math.ceil(array.length / chunkSize) }, (_, i) =>
+    array.slice(i * chunkSize, i * chunkSize + chunkSize)
+  );
+
+const methodToVerb = Object.freeze({
+  get: 'Get',
+  post: 'Create',
+  put: 'Replace',
+  patch: 'Update',
+  delete: 'Delete',
+  options: 'Options',
+  head: 'Head',
+  trace: 'Trace',
+} satisfies Record<OpenAPIV3.HttpMethods, string>);
+
+type RouteDictionary = Record<`${OpenAPIV3.HttpMethods} ${string}`, string>;
+
+const devFeatureCustomRoutes: RouteDictionary = Object.freeze({
+  // Security
+  'post /security/subject-tokens': 'CreateSubjectToken',
+});
+
+export const customRoutes: Readonly<RouteDictionary> = Object.freeze({
+  // Authn
+  'get /authn/hasura': 'GetHasuraAuth',
+  'post /authn/saml/:connectorId': 'AssertSaml',
+  'post /authn/single-sign-on/saml/:connectorId': 'AssertSingleSignOnSaml',
+  // Organization users
+  'post /organizations/:id/users': 'AddOrganizationUsers',
+  'post /organizations/:id/users/roles': 'AssignOrganizationRolesToUsers',
+  'post /organizations/:id/users/:userId/roles': 'AssignOrganizationRolesToUser',
+  // Organization applications
+  'post /organizations/:id/applications': 'AddOrganizationApplications',
+  'post /organizations/:id/applications/roles': 'AssignOrganizationRolesToApplications',
+  'post /organizations/:id/applications/:applicationId/roles':
+    'AssignOrganizationRolesToApplication',
+  // Configs
+  'get /configs/jwt-customizer': 'ListJwtCustomizers',
+  'put /configs/jwt-customizer/:tokenTypePath': 'UpsertJwtCustomizer',
+  'patch /configs/jwt-customizer/:tokenTypePath': 'UpdateJwtCustomizer',
+  'get /configs/jwt-customizer/:tokenTypePath': 'GetJwtCustomizer',
+  'delete /configs/jwt-customizer/:tokenTypePath': 'DeleteJwtCustomizer',
+  'post /configs/jwt-customizer/test': 'TestJwtCustomizer',
+  'get /configs/oidc/:keyType': 'GetOidcKeys',
+  'delete /configs/oidc/:keyType/:keyId': 'DeleteOidcKey',
+  'post /configs/oidc/:keyType/rotate': 'RotateOidcKeys',
+  'get /configs/admin-console': 'GetAdminConsoleConfig',
+  'patch /configs/admin-console': 'UpdateAdminConsoleConfig',
+  // Systems
+  'get /systems/application': 'GetSystemApplicationConfig',
+  // Applications
+  'post /applications/:applicationId/roles': 'AssignApplicationRoles',
+  'get /applications/:id/protected-app-metadata/custom-domains':
+    'ListApplicationProtectedAppMetadataCustomDomains',
+  'post /applications/:id/protected-app-metadata/custom-domains':
+    'CreateApplicationProtectedAppMetadataCustomDomain',
+  'delete /applications/:id/protected-app-metadata/custom-domains/:domain':
+    'DeleteApplicationProtectedAppMetadataCustomDomain',
+  'delete /applications/:applicationId/user-consent-scopes/:scopeType/:scopeId':
+    'DeleteApplicationUserConsentScope',
+  // Users
+  'post /users/:userId/roles': 'AssignUserRoles',
+  'post /users/:userId/password/verify': 'VerifyUserPassword',
+  // Dashboard
+  'get /dashboard/users/total': 'GetTotalUserCount',
+  'get /dashboard/users/new': 'GetNewUserCounts',
+  'get /dashboard/users/active': 'GetActiveUserCounts',
+  // Verification code
+  'post /verification-codes/verify': 'VerifyVerificationCode',
+  // User assets
+  'get /user-assets/service-status': 'GetUserAssetServiceStatus',
+  // Well-known
+  'get /.well-known/phrases': 'GetSignInExperiencePhrases',
+  'get /.well-known/sign-in-exp': 'GetSignInExperienceConfig',
+  ...(EnvSet.values.isDevFeaturesEnabled ? devFeatureCustomRoutes : {}),
+} satisfies RouteDictionary); // Key assertion doesn't work without `satisfies`
+
+/**
+ * Given a set of built custom routes, throws an error if there are any differences between the
+ * built routes and the routes defined in `customRoutes`.
+ */
+export const throwByDifference = (builtCustomRoutes: Set<string>) => {
+  // Unit tests are hard to cover the full list of custom routes, skip the check.
+  if (EnvSet.values.isUnitTest) {
+    return;
+  }
+
+  if (shouldThrow() && builtCustomRoutes.size !== Object.keys(customRoutes).length) {
+    const missingRoutes = Object.entries(customRoutes).filter(
+      ([path]) => !builtCustomRoutes.has(path)
+    );
+
+    if (missingRoutes.length > 0) {
+      throw new Error(
+        'Not all custom routes are built.\n' +
+          `Missing routes: ${missingRoutes.map(([path]) => path).join(', ')}.`
+      );
+    }
+
+    const extraRoutes = [...builtCustomRoutes].filter(
+      (path) => !Object.keys(customRoutes).includes(path)
+    );
+
+    if (extraRoutes.length > 0) {
+      throw new Error(
+        'There are extra routes that are built but not defined in `customRoutes`.\n' +
+          `Extra routes: ${extraRoutes.join(', ')}.`
+      );
+    }
+  }
+};
+
+/** Path segments that are treated as namespace prefixes. */
+const namespacePrefixes = Object.freeze(['jit', '.well-known']);
+
+const isPathParameter = (segment?: string) =>
+  Boolean(segment && (segment.startsWith(':') || segment.startsWith('{')));
+
+const throwIfNeeded = (method: OpenAPIV3.HttpMethods, path: string) => {
+  if (shouldThrow()) {
+    throw new Error(`Invalid path for generating operation ID: ${method} ${path}`);
+  }
+};
+
+/**
+ * Given a method and a path, generates an operation ID that is friendly for creating client SDKs.
+ *
+ * The generated operation ID is in the format of `VerbNounNoun...` where `Verb` is translated from
+ * the HTTP method and `Noun` is the path segment in PascalCase. Some exceptions:
+ *
+ * 1. If an override is found in `customRoutes`, it will be used instead.
+ * 2. If the HTTP method is `GET` and the path does not end with a path parameter, the verb will be
+ * `List`.
+ * 3. If the path segment is a namespace prefix, the trailing `/` will be replaced with `-`.
+ *
+ * @example
+ * buildOperationId('get', '/foo/:fooId/bar/:barId') // GetFooBar
+ * buildOperationId('post', '/foo/:fooId/bar') // CreateFooBar
+ * buildOperationId('get', '/foo/:fooId/bar') // ListFooBars
+ * buildOperationId('get', '/jit/foo') // GetJitFoo
+ *
+ * @see {@link customRoutes} for the full list of overrides.
+ * @see {@link methodToVerb} for the mapping of HTTP methods to verbs.
+ * @see {@link namespacePrefixes} for the list of namespace prefixes.
+ */
+export const buildOperationId = (method: OpenAPIV3.HttpMethods, path: string) => {
+  const customOperationId = customRoutes[`${method} ${path}`];
+
+  if (customOperationId) {
+    return customOperationId;
+  }
+
+  // Skip interactions APIs as they are going to replaced by the new APIs soon.
+  if (path.startsWith('/interaction')) {
+    return;
+  }
+
+  const splitted = namespacePrefixes
+    .reduce((accumulator, prefix) => accumulator.replace(prefix + '/', prefix + '-'), path)
+    .split('/');
+
+  const lastItem = splitted.at(-1);
+
+  if (!lastItem) {
+    throwIfNeeded(method, path);
+    return;
+  }
+
+  const isForSingleItem = isPathParameter(lastItem);
+  const items = chunk(splitted.slice(1, isForSingleItem ? undefined : -1), 2);
+
+  // Check if all items have the pattern of `[name, parameter]`
+  if (
+    !items.every((values): values is [string, string] =>
+      Boolean(values[0] && values[1] && !isPathParameter(values[0]) && isPathParameter(values[1]))
+    )
+  ) {
+    throwIfNeeded(method, path);
+    return;
+  }
+
+  const itemTypes = items.map(([name]) =>
+    camelcase(pluralize.singular(name), { pascalCase: true })
+  );
+
+  const verb =
+    !isForSingleItem && method === OpenAPIV3.HttpMethods.GET && pluralize.isPlural(lastItem)
+      ? 'List'
+      : methodToVerb[method];
+
+  if (isForSingleItem) {
+    return verb + itemTypes.join('');
+  }
+
+  return (
+    verb +
+    itemTypes.join('') +
+    camelcase(method === OpenAPIV3.HttpMethods.POST ? pluralize.singular(lastItem) : lastItem, {
+      pascalCase: true,
+    })
+  );
+};

packages/integration-tests/src/api/index.ts

@@ -9,4 +9,4 @@ export * from './interaction.js';
 export * from './logto-config.js';
 export * from './domain.js';
 
-export { default as api, authedAdminApi } from './api.js';
+export { default as api, authedAdminApi, adminTenantApi } from './api.js';

packages/integration-tests/src/tests/api/swagger-check.test.ts

@@ -2,22 +2,28 @@ import * as SwaggerParser from '@apidevtools/swagger-parser';
 import Validator from 'openapi-schema-validator';
 import type { OpenAPI } from 'openapi-types';
 
-import { api } from '#src/api/index.js';
+import * as apis from '#src/api/index.js';
 
 const { default: OpenApiSchemaValidator } = Validator;
 
 describe('Swagger check', () => {
-  it('should provide a valid swagger.json', async () => {
-    const response = await api.get('swagger.json');
-    expect(response).toHaveProperty('status', 200);
-    expect(response.headers.get('content-type')).toContain('application/json');
+  it.each(['api', 'adminTenantApi'] as const)(
+    'should provide a valid swagger.json for %s',
+    async (apiName) => {
+      const api = apis[apiName];
+      const response = await api.get('swagger.json');
+      expect(response).toHaveProperty('status', 200);
+      expect(response.headers.get('content-type')).toContain('application/json');
 
-    // Use multiple validators to be more confident
-    const object: unknown = await response.json();
+      // Use multiple validators to be more confident
+      const object: unknown = await response.json();
 
-    const validator = new OpenApiSchemaValidator({ version: 3 });
-    const result = validator.validate(object as OpenAPI.Document);
-    expect(result.errors).toEqual([]);
-    await expect(SwaggerParser.default.validate(object as OpenAPI.Document)).resolves.not.toThrow();
-  });
+      const validator = new OpenApiSchemaValidator({ version: 3 });
+      const result = validator.validate(object as OpenAPI.Document);
+      expect(result.errors).toEqual([]);
+      await expect(
+        SwaggerParser.default.validate(object as OpenAPI.Document)
+      ).resolves.not.toThrow();
+    }
+  );
 });