From b013de3f502ac420163db1aa3ff6bfc350f56cad Mon Sep 17 00:00:00 2001 From: Hasan Oezdemir <21654050+nodify-at@users.noreply.github.com> Date: Wed, 6 Apr 2022 13:25:05 +0200 Subject: [PATCH 01/10] feature: provide access token to JenkinsInstanceConfig. It can be passed to other backend calls if authentication enabled. DefaultJenkinsInfoProvider sends always this token to catalog api if access token exists. Signed-off-by: Hasan Oezdemir <21654050+nodify-at@users.noreply.github.com> --- .changeset/rare-emus-agree.md | 5 +++ plugins/jenkins-backend/api-report.md | 2 ++ .../src/service/jenkinsInfoProvider.test.ts | 32 ++++++++++++++----- .../src/service/jenkinsInfoProvider.ts | 7 +++- plugins/jenkins-backend/src/service/router.ts | 8 +++++ 5 files changed, 45 insertions(+), 9 deletions(-) create mode 100644 .changeset/rare-emus-agree.md diff --git a/.changeset/rare-emus-agree.md b/.changeset/rare-emus-agree.md new file mode 100644 index 0000000000..423d4c6259 --- /dev/null +++ b/.changeset/rare-emus-agree.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-jenkins-backend': patch +--- + +feature: provide access token to JenkinsInstanceConfig. It can be passed to other backend calls if authentication enabled. DefaultJenkinsInfoProvider sends always this token to catalog api if access token exists. diff --git a/plugins/jenkins-backend/api-report.md b/plugins/jenkins-backend/api-report.md index fb155bafe2..fd20cd79cf 100644 --- a/plugins/jenkins-backend/api-report.md +++ b/plugins/jenkins-backend/api-report.md @@ -28,6 +28,7 @@ export class DefaultJenkinsInfoProvider implements JenkinsInfoProvider { getInstance(opt: { entityRef: CompoundEntityRef; jobFullName?: string; + token?: string; }): Promise; // (undocumented) static readonly NEW_JENKINS_ANNOTATION = 'jenkins.io/job-full-name'; @@ -68,6 +69,7 @@ export interface JenkinsInfoProvider { getInstance(options: { entityRef: CompoundEntityRef; jobFullName?: string; + token?: string; }): Promise; } diff --git a/plugins/jenkins-backend/src/service/jenkinsInfoProvider.test.ts b/plugins/jenkins-backend/src/service/jenkinsInfoProvider.test.ts index a3a5ba069d..e2d34f59b6 100644 --- a/plugins/jenkins-backend/src/service/jenkinsInfoProvider.test.ts +++ b/plugins/jenkins-backend/src/service/jenkinsInfoProvider.test.ts @@ -185,7 +185,9 @@ describe('DefaultJenkinsInfoProvider', () => { const provider = configureProvider({ jenkins: {} }, undefined); await expect(provider.getInstance({ entityRef })).rejects.toThrowError(); - expect(mockCatalog.getEntityByRef).toBeCalledWith(entityRef); + expect(mockCatalog.getEntityByRef).toBeCalledWith(entityRef, { + token: undefined, + }); }); it('Reads simple config and annotation', async () => { @@ -207,7 +209,9 @@ describe('DefaultJenkinsInfoProvider', () => { ); const info: JenkinsInfo = await provider.getInstance({ entityRef }); - expect(mockCatalog.getEntityByRef).toBeCalledWith(entityRef); + expect(mockCatalog.getEntityByRef).toBeCalledWith(entityRef, { + token: undefined, + }); expect(info).toStrictEqual({ baseUrl: 'https://jenkins.example.com', crumbIssuer: undefined, @@ -243,7 +247,9 @@ describe('DefaultJenkinsInfoProvider', () => { ); const info: JenkinsInfo = await provider.getInstance({ entityRef }); - expect(mockCatalog.getEntityByRef).toBeCalledWith(entityRef); + expect(mockCatalog.getEntityByRef).toBeCalledWith(entityRef, { + token: undefined, + }); expect(info).toMatchObject({ baseUrl: 'https://jenkins.example.com', jobFullName: 'teamA/artistLookup-build', @@ -280,7 +286,9 @@ describe('DefaultJenkinsInfoProvider', () => { ); const info: JenkinsInfo = await provider.getInstance({ entityRef }); - expect(mockCatalog.getEntityByRef).toBeCalledWith(entityRef); + expect(mockCatalog.getEntityByRef).toBeCalledWith(entityRef, { + token: undefined, + }); expect(info).toMatchObject({ baseUrl: 'https://jenkins.example.com', jobFullName: 'teamA/artistLookup-build', @@ -317,7 +325,9 @@ describe('DefaultJenkinsInfoProvider', () => { ); const info: JenkinsInfo = await provider.getInstance({ entityRef }); - expect(mockCatalog.getEntityByRef).toBeCalledWith(entityRef); + expect(mockCatalog.getEntityByRef).toBeCalledWith(entityRef, { + token: undefined, + }); expect(info).toMatchObject({ baseUrl: 'https://jenkins-other.example.com', jobFullName: 'teamA/artistLookup-build', @@ -343,7 +353,9 @@ describe('DefaultJenkinsInfoProvider', () => { ); const info: JenkinsInfo = await provider.getInstance({ entityRef }); - expect(mockCatalog.getEntityByRef).toBeCalledWith(entityRef); + expect(mockCatalog.getEntityByRef).toBeCalledWith(entityRef, { + token: undefined, + }); expect(info).toMatchObject({ baseUrl: 'https://jenkins.example.com', jobFullName: 'teamA/artistLookup-build', @@ -369,7 +381,9 @@ describe('DefaultJenkinsInfoProvider', () => { ); const info: JenkinsInfo = await provider.getInstance({ entityRef }); - expect(mockCatalog.getEntityByRef).toBeCalledWith(entityRef); + expect(mockCatalog.getEntityByRef).toBeCalledWith(entityRef, { + token: undefined, + }); expect(info).toMatchObject({ baseUrl: 'https://jenkins.example.com', jobFullName: 'teamA/artistLookup-build', @@ -400,7 +414,9 @@ describe('DefaultJenkinsInfoProvider', () => { ); const info: JenkinsInfo = await provider.getInstance({ entityRef }); - expect(mockCatalog.getEntityByRef).toBeCalledWith(entityRef); + expect(mockCatalog.getEntityByRef).toBeCalledWith(entityRef, { + token: undefined, + }); expect(info).toMatchObject({ baseUrl: 'https://jenkins-other.example.com', jobFullName: 'teamA/artistLookup-build', diff --git a/plugins/jenkins-backend/src/service/jenkinsInfoProvider.ts b/plugins/jenkins-backend/src/service/jenkinsInfoProvider.ts index d9a97e2f2d..221acb3c51 100644 --- a/plugins/jenkins-backend/src/service/jenkinsInfoProvider.ts +++ b/plugins/jenkins-backend/src/service/jenkinsInfoProvider.ts @@ -32,6 +32,8 @@ export interface JenkinsInfoProvider { * A specific job to get. This is only passed in when we know about a job name we are interested in. */ jobFullName?: string; + + token?: string; }): Promise; } @@ -184,9 +186,12 @@ export class DefaultJenkinsInfoProvider implements JenkinsInfoProvider { async getInstance(opt: { entityRef: CompoundEntityRef; jobFullName?: string; + token?: string; }): Promise { // load entity - const entity = await this.catalog.getEntityByRef(opt.entityRef); + const entity = await this.catalog.getEntityByRef(opt.entityRef, { + token: opt.token, + }); if (!entity) { throw new Error( `Couldn't find entity with name: ${stringifyEntityRef(opt.entityRef)}`, diff --git a/plugins/jenkins-backend/src/service/router.ts b/plugins/jenkins-backend/src/service/router.ts index 08712215e4..3e53f27e4b 100644 --- a/plugins/jenkins-backend/src/service/router.ts +++ b/plugins/jenkins-backend/src/service/router.ts @@ -44,6 +44,9 @@ export async function createRouter( '/v1/entity/:namespace/:kind/:name/projects', async (request, response) => { const { namespace, kind, name } = request.params; + const token = getBearerTokenFromAuthorizationHeader( + request.header('authorization'), + ); const branch = request.query.branch; let branchStr: string | undefined; @@ -67,6 +70,7 @@ export async function createRouter( namespace, name, }, + token, }); const projects = await jenkinsApi.getProjects(jenkinsInfo, branchStr); @@ -79,6 +83,9 @@ export async function createRouter( router.get( '/v1/entity/:namespace/:kind/:name/job/:jobFullName/:buildNumber', async (request, response) => { + const token = getBearerTokenFromAuthorizationHeader( + request.header('authorization'), + ); const { namespace, kind, name, jobFullName, buildNumber } = request.params; @@ -89,6 +96,7 @@ export async function createRouter( name, }, jobFullName, + token, }); const build = await jenkinsApi.getBuild( From cc8f0076d81b347d4ee749144b3490dd7212b331 Mon Sep 17 00:00:00 2001 From: Hasan Oezdemir <21654050+nodify-at@users.noreply.github.com> Date: Wed, 6 Apr 2022 16:18:43 +0200 Subject: [PATCH 02/10] feature: rename token to backstageToken and update the documentation Signed-off-by: Hasan Oezdemir <21654050+nodify-at@users.noreply.github.com> --- plugins/jenkins-backend/api-report.md | 4 ++-- .../src/service/jenkinsInfoProvider.test.ts | 16 ++++++++-------- .../src/service/jenkinsInfoProvider.ts | 6 +++--- plugins/jenkins-backend/src/service/router.ts | 4 ++-- 4 files changed, 15 insertions(+), 15 deletions(-) diff --git a/plugins/jenkins-backend/api-report.md b/plugins/jenkins-backend/api-report.md index fd20cd79cf..37e8f80a80 100644 --- a/plugins/jenkins-backend/api-report.md +++ b/plugins/jenkins-backend/api-report.md @@ -28,7 +28,7 @@ export class DefaultJenkinsInfoProvider implements JenkinsInfoProvider { getInstance(opt: { entityRef: CompoundEntityRef; jobFullName?: string; - token?: string; + backstageToken?: string; }): Promise; // (undocumented) static readonly NEW_JENKINS_ANNOTATION = 'jenkins.io/job-full-name'; @@ -69,7 +69,7 @@ export interface JenkinsInfoProvider { getInstance(options: { entityRef: CompoundEntityRef; jobFullName?: string; - token?: string; + backstageToken?: string; }): Promise; } diff --git a/plugins/jenkins-backend/src/service/jenkinsInfoProvider.test.ts b/plugins/jenkins-backend/src/service/jenkinsInfoProvider.test.ts index e2d34f59b6..5e6198e1cd 100644 --- a/plugins/jenkins-backend/src/service/jenkinsInfoProvider.test.ts +++ b/plugins/jenkins-backend/src/service/jenkinsInfoProvider.test.ts @@ -186,7 +186,7 @@ describe('DefaultJenkinsInfoProvider', () => { await expect(provider.getInstance({ entityRef })).rejects.toThrowError(); expect(mockCatalog.getEntityByRef).toBeCalledWith(entityRef, { - token: undefined, + backstageToken: undefined, }); }); @@ -210,7 +210,7 @@ describe('DefaultJenkinsInfoProvider', () => { const info: JenkinsInfo = await provider.getInstance({ entityRef }); expect(mockCatalog.getEntityByRef).toBeCalledWith(entityRef, { - token: undefined, + backstageToken: undefined, }); expect(info).toStrictEqual({ baseUrl: 'https://jenkins.example.com', @@ -248,7 +248,7 @@ describe('DefaultJenkinsInfoProvider', () => { const info: JenkinsInfo = await provider.getInstance({ entityRef }); expect(mockCatalog.getEntityByRef).toBeCalledWith(entityRef, { - token: undefined, + backstageToken: undefined, }); expect(info).toMatchObject({ baseUrl: 'https://jenkins.example.com', @@ -287,7 +287,7 @@ describe('DefaultJenkinsInfoProvider', () => { const info: JenkinsInfo = await provider.getInstance({ entityRef }); expect(mockCatalog.getEntityByRef).toBeCalledWith(entityRef, { - token: undefined, + backstageToken: undefined, }); expect(info).toMatchObject({ baseUrl: 'https://jenkins.example.com', @@ -326,7 +326,7 @@ describe('DefaultJenkinsInfoProvider', () => { const info: JenkinsInfo = await provider.getInstance({ entityRef }); expect(mockCatalog.getEntityByRef).toBeCalledWith(entityRef, { - token: undefined, + backstageToken: undefined, }); expect(info).toMatchObject({ baseUrl: 'https://jenkins-other.example.com', @@ -354,7 +354,7 @@ describe('DefaultJenkinsInfoProvider', () => { const info: JenkinsInfo = await provider.getInstance({ entityRef }); expect(mockCatalog.getEntityByRef).toBeCalledWith(entityRef, { - token: undefined, + backstageToken: undefined, }); expect(info).toMatchObject({ baseUrl: 'https://jenkins.example.com', @@ -382,7 +382,7 @@ describe('DefaultJenkinsInfoProvider', () => { const info: JenkinsInfo = await provider.getInstance({ entityRef }); expect(mockCatalog.getEntityByRef).toBeCalledWith(entityRef, { - token: undefined, + backstageToken: undefined, }); expect(info).toMatchObject({ baseUrl: 'https://jenkins.example.com', @@ -415,7 +415,7 @@ describe('DefaultJenkinsInfoProvider', () => { const info: JenkinsInfo = await provider.getInstance({ entityRef }); expect(mockCatalog.getEntityByRef).toBeCalledWith(entityRef, { - token: undefined, + backstageToken: undefined, }); expect(info).toMatchObject({ baseUrl: 'https://jenkins-other.example.com', diff --git a/plugins/jenkins-backend/src/service/jenkinsInfoProvider.ts b/plugins/jenkins-backend/src/service/jenkinsInfoProvider.ts index 221acb3c51..fcce944dc8 100644 --- a/plugins/jenkins-backend/src/service/jenkinsInfoProvider.ts +++ b/plugins/jenkins-backend/src/service/jenkinsInfoProvider.ts @@ -33,7 +33,7 @@ export interface JenkinsInfoProvider { */ jobFullName?: string; - token?: string; + backstageToken?: string; }): Promise; } @@ -186,11 +186,11 @@ export class DefaultJenkinsInfoProvider implements JenkinsInfoProvider { async getInstance(opt: { entityRef: CompoundEntityRef; jobFullName?: string; - token?: string; + backstageToken?: string; }): Promise { // load entity const entity = await this.catalog.getEntityByRef(opt.entityRef, { - token: opt.token, + token: opt.backstageToken, }); if (!entity) { throw new Error( diff --git a/plugins/jenkins-backend/src/service/router.ts b/plugins/jenkins-backend/src/service/router.ts index 3e53f27e4b..03462b923f 100644 --- a/plugins/jenkins-backend/src/service/router.ts +++ b/plugins/jenkins-backend/src/service/router.ts @@ -70,7 +70,7 @@ export async function createRouter( namespace, name, }, - token, + backstageToken: token, }); const projects = await jenkinsApi.getProjects(jenkinsInfo, branchStr); @@ -96,7 +96,7 @@ export async function createRouter( name, }, jobFullName, - token, + backstageToken: token, }); const build = await jenkinsApi.getBuild( From 0a63e99a2643aafdda23053222fdd02669bac6e7 Mon Sep 17 00:00:00 2001 From: Phil Kuang Date: Wed, 23 Feb 2022 16:11:50 -0500 Subject: [PATCH 03/10] feat(search): handle search indexing coordination among nodes Signed-off-by: Phil Kuang --- .changeset/ninety-eggs-argue.md | 84 ++++++++++ docs/features/search/concepts.md | 4 +- docs/features/search/getting-started.md | 82 ++++++++-- packages/backend/package.json | 4 +- packages/backend/src/plugins/search.ts | 17 +- .../packages/backend/package.json.hbs | 4 +- .../backend/src/plugins/search.ts.hbs | 17 +- plugins/search-backend-node/api-report.md | 21 ++- plugins/search-backend-node/package.json | 2 + .../src/IndexBuilder.test.ts | 22 +-- .../search-backend-node/src/IndexBuilder.ts | 108 +++++++------ .../search-backend-node/src/Scheduler.test.ts | 145 ++++++++++++++++-- plugins/search-backend-node/src/Scheduler.ts | 50 ++++-- plugins/search-backend-node/src/index.ts | 2 + .../src/runPeriodically.test.ts | 84 ---------- .../src/runPeriodically.ts | 54 ------- plugins/search-backend-node/src/types.ts | 7 +- 17 files changed, 438 insertions(+), 269 deletions(-) create mode 100644 .changeset/ninety-eggs-argue.md delete mode 100644 plugins/search-backend-node/src/runPeriodically.test.ts delete mode 100644 plugins/search-backend-node/src/runPeriodically.ts diff --git a/.changeset/ninety-eggs-argue.md b/.changeset/ninety-eggs-argue.md new file mode 100644 index 0000000000..01f861fe4b --- /dev/null +++ b/.changeset/ninety-eggs-argue.md @@ -0,0 +1,84 @@ +--- +'@backstage/plugin-search-backend-node': minor +'@backstage/create-app': patch +--- + +**BREAKING**: `IndexBuilder.addCollator()` now requires a `schedule` parameter (replacing `defaultRefreshIntervalSeconds`) which is expected to be a `TaskRunner` that is configured with the desired search indexing schedule for the given collator. + +`Scheduler.addToSchedule()` now takes a new parameter object (`ScheduleTaskParameters`) with two new options `id` and `scheduledRunner` in addition to the migrated `task` argument. + +NOTE: The search backend plugin now creates a dedicated database for coordinating indexing tasks. + +To make this change to an existing app, make the following changes to `packages/backend/src/plugins/search.ts`: + +```diff ++import { Duration } from 'luxon'; + +/* ... */ + ++ const schedule = env.scheduler.createScheduledTaskRunner({ ++ frequency: Duration.fromObject({ seconds: 600 }), ++ timeout: Duration.fromObject({ seconds: 900 }), ++ initialDelay: Duration.fromObject({ seconds: 3 }), ++ }); + + indexBuilder.addCollator({ +- defaultRefreshIntervalSeconds: 600, ++ schedule, + factory: DefaultCatalogCollatorFactory.fromConfig(env.config, { + discovery: env.discovery, + tokenManager: env.tokenManager, + }), + }); + + indexBuilder.addCollator({ +- defaultRefreshIntervalSeconds: 600, ++ schedule, + factory: DefaultTechDocsCollatorFactory.fromConfig(env.config, { + discovery: env.discovery, + tokenManager: env.tokenManager, + }), + }); + + const { scheduler } = await indexBuilder.build(); +- setTimeout(() => scheduler.start(), 3000); ++ scheduler.start(); +/* ... */ +``` + +NOTE: For scenarios where the `lunr` search engine is used in a multi-node configuration, a non-distributed `TaskRunner` like the following should be implemented to ensure consistency across nodes (alternatively, you can configure +the search plugin to use a non-distributed DB such as [SQLite](https://backstage.io/docs/tutorials/configuring-plugin-databases#postgresql-and-sqlite-3)): + +```diff ++import { TaskInvocationDefinition, TaskRunner } from '@backstage/backend-tasks'; + +/* ... */ + ++ const schedule: TaskRunner = { ++ run: async (task: TaskInvocationDefinition) => { ++ const startRefresh = async () => { ++ while (!task.signal?.aborted) { ++ try { ++ await task.fn(task.signal); ++ } catch { ++ // ignore intentionally ++ } ++ ++ await new Promise(resolve => setTimeout(resolve, 600 * 1000)); ++ } ++ }; ++ startRefresh(); ++ }, ++ }; + + indexBuilder.addCollator({ +- defaultRefreshIntervalSeconds: 600, ++ schedule, + factory: DefaultCatalogCollatorFactory.fromConfig(env.config, { + discovery: env.discovery, + tokenManager: env.tokenManager, + }), + }); + +/* ... */ +``` diff --git a/docs/features/search/concepts.md b/docs/features/search/concepts.md index 052f71376f..57ab061974 100644 --- a/docs/features/search/concepts.md +++ b/docs/features/search/concepts.md @@ -84,7 +84,9 @@ index-time. There are many ways a search index could be built and maintained, but Backstage Search chooses to completely rebuild indices on a schedule. Different collators can be configured to refresh at different intervals, depending on how often the -source information is updated. +source information is updated. When search indexing is distributed among multiple +backend nodes, coordination to prevent clashes is typically handled by a +distributed `TaskRunner`. ### The Search Page diff --git a/docs/features/search/getting-started.md b/docs/features/search/getting-started.md index f38d02a3d7..bc17b1eea2 100644 --- a/docs/features/search/getting-started.md +++ b/docs/features/search/getting-started.md @@ -149,6 +149,7 @@ import { import { PluginEnvironment } from '../types'; import { DefaultCatalogCollator } from '@backstage/plugin-catalog-backend'; import { Router } from 'express'; +import { Duration } from 'luxon'; export default async function createPlugin( env: PluginEnvironment, @@ -161,9 +162,15 @@ export default async function createPlugin( searchEngine, }); + const every10MinutesSchedule = env.scheduler.createScheduledTaskRunner({ + frequency: Duration.fromObject({ seconds: 600 }), + timeout: Duration.fromObject({ seconds: 900 }), + initialDelay: Duration.fromObject({ seconds: 3 }), + }); + indexBuilder.addCollator({ - defaultRefreshIntervalSeconds: 600, - collator: new DefaultCatalogCollator({ + schedule: every10MinutesSchedule, + factory: DefaultCatalogCollatorFactory.fromConfig(env.config, { discovery: env.discovery, tokenManager: env.tokenManager, }), @@ -287,32 +294,87 @@ which are responsible for providing documents number of collators with the `IndexBuilder` like this: ```typescript +import { Duration } from 'luxon'; + const indexBuilder = new IndexBuilder({ logger: env.logger, searchEngine }); +const every10MinutesSchedule = env.scheduler.createScheduledTaskRunner({ + frequency: Duration.fromObject({ seconds: 600 }), + timeout: Duration.fromObject({ seconds: 900 }), + initialDelay: Duration.fromObject({ seconds: 3 }), +}); + +const everyHourSchedule = env.scheduler.createScheduledTaskRunner({ + frequency: Duration.fromObject({ seconds: 3600 }), + timeout: Duration.fromObject({ seconds: 5400 }), + initialDelay: Duration.fromObject({ seconds: 3 }), +}); + indexBuilder.addCollator({ - defaultRefreshIntervalSeconds: 600, - collator: new DefaultCatalogCollator({ + schedule: every10MinutesSchedule, + factory: DefaultCatalogCollatorFactory.fromConfig(env.config, { discovery: env.discovery, tokenManager: env.tokenManager, }), }); indexBuilder.addCollator({ - defaultRefreshIntervalSeconds: 3600, - collator: new MyCustomCollator(), + schedule: everyHourSchedule, + factory: new MyCustomCollatorFactory(), }); ``` Backstage Search builds and maintains its index [on a schedule](./concepts.md#the-scheduler). You can change how often the indexes are rebuilt for a given type of document. You may want to do this if -your documents are updated more or less frequently. You can do so by modifying -its `defaultRefreshIntervalSeconds` value, like this: +your documents are updated more or less frequently. You can do so by configuring +a scheduled `TaskRunner` to pass into the `schedule` value, like this: ```typescript {3} +const every10MinutesSchedule = env.scheduler.createScheduledTaskRunner({ + frequency: Duration.fromObject({ seconds: 600 }), + timeout: Duration.fromObject({ seconds: 900 }), + initialDelay: Duration.fromObject({ seconds: 3 }), +}); + indexBuilder.addCollator({ - defaultRefreshIntervalSeconds: 600, - collator: new DefaultCatalogCollator({ + schedule: every10MinutesSchedule, + factory: DefaultCatalogCollatorFactory.fromConfig(env.config, { + discovery: env.discovery, + tokenManager: env.tokenManager, + }), +}); +``` + +Note: if you are using the in-memory Lunr search engine, you probably want to +implement a non-distributed `TaskRunner` like the following to ensure consistency +if you're running multiple search backend nodes (alternatively, you can configure +the search plugin to use a non-distributed database such as +[SQLite](../../tutorials/configuring-plugin-databases.md#postgresql-and-sqlite-3)): + +```typescript +import { TaskInvocationDefinition, TaskRunner } from '@backstage/backend-tasks'; + +const schedule: TaskRunner = { + run: async (task: TaskInvocationDefinition) => { + const startRefresh = async () => { + while (!task.signal?.aborted) { + try { + await task.fn(task.signal); + } catch { + // ignore intentionally + } + + await new Promise(resolve => setTimeout(resolve, 600 * 1000)); + } + }; + startRefresh(); + }, +}; + +indexBuilder.addCollator({ + schedule, + factory: DefaultCatalogCollatorFactory.fromConfig(env.config, { discovery: env.discovery, tokenManager: env.tokenManager, }), diff --git a/packages/backend/package.json b/packages/backend/package.json index 3fde1cdc11..aed934a028 100644 --- a/packages/backend/package.json +++ b/packages/backend/package.json @@ -68,6 +68,7 @@ "express": "^4.17.1", "express-promise-router": "^4.1.0", "express-prom-bundle": "^6.3.6", + "luxon": "^2.0.2", "pg": "^8.3.0", "pg-connection-string": "^2.3.0", "prom-client": "^14.0.1", @@ -77,7 +78,8 @@ "@backstage/cli": "^0.17.0-next.1", "@types/dockerode": "^3.3.0", "@types/express": "^4.17.6", - "@types/express-serve-static-core": "^4.17.5" + "@types/express-serve-static-core": "^4.17.5", + "@types/luxon": "^2.0.4" }, "files": [ "dist" diff --git a/packages/backend/src/plugins/search.ts b/packages/backend/src/plugins/search.ts index cb675a842c..052b7380c3 100644 --- a/packages/backend/src/plugins/search.ts +++ b/packages/backend/src/plugins/search.ts @@ -26,6 +26,7 @@ import { } from '@backstage/plugin-search-backend-node'; import { DefaultTechDocsCollatorFactory } from '@backstage/plugin-techdocs-backend'; import { Router } from 'express'; +import { Duration } from 'luxon'; import { PluginEnvironment } from '../types'; async function createSearchEngine( @@ -55,10 +56,18 @@ export default async function createPlugin( searchEngine, }); + const schedule = env.scheduler.createScheduledTaskRunner({ + frequency: Duration.fromObject({ seconds: 600 }), + timeout: Duration.fromObject({ seconds: 900 }), + // A 3 second delay gives the backend server a chance to initialize before + // any collators are executed, which may attempt requests against the API. + initialDelay: Duration.fromObject({ seconds: 3 }), + }); + // Collators are responsible for gathering documents known to plugins. This // particular collator gathers entities from the software catalog. indexBuilder.addCollator({ - defaultRefreshIntervalSeconds: 600, + schedule, factory: DefaultCatalogCollatorFactory.fromConfig(env.config, { discovery: env.discovery, tokenManager: env.tokenManager, @@ -66,7 +75,7 @@ export default async function createPlugin( }); indexBuilder.addCollator({ - defaultRefreshIntervalSeconds: 600, + schedule, factory: DefaultTechDocsCollatorFactory.fromConfig(env.config, { discovery: env.discovery, logger: env.logger, @@ -77,10 +86,8 @@ export default async function createPlugin( // The scheduler controls when documents are gathered from collators and sent // to the search engine for indexing. const { scheduler } = await indexBuilder.build(); + scheduler.start(); - // A 3 second delay gives the backend server a chance to initialize before - // any collators are executed, which may attempt requests against the API. - setTimeout(() => scheduler.start(), 3000); useHotCleanup(module, () => scheduler.stop()); return await createRouter({ diff --git a/packages/create-app/templates/default-app/packages/backend/package.json.hbs b/packages/create-app/templates/default-app/packages/backend/package.json.hbs index cd4d3e1222..b91366dee8 100644 --- a/packages/create-app/templates/default-app/packages/backend/package.json.hbs +++ b/packages/create-app/templates/default-app/packages/backend/package.json.hbs @@ -40,6 +40,7 @@ "dockerode": "^3.3.1", "express": "^4.17.1", "express-promise-router": "^4.1.0", + "luxon": "^2.0.2", {{#if dbTypePG}} "pg": "^8.3.0", {{/if}} @@ -52,7 +53,8 @@ "@backstage/cli": "^{{version '@backstage/cli'}}", "@types/dockerode": "^3.3.0", "@types/express": "^4.17.6", - "@types/express-serve-static-core": "^4.17.5" + "@types/express-serve-static-core": "^4.17.5", + "@types/luxon": "^2.0.4" }, "files": [ "dist" diff --git a/packages/create-app/templates/default-app/packages/backend/src/plugins/search.ts.hbs b/packages/create-app/templates/default-app/packages/backend/src/plugins/search.ts.hbs index 8f44a35b16..bda10ae21b 100644 --- a/packages/create-app/templates/default-app/packages/backend/src/plugins/search.ts.hbs +++ b/packages/create-app/templates/default-app/packages/backend/src/plugins/search.ts.hbs @@ -11,6 +11,7 @@ import { PluginEnvironment } from '../types'; import { DefaultCatalogCollatorFactory } from '@backstage/plugin-catalog-backend'; import { DefaultTechDocsCollatorFactory } from '@backstage/plugin-techdocs-backend'; import { Router } from 'express'; +import { Duration } from 'luxon'; export default async function createPlugin( env: PluginEnvironment, @@ -31,10 +32,18 @@ export default async function createPlugin( searchEngine, }); + const schedule = env.scheduler.createScheduledTaskRunner({ + frequency: Duration.fromObject({ seconds: 600 }), + timeout: Duration.fromObject({ seconds: 900 }), + // A 3 second delay gives the backend server a chance to initialize before + // any collators are executed, which may attempt requests against the API. + initialDelay: Duration.fromObject({ seconds: 3 }), + }); + // Collators are responsible for gathering documents known to plugins. This // collator gathers entities from the software catalog. indexBuilder.addCollator({ - defaultRefreshIntervalSeconds: 600, + schedule, factory: DefaultCatalogCollatorFactory.fromConfig(env.config, { discovery: env.discovery, tokenManager: env.tokenManager, @@ -43,7 +52,7 @@ export default async function createPlugin( // collator gathers entities from techdocs. indexBuilder.addCollator({ - defaultRefreshIntervalSeconds: 600, + schedule, factory: DefaultTechDocsCollatorFactory.fromConfig(env.config, { discovery: env.discovery, logger: env.logger, @@ -54,10 +63,8 @@ export default async function createPlugin( // The scheduler controls when documents are gathered from collators and sent // to the search engine for indexing. const { scheduler } = await indexBuilder.build(); + scheduler.start(); - // A 3 second delay gives the backend server a chance to initialize before - // any collators are executed, which may attempt requests against the API. - setTimeout(() => scheduler.start(), 3000); useHotCleanup(module, () => scheduler.stop()); return await createRouter({ diff --git a/plugins/search-backend-node/api-report.md b/plugins/search-backend-node/api-report.md index ebc49df280..61b8a124f7 100644 --- a/plugins/search-backend-node/api-report.md +++ b/plugins/search-backend-node/api-report.md @@ -16,6 +16,8 @@ import { QueryTranslator } from '@backstage/plugin-search-common'; import { Readable } from 'stream'; import { SearchEngine } from '@backstage/plugin-search-common'; import { SearchQuery } from '@backstage/plugin-search-common'; +import { TaskFunction } from '@backstage/backend-tasks'; +import { TaskRunner } from '@backstage/backend-tasks'; import { Transform } from 'stream'; import { Writable } from 'stream'; @@ -52,10 +54,7 @@ export abstract class DecoratorBase extends Transform { // @beta (undocumented) export class IndexBuilder { constructor({ logger, searchEngine }: IndexBuilderOptions); - addCollator({ - factory, - defaultRefreshIntervalSeconds, - }: RegisterCollatorParameters): void; + addCollator({ factory, schedule }: RegisterCollatorParameters): void; addDecorator({ factory }: RegisterDecoratorParameters): void; build(): Promise<{ scheduler: Scheduler; @@ -111,8 +110,8 @@ export class LunrSearchEngineIndexer extends BatchSearchEngineIndexer { // @beta export interface RegisterCollatorParameters { - defaultRefreshIntervalSeconds: number; factory: DocumentCollatorFactory; + schedule: TaskRunner; } // @beta @@ -123,11 +122,21 @@ export interface RegisterDecoratorParameters { // @beta (undocumented) export class Scheduler { constructor({ logger }: { logger: Logger }); - addToSchedule(task: Function, interval: number): void; + addToSchedule({ id, task, scheduledRunner }: ScheduleTaskParameters): void; start(): void; stop(): void; } +// @public +export interface ScheduleTaskParameters { + // (undocumented) + id: string; + // (undocumented) + scheduledRunner: TaskRunner; + // (undocumented) + task: TaskFunction; +} + export { SearchEngine }; // @beta diff --git a/plugins/search-backend-node/package.json b/plugins/search-backend-node/package.json index e6fcac1398..3eea462c20 100644 --- a/plugins/search-backend-node/package.json +++ b/plugins/search-backend-node/package.json @@ -23,11 +23,13 @@ "clean": "backstage-cli package clean" }, "dependencies": { + "@backstage/backend-tasks": "^0.3.0-next.2", "@backstage/errors": "^1.0.0", "@backstage/plugin-search-common": "^0.3.3-next.1", "@types/lunr": "^2.3.3", "lodash": "^4.17.21", "lunr": "^2.3.9", + "node-abort-controller": "^3.0.1", "winston": "^3.2.1" }, "devDependencies": { diff --git a/plugins/search-backend-node/src/IndexBuilder.test.ts b/plugins/search-backend-node/src/IndexBuilder.test.ts index 6a32343205..70d00f6974 100644 --- a/plugins/search-backend-node/src/IndexBuilder.test.ts +++ b/plugins/search-backend-node/src/IndexBuilder.test.ts @@ -15,6 +15,7 @@ */ import { getVoidLogger } from '@backstage/backend-common'; +import { TaskInvocationDefinition, TaskRunner } from '@backstage/backend-tasks'; import { DocumentCollatorFactory, DocumentDecoratorFactory, @@ -53,9 +54,15 @@ class DifferentlyTypedDocumentDecoratorFactory extends TestDocumentDecoratorFact describe('IndexBuilder', () => { let testSearchEngine: SearchEngine; let testIndexBuilder: IndexBuilder; + let testScheduledTaskRunner: TaskRunner; beforeEach(() => { const logger = getVoidLogger(); + testScheduledTaskRunner = { + run: async (task: TaskInvocationDefinition & { fn: () => void }) => { + task.fn(); + }, + }; testSearchEngine = new LunrSearchEngine({ logger }); testIndexBuilder = new IndexBuilder({ logger, @@ -65,35 +72,32 @@ describe('IndexBuilder', () => { describe('addCollator', () => { it('adds a collator', async () => { - jest.useFakeTimers(); const testCollatorFactory = new TestDocumentCollatorFactory(); const collatorSpy = jest.spyOn(testCollatorFactory, 'getCollator'); // Add a collator. testIndexBuilder.addCollator({ - defaultRefreshIntervalSeconds: 6, factory: testCollatorFactory, + schedule: testScheduledTaskRunner, }); // Build the index and ensure the collator was invoked. const { scheduler } = await testIndexBuilder.build(); scheduler.start(); - jest.advanceTimersByTime(6000); expect(collatorSpy).toHaveBeenCalled(); }); }); describe('addDecorator', () => { it('adds a decorator', async () => { - jest.useFakeTimers(); const testCollatorFactory = new TestDocumentCollatorFactory(); const testDecoratorFactory = new TestDocumentDecoratorFactory(); const decoratorSpy = jest.spyOn(testDecoratorFactory, 'getDecorator'); // Add a collator. testIndexBuilder.addCollator({ - defaultRefreshIntervalSeconds: 6, factory: testCollatorFactory, + schedule: testScheduledTaskRunner, }); // Add a decorator. @@ -104,14 +108,12 @@ describe('IndexBuilder', () => { // Build the index and ensure the decorator was invoked. const { scheduler } = await testIndexBuilder.build(); scheduler.start(); - jest.advanceTimersByTime(6000); // wait for async decorator execution await Promise.resolve(); expect(decoratorSpy).toHaveBeenCalled(); }); it('adds a type-specific decorator', async () => { - jest.useFakeTimers(); const testCollatorFactory = new TypedDocumentCollatorFactory(); const testDecoratorFactory = new TypedDocumentDecoratorFactory(); jest.spyOn(testCollatorFactory, 'getCollator'); @@ -119,8 +121,8 @@ describe('IndexBuilder', () => { // Add a collator. testIndexBuilder.addCollator({ - defaultRefreshIntervalSeconds: 6, factory: testCollatorFactory, + schedule: testScheduledTaskRunner, }); // Add a decorator for the same type. @@ -131,7 +133,6 @@ describe('IndexBuilder', () => { // Build the index and ensure the decorator was invoked. const { scheduler } = await testIndexBuilder.build(); scheduler.start(); - jest.advanceTimersByTime(6000); // wait for async decorator execution await Promise.resolve(); expect(decoratorSpy).toHaveBeenCalled(); @@ -146,8 +147,8 @@ describe('IndexBuilder', () => { // Add a collator. testIndexBuilder.addCollator({ - defaultRefreshIntervalSeconds: 6, factory: testCollatorFactory, + schedule: testScheduledTaskRunner, }); // Add a decorator for a different type. @@ -158,7 +159,6 @@ describe('IndexBuilder', () => { // Build the index and ensure the decorator was not invoked. const { scheduler } = await testIndexBuilder.build(); scheduler.start(); - jest.advanceTimersByTime(6000); expect(collatorSpy).toHaveBeenCalled(); expect(decoratorSpy).not.toHaveBeenCalled(); }); diff --git a/plugins/search-backend-node/src/IndexBuilder.ts b/plugins/search-backend-node/src/IndexBuilder.ts index 69c1b2e6fc..235bac905a 100644 --- a/plugins/search-backend-node/src/IndexBuilder.ts +++ b/plugins/search-backend-node/src/IndexBuilder.ts @@ -13,32 +13,25 @@ * See the License for the specific language governing permissions and * limitations under the License. */ - import { - DocumentCollatorFactory, DocumentDecoratorFactory, DocumentTypeInfo, SearchEngine, } from '@backstage/plugin-search-common'; import { Transform, pipeline } from 'stream'; import { Logger } from 'winston'; -import { Scheduler } from './index'; +import { Scheduler } from './Scheduler'; import { IndexBuilderOptions, RegisterCollatorParameters, RegisterDecoratorParameters, } from './types'; -interface CollatorEnvelope { - factory: DocumentCollatorFactory; - refreshInterval: number; -} - /** * @beta */ export class IndexBuilder { - private collators: Record; + private collators: Record; private decorators: Record; private documentTypes: Record; private searchEngine: SearchEngine; @@ -64,16 +57,13 @@ export class IndexBuilder { * Makes the index builder aware of a collator that should be executed at the * given refresh interval. */ - addCollator({ - factory, - defaultRefreshIntervalSeconds, - }: RegisterCollatorParameters): void { + addCollator({ factory, schedule }: RegisterCollatorParameters): void { this.logger.info( `Added ${factory.constructor.name} collator factory for type ${factory.type}`, ); this.collators[factory.type] = { - refreshInterval: defaultRefreshIntervalSeconds, factory, + schedule, }; this.documentTypes[factory.type] = { visibilityPermission: factory.visibilityPermission, @@ -106,51 +96,57 @@ export class IndexBuilder { * scheduler returned to the caller. */ async build(): Promise<{ scheduler: Scheduler }> { - const scheduler = new Scheduler({ logger: this.logger }); + const scheduler = new Scheduler({ + logger: this.logger, + }); Object.keys(this.collators).forEach(type => { - scheduler.addToSchedule(async () => { - // Instantiate the collator. - const collator = await this.collators[type].factory.getCollator(); - this.logger.info( - `Collating documents for ${type} via ${this.collators[type].factory.constructor.name}`, - ); - - // Instantiate all relevant decorators. - const decorators: Transform[] = await Promise.all( - (this.decorators['*'] || []) - .concat(this.decorators[type] || []) - .map(async factory => { - const decorator = await factory.getDecorator(); - this.logger.info( - `Attached decorator via ${factory.constructor.name} to ${type} index pipeline.`, - ); - return decorator; - }), - ); - - // Instantiate the indexer. - const indexer = await this.searchEngine.getIndexer(type); - - // Compose collator/decorators/indexer into a pipeline - return new Promise(done => { - pipeline( - [collator, ...decorators, indexer], - (error: NodeJS.ErrnoException | null) => { - if (error) { - this.logger.error( - `Collating documents for ${type} failed: ${error}`, - ); - } else { - this.logger.info(`Collating documents for ${type} succeeded`); - } - - // Signal index pipeline completion! - done(); - }, + scheduler.addToSchedule({ + id: `search_index_${type.replace('-', '_').toLocaleLowerCase('en-US')}`, + scheduledRunner: this.collators[type].schedule, + task: async () => { + // Instantiate the collator. + const collator = await this.collators[type].factory.getCollator(); + this.logger.info( + `Collating documents for ${type} via ${this.collators[type].factory.constructor.name}`, ); - }); - }, this.collators[type].refreshInterval * 1000); + + // Instantiate all relevant decorators. + const decorators: Transform[] = await Promise.all( + (this.decorators['*'] || []) + .concat(this.decorators[type] || []) + .map(async factory => { + const decorator = await factory.getDecorator(); + this.logger.info( + `Attached decorator via ${factory.constructor.name} to ${type} index pipeline.`, + ); + return decorator; + }), + ); + + // Instantiate the indexer. + const indexer = await this.searchEngine.getIndexer(type); + + // Compose collator/decorators/indexer into a pipeline + return new Promise(done => { + pipeline( + [collator, ...decorators, indexer], + (error: NodeJS.ErrnoException | null) => { + if (error) { + this.logger.error( + `Collating documents for ${type} failed: ${error}`, + ); + } else { + this.logger.info(`Collating documents for ${type} succeeded`); + } + + // Signal index pipeline completion! + done(); + }, + ); + }); + }, + }); }); return { diff --git a/plugins/search-backend-node/src/Scheduler.test.ts b/plugins/search-backend-node/src/Scheduler.test.ts index de6add13fe..7eaa6e8c9d 100644 --- a/plugins/search-backend-node/src/Scheduler.test.ts +++ b/plugins/search-backend-node/src/Scheduler.test.ts @@ -29,31 +29,107 @@ describe('Scheduler', () => { describe('addToSchedule', () => { it('should not add a task and interval to schedule, if already started', async () => { - jest.useFakeTimers(); const mockTask1 = jest.fn(); const mockTask2 = jest.fn(); + const mockScheduledTaskRunner1 = { + run: jest.fn(), + }; + const mockScheduledTaskRunner2 = { + run: jest.fn(), + }; // Add a task and interval to schedule - testScheduler.addToSchedule(mockTask1, 2); + testScheduler.addToSchedule({ + id: 'id1', + task: mockTask1, + scheduledRunner: mockScheduledTaskRunner1, + }); // Starts scheduling process testScheduler.start(); // Throws Error if task and interval is added to a already started schedule - expect(() => testScheduler.addToSchedule(mockTask2, 2)).toThrowError(); + expect(() => + testScheduler.addToSchedule({ + id: 'id2', + task: mockTask2, + scheduledRunner: mockScheduledTaskRunner2, + }), + ).toThrowError(); - jest.runOnlyPendingTimers(); - expect(mockTask1).toHaveBeenCalled(); - expect(mockTask2).not.toHaveBeenCalled(); + expect(mockScheduledTaskRunner1.run).toHaveBeenCalledWith( + expect.objectContaining({ + id: 'id1', + fn: mockTask1, + }), + ); + expect(mockScheduledTaskRunner2.run).not.toHaveBeenCalledWith( + expect.objectContaining({ + id: 'id2', + fn: mockTask2, + }), + ); + }); + + it('should not add a task to schedule, if it already exists', async () => { + const mockTask1 = jest.fn(); + const mockTask2 = jest.fn(); + const mockScheduledTaskRunner1 = { + run: jest.fn(), + }; + const mockScheduledTaskRunner2 = { + run: jest.fn(), + }; + + // Add a task and interval to schedule + testScheduler.addToSchedule({ + id: 'id1', + task: mockTask1, + scheduledRunner: mockScheduledTaskRunner1, + }); + + // Throws Error if task and interval is added to a already started schedule + expect(() => + testScheduler.addToSchedule({ + id: 'id1', + task: mockTask2, + scheduledRunner: mockScheduledTaskRunner2, + }), + ).toThrowError(); + + // Starts scheduling process + testScheduler.start(); + + expect(mockScheduledTaskRunner1.run).toHaveBeenCalledWith( + expect.objectContaining({ + id: 'id1', + fn: mockTask1, + }), + ); + expect(mockScheduledTaskRunner2.run).not.toHaveBeenCalledWith( + expect.objectContaining({ + id: 'id2', + fn: mockTask2, + }), + ); }); it('should be possible to add a task and interval to schedule, if already started, but stopped in between', async () => { - jest.useFakeTimers(); const mockTask1 = jest.fn(); const mockTask2 = jest.fn(); + const mockScheduledTaskRunner1 = { + run: jest.fn(), + }; + const mockScheduledTaskRunner2 = { + run: jest.fn(), + }; // Add a task and interval to schedule - testScheduler.addToSchedule(mockTask1, 2); + testScheduler.addToSchedule({ + id: 'id1', + task: mockTask1, + scheduledRunner: mockScheduledTaskRunner1, + }); // Starts scheduling process testScheduler.start(); @@ -63,15 +139,28 @@ describe('Scheduler', () => { // Shouldn't throw error, as it is stopped. expect(() => - testScheduler.addToSchedule(mockTask2, 4), + testScheduler.addToSchedule({ + id: 'id2', + task: mockTask2, + scheduledRunner: mockScheduledTaskRunner2, + }), ).not.toThrowError(); // Starts scheduling process testScheduler.start(); - jest.runOnlyPendingTimers(); - expect(mockTask1).toHaveBeenCalled(); - expect(mockTask2).toHaveBeenCalled(); + expect(mockScheduledTaskRunner1.run).toHaveBeenCalledWith( + expect.objectContaining({ + id: 'id1', + fn: mockTask1, + }), + ); + expect(mockScheduledTaskRunner2.run).toHaveBeenCalledWith( + expect.objectContaining({ + id: 'id2', + fn: mockTask2, + }), + ); }); }); @@ -79,16 +168,40 @@ describe('Scheduler', () => { it('should execute tasks on start', () => { const mockTask1 = jest.fn(); const mockTask2 = jest.fn(); + const mockScheduledTaskRunner1 = { + run: jest.fn(), + }; + const mockScheduledTaskRunner2 = { + run: jest.fn(), + }; // Add tasks and interval to schedule - testScheduler.addToSchedule(mockTask1, 2); - testScheduler.addToSchedule(mockTask2, 2); + testScheduler.addToSchedule({ + id: 'id1', + task: mockTask1, + scheduledRunner: mockScheduledTaskRunner1, + }); + testScheduler.addToSchedule({ + id: 'id2', + task: mockTask2, + scheduledRunner: mockScheduledTaskRunner2, + }); // Starts scheduling process testScheduler.start(); - expect(mockTask1).toHaveBeenCalled(); - expect(mockTask2).toHaveBeenCalled(); + expect(mockScheduledTaskRunner1.run).toHaveBeenCalledWith( + expect.objectContaining({ + id: 'id1', + fn: mockTask1, + }), + ); + expect(mockScheduledTaskRunner2.run).toHaveBeenCalledWith( + expect.objectContaining({ + id: 'id2', + fn: mockTask2, + }), + ); }); }); }); diff --git a/plugins/search-backend-node/src/Scheduler.ts b/plugins/search-backend-node/src/Scheduler.ts index 6debaa3dcd..9f3d95b112 100644 --- a/plugins/search-backend-node/src/Scheduler.ts +++ b/plugins/search-backend-node/src/Scheduler.ts @@ -14,29 +14,38 @@ * limitations under the License. */ +import { AbortController } from 'node-abort-controller'; import { Logger } from 'winston'; -import { runPeriodically } from './runPeriodically'; +import { TaskFunction, TaskRunner } from '@backstage/backend-tasks'; type TaskEnvelope = { - task: Function; - interval: number; + task: TaskFunction; + scheduledRunner: TaskRunner; }; /** - * TODO: coordination, error handling + * @public ScheduleTaskParameters */ +export interface ScheduleTaskParameters { + id: string; + task: TaskFunction; + scheduledRunner: TaskRunner; +} /** * @beta */ export class Scheduler { private logger: Logger; - private schedule: TaskEnvelope[]; - private runningTasks: Function[] = []; + private schedule: { [id: string]: TaskEnvelope }; + private abortController: AbortController; + private isRunning: boolean; constructor({ logger }: { logger: Logger }) { this.logger = logger; - this.schedule = []; + this.schedule = {}; + this.abortController = new AbortController(); + this.isRunning = false; } /** @@ -44,13 +53,18 @@ export class Scheduler { * When running the tasks, the scheduler waits at least for the time specified * in the interval once the task was completed, before running it again. */ - addToSchedule(task: Function, interval: number) { - if (this.runningTasks.length) { + addToSchedule({ id, task, scheduledRunner }: ScheduleTaskParameters) { + if (this.isRunning) { throw new Error( 'Cannot add task to schedule that has already been started.', ); } - this.schedule.push({ task, interval }); + + if (this.schedule[id]) { + throw new Error(`Task with id ${id} already exists.`); + } + + this.schedule[id] = { task, scheduledRunner }; } /** @@ -58,8 +72,14 @@ export class Scheduler { */ start() { this.logger.info('Starting all scheduled search tasks.'); - this.schedule.forEach(({ task, interval }) => { - this.runningTasks.push(runPeriodically(() => task(), interval)); + this.isRunning = true; + Object.keys(this.schedule).forEach(id => { + const { task, scheduledRunner } = this.schedule[id]; + scheduledRunner.run({ + id, + fn: task, + signal: this.abortController.signal, + }); }); } @@ -68,9 +88,7 @@ export class Scheduler { */ stop() { this.logger.info('Stopping all scheduled search tasks.'); - this.runningTasks.forEach(cancel => { - cancel(); - }); - this.runningTasks = []; + this.abortController.abort(); + this.isRunning = false; } } diff --git a/plugins/search-backend-node/src/index.ts b/plugins/search-backend-node/src/index.ts index e84c5d6a4d..e5202ac640 100644 --- a/plugins/search-backend-node/src/index.ts +++ b/plugins/search-backend-node/src/index.ts @@ -36,6 +36,8 @@ export type { export * from './indexing'; export * from './test-utils'; +export type { ScheduleTaskParameters } from './Scheduler'; + /** * @deprecated Import from @backstage/plugin-search-common instead */ diff --git a/plugins/search-backend-node/src/runPeriodically.test.ts b/plugins/search-backend-node/src/runPeriodically.test.ts deleted file mode 100644 index 0f2ee44b8d..0000000000 --- a/plugins/search-backend-node/src/runPeriodically.test.ts +++ /dev/null @@ -1,84 +0,0 @@ -/* - * Copyright 2020 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 { runPeriodically } from './runPeriodically'; - -jest.useFakeTimers(); - -describe('runPeriodically', () => { - const flushPromises = async () => { - const promise = new Promise(resolve => process.nextTick(resolve)); - jest.runAllTicks(); - await promise; - }; - const advanceTimersByTime = async (time: number) => { - jest.advanceTimersByTime(time); - // Advancing the time with jest doesn't run all promises, but only sync code - await flushPromises(); - }; - - it('runs task initially', async () => { - const task = jest.fn(async () => {}); - const cancel = runPeriodically(task, 1000); - expect(task).toHaveBeenCalledTimes(1); - cancel(); - }); - - it('runs at requested interval', async () => { - const task = jest.fn(async () => {}); - const cancel = runPeriodically(task, 1000); - await flushPromises(); - await advanceTimersByTime(1000); - await advanceTimersByTime(1000); - expect(task).toHaveBeenCalledTimes(3); - cancel(); - }); - - it('stops after being canceled', async () => { - const task = jest.fn(async () => {}); - const cancel = runPeriodically(task, 1000); - await flushPromises(); - cancel(); - await advanceTimersByTime(1000); - await advanceTimersByTime(1000); - expect(task).toHaveBeenCalledTimes(1); - }); - - it('continues running after failures', async () => { - const task = jest.fn(async () => { - throw new Error(); - }); - const cancel = runPeriodically(task, 1000); - await flushPromises(); - await advanceTimersByTime(1000); - await advanceTimersByTime(1000); - expect(task).toHaveBeenCalledTimes(3); - cancel(); - }); - - it('waits till a long running task is completed', async () => { - const task = jest.fn( - () => new Promise(resolve => setTimeout(resolve, 10000)), - ); - const cancel = runPeriodically(task, 1000); - await flushPromises(); - await advanceTimersByTime(1000); - expect(task).toHaveBeenCalledTimes(1); - await advanceTimersByTime(9000); - await advanceTimersByTime(1000); - expect(task).toHaveBeenCalledTimes(2); - cancel(); - }); -}); diff --git a/plugins/search-backend-node/src/runPeriodically.ts b/plugins/search-backend-node/src/runPeriodically.ts deleted file mode 100644 index 2f3104e221..0000000000 --- a/plugins/search-backend-node/src/runPeriodically.ts +++ /dev/null @@ -1,54 +0,0 @@ -/* - * Copyright 2020 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. - */ - -/** - * Runs a function repeatedly, with a fixed wait between invocations. - * - * Supports async functions, and silently ignores exceptions and rejections. - * - * @param fn - The function to run. May return a Promise. - * @param delayMs - The delay between a completed function invocation and the - * next. - * @returns A function that, when called, stops the invocation loop. - */ -export function runPeriodically(fn: () => any, delayMs: number): () => void { - let cancel: () => void; - let cancelled = false; - const cancellationPromise = new Promise(resolve => { - cancel = () => { - resolve(); - cancelled = true; - }; - }); - - const startRefresh = async () => { - while (!cancelled) { - try { - await fn(); - } catch { - // ignore intentionally - } - - await Promise.race([ - new Promise(resolve => setTimeout(resolve, delayMs)), - cancellationPromise, - ]); - } - }; - startRefresh(); - - return cancel!; -} diff --git a/plugins/search-backend-node/src/types.ts b/plugins/search-backend-node/src/types.ts index d21bfaf453..a92398e8c8 100644 --- a/plugins/search-backend-node/src/types.ts +++ b/plugins/search-backend-node/src/types.ts @@ -14,6 +14,7 @@ * limitations under the License. */ +import { TaskRunner } from '@backstage/backend-tasks'; import { DocumentCollatorFactory, DocumentDecoratorFactory, @@ -35,10 +36,10 @@ export type IndexBuilderOptions = { */ export interface RegisterCollatorParameters { /** - * The default interval (in seconds) that the provided collator will be called (can be overridden in config). + * The schedule for which the provided collator will be called, commonly the result of + * {@link @backstage/backend-tasks#PluginTaskScheduler.createScheduledTaskRunner} */ - defaultRefreshIntervalSeconds: number; - + schedule: TaskRunner; /** * The class responsible for returning the document collator of the given type. */ From 04575786b0a8868b42fd5accbad4eaad3f41367a Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Thu, 7 Apr 2022 12:06:53 +0200 Subject: [PATCH 04/10] scripts: add script to verify local dependency ranges Signed-off-by: Patrik Oldsberg --- .github/workflows/ci.yml | 3 + scripts/verify-local-dependencies.js | 101 +++++++++++++++++++++++++++ 2 files changed, 104 insertions(+) create mode 100755 scripts/verify-local-dependencies.js diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 1f0c6e7602..5ca77b96bb 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -137,6 +137,9 @@ jobs: - name: verify doc links run: node scripts/verify-links.js + - name: verify local dependency ranges + run: node scripts/verify-local-dependencies.js + - name: build changed packages if: ${{ steps.yarn-lock.outcome == 'success' }} run: yarn backstage-cli repo build --all --since origin/master diff --git a/scripts/verify-local-dependencies.js b/scripts/verify-local-dependencies.js new file mode 100755 index 0000000000..e11ae2e55d --- /dev/null +++ b/scripts/verify-local-dependencies.js @@ -0,0 +1,101 @@ +#!/usr/bin/env node +/* + * Copyright 2020 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. + */ + +const fs = require('fs-extra'); +const semver = require('semver'); +const { getPackages } = require('@manypkg/get-packages'); +const { + resolve: resolvePath, + relative: relativePath, + join: joinPath, +} = require('path'); + +/** + * This script checks that all local package dependencies within the repo + * point to the correct version ranges. + * + * It can be run with a `--fix` flag to a automatically fix any issues. + */ + +const depTypes = [ + 'dependencies', + 'devDependencies', + 'peerDependencies', + 'optionalDependencies', +]; + +async function main(args) { + const shouldFix = args.includes('--fix'); + const rootPath = resolvePath(__dirname, '..'); + const { packages } = await getPackages(rootPath); + + let hadErrors = false; + + const pkgMap = new Map(packages.map(pkg => [pkg.packageJson.name, pkg])); + + for (const pkg of packages) { + let fixed = false; + + for (const depType of depTypes) { + const deps = pkg.packageJson[depType]; + + for (const [dep, range] of Object.entries(deps || {})) { + if (range === '' || range.startsWith('link:')) { + continue; + } + const localPackage = pkgMap.get(dep); + if (localPackage) { + const localVersion = localPackage.packageJson.version; + if (!semver.satisfies(localVersion, range)) { + const path = joinPath( + relativePath(rootPath, pkg.dir), + 'package.json', + ); + console.log( + `${path} depends on the wrong version of ${dep}: ${range} does not satisfy ${localVersion}`, + ); + hadErrors = true; + + fixed = true; + pkg.packageJson[depType][dep] = `^${localVersion}`; + } + } + } + } + + if (shouldFix && fixed) { + await fs.writeJson(joinPath(pkg.dir, 'package.json'), pkg.packageJson, { + spaces: 2, + }); + } + } + + if (!shouldFix && hadErrors) { + console.error(); + console.error('At least one package has an invalid local dependency'); + console.error( + 'Run `node scripts/verify-local-dependencies.js --fix` to fix', + ); + + process.exit(2); + } +} + +main(process.argv.slice(2)).catch(error => { + console.error(error.stack || error); + process.exit(1); +}); From 9b5877584917bf9c9445f8338f17c8e85a648bcf Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Thu, 7 Apr 2022 12:07:11 +0200 Subject: [PATCH 05/10] home: fix outdated dependency on @backstage/config Signed-off-by: Patrik Oldsberg --- .changeset/silly-forks-study.md | 5 +++++ plugins/home/package.json | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) create mode 100644 .changeset/silly-forks-study.md diff --git a/.changeset/silly-forks-study.md b/.changeset/silly-forks-study.md new file mode 100644 index 0000000000..d337aecec6 --- /dev/null +++ b/.changeset/silly-forks-study.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-home': patch +--- + +Updated the dependency on `@backstage/config` to `^1.0.0`. diff --git a/plugins/home/package.json b/plugins/home/package.json index 5358f83b55..43b474704a 100644 --- a/plugins/home/package.json +++ b/plugins/home/package.json @@ -41,7 +41,7 @@ "@backstage/plugin-search": "^0.7.5-next.0", "@backstage/plugin-stack-overflow": "^0.1.0-next.0", "@backstage/theme": "^0.2.15", - "@backstage/config": "^0.1.15", + "@backstage/config": "^1.0.0", "@material-ui/core": "^4.12.2", "@material-ui/icons": "^4.9.1", "@material-ui/lab": "4.0.0-alpha.57", From e838a7060acc29cd94ab4cf847192733b9aa1188 Mon Sep 17 00:00:00 2001 From: Johan Haals Date: Fri, 8 Apr 2022 10:58:08 +0200 Subject: [PATCH 06/10] Add package resolution for @types/react Signed-off-by: Johan Haals --- .changeset/little-moles-pull.md | 5 +++++ package.json | 3 ++- .../templates/default-app/package.json.hbs | 3 +++ yarn.lock | 21 ++++--------------- 4 files changed, 14 insertions(+), 18 deletions(-) create mode 100644 .changeset/little-moles-pull.md diff --git a/.changeset/little-moles-pull.md b/.changeset/little-moles-pull.md new file mode 100644 index 0000000000..8a8d6da972 --- /dev/null +++ b/.changeset/little-moles-pull.md @@ -0,0 +1,5 @@ +--- +'@backstage/create-app': patch +--- + +Add resolution for version 17 `@types/react` due to breaking changes introduced in version 18. diff --git a/package.json b/package.json index 13ba3c34ea..3154188414 100644 --- a/package.json +++ b/package.json @@ -45,7 +45,8 @@ ] }, "resolutions": { - "**/@graphql-codegen/cli/**/ws": "^7.4.6" + "**/@graphql-codegen/cli/**/ws": "^7.4.6", + "@types/react": "^17.0.39" }, "version": "1.1.0-next.2", "dependencies": { diff --git a/packages/create-app/templates/default-app/package.json.hbs b/packages/create-app/templates/default-app/package.json.hbs index 884eaf23cf..55a1b63aa9 100644 --- a/packages/create-app/templates/default-app/package.json.hbs +++ b/packages/create-app/templates/default-app/package.json.hbs @@ -37,6 +37,9 @@ "prettier": "^2.3.2", "typescript": "~4.5.4" }, + "resolutions": { + "@types/react": "^17.0.39" + }, "prettier": "@spotify/prettier-config", "lint-staged": { "*.{js,jsx,ts,tsx,mjs,cjs}": [ diff --git a/yarn.lock b/yarn.lock index 63cd4710f5..21e7f59caa 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1478,14 +1478,6 @@ lodash "^4.17.21" uuid "^8.0.0" -"@backstage/config@^0.1.15": - version "0.1.15" - resolved "https://registry.npmjs.org/@backstage/config/-/config-0.1.15.tgz#4bad122ad861be5bd61a60639f92d2494fa245c5" - integrity sha512-eNJEYYSEu9MkrkBYiMpUBWEc3Bu64YgB9pZZGCMW7/9350tV2wbylEdoBJHslilJlJhiUyTXBckn8Ua7DOH7rw== - dependencies: - "@backstage/types" "^0.1.3" - lodash "^4.17.21" - "@backstage/core-components@^0.9.0", "@backstage/core-components@^0.9.2": version "0.9.2" resolved "https://registry.npmjs.org/@backstage/core-components/-/core-components-0.9.2.tgz#9a3d79a15039256bbc007e5daa08c983050e0238" @@ -1610,11 +1602,6 @@ react-use "^17.2.4" swr "^1.1.2" -"@backstage/types@^0.1.3": - version "0.1.3" - resolved "https://registry.npmjs.org/@backstage/types/-/types-0.1.3.tgz#6613d8cbdf97d42d31cd1e66a833df533e7ccf14" - integrity sha512-fJVi4oVrlO+G3PRv1fYSll9/X4pE11HLnkI//Geare9sP6wSfp/2zXpLYfKVsG0e24jOl7Swkc8lwLkQ90zMaQ== - "@balena/dockerignore@^1.0.2": version "1.0.2" resolved "https://registry.npmjs.org/@balena/dockerignore/-/dockerignore-1.0.2.tgz#9ffe4726915251e8eb69f44ef3547e0da2c03e0d" @@ -6652,10 +6639,10 @@ dependencies: "@types/react" "*" -"@types/react@*", "@types/react@>=16.9.0", "@types/react@^16.13.1 || ^17.0.0": - version "17.0.43" - resolved "https://registry.npmjs.org/@types/react/-/react-17.0.43.tgz#4adc142887dd4a2601ce730bc56c3436fdb07a55" - integrity sha512-8Q+LNpdxf057brvPu1lMtC5Vn7J119xrP1aq4qiaefNioQUYANF/CYeK4NsKorSZyUGJ66g0IM+4bbjwx45o2A== +"@types/react@*", "@types/react@>=16.9.0", "@types/react@^16.13.1 || ^17.0.0", "@types/react@^17.0.39": + version "17.0.44" + resolved "https://registry.npmjs.org/@types/react/-/react-17.0.44.tgz#c3714bd34dd551ab20b8015d9d0dbec812a51ec7" + integrity sha512-Ye0nlw09GeMp2Suh8qoOv0odfgCoowfM/9MG6WeRD60Gq9wS90bdkdRtYbRkNhXOpG4H+YXGvj4wOWhAC0LJ1g== dependencies: "@types/prop-types" "*" "@types/scheduler" "*" From 8b669400ea8340a1f2c7595b186e33837c3143cc Mon Sep 17 00:00:00 2001 From: Johan Haals Date: Fri, 8 Apr 2022 11:05:43 +0200 Subject: [PATCH 07/10] include react-dom types, update changeset Signed-off-by: Johan Haals --- .changeset/little-moles-pull.md | 20 ++++++++++++++++++- package.json | 3 ++- .../templates/default-app/package.json.hbs | 3 ++- 3 files changed, 23 insertions(+), 3 deletions(-) diff --git a/.changeset/little-moles-pull.md b/.changeset/little-moles-pull.md index 8a8d6da972..7fee7e5148 100644 --- a/.changeset/little-moles-pull.md +++ b/.changeset/little-moles-pull.md @@ -2,4 +2,22 @@ '@backstage/create-app': patch --- -Add resolution for version 17 `@types/react` due to breaking changes introduced in version 18. +Add resolution for version 17 `@types/react` and `types/react-dom` due to breaking changes introduced in version 18. + +To apply these changes to your existing installation. Add a resolutions block to your `package.json` + +```json + "resolutions": { + "@types/react": "^17", + "@types/react-dom": "^17" + }, +``` + +If your depending on react 16 in use this resolution block instead. + +```json + "resolutions": { + "@types/react": "^16", + "@types/react-dom": "^16" + }, +``` diff --git a/package.json b/package.json index 3154188414..dc72b8974b 100644 --- a/package.json +++ b/package.json @@ -46,7 +46,8 @@ }, "resolutions": { "**/@graphql-codegen/cli/**/ws": "^7.4.6", - "@types/react": "^17.0.39" + "@types/react": "^17", + "@types/react-dom": "^17" }, "version": "1.1.0-next.2", "dependencies": { diff --git a/packages/create-app/templates/default-app/package.json.hbs b/packages/create-app/templates/default-app/package.json.hbs index 55a1b63aa9..5bdbec7971 100644 --- a/packages/create-app/templates/default-app/package.json.hbs +++ b/packages/create-app/templates/default-app/package.json.hbs @@ -38,7 +38,8 @@ "typescript": "~4.5.4" }, "resolutions": { - "@types/react": "^17.0.39" + "@types/react": "^17", + "@types/react-dom": "^17" }, "prettier": "@spotify/prettier-config", "lint-staged": { From 7d0a1f0199751abac40735a016cf6bed39715a52 Mon Sep 17 00:00:00 2001 From: Johan Haals Date: Fri, 8 Apr 2022 11:10:09 +0200 Subject: [PATCH 08/10] fix typo Signed-off-by: Johan Haals --- .changeset/little-moles-pull.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.changeset/little-moles-pull.md b/.changeset/little-moles-pull.md index 7fee7e5148..8b9f3ca446 100644 --- a/.changeset/little-moles-pull.md +++ b/.changeset/little-moles-pull.md @@ -2,7 +2,7 @@ '@backstage/create-app': patch --- -Add resolution for version 17 `@types/react` and `types/react-dom` due to breaking changes introduced in version 18. +Add type resolutions for `@types/react` and `types/react-dom`. This is an To apply these changes to your existing installation. Add a resolutions block to your `package.json` @@ -13,7 +13,7 @@ To apply these changes to your existing installation. Add a resolutions block to }, ``` -If your depending on react 16 in use this resolution block instead. +If your existing app depend on react 16 use this resolution block instead. ```json "resolutions": { From 1d031b536797bb5ac16c556dc399b6bfa4789fb2 Mon Sep 17 00:00:00 2001 From: Johan Haals Date: Fri, 8 Apr 2022 11:18:27 +0200 Subject: [PATCH 09/10] fix wording Signed-off-by: Johan Haals --- .changeset/little-moles-pull.md | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/.changeset/little-moles-pull.md b/.changeset/little-moles-pull.md index 8b9f3ca446..592fc13ab6 100644 --- a/.changeset/little-moles-pull.md +++ b/.changeset/little-moles-pull.md @@ -2,7 +2,9 @@ '@backstage/create-app': patch --- -Add type resolutions for `@types/react` and `types/react-dom`. This is an +Add type resolutions for `@types/react` and `types/react-dom`. + +The reason for this is the usage of `"@types/react": "*"` as a dependency which is very common practice in react packages. This recently resolves to react 18 which introduces several breaking changes in both internal and external packages. To apply these changes to your existing installation. Add a resolutions block to your `package.json` @@ -13,7 +15,7 @@ To apply these changes to your existing installation. Add a resolutions block to }, ``` -If your existing app depend on react 16 use this resolution block instead. +If your existing app depends on react 16, use this resolution block instead. ```json "resolutions": { From 48129cf82642e071089c0f18dd42880dd1dd7858 Mon Sep 17 00:00:00 2001 From: Johan Haals Date: Fri, 8 Apr 2022 11:19:56 +0200 Subject: [PATCH 10/10] reformat Signed-off-by: Johan Haals --- .changeset/little-moles-pull.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/little-moles-pull.md b/.changeset/little-moles-pull.md index 592fc13ab6..f52bc24bb2 100644 --- a/.changeset/little-moles-pull.md +++ b/.changeset/little-moles-pull.md @@ -6,7 +6,7 @@ Add type resolutions for `@types/react` and `types/react-dom`. The reason for this is the usage of `"@types/react": "*"` as a dependency which is very common practice in react packages. This recently resolves to react 18 which introduces several breaking changes in both internal and external packages. -To apply these changes to your existing installation. Add a resolutions block to your `package.json` +To apply these changes to your existing installation, add a resolutions block to your `package.json` ```json "resolutions": {