From df9292ff534877fc3580d0560c44acb75fd9dae4 Mon Sep 17 00:00:00 2001
From: "weekwith.me" <63915557+0417taehyun@users.noreply.github.com>
Date: Fri, 20 Dec 2024 18:53:33 +0900
Subject: [PATCH] fix: Change Open API validation middleware to specify and use
path parameters (#8913)
## About the changes
Moved Open API validation handler to the controller layer to reuse on
all services such as project and segments, and also removed unnecessary
middleware at the top level, `app.ts`, and method, `useErrorHandler` in
`openapi-service.ts`.
### Important files
#### Before
Express cant' parse the path parameter because it doesn't be specified
on the `use` method. Therefore, it returns `undefined` as an error
message.
#### After
Express can parse the path parameter because I change to specify it on
the controller layer. Accordingly, it returns `test`.
---
src/lib/app.ts | 4 ----
src/lib/routes/controller.ts | 13 +++++++++++++
src/lib/services/openapi-service.ts | 16 ----------------
3 files changed, 13 insertions(+), 20 deletions(-)
diff --git a/src/lib/app.ts b/src/lib/app.ts
index f3c1483904..2d407d7405 100644
--- a/src/lib/app.ts
+++ b/src/lib/app.ts
@@ -197,10 +197,6 @@ export default async function getApp(
// Setup API routes
app.use(`${baseUriPath}/`, new IndexRouter(config, services, db).router);
- if (services.openApiService) {
- services.openApiService.useErrorHandler(app);
- }
-
if (process.env.NODE_ENV !== 'production') {
app.use(errorHandler());
} else {
diff --git a/src/lib/routes/controller.ts b/src/lib/routes/controller.ts
index d9ba28aa2f..0b3f36f153 100644
--- a/src/lib/routes/controller.ts
+++ b/src/lib/routes/controller.ts
@@ -10,6 +10,7 @@ import { type IUnleashConfig, NONE } from '../types';
import { handleErrors } from './util';
import requireContentType from '../middleware/content_type_checker';
import { PermissionError } from '../error';
+import { fromOpenApiValidationErrors } from '../error/bad-data-error';
import { storeRequestedRoute } from '../middleware/response-time-metrics';
type IRequestHandler
= (
@@ -64,6 +65,16 @@ const checkPrivateProjectPermissions = () => async (req, res, next) => {
return res.status(404).end();
};
+const openAPIValidationMiddleware = async (err, req, res, next) => {
+ if (err?.status && err.validationErrors) {
+ const apiError = fromOpenApiValidationErrors(req, err.validationErrors);
+
+ res.status(apiError.statusCode).json(apiError);
+ } else {
+ next(err);
+ }
+};
+
/**
* Base class for Controllers to standardize binding to express Router.
*
@@ -114,6 +125,8 @@ export default class Controller {
this.useContentTypeMiddleware(options),
this.useRouteErrorHandler(options.handler.bind(this)),
);
+
+ this.app.use(options.path, openAPIValidationMiddleware);
}
get(
diff --git a/src/lib/services/openapi-service.ts b/src/lib/services/openapi-service.ts
index f678bfb413..4cec326990 100644
--- a/src/lib/services/openapi-service.ts
+++ b/src/lib/services/openapi-service.ts
@@ -11,7 +11,6 @@ import type { ApiOperation } from '../openapi/util/api-operation';
import type { Logger } from '../logger';
import { validateSchema } from '../openapi/validate';
import type { IFlagResolver } from '../types';
-import { fromOpenApiValidationErrors } from '../error/bad-data-error';
export class OpenApiService {
private readonly config: IUnleashConfig;
@@ -60,21 +59,6 @@ export class OpenApiService {
});
}
- useErrorHandler(app: Express): void {
- app.use((err, req, res, next) => {
- if (err?.status && err.validationErrors) {
- const apiError = fromOpenApiValidationErrors(
- req,
- err.validationErrors,
- );
-
- res.status(apiError.statusCode).json(apiError);
- } else {
- next(err);
- }
- });
- }
-
respondWithValidation