From 0363bf16006ced122e672f36ead62365877c1879 Mon Sep 17 00:00:00 2001 From: Camila Belo Date: Mon, 19 Aug 2024 12:14:51 +0200 Subject: [PATCH] refactor: apply review suggestions Signed-off-by: Camila Belo --- .changeset/heavy-suits-judge.md | 2 +- .changeset/metal-rivers-grin.md | 5 +++ packages/backend-test-utils/api-report.md | 14 +++++++ packages/backend-test-utils/package.json | 3 ++ packages/backend-test-utils/src/index.ts | 1 + .../src/util/errorHandler.ts | 29 +++++++++++++ packages/backend-test-utils/src/util/index.ts | 1 + plugins/user-settings-backend/package.json | 2 - .../user-settings-backend/src/deprecated.ts | 42 ++----------------- .../src/service/router.test.ts | 14 +++---- yarn.lock | 13 +++--- 11 files changed, 70 insertions(+), 56 deletions(-) create mode 100644 .changeset/metal-rivers-grin.md create mode 100644 packages/backend-test-utils/src/util/errorHandler.ts diff --git a/.changeset/heavy-suits-judge.md b/.changeset/heavy-suits-judge.md index f3b16a25f3..d5c00184c7 100644 --- a/.changeset/heavy-suits-judge.md +++ b/.changeset/heavy-suits-judge.md @@ -1,5 +1,5 @@ --- -'@backstage/plugin-user-settings-backend': minor +'@backstage/plugin-user-settings-backend': patch --- In preparation to stop supporting to the legacy backend system, the `createRouter` function is now deprecated and we strongly recommend you [migrate](https://backstage.io/docs/backend-system/building-backends/migrating) your backend to the new system. diff --git a/.changeset/metal-rivers-grin.md b/.changeset/metal-rivers-grin.md new file mode 100644 index 0000000000..c222c04a76 --- /dev/null +++ b/.changeset/metal-rivers-grin.md @@ -0,0 +1,5 @@ +--- +'@backstage/backend-test-utils': minor +--- + +There is a new `mockErrorHandler` utility to help in mocking the error middleware in tests. diff --git a/packages/backend-test-utils/api-report.md b/packages/backend-test-utils/api-report.md index cbc1cbde26..335857309a 100644 --- a/packages/backend-test-utils/api-report.md +++ b/packages/backend-test-utils/api-report.md @@ -3,8 +3,10 @@ > Do not edit this file. It is a report generated by [API Extractor](https://api-extractor.com/). ```ts +/// /// /// +/// import { AuthService } from '@backstage/backend-plugin-api'; import { Backend } from '@backstage/backend-app-api'; @@ -18,6 +20,7 @@ import { BackstageUserPrincipal } from '@backstage/backend-plugin-api'; import { CacheService } from '@backstage/backend-plugin-api'; import { DatabaseService } from '@backstage/backend-plugin-api'; import { DiscoveryService } from '@backstage/backend-plugin-api'; +import { ErrorRequestHandler } from 'express'; import { EventsService } from '@backstage/plugin-events-node'; import { ExtendedHttpServer } from '@backstage/backend-defaults/rootHttpRouter'; import { ExtensionPoint } from '@backstage/backend-plugin-api'; @@ -29,6 +32,8 @@ import Keyv from 'keyv'; import { Knex } from 'knex'; import { LifecycleService } from '@backstage/backend-plugin-api'; import { LoggerService } from '@backstage/backend-plugin-api'; +import { ParamsDictionary } from 'express-serve-static-core'; +import { ParsedQs } from 'qs'; import { PermissionsService } from '@backstage/backend-plugin-api'; import { RootConfigService } from '@backstage/backend-plugin-api'; import { RootHealthService } from '@backstage/backend-plugin-api'; @@ -139,6 +144,15 @@ export interface MockDirectoryContentOptions { shouldReadAsText?: boolean | ((path: string, buffer: Buffer) => boolean); } +// @public +export function mockErrorHandler(): ErrorRequestHandler< + ParamsDictionary, + any, + any, + ParsedQs, + Record +>; + // @public (undocumented) export namespace mockServices { // (undocumented) diff --git a/packages/backend-test-utils/package.json b/packages/backend-test-utils/package.json index 682ec3067e..7c0ac1b84c 100644 --- a/packages/backend-test-utils/package.json +++ b/packages/backend-test-utils/package.json @@ -55,7 +55,10 @@ "@backstage/types": "workspace:^", "@keyv/memcache": "^1.3.5", "@keyv/redis": "^2.5.3", + "@types/express": "^4.17.6", + "@types/express-serve-static-core": "^4.17.5", "@types/keyv": "^4.2.0", + "@types/qs": "^6.9.6", "better-sqlite3": "^11.0.0", "cookie": "^0.6.0", "express": "^4.17.1", diff --git a/packages/backend-test-utils/src/index.ts b/packages/backend-test-utils/src/index.ts index d1d80bf387..d8e0e2c4a6 100644 --- a/packages/backend-test-utils/src/index.ts +++ b/packages/backend-test-utils/src/index.ts @@ -25,3 +25,4 @@ export * from './database'; export * from './msw'; export * from './filesystem'; export * from './next'; +export { mockErrorHandler } from './util'; diff --git a/packages/backend-test-utils/src/util/errorHandler.ts b/packages/backend-test-utils/src/util/errorHandler.ts new file mode 100644 index 0000000000..f534823f50 --- /dev/null +++ b/packages/backend-test-utils/src/util/errorHandler.ts @@ -0,0 +1,29 @@ +/* + * Copyright 2024 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { MiddlewareFactory } from '@backstage/backend-defaults/rootHttpRouter'; +import { mockServices } from '../next'; + +/** + * A mock for error handler middleware that can be used in router tests. + * @public + */ +export function mockErrorHandler() { + return MiddlewareFactory.create({ + config: mockServices.rootConfig(), + logger: mockServices.rootLogger(), + }).error(); +} diff --git a/packages/backend-test-utils/src/util/index.ts b/packages/backend-test-utils/src/util/index.ts index a6cdc621d4..621c36ba8f 100644 --- a/packages/backend-test-utils/src/util/index.ts +++ b/packages/backend-test-utils/src/util/index.ts @@ -14,4 +14,5 @@ * limitations under the License. */ +export { mockErrorHandler } from './errorHandler'; export { isDockerDisabledForTests } from './isDockerDisabledForTests'; diff --git a/plugins/user-settings-backend/package.json b/plugins/user-settings-backend/package.json index c26c1cd071..93d2d5a2c5 100644 --- a/plugins/user-settings-backend/package.json +++ b/plugins/user-settings-backend/package.json @@ -64,8 +64,6 @@ "express": "^4.17.1", "express-promise-router": "^4.1.0", "knex": "^3.0.0", - "lodash": "^4.17.21", - "winston": "^3.2.1", "yn": "^4.0.0" }, "devDependencies": { diff --git a/plugins/user-settings-backend/src/deprecated.ts b/plugins/user-settings-backend/src/deprecated.ts index 04e75df97a..2631236d44 100644 --- a/plugins/user-settings-backend/src/deprecated.ts +++ b/plugins/user-settings-backend/src/deprecated.ts @@ -15,17 +15,11 @@ */ import express from 'express'; -import { merge } from 'lodash'; -import * as winston from 'winston'; - -import { ConfigReader } from '@backstage/config'; import { DatabaseService } from '@backstage/backend-plugin-api'; -import { WinstonLogger } from '@backstage/backend-defaults/rootLogger'; -import { MiddlewareFactory } from '@backstage/backend-defaults/rootHttpRouter'; -import { IdentityApi } from '@backstage/plugin-auth-node'; import { SignalsService } from '@backstage/plugin-signals-node'; -import { createRouter as _createRouter } from './service'; +import { createRouter as internalCreateRouter } from './service'; +import { IdentityApi } from '@backstage/plugin-auth-node'; /** * Type for the options passed to the "createRouter" function. @@ -49,35 +43,5 @@ export type RouterOptions = { export async function createRouter( options: RouterOptions, ): Promise { - const router = await _createRouter(options); - const config = new ConfigReader({}); - const logger = winston - .createLogger( - merge( - { - level: process.env.LOG_LEVEL || 'info', - format: winston.format.combine( - WinstonLogger.redacter().format, - process.env.NODE_ENV === 'production' - ? winston.format.json() - : WinstonLogger.colorFormat(), - ), - transports: [ - new winston.transports.Console({ - silent: - process.env.JEST_WORKER_ID !== undefined && - !process.env.LOG_LEVEL, - }), - ], - }, - {}, - ), - ) - .child({ service: 'backstage' }); - const middleware = MiddlewareFactory.create({ - config, - logger, - }); - router.use(middleware.error()); - return router; + return await internalCreateRouter(options); } diff --git a/plugins/user-settings-backend/src/service/router.test.ts b/plugins/user-settings-backend/src/service/router.test.ts index a690096e15..366d3828a0 100644 --- a/plugins/user-settings-backend/src/service/router.test.ts +++ b/plugins/user-settings-backend/src/service/router.test.ts @@ -19,8 +19,11 @@ import request from 'supertest'; import { UserSettingsStore } from '../database/UserSettingsStore'; import { createRouterInternal } from './router'; import { SignalsService } from '@backstage/plugin-signals-node'; -import { mockCredentials, mockServices } from '@backstage/backend-test-utils'; -import { MiddlewareFactory } from '@backstage/backend-defaults/rootHttpRouter'; +import { + mockCredentials, + mockServices, + mockErrorHandler, +} from '@backstage/backend-test-utils'; describe('createRouter', () => { const userSettingsStore: jest.Mocked = { @@ -41,12 +44,7 @@ describe('createRouter', () => { signals: signalService as SignalsService, }); - const errorHandler = MiddlewareFactory.create({ - config: mockServices.rootConfig(), - logger: mockServices.rootLogger(), - }).error(); - - app = express().use(router).use(errorHandler); + app = express().use(router).use(mockErrorHandler()); }); afterEach(() => { diff --git a/yarn.lock b/yarn.lock index 9bf0c2a708..4b498545b0 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3821,7 +3821,10 @@ __metadata: "@backstage/types": "workspace:^" "@keyv/memcache": ^1.3.5 "@keyv/redis": ^2.5.3 + "@types/express": ^4.17.6 + "@types/express-serve-static-core": ^4.17.5 "@types/keyv": ^4.2.0 + "@types/qs": ^6.9.6 "@types/supertest": ^2.0.8 better-sqlite3: ^11.0.0 cookie: ^0.6.0 @@ -7781,9 +7784,7 @@ __metadata: express: ^4.17.1 express-promise-router: ^4.1.0 knex: ^3.0.0 - lodash: ^4.17.21 supertest: ^6.1.3 - winston: ^3.2.1 yn: ^4.0.0 languageName: unknown linkType: soft @@ -18471,10 +18472,10 @@ __metadata: languageName: node linkType: hard -"@types/qs@npm:*": - version: 6.9.6 - resolution: "@types/qs@npm:6.9.6" - checksum: 01871b1cf7062717ec76fcb9b29ddae1e04fcfadc1c76d86ec2571e72f27bf09ff31b094b295be8d4ca664aeec9b8965563680b31fcab7aba1ed93afac5181cd +"@types/qs@npm:*, @types/qs@npm:^6.9.6": + version: 6.9.15 + resolution: "@types/qs@npm:6.9.15" + checksum: 97d8208c2b82013b618e7a9fc14df6bd40a73e1385ac479b6896bafc7949a46201c15f42afd06e86a05e914f146f495f606b6fb65610cc60cf2e0ff743ec38a2 languageName: node linkType: hard