From 43f6767e690c415b2cc7f5ca6193d4cf49ec0840 Mon Sep 17 00:00:00 2001 From: Mostafa Moradian Date: Tue, 25 Jun 2024 20:20:55 +0200 Subject: [PATCH 1/5] feat: add operationId to HTTP methods on paths --- packages/core/src/routes/swagger/index.ts | 56 ++++++++++++++++++++++- 1 file changed, 54 insertions(+), 2 deletions(-) diff --git a/packages/core/src/routes/swagger/index.ts b/packages/core/src/routes/swagger/index.ts index b27831f8c47..7ac9ba25daf 100644 --- a/packages/core/src/routes/swagger/index.ts +++ b/packages/core/src/routes/swagger/index.ts @@ -3,11 +3,13 @@ import { fileURLToPath } from 'node:url'; import { httpCodeToMessage } from '@logto/core-kit'; import { cond, condArray, condString, conditionalArray, deduplicate } from '@silverhand/essentials'; +import camelcase from 'camelcase'; import deepmerge from 'deepmerge'; import { findUp } from 'find-up'; import type { IMiddleware } from 'koa-router'; import type Router from 'koa-router'; -import { type OpenAPIV3 } from 'openapi-types'; +import { OpenAPIV3 } from 'openapi-types'; +import pluralize from 'pluralize'; import { EnvSet } from '#src/env-set/index.js'; import { isKoaAuthMiddleware } from '#src/middleware/koa-auth/index.js'; @@ -53,7 +55,56 @@ type RouteObject = { operation: OpenAPIV3.OperationObject; }; +const methodMap = new Map([ + [OpenAPIV3.HttpMethods.GET, 'Get'], + [OpenAPIV3.HttpMethods.POST, 'Create'], + [OpenAPIV3.HttpMethods.PUT, 'Replace'], + [OpenAPIV3.HttpMethods.PATCH, 'Update'], + [OpenAPIV3.HttpMethods.DELETE, 'Delete'], +]); + +const customRoutes = new Map([ + ['post:/organizations/:id/users/roles', 'AssignRolesToUsers'], + ['post:/organizations/:id/users/:userId/roles', 'AssignRolesToSpecificUser'], + ['get:/configs/jwt-customizer', 'ListJwtCustomizers'], + ['get:/organizations/:id/users/:userId/roles', 'ListRolesForSpecificUser'], + [ + 'delete:/organizations/:id/users/:userId/roles/:organizationRoleId', + 'RemoveRolesFromSpecificUser', + ], + ['put:/organizations/:id/users/:userId/roles', 'ReplaceRolesForSpecificUser'], +]); + +export const buildOperationId = (method: OpenAPIV3.HttpMethods, path: string) => { + const customAction = customRoutes.get(`${method}:${path}`); + if (customAction) { + return customAction; + } + + const singleItem = + path + .split('/') + .slice(-1) + .filter((segment) => segment.startsWith(':') || segment.startsWith('{')).length === 1; + const action = path + .split('/') + .filter((segment) => !segment.startsWith('{') && !segment.startsWith(':')) + .slice(-2) + .map((segment) => camelcase(segment, { pascalCase: true })) + .join(''); + const verb = + !singleItem && method === OpenAPIV3.HttpMethods.GET && !pluralize.isSingular(action) + ? 'List' + : methodMap.get(method); + + return ( + verb + + (method === OpenAPIV3.HttpMethods.POST || singleItem ? pluralize.singular(action) : action) + ); +}; + const buildOperation = ( + method: OpenAPIV3.HttpMethods, stack: IMiddleware[], path: string, isAuthGuarded: boolean @@ -111,6 +162,7 @@ const buildOperation = ( const [firstSegment] = path.split('/').slice(1); return { + operationId: buildOperationId(method, path), tags: [buildTag(path)], parameters: [...pathParameters, ...queryParameters], requestBody, @@ -184,7 +236,7 @@ export default function swaggerRoutes method !== 'head') .map((httpMethod) => { const path = normalizePath(routerPath); - const operation = buildOperation(stack, routerPath, isAuthGuarded); + const operation = buildOperation(httpMethod, stack, routerPath, isAuthGuarded); return { path, From 9ad4d9d6e1503df0717b9f2969a29dbb3f1a89bf Mon Sep 17 00:00:00 2001 From: Gao Sun Date: Tue, 2 Jul 2024 23:06:44 +0800 Subject: [PATCH 2/5] refactor(core): strictly handle routes for building operation id --- packages/core/src/routes/swagger/index.ts | 69 ++---- .../core/src/routes/swagger/utils/general.ts | 2 + .../routes/swagger/utils/operation-id.test.ts | 41 ++++ .../src/routes/swagger/utils/operation-id.ts | 196 ++++++++++++++++++ 4 files changed, 256 insertions(+), 52 deletions(-) create mode 100644 packages/core/src/routes/swagger/utils/operation-id.test.ts create mode 100644 packages/core/src/routes/swagger/utils/operation-id.ts diff --git a/packages/core/src/routes/swagger/index.ts b/packages/core/src/routes/swagger/index.ts index 7ac9ba25daf..ffbc37d6626 100644 --- a/packages/core/src/routes/swagger/index.ts +++ b/packages/core/src/routes/swagger/index.ts @@ -3,13 +3,11 @@ import { fileURLToPath } from 'node:url'; import { httpCodeToMessage } from '@logto/core-kit'; import { cond, condArray, condString, conditionalArray, deduplicate } from '@silverhand/essentials'; -import camelcase from 'camelcase'; import deepmerge from 'deepmerge'; import { findUp } from 'find-up'; import type { IMiddleware } from 'koa-router'; import type Router from 'koa-router'; -import { OpenAPIV3 } from 'openapi-types'; -import pluralize from 'pluralize'; +import { type OpenAPIV3 } from 'openapi-types'; import { EnvSet } from '#src/env-set/index.js'; import { isKoaAuthMiddleware } from '#src/middleware/koa-auth/index.js'; @@ -30,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, @@ -55,54 +55,6 @@ type RouteObject = { operation: OpenAPIV3.OperationObject; }; -const methodMap = new Map([ - [OpenAPIV3.HttpMethods.GET, 'Get'], - [OpenAPIV3.HttpMethods.POST, 'Create'], - [OpenAPIV3.HttpMethods.PUT, 'Replace'], - [OpenAPIV3.HttpMethods.PATCH, 'Update'], - [OpenAPIV3.HttpMethods.DELETE, 'Delete'], -]); - -const customRoutes = new Map([ - ['post:/organizations/:id/users/roles', 'AssignRolesToUsers'], - ['post:/organizations/:id/users/:userId/roles', 'AssignRolesToSpecificUser'], - ['get:/configs/jwt-customizer', 'ListJwtCustomizers'], - ['get:/organizations/:id/users/:userId/roles', 'ListRolesForSpecificUser'], - [ - 'delete:/organizations/:id/users/:userId/roles/:organizationRoleId', - 'RemoveRolesFromSpecificUser', - ], - ['put:/organizations/:id/users/:userId/roles', 'ReplaceRolesForSpecificUser'], -]); - -export const buildOperationId = (method: OpenAPIV3.HttpMethods, path: string) => { - const customAction = customRoutes.get(`${method}:${path}`); - if (customAction) { - return customAction; - } - - const singleItem = - path - .split('/') - .slice(-1) - .filter((segment) => segment.startsWith(':') || segment.startsWith('{')).length === 1; - const action = path - .split('/') - .filter((segment) => !segment.startsWith('{') && !segment.startsWith(':')) - .slice(-2) - .map((segment) => camelcase(segment, { pascalCase: true })) - .join(''); - const verb = - !singleItem && method === OpenAPIV3.HttpMethods.GET && !pluralize.isSingular(action) - ? 'List' - : methodMap.get(method); - - return ( - verb + - (method === OpenAPIV3.HttpMethods.POST || singleItem ? pluralize.singular(action) : action) - ); -}; - const buildOperation = ( method: OpenAPIV3.HttpMethods, stack: IMiddleware[], @@ -222,6 +174,12 @@ export default function swaggerRoutes { + /** + * A set to store all custom routes that have been built. + * @see {@link customRoutes} + */ + const builtCustomRoutes = new Set(); + const routes = allRouters.flatMap((router) => { const isAuthGuarded = isManagementApiRouter(router); @@ -238,6 +196,10 @@ export default function swaggerRoutes(); const tags = new Set(); @@ -338,7 +303,7 @@ export default function swaggerRoutes !EnvSet.values.isProduction || EnvSet.values.isIntegrationTest; diff --git a/packages/core/src/routes/swagger/utils/operation-id.test.ts b/packages/core/src/routes/swagger/utils/operation-id.test.ts new file mode 100644 index 00000000000..27e137fc527 --- /dev/null +++ b/packages/core/src/routes/swagger/utils/operation-id.test.ts @@ -0,0 +1,41 @@ +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' + ); + }); +}); diff --git a/packages/core/src/routes/swagger/utils/operation-id.ts b/packages/core/src/routes/swagger/utils/operation-id.ts new file mode 100644 index 00000000000..bd84bd997a7 --- /dev/null +++ b/packages/core/src/routes/swagger/utils/operation-id.ts @@ -0,0 +1,196 @@ +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 = (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); + +type RouteDictionary = Record<`${OpenAPIV3.HttpMethods} ${string}`, string>; + +const devFeatureCustomRoutes: RouteDictionary = Object.freeze({ + // Security + 'post /security/subject-tokens': 'CreateSubjectToken', +}); + +export const customRoutes: Readonly = 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/endpoints/:tenantId': 'GetTenantEndpoint', + '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) => { + 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(', ')}.` + ); + } + } +}; + +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. If the HTTP method is `GET` and the + * path does not end with a path parameter, the verb will be `List`. + * + * If an override is found in `customRoutes`, it will be used instead. + * + * @example + * buildOperationId('get', '/foo/:fooId/bar/:barId') // GetFooBar + * buildOperationId('post', '/foo/:fooId/bar') // CreateFooBar + * buildOperationId('get', '/foo/:fooId/bar') // ListFooBars + * + * @see {@link customRoutes} for the full list of overrides. + * @see {@link methodToVerb} for the mapping of HTTP methods to verbs. + */ +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; + } + + // Special case for JIT APIs since `jit/` is more like a namespace. + const splitted = path.replace('jit/', 'jit-').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 ? 'List' : methodToVerb[method]; + + if (isForSingleItem) { + return verb + itemTypes.join(''); + } + + return ( + verb + + itemTypes.join('') + + camelcase(method === OpenAPIV3.HttpMethods.POST ? pluralize.singular(lastItem) : lastItem, { + pascalCase: true, + }) + ); +}; From e62463538ce0eea3fdbf123bdc59f0daf5f9ce66 Mon Sep 17 00:00:00 2001 From: Gao Sun Date: Tue, 2 Jul 2024 23:09:18 +0800 Subject: [PATCH 3/5] chore: add changeset --- .changeset/brown-cobras-know.md | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) create mode 100644 .changeset/brown-cobras-know.md diff --git a/.changeset/brown-cobras-know.md b/.changeset/brown-cobras-know.md new file mode 100644 index 00000000000..251ad3fe0c5 --- /dev/null +++ b/.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.ListOrganizationsUsers(ctx, req.GetId()).Execute() +``` From 00c9442518add7a9e0c015c9b379b2a12f14e67e Mon Sep 17 00:00:00 2001 From: Gao Sun Date: Wed, 3 Jul 2024 11:57:56 +0800 Subject: [PATCH 4/5] refactor: reorg code --- .changeset/brown-cobras-know.md | 2 +- .../core/src/routes/swagger/index.test.ts | 8 ++--- .../src/routes/swagger/utils/operation-id.ts | 25 ++++++++++++---- packages/integration-tests/src/api/index.ts | 2 +- .../src/tests/api/swagger-check.test.ts | 30 +++++++++++-------- 5 files changed, 43 insertions(+), 24 deletions(-) diff --git a/.changeset/brown-cobras-know.md b/.changeset/brown-cobras-know.md index 251ad3fe0c5..ce616fd9736 100644 --- a/.changeset/brown-cobras-know.md +++ b/.changeset/brown-cobras-know.md @@ -17,5 +17,5 @@ This greatly simplifies the creation of client SDKs in different languages, beca ```diff - users, _, err := s.Client.OrganizationsAPI.ApiOrganizationsIdUsersGet(ctx, req.GetId()).Execute() -+ users, _, err := s.Client.OrganizationsAPI.ListOrganizationsUsers(ctx, req.GetId()).Execute() ++ users, _, err := s.Client.OrganizationsAPI.ListOrganizationUsers(ctx, req.GetId()).Execute() ``` diff --git a/packages/core/src/routes/swagger/index.test.ts b/packages/core/src/routes/swagger/index.test.ts index 7444f2d6761..f339596d0e8 100644 --- a/packages/core/src/routes/swagger/index.test.ts +++ b/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: [ { diff --git a/packages/core/src/routes/swagger/utils/operation-id.ts b/packages/core/src/routes/swagger/utils/operation-id.ts index bd84bd997a7..b7fd2d44c8d 100644 --- a/packages/core/src/routes/swagger/utils/operation-id.ts +++ b/packages/core/src/routes/swagger/utils/operation-id.ts @@ -79,7 +79,6 @@ export const customRoutes: Readonly = Object.freeze({ // User assets 'get /user-assets/service-status': 'GetUserAssetServiceStatus', // Well-known - 'get /.well-known/endpoints/:tenantId': 'GetTenantEndpoint', 'get /.well-known/phrases': 'GetSignInExperiencePhrases', 'get /.well-known/sign-in-exp': 'GetSignInExperienceConfig', ...(EnvSet.values.isDevFeaturesEnabled ? devFeatureCustomRoutes : {}), @@ -90,6 +89,11 @@ export const customRoutes: Readonly = Object.freeze({ * built routes and the routes defined in `customRoutes`. */ export const throwByDifference = (builtCustomRoutes: Set) => { + // 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) @@ -115,6 +119,9 @@ export const throwByDifference = (builtCustomRoutes: Set) => { } }; +/** 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('{'))); @@ -128,18 +135,22 @@ const throwIfNeeded = (method: OpenAPIV3.HttpMethods, path: string) => { * 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. If the HTTP method is `GET` and the - * path does not end with a path parameter, the verb will be `List`. + * the HTTP method and `Noun` is the path segment in PascalCase. Some exceptions: * - * If an override is found in `customRoutes`, it will be used instead. + * 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}`]; @@ -153,8 +164,10 @@ export const buildOperationId = (method: OpenAPIV3.HttpMethods, path: string) => return; } - // Special case for JIT APIs since `jit/` is more like a namespace. - const splitted = path.replace('jit/', 'jit-').split('/'); + const splitted = namespacePrefixes + .reduce((accumulator, prefix) => accumulator.replace(prefix + '/', prefix + '-'), path) + .split('/'); + const lastItem = splitted.at(-1); if (!lastItem) { diff --git a/packages/integration-tests/src/api/index.ts b/packages/integration-tests/src/api/index.ts index d1147343415..160d2a705d1 100644 --- a/packages/integration-tests/src/api/index.ts +++ b/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'; diff --git a/packages/integration-tests/src/tests/api/swagger-check.test.ts b/packages/integration-tests/src/tests/api/swagger-check.test.ts index 22b17267889..e2a2b3c49ff 100644 --- a/packages/integration-tests/src/tests/api/swagger-check.test.ts +++ b/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(); + } + ); }); From 933b1d0b9e82b57fe2b89f4e91f54e3c07708619 Mon Sep 17 00:00:00 2001 From: Gao Sun Date: Wed, 3 Jul 2024 13:05:23 +0800 Subject: [PATCH 5/5] refactor: use get as verb for singular items --- .../src/routes/swagger/utils/operation-id.test.ts | 12 ++++++++++++ .../core/src/routes/swagger/utils/operation-id.ts | 4 +++- 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/packages/core/src/routes/swagger/utils/operation-id.test.ts b/packages/core/src/routes/swagger/utils/operation-id.test.ts index 27e137fc527..12ea4195ef7 100644 --- a/packages/core/src/routes/swagger/utils/operation-id.test.ts +++ b/packages/core/src/routes/swagger/utils/operation-id.test.ts @@ -38,4 +38,16 @@ describe('buildOperationId', () => { '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' + ); + }); }); diff --git a/packages/core/src/routes/swagger/utils/operation-id.ts b/packages/core/src/routes/swagger/utils/operation-id.ts index b7fd2d44c8d..fe3df4002eb 100644 --- a/packages/core/src/routes/swagger/utils/operation-id.ts +++ b/packages/core/src/routes/swagger/utils/operation-id.ts @@ -193,7 +193,9 @@ export const buildOperationId = (method: OpenAPIV3.HttpMethods, path: string) => ); const verb = - !isForSingleItem && method === OpenAPIV3.HttpMethods.GET ? 'List' : methodToVerb[method]; + !isForSingleItem && method === OpenAPIV3.HttpMethods.GET && pluralize.isPlural(lastItem) + ? 'List' + : methodToVerb[method]; if (isForSingleItem) { return verb + itemTypes.join('');