From 164ce3ee258af2fca57677f4619db0ed1e6b367d Mon Sep 17 00:00:00 2001 From: Camila Belo Date: Thu, 8 Aug 2024 11:00:36 +0200 Subject: [PATCH 1/3] refactor(user-settings-backend): deprecate legacy create Router Signed-off-by: Camila Belo --- .changeset/heavy-suits-judge.md | 5 ++ plugins/user-settings-backend/api-report.md | 11 +-- plugins/user-settings-backend/package.json | 3 +- .../src/database/DatabaseUserSettingsStore.ts | 8 +- .../user-settings-backend/src/deprecated.ts | 83 +++++++++++++++++++ plugins/user-settings-backend/src/index.ts | 2 +- .../src/service/router.test.ts | 8 +- .../src/service/router.ts | 3 - yarn.lock | 2 +- 9 files changed, 108 insertions(+), 17 deletions(-) create mode 100644 .changeset/heavy-suits-judge.md create mode 100644 plugins/user-settings-backend/src/deprecated.ts diff --git a/.changeset/heavy-suits-judge.md b/.changeset/heavy-suits-judge.md new file mode 100644 index 0000000000..f3b16a25f3 --- /dev/null +++ b/.changeset/heavy-suits-judge.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-user-settings-backend': minor +--- + +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/plugins/user-settings-backend/api-report.md b/plugins/user-settings-backend/api-report.md index 06dad6a82a..04aecca574 100644 --- a/plugins/user-settings-backend/api-report.md +++ b/plugins/user-settings-backend/api-report.md @@ -8,18 +8,15 @@ import express from 'express'; import { IdentityApi } from '@backstage/plugin-auth-node'; import { SignalsService } from '@backstage/plugin-signals-node'; -// @public +// @public @deprecated export function createRouter(options: RouterOptions): Promise; -// @public (undocumented) -export interface RouterOptions { - // (undocumented) +// @public @deprecated +export type RouterOptions = { database: DatabaseService; - // (undocumented) identity: IdentityApi; - // (undocumented) signals?: SignalsService; -} +}; // (No @packageDocumentation comment for this package) ``` diff --git a/plugins/user-settings-backend/package.json b/plugins/user-settings-backend/package.json index ab91735143..c26c1cd071 100644 --- a/plugins/user-settings-backend/package.json +++ b/plugins/user-settings-backend/package.json @@ -52,7 +52,7 @@ "test": "backstage-cli package test" }, "dependencies": { - "@backstage/backend-common": "workspace:^", + "@backstage/backend-defaults": "workspace:^", "@backstage/backend-plugin-api": "workspace:^", "@backstage/config": "workspace:^", "@backstage/errors": "workspace:^", @@ -64,6 +64,7 @@ "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" }, diff --git a/plugins/user-settings-backend/src/database/DatabaseUserSettingsStore.ts b/plugins/user-settings-backend/src/database/DatabaseUserSettingsStore.ts index 99c4b2cff4..5a051e8806 100644 --- a/plugins/user-settings-backend/src/database/DatabaseUserSettingsStore.ts +++ b/plugins/user-settings-backend/src/database/DatabaseUserSettingsStore.ts @@ -14,8 +14,10 @@ * limitations under the License. */ -import { PluginDatabaseManager } from '@backstage/backend-common'; -import { resolvePackagePath } from '@backstage/backend-plugin-api'; +import { + resolvePackagePath, + DatabaseService, +} from '@backstage/backend-plugin-api'; import { NotFoundError } from '@backstage/errors'; import { JsonValue } from '@backstage/types'; import { Knex } from 'knex'; @@ -43,7 +45,7 @@ export type RawDbUserSettingsRow = { */ export class DatabaseUserSettingsStore implements UserSettingsStore { static async create(options: { - database: PluginDatabaseManager; + database: DatabaseService; }): Promise { const { database } = options; const client = await database.getClient(); diff --git a/plugins/user-settings-backend/src/deprecated.ts b/plugins/user-settings-backend/src/deprecated.ts new file mode 100644 index 0000000000..04e75df97a --- /dev/null +++ b/plugins/user-settings-backend/src/deprecated.ts @@ -0,0 +1,83 @@ +/* + * 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 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'; + +/** + * Type for the options passed to the "createRouter" function. + * + * @public + * @deprecated This type is only exported for legacy reasons and will be removed in the future. + */ +export type RouterOptions = { + database: DatabaseService; + identity: IdentityApi; + signals?: SignalsService; +}; + +/** + * Create the user settings backend routes. + * + * @public + * @deprecated This function is only exported for legacy reasons and will be removed in the future. + * Please {@link https://backstage.io/docs/backend-system/building-backends/migrating | migrate } to use the new backend system and follow these {@link https://github.com/backstage/backstage/tree/master/plugins/user-settings-backend#new-backend | instructions } to install the user settings backend plugin. + */ +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; +} diff --git a/plugins/user-settings-backend/src/index.ts b/plugins/user-settings-backend/src/index.ts index 04d8accdaf..395a8570bf 100644 --- a/plugins/user-settings-backend/src/index.ts +++ b/plugins/user-settings-backend/src/index.ts @@ -14,5 +14,5 @@ * limitations under the License. */ -export * from './service'; +export * from './deprecated'; export * from './database'; diff --git a/plugins/user-settings-backend/src/service/router.test.ts b/plugins/user-settings-backend/src/service/router.test.ts index 90b6fb587f..a690096e15 100644 --- a/plugins/user-settings-backend/src/service/router.test.ts +++ b/plugins/user-settings-backend/src/service/router.test.ts @@ -20,6 +20,7 @@ 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'; describe('createRouter', () => { const userSettingsStore: jest.Mocked = { @@ -40,7 +41,12 @@ describe('createRouter', () => { signals: signalService as SignalsService, }); - app = express().use(router); + const errorHandler = MiddlewareFactory.create({ + config: mockServices.rootConfig(), + logger: mockServices.rootLogger(), + }).error(); + + app = express().use(router).use(errorHandler); }); afterEach(() => { diff --git a/plugins/user-settings-backend/src/service/router.ts b/plugins/user-settings-backend/src/service/router.ts index 3efb7da0f9..eacc35a5ae 100644 --- a/plugins/user-settings-backend/src/service/router.ts +++ b/plugins/user-settings-backend/src/service/router.ts @@ -14,7 +14,6 @@ * limitations under the License. */ -import { errorHandler } from '@backstage/backend-common'; import { AuthenticationError, InputError } from '@backstage/errors'; import { IdentityApi } from '@backstage/plugin-auth-node'; import express, { Request } from 'express'; @@ -156,7 +155,5 @@ export async function createRouterInternal( res.status(204).end(); }); - router.use(errorHandler()); - return router; } diff --git a/yarn.lock b/yarn.lock index f09d8e9695..9bf0c2a708 100644 --- a/yarn.lock +++ b/yarn.lock @@ -7766,7 +7766,6 @@ __metadata: version: 0.0.0-use.local resolution: "@backstage/plugin-user-settings-backend@workspace:plugins/user-settings-backend" dependencies: - "@backstage/backend-common": "workspace:^" "@backstage/backend-defaults": "workspace:^" "@backstage/backend-plugin-api": "workspace:^" "@backstage/backend-test-utils": "workspace:^" @@ -7782,6 +7781,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 From 0363bf16006ced122e672f36ead62365877c1879 Mon Sep 17 00:00:00 2001 From: Camila Belo Date: Mon, 19 Aug 2024 12:14:51 +0200 Subject: [PATCH 2/3] 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 From c58ef0dc055776f042d7cec84076b61cb41130a5 Mon Sep 17 00:00:00 2001 From: Camila Belo Date: Mon, 26 Aug 2024 11:22:32 +0200 Subject: [PATCH 3/3] Update .changeset/metal-rivers-grin.md Co-authored-by: Patrik Oldsberg Signed-off-by: Camila Belo --- .changeset/metal-rivers-grin.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/metal-rivers-grin.md b/.changeset/metal-rivers-grin.md index c222c04a76..b70750c98d 100644 --- a/.changeset/metal-rivers-grin.md +++ b/.changeset/metal-rivers-grin.md @@ -1,5 +1,5 @@ --- -'@backstage/backend-test-utils': minor +'@backstage/backend-test-utils': patch --- There is a new `mockErrorHandler` utility to help in mocking the error middleware in tests.