Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add operationId to HTTP methods on paths #6108

Merged
merged 5 commits into from
Jul 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions .changeset/brown-cobras-know.md
Original file line number Diff line number Diff line change
@@ -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()
```
8 changes: 4 additions & 4 deletions packages/core/src/routes/swagger/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand All @@ -103,7 +103,7 @@ describe('GET /swagger.json', () => {
);
// Test plural
queryParametersRouter.get(
'/mocks/:id/:field',
'/mocks/:id/fields/:field',
koaGuard({
params: object({
id: number(),
Expand All @@ -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: [
{
Expand All @@ -131,7 +131,7 @@ describe('GET /swagger.json', () => {
],
},
},
'/api/mocks/{id}/{field}': {
'/api/mocks/{id}/fields/{field}': {
get: {
parameters: [
{
Expand Down
21 changes: 19 additions & 2 deletions packages/core/src/routes/swagger/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,11 @@
findSupplementFiles,
normalizePath,
removeUnnecessaryOperations,
shouldThrow,
validateSupplement,
validateSwaggerDocument,
} from './utils/general.js';
import { buildOperationId, customRoutes, throwByDifference } from './utils/operation-id.js';
import {
buildParameters,
paginationParameters,
Expand All @@ -54,6 +56,7 @@
};

const buildOperation = (
method: OpenAPIV3.HttpMethods,
stack: IMiddleware[],
path: string,
isAuthGuarded: boolean
Expand Down Expand Up @@ -111,6 +114,7 @@
const [firstSegment] = path.split('/').slice(1);

return {
operationId: buildOperationId(method, path),
tags: [buildTag(path)],
parameters: [...pathParameters, ...queryParameters],
requestBody,
Expand Down Expand Up @@ -170,6 +174,12 @@
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);

Expand All @@ -184,7 +194,11 @@
.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}`);
}

Check warning on line 201 in packages/core/src/routes/swagger/index.ts

View check run for this annotation

Codecov / codecov/patch

packages/core/src/routes/swagger/index.ts#L200-L201

Added lines #L200 - L201 were not covered by tests

return {
path,
Expand All @@ -196,6 +210,9 @@
);
});

// Ensure all custom routes are built.
throwByDifference(builtCustomRoutes);

const pathMap = new Map<string, OpenAPIV3.PathItemObject>();
const tags = new Set<string>();

Expand Down Expand Up @@ -286,7 +303,7 @@
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()) {

Check warning on line 306 in packages/core/src/routes/swagger/index.ts

View check run for this annotation

Codecov / codecov/patch

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

Added line #L306 was not covered by tests
for (const document of supplementDocuments) {
validateSupplement(baseDocument, document);
}
Expand Down
2 changes: 2 additions & 0 deletions packages/core/src/routes/swagger/utils/general.ts
Original file line number Diff line number Diff line change
Expand Up @@ -262,3 +262,5 @@ export const removeUnnecessaryOperations = (

return document;
};

export const shouldThrow = () => !EnvSet.values.isProduction || EnvSet.values.isIntegrationTest;
53 changes: 53 additions & 0 deletions packages/core/src/routes/swagger/utils/operation-id.test.ts
Original file line number Diff line number Diff line change
@@ -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'
);
});
});
Loading
Loading