From 23072102a2859f33f31304c987b0b50be4b6248e Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Thu, 29 Dec 2022 14:40:01 +0100 Subject: [PATCH 01/11] backend-plugin-api: add RootHttpRouterService definition MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Fredrik Adelöw Co-authored-by: blam Co-authored-by: Johan Haals Signed-off-by: Patrik Oldsberg --- packages/backend-plugin-api/api-report.md | 6 ++++ .../definitions/RootHttpRouterService.ts | 28 +++++++++++++++++++ .../src/services/definitions/coreServices.ts | 9 ++++++ .../src/services/definitions/index.ts | 1 + 4 files changed, 44 insertions(+) create mode 100644 packages/backend-plugin-api/src/services/definitions/RootHttpRouterService.ts diff --git a/packages/backend-plugin-api/api-report.md b/packages/backend-plugin-api/api-report.md index 9c6ffe6dd5..165f5dc738 100644 --- a/packages/backend-plugin-api/api-report.md +++ b/packages/backend-plugin-api/api-report.md @@ -82,6 +82,7 @@ export namespace coreServices { const logger: ServiceRef; const permissions: ServiceRef; const pluginMetadata: ServiceRef; + const rootHttpRouter: ServiceRef; const rootLifecycle: ServiceRef; const rootLogger: ServiceRef; const scheduler: ServiceRef; @@ -265,6 +266,11 @@ export type ReadUrlResponse = { etag?: string; }; +// @public (undocumented) +export interface RootHttpRouterService { + use(path: string, handler: Handler): void; +} + // @public (undocumented) export interface RootLifecycleService extends LifecycleService {} diff --git a/packages/backend-plugin-api/src/services/definitions/RootHttpRouterService.ts b/packages/backend-plugin-api/src/services/definitions/RootHttpRouterService.ts new file mode 100644 index 0000000000..e583e978fa --- /dev/null +++ b/packages/backend-plugin-api/src/services/definitions/RootHttpRouterService.ts @@ -0,0 +1,28 @@ +/* + * Copyright 2022 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 { Handler } from 'express'; + +/** + * @public + */ +export interface RootHttpRouterService { + /** + * Registers a handler at the root of the backend router. + * The path is required and may not be empty. + */ + use(path: string, handler: Handler): void; +} diff --git a/packages/backend-plugin-api/src/services/definitions/coreServices.ts b/packages/backend-plugin-api/src/services/definitions/coreServices.ts index fc96ec8be9..7de92d6d01 100644 --- a/packages/backend-plugin-api/src/services/definitions/coreServices.ts +++ b/packages/backend-plugin-api/src/services/definitions/coreServices.ts @@ -103,6 +103,15 @@ export namespace coreServices { import('./PluginMetadataService').PluginMetadataService >({ id: 'core.pluginMetadata' }); + /** + * The service reference for the root scoped {@link RootHttpRouterService}. + * + * @public + */ + export const rootHttpRouter = createServiceRef< + import('./RootHttpRouterService').RootHttpRouterService + >({ id: 'core.rootHttpRouter', scope: 'root' }); + /** * The service reference for the root scoped {@link RootLifecycleService}. * diff --git a/packages/backend-plugin-api/src/services/definitions/index.ts b/packages/backend-plugin-api/src/services/definitions/index.ts index dedfd6223e..82b16efda2 100644 --- a/packages/backend-plugin-api/src/services/definitions/index.ts +++ b/packages/backend-plugin-api/src/services/definitions/index.ts @@ -27,6 +27,7 @@ export type { export type { LoggerService, LogMeta } from './LoggerService'; export type { PermissionsService } from './PermissionsService'; export type { PluginMetadataService } from './PluginMetadataService'; +export type { RootHttpRouterService } from './RootHttpRouterService'; export type { RootLifecycleService } from './RootLifecycleService'; export type { RootLoggerService } from './RootLoggerService'; export type { SchedulerService } from './SchedulerService'; From a083d5d38ac27d626ead0bacb0c0b561790a0947 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Thu, 29 Dec 2022 15:43:47 +0100 Subject: [PATCH 02/11] backend-app-api: add root http router service + adapt http router service Co-authored-by: blam Signed-off-by: Patrik Oldsberg --- packages/backend-app-api/api-report.md | 15 ++- .../implementations/httpRouterService.ts | 30 ++---- .../src/services/implementations/index.ts | 2 + .../rootHttpRouterService.test.ts | 31 ++++++ .../implementations/rootHttpRouterService.ts | 102 ++++++++++++++++++ 5 files changed, 156 insertions(+), 24 deletions(-) create mode 100644 packages/backend-app-api/src/services/implementations/rootHttpRouterService.test.ts create mode 100644 packages/backend-app-api/src/services/implementations/rootHttpRouterService.ts diff --git a/packages/backend-app-api/api-report.md b/packages/backend-app-api/api-report.md index fa3f3bec2e..d7150fc9e8 100644 --- a/packages/backend-app-api/api-report.md +++ b/packages/backend-app-api/api-report.md @@ -8,11 +8,13 @@ import { CacheService } from '@backstage/backend-plugin-api'; import { ConfigService } from '@backstage/backend-plugin-api'; import { DatabaseService } from '@backstage/backend-plugin-api'; import { ExtensionPoint } from '@backstage/backend-plugin-api'; +import { Handler } from 'express'; import { HttpRouterService } from '@backstage/backend-plugin-api'; import { LifecycleService } from '@backstage/backend-plugin-api'; import { LoggerService } from '@backstage/backend-plugin-api'; import { PermissionsService } from '@backstage/backend-plugin-api'; import { PluginEndpointDiscovery } from '@backstage/backend-common'; +import { RootHttpRouterService } from '@backstage/backend-plugin-api'; import { RootLifecycleService } from '@backstage/backend-plugin-api'; import { RootLoggerService } from '@backstage/backend-plugin-api'; import { SchedulerService } from '@backstage/backend-plugin-api'; @@ -69,7 +71,7 @@ export const httpRouterFactory: ( // @public (undocumented) export type HttpRouterFactoryOptions = { - indexPlugin?: string; + pathPrefix?: string; }; // @public @@ -87,6 +89,17 @@ export const permissionsFactory: ( options?: undefined, ) => ServiceFactory; +// @public (undocumented) +export const rootHttpRouterFactory: ( + options?: RootHttpRouterFactoryOptions | undefined, +) => ServiceFactory; + +// @public (undocumented) +export type RootHttpRouterFactoryOptions = { + indexPath?: string | false; + middleware?: Handler[]; +}; + // @public export const rootLifecycleFactory: ( options?: undefined, diff --git a/packages/backend-app-api/src/services/implementations/httpRouterService.ts b/packages/backend-app-api/src/services/implementations/httpRouterService.ts index 2021589a50..78d04c988a 100644 --- a/packages/backend-app-api/src/services/implementations/httpRouterService.ts +++ b/packages/backend-app-api/src/services/implementations/httpRouterService.ts @@ -18,49 +18,33 @@ import { createServiceFactory, coreServices, } from '@backstage/backend-plugin-api'; -import Router from 'express-promise-router'; import { Handler } from 'express'; -import { createServiceBuilder } from '@backstage/backend-common'; /** * @public */ export type HttpRouterFactoryOptions = { /** - * The plugin ID used for the index route. Defaults to 'app' + * The path prefix used for each plugin, defaults to `/api/`. */ - indexPlugin?: string; + pathPrefix?: string; }; /** @public */ export const httpRouterFactory = createServiceFactory({ service: coreServices.httpRouter, deps: { - config: coreServices.config, plugin: coreServices.pluginMetadata, + rootHttpRouter: coreServices.rootHttpRouter, }, - async factory({ config }, options?: HttpRouterFactoryOptions) { - const defaultPluginId = options?.indexPlugin ?? 'app'; - - const apiRouter = Router(); - const rootRouter = Router(); - - const service = createServiceBuilder(module) - .loadConfig(config) - .addRouter('/api', apiRouter) - .addRouter('', rootRouter); - - await service.start(); + async factory({ rootHttpRouter }, options?: HttpRouterFactoryOptions) { + const pathPrefix = options?.pathPrefix ?? '/api/'; return async ({ plugin }) => { - const pluginId = plugin.getId(); + const path = pathPrefix + plugin.getId(); return { use(handler: Handler) { - if (pluginId === defaultPluginId) { - rootRouter.use(handler); - } else { - apiRouter.use(`/${pluginId}`, handler); - } + rootHttpRouter.use(path, handler); }, }; }; diff --git a/packages/backend-app-api/src/services/implementations/index.ts b/packages/backend-app-api/src/services/implementations/index.ts index c87011d570..2365ff36f4 100644 --- a/packages/backend-app-api/src/services/implementations/index.ts +++ b/packages/backend-app-api/src/services/implementations/index.ts @@ -25,6 +25,8 @@ export { schedulerFactory } from './schedulerService'; export { tokenManagerFactory } from './tokenManagerService'; export { urlReaderFactory } from './urlReaderService'; export { httpRouterFactory } from './httpRouterService'; +export { rootHttpRouterFactory } from './rootHttpRouterService'; export { lifecycleFactory } from './lifecycleService'; export { rootLifecycleFactory } from './rootLifecycleService'; export type { HttpRouterFactoryOptions } from './httpRouterService'; +export type { RootHttpRouterFactoryOptions } from './rootHttpRouterService'; diff --git a/packages/backend-app-api/src/services/implementations/rootHttpRouterService.test.ts b/packages/backend-app-api/src/services/implementations/rootHttpRouterService.test.ts new file mode 100644 index 0000000000..fea99b0df9 --- /dev/null +++ b/packages/backend-app-api/src/services/implementations/rootHttpRouterService.test.ts @@ -0,0 +1,31 @@ +/* + * Copyright 2022 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 { findConflictingPath } from './rootHttpRouterService'; + +describe('findConflictingPath', () => { + it('finds conflicts when present', () => { + expect(findConflictingPath(['/a'], '/a')).toBe('/a'); + expect(findConflictingPath(['/b'], '/a')).toBe(undefined); + expect(findConflictingPath(['/a'], '/a/b')).toBe('/a'); + expect(findConflictingPath(['/a'], '/aa/b')).toBe(undefined); + expect(findConflictingPath(['/aa'], '/a/b')).toBe(undefined); + expect(findConflictingPath(['/a/b'], '/a')).toBe('/a/b'); + expect(findConflictingPath(['/a/b'], '/aa')).toBe(undefined); + expect(findConflictingPath(['/b/a'], '/a')).toBe(undefined); + expect(findConflictingPath(['/a'], '/aa')).toBe(undefined); + }); +}); diff --git a/packages/backend-app-api/src/services/implementations/rootHttpRouterService.ts b/packages/backend-app-api/src/services/implementations/rootHttpRouterService.ts new file mode 100644 index 0000000000..19108a00d9 --- /dev/null +++ b/packages/backend-app-api/src/services/implementations/rootHttpRouterService.ts @@ -0,0 +1,102 @@ +/* + * Copyright 2022 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 { + createServiceFactory, + coreServices, +} from '@backstage/backend-plugin-api'; +import Router from 'express-promise-router'; +import { Handler } from 'express'; +import { createServiceBuilder } from '@backstage/backend-common'; + +/** + * @public + */ +export type RootHttpRouterFactoryOptions = { + /** + * The path to forward all unmatched requests to. Defaults to '/api/app' + */ + indexPath?: string | false; + + /** + * Middlewares that are added before all other routes. + */ + middleware?: Handler[]; +}; + +/** @public */ +export const rootHttpRouterFactory = createServiceFactory({ + service: coreServices.rootHttpRouter, + deps: { + config: coreServices.config, + }, + async factory({ config }, options?: RootHttpRouterFactoryOptions) { + const indexPath = options?.indexPath ?? '/api/app'; + + const namedRouter = Router(); + const indexRouter = Router(); + + const service = createServiceBuilder(module).loadConfig(config); + + for (const middleware of options?.middleware ?? []) { + service.addRouter('', middleware); + } + + service.addRouter('', namedRouter).addRouter('', indexRouter); + + await service.start(); + + const existingPaths = new Array(); + + return { + use: (path: string, handler: Handler) => { + const conflictingPath = findConflictingPath(existingPaths, path); + if (conflictingPath) { + throw new Error( + `Path ${path} conflicts with the existing path ${conflictingPath}`, + ); + } + existingPaths.push(path); + namedRouter.use(path, handler); + + if (indexPath === path) { + indexRouter.use(handler); + } + }, + }; + }, +}); + +function normalizePath(path: string): string { + return path.replace(/\/*$/, '/'); +} + +export function findConflictingPath( + paths: string[], + newPath: string, +): string | undefined { + const normalizedNewPath = normalizePath(newPath); + for (const path of paths) { + const normalizedPath = normalizePath(path); + if (normalizedPath.startsWith(normalizedNewPath)) { + return path; + } + if (normalizedNewPath.startsWith(normalizedPath)) { + return path; + } + } + return undefined; +} From 3cf507490315ac16ea78775a93663b7845742a7c Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Thu, 29 Dec 2022 15:48:07 +0100 Subject: [PATCH 03/11] backend-defaults: add root http router Co-authored-by: blam Signed-off-by: Patrik Oldsberg --- packages/backend-defaults/src/CreateBackend.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/backend-defaults/src/CreateBackend.ts b/packages/backend-defaults/src/CreateBackend.ts index a37a8c32b2..20edfc85a8 100644 --- a/packages/backend-defaults/src/CreateBackend.ts +++ b/packages/backend-defaults/src/CreateBackend.ts @@ -22,6 +22,7 @@ import { databaseFactory, discoveryFactory, httpRouterFactory, + rootHttpRouterFactory, lifecycleFactory, rootLifecycleFactory, loggerFactory, @@ -45,6 +46,7 @@ export const defaultServiceFactories = [ tokenManagerFactory, urlReaderFactory, httpRouterFactory, + rootHttpRouterFactory, lifecycleFactory, rootLifecycleFactory, ]; From 02b119ff9333c2050589c355798afb3f4fcf49ed Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Thu, 29 Dec 2022 15:53:23 +0100 Subject: [PATCH 04/11] changesets: added changesets for the new root http router service Co-authored-by: blam Signed-off-by: Patrik Oldsberg --- .changeset/fair-ants-work.md | 7 +++++++ .changeset/gorgeous-days-applaud.md | 5 +++++ .changeset/tiny-cooks-hide.md | 5 +++++ .../src/services/implementations/rootHttpRouterService.ts | 3 +++ 4 files changed, 20 insertions(+) create mode 100644 .changeset/fair-ants-work.md create mode 100644 .changeset/gorgeous-days-applaud.md create mode 100644 .changeset/tiny-cooks-hide.md diff --git a/.changeset/fair-ants-work.md b/.changeset/fair-ants-work.md new file mode 100644 index 0000000000..ac3a105d52 --- /dev/null +++ b/.changeset/fair-ants-work.md @@ -0,0 +1,7 @@ +--- +'@backstage/backend-app-api': minor +--- + +**BREAKING**: The `httpRouterFactory` now accepts a `pathPrefix` option rather than `indexPlugin`. To set up custom index path, configure the new `rootHttpRouterFactory` with a custom `indexPath` instead. + +Added an implementation for the new `rootHttpRouterServiceRef`. diff --git a/.changeset/gorgeous-days-applaud.md b/.changeset/gorgeous-days-applaud.md new file mode 100644 index 0000000000..a34307d720 --- /dev/null +++ b/.changeset/gorgeous-days-applaud.md @@ -0,0 +1,5 @@ +--- +'@backstage/backend-plugin-api': patch +--- + +Added a new `rootHttpRouterServiceRef` and `RootHttpRouterService` interface. diff --git a/.changeset/tiny-cooks-hide.md b/.changeset/tiny-cooks-hide.md new file mode 100644 index 0000000000..feab4b65d2 --- /dev/null +++ b/.changeset/tiny-cooks-hide.md @@ -0,0 +1,5 @@ +--- +'@backstage/backend-defaults': patch +--- + +The new root HTTP router service is now installed by default. diff --git a/packages/backend-app-api/src/services/implementations/rootHttpRouterService.ts b/packages/backend-app-api/src/services/implementations/rootHttpRouterService.ts index 19108a00d9..d1c8643244 100644 --- a/packages/backend-app-api/src/services/implementations/rootHttpRouterService.ts +++ b/packages/backend-app-api/src/services/implementations/rootHttpRouterService.ts @@ -63,6 +63,9 @@ export const rootHttpRouterFactory = createServiceFactory({ return { use: (path: string, handler: Handler) => { + if (path.match(/^[/\s]*$/)) { + throw new Error(`Root router path may not be empty`); + } const conflictingPath = findConflictingPath(existingPaths, path); if (conflictingPath) { throw new Error( From 6518fbb3c851e7ab1d26bd9ab1af449d06b8533e Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Fri, 30 Dec 2022 10:53:06 +0100 Subject: [PATCH 05/11] backend-app-api: add missing @types/express dep Signed-off-by: Patrik Oldsberg --- packages/backend-app-api/package.json | 3 ++- yarn.lock | 1 + 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/backend-app-api/package.json b/packages/backend-app-api/package.json index 93230341df..abbcdf4201 100644 --- a/packages/backend-app-api/package.json +++ b/packages/backend-app-api/package.json @@ -43,7 +43,8 @@ "winston": "^3.2.1" }, "devDependencies": { - "@backstage/cli": "workspace:^" + "@backstage/cli": "workspace:^", + "@types/express": "^4.17.6" }, "files": [ "dist", diff --git a/yarn.lock b/yarn.lock index 41202619bc..ad653b416a 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3382,6 +3382,7 @@ __metadata: "@backstage/cli": "workspace:^" "@backstage/errors": "workspace:^" "@backstage/plugin-permission-node": "workspace:^" + "@types/express": ^4.17.6 express: ^4.17.1 express-promise-router: ^4.1.0 winston: ^3.2.1 From a3ec2f32ea439c43532121b5d0b35588fab49985 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Fri, 30 Dec 2022 10:54:14 +0100 Subject: [PATCH 06/11] backend-test-utils: include all core services in test backend Signed-off-by: Patrik Oldsberg --- .changeset/little-beans-hammer.md | 5 ++++ .../src/next/wiring/TestBackend.ts | 26 ++++++++++++++++--- .../app-backend/src/service/appPlugin.test.ts | 10 ------- 3 files changed, 28 insertions(+), 13 deletions(-) create mode 100644 .changeset/little-beans-hammer.md diff --git a/.changeset/little-beans-hammer.md b/.changeset/little-beans-hammer.md new file mode 100644 index 0000000000..d998f5f5ba --- /dev/null +++ b/.changeset/little-beans-hammer.md @@ -0,0 +1,5 @@ +--- +'@backstage/backend-test-utils': patch +--- + +The `startTestBackend` setup now includes default implementations for all core services. diff --git a/packages/backend-test-utils/src/next/wiring/TestBackend.ts b/packages/backend-test-utils/src/next/wiring/TestBackend.ts index 647ebc9dda..f21cad6049 100644 --- a/packages/backend-test-utils/src/next/wiring/TestBackend.ts +++ b/packages/backend-test-utils/src/next/wiring/TestBackend.ts @@ -17,10 +17,20 @@ import { Backend, createSpecializedBackend, + cacheFactory, + configFactory, + databaseFactory, + discoveryFactory, + httpRouterFactory, lifecycleFactory, - rootLifecycleFactory, loggerFactory, + permissionsFactory, + rootHttpRouterFactory, + rootLifecycleFactory, rootLoggerFactory, + schedulerFactory, + tokenManagerFactory, + urlReaderFactory, } from '@backstage/backend-app-api'; import { ServiceFactory, @@ -55,10 +65,20 @@ export interface TestBackendOptions< } const defaultServiceFactories = [ - rootLoggerFactory(), - loggerFactory(), + cacheFactory(), + configFactory(), + databaseFactory(), + discoveryFactory(), + httpRouterFactory(), lifecycleFactory(), + loggerFactory(), + permissionsFactory(), + rootHttpRouterFactory(), rootLifecycleFactory(), + rootLoggerFactory(), + schedulerFactory(), + tokenManagerFactory(), + urlReaderFactory(), ]; const backendInstancesToCleanUp = new Array(); diff --git a/plugins/app-backend/src/service/appPlugin.test.ts b/plugins/app-backend/src/service/appPlugin.test.ts index 14921622c9..d911919425 100644 --- a/plugins/app-backend/src/service/appPlugin.test.ts +++ b/plugins/app-backend/src/service/appPlugin.test.ts @@ -20,12 +20,6 @@ import fetch from 'node-fetch'; import { coreServices } from '@backstage/backend-plugin-api'; import { startTestBackend } from '@backstage/backend-test-utils'; import { appPlugin } from './appPlugin'; -import { - databaseFactory, - httpRouterFactory, - loggerFactory, - rootLoggerFactory, -} from '@backstage/backend-app-api'; import { ConfigReader } from '@backstage/config'; import getPort from 'get-port'; @@ -59,10 +53,6 @@ describe('appPlugin', () => { }, }), ], - loggerFactory(), - rootLoggerFactory(), - databaseFactory(), - httpRouterFactory(), ], features: [ appPlugin({ From 6cf3090427e1938df6f25dedff63ce0ef53f520c Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Mon, 2 Jan 2023 14:50:18 +0100 Subject: [PATCH 07/11] backend-app-api: move @types/express to deps Signed-off-by: Patrik Oldsberg --- packages/backend-app-api/package.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/backend-app-api/package.json b/packages/backend-app-api/package.json index abbcdf4201..80598ec93d 100644 --- a/packages/backend-app-api/package.json +++ b/packages/backend-app-api/package.json @@ -38,13 +38,13 @@ "@backstage/backend-tasks": "workspace:^", "@backstage/errors": "workspace:^", "@backstage/plugin-permission-node": "workspace:^", + "@types/express": "^4.17.6", "express": "^4.17.1", "express-promise-router": "^4.1.0", "winston": "^3.2.1" }, "devDependencies": { - "@backstage/cli": "workspace:^", - "@types/express": "^4.17.6" + "@backstage/cli": "workspace:^" }, "files": [ "dist", From bd195c5c772ac1ee8df85f48f81cd7bc856ba0c6 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Mon, 2 Jan 2023 15:57:20 +0100 Subject: [PATCH 08/11] backend-app-api: use root lifecycle service to shut down root http service Signed-off-by: Patrik Oldsberg --- .../implementations/rootHttpRouterService.ts | 24 +++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/packages/backend-app-api/src/services/implementations/rootHttpRouterService.ts b/packages/backend-app-api/src/services/implementations/rootHttpRouterService.ts index d1c8643244..dae7474793 100644 --- a/packages/backend-app-api/src/services/implementations/rootHttpRouterService.ts +++ b/packages/backend-app-api/src/services/implementations/rootHttpRouterService.ts @@ -42,8 +42,9 @@ export const rootHttpRouterFactory = createServiceFactory({ service: coreServices.rootHttpRouter, deps: { config: coreServices.config, + lifecycle: coreServices.rootLifecycle, }, - async factory({ config }, options?: RootHttpRouterFactoryOptions) { + async factory({ config, lifecycle }, options?: RootHttpRouterFactoryOptions) { const indexPath = options?.indexPath ?? '/api/app'; const namedRouter = Router(); @@ -57,7 +58,26 @@ export const rootHttpRouterFactory = createServiceFactory({ service.addRouter('', namedRouter).addRouter('', indexRouter); - await service.start(); + const server = await service.start(); + // Stop method isn't part of the public API, let's fix that once we move the implementation here. + const stoppableServer = server as typeof server & { + stop: (cb: (error?: Error) => void) => void; + }; + + lifecycle.addShutdownHook({ + async fn() { + await new Promise((resolve, reject) => { + stoppableServer.stop((error?: Error) => { + if (error) { + reject(error); + } else { + resolve(); + } + }); + }); + }, + labels: { service: 'rootHttpRouter' }, + }); const existingPaths = new Array(); From d677dcf59d4a224d004bce4b948d00f12bd2a02a Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Mon, 2 Jan 2023 16:25:26 +0100 Subject: [PATCH 09/11] backend-test-utils: use empty config instead of trying to load Signed-off-by: Patrik Oldsberg --- .../backend-test-utils/src/next/wiring/TestBackend.ts | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/packages/backend-test-utils/src/next/wiring/TestBackend.ts b/packages/backend-test-utils/src/next/wiring/TestBackend.ts index f21cad6049..ea7b492543 100644 --- a/packages/backend-test-utils/src/next/wiring/TestBackend.ts +++ b/packages/backend-test-utils/src/next/wiring/TestBackend.ts @@ -18,7 +18,6 @@ import { Backend, createSpecializedBackend, cacheFactory, - configFactory, databaseFactory, discoveryFactory, httpRouterFactory, @@ -38,7 +37,9 @@ import { createServiceFactory, BackendFeature, ExtensionPoint, + coreServices, } from '@backstage/backend-plugin-api'; +import { ConfigReader } from '@backstage/config'; /** @alpha */ export interface TestBackendOptions< @@ -65,8 +66,12 @@ export interface TestBackendOptions< } const defaultServiceFactories = [ + createServiceFactory({ + service: coreServices.config, + deps: {}, + factory: async () => new ConfigReader({}, 'test'), + })(), cacheFactory(), - configFactory(), databaseFactory(), discoveryFactory(), httpRouterFactory(), From eaa705f754ee7e75382fa518f7c0242cfd29368a Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Mon, 2 Jan 2023 16:47:58 +0100 Subject: [PATCH 10/11] backend-test-utils: roll back default factories change Signed-off-by: Patrik Oldsberg --- .../src/next/wiring/TestBackend.ts | 33 +++---------------- .../app-backend/src/service/appPlugin.test.ts | 12 +++++++ 2 files changed, 16 insertions(+), 29 deletions(-) diff --git a/packages/backend-test-utils/src/next/wiring/TestBackend.ts b/packages/backend-test-utils/src/next/wiring/TestBackend.ts index ea7b492543..647ebc9dda 100644 --- a/packages/backend-test-utils/src/next/wiring/TestBackend.ts +++ b/packages/backend-test-utils/src/next/wiring/TestBackend.ts @@ -17,19 +17,10 @@ import { Backend, createSpecializedBackend, - cacheFactory, - databaseFactory, - discoveryFactory, - httpRouterFactory, lifecycleFactory, - loggerFactory, - permissionsFactory, - rootHttpRouterFactory, rootLifecycleFactory, + loggerFactory, rootLoggerFactory, - schedulerFactory, - tokenManagerFactory, - urlReaderFactory, } from '@backstage/backend-app-api'; import { ServiceFactory, @@ -37,9 +28,7 @@ import { createServiceFactory, BackendFeature, ExtensionPoint, - coreServices, } from '@backstage/backend-plugin-api'; -import { ConfigReader } from '@backstage/config'; /** @alpha */ export interface TestBackendOptions< @@ -66,24 +55,10 @@ export interface TestBackendOptions< } const defaultServiceFactories = [ - createServiceFactory({ - service: coreServices.config, - deps: {}, - factory: async () => new ConfigReader({}, 'test'), - })(), - cacheFactory(), - databaseFactory(), - discoveryFactory(), - httpRouterFactory(), - lifecycleFactory(), - loggerFactory(), - permissionsFactory(), - rootHttpRouterFactory(), - rootLifecycleFactory(), rootLoggerFactory(), - schedulerFactory(), - tokenManagerFactory(), - urlReaderFactory(), + loggerFactory(), + lifecycleFactory(), + rootLifecycleFactory(), ]; const backendInstancesToCleanUp = new Array(); diff --git a/plugins/app-backend/src/service/appPlugin.test.ts b/plugins/app-backend/src/service/appPlugin.test.ts index d911919425..80408694d6 100644 --- a/plugins/app-backend/src/service/appPlugin.test.ts +++ b/plugins/app-backend/src/service/appPlugin.test.ts @@ -20,6 +20,13 @@ import fetch from 'node-fetch'; import { coreServices } from '@backstage/backend-plugin-api'; import { startTestBackend } from '@backstage/backend-test-utils'; import { appPlugin } from './appPlugin'; +import { + databaseFactory, + httpRouterFactory, + rootHttpRouterFactory, + loggerFactory, + rootLoggerFactory, +} from '@backstage/backend-app-api'; import { ConfigReader } from '@backstage/config'; import getPort from 'get-port'; @@ -53,6 +60,11 @@ describe('appPlugin', () => { }, }), ], + loggerFactory(), + rootLoggerFactory(), + databaseFactory(), + httpRouterFactory(), + rootHttpRouterFactory(), ], features: [ appPlugin({ From 0d5c70d1f69a875fb6aedd6c5e489eaf82fc1eaf Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Tue, 3 Jan 2023 10:16:56 +0100 Subject: [PATCH 11/11] backend-app-api: switch out http router pathPrefix option for getPath + tests Signed-off-by: Patrik Oldsberg --- .changeset/fair-ants-work.md | 2 +- packages/backend-app-api/api-report.md | 2 +- .../implementations/httpRouterService.test.ts | 70 +++++++++++++++++++ .../implementations/httpRouterService.ts | 8 +-- 4 files changed, 76 insertions(+), 6 deletions(-) create mode 100644 packages/backend-app-api/src/services/implementations/httpRouterService.test.ts diff --git a/.changeset/fair-ants-work.md b/.changeset/fair-ants-work.md index ac3a105d52..4cf3c80488 100644 --- a/.changeset/fair-ants-work.md +++ b/.changeset/fair-ants-work.md @@ -2,6 +2,6 @@ '@backstage/backend-app-api': minor --- -**BREAKING**: The `httpRouterFactory` now accepts a `pathPrefix` option rather than `indexPlugin`. To set up custom index path, configure the new `rootHttpRouterFactory` with a custom `indexPath` instead. +**BREAKING**: The `httpRouterFactory` now accepts a `getPath` option rather than `indexPlugin`. To set up custom index path, configure the new `rootHttpRouterFactory` with a custom `indexPath` instead. Added an implementation for the new `rootHttpRouterServiceRef`. diff --git a/packages/backend-app-api/api-report.md b/packages/backend-app-api/api-report.md index d7150fc9e8..dcf3ced5e8 100644 --- a/packages/backend-app-api/api-report.md +++ b/packages/backend-app-api/api-report.md @@ -71,7 +71,7 @@ export const httpRouterFactory: ( // @public (undocumented) export type HttpRouterFactoryOptions = { - pathPrefix?: string; + getPath(pluginId: string): string; }; // @public diff --git a/packages/backend-app-api/src/services/implementations/httpRouterService.test.ts b/packages/backend-app-api/src/services/implementations/httpRouterService.test.ts new file mode 100644 index 0000000000..89f6db2ac8 --- /dev/null +++ b/packages/backend-app-api/src/services/implementations/httpRouterService.test.ts @@ -0,0 +1,70 @@ +/* + * Copyright 2022 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 { + HttpRouterService, + ServiceFactory, +} from '@backstage/backend-plugin-api'; +import { httpRouterFactory } from './httpRouterService'; + +describe('httpRouterFactory', () => { + it('should register plugin paths', async () => { + const rootHttpRouter = { use: jest.fn() }; + const factory = httpRouterFactory() as Exclude< + ServiceFactory, + { scope: 'root' } + >; + const innerFactory = await factory.factory({ rootHttpRouter }); + + const handler1 = () => {}; + const router1 = await innerFactory({ plugin: { getId: () => 'test1' } }); + router1.use(handler1); + expect(rootHttpRouter.use).toHaveBeenCalledTimes(1); + expect(rootHttpRouter.use).toHaveBeenCalledWith('/api/test1', handler1); + + const handler2 = () => {}; + const router2 = await innerFactory({ plugin: { getId: () => 'test2' } }); + router2.use(handler2); + expect(rootHttpRouter.use).toHaveBeenCalledTimes(2); + expect(rootHttpRouter.use).toHaveBeenCalledWith('/api/test2', handler2); + }); + + it('should use custom path generator', async () => { + const rootHttpRouter = { use: jest.fn() }; + const factory = httpRouterFactory({ + getPath: id => `/some/${id}/path`, + }) as Exclude, { scope: 'root' }>; + const innerFactory = await factory.factory({ rootHttpRouter }); + + const handler1 = () => {}; + const router1 = await innerFactory({ plugin: { getId: () => 'test1' } }); + router1.use(handler1); + expect(rootHttpRouter.use).toHaveBeenCalledTimes(1); + expect(rootHttpRouter.use).toHaveBeenCalledWith( + '/some/test1/path', + handler1, + ); + + const handler2 = () => {}; + const router2 = await innerFactory({ plugin: { getId: () => 'test2' } }); + router2.use(handler2); + expect(rootHttpRouter.use).toHaveBeenCalledTimes(2); + expect(rootHttpRouter.use).toHaveBeenCalledWith( + '/some/test2/path', + handler2, + ); + }); +}); diff --git a/packages/backend-app-api/src/services/implementations/httpRouterService.ts b/packages/backend-app-api/src/services/implementations/httpRouterService.ts index 78d04c988a..f1fc089ea3 100644 --- a/packages/backend-app-api/src/services/implementations/httpRouterService.ts +++ b/packages/backend-app-api/src/services/implementations/httpRouterService.ts @@ -25,9 +25,9 @@ import { Handler } from 'express'; */ export type HttpRouterFactoryOptions = { /** - * The path prefix used for each plugin, defaults to `/api/`. + * A callback used to generate the path for each plugin, defaults to `/api/{pluginId}`. */ - pathPrefix?: string; + getPath(pluginId: string): string; }; /** @public */ @@ -38,10 +38,10 @@ export const httpRouterFactory = createServiceFactory({ rootHttpRouter: coreServices.rootHttpRouter, }, async factory({ rootHttpRouter }, options?: HttpRouterFactoryOptions) { - const pathPrefix = options?.pathPrefix ?? '/api/'; + const getPath = options?.getPath ?? (id => `/api/${id}`); return async ({ plugin }) => { - const path = pathPrefix + plugin.getId(); + const path = getPath(plugin.getId()); return { use(handler: Handler) { rootHttpRouter.use(path, handler);