From 2a865343c2bcaa6c70fe123b0eff32debde42a0d Mon Sep 17 00:00:00 2001 From: Nikolas Skoufis Date: Wed, 16 Feb 2022 16:42:33 +1100 Subject: [PATCH] Add a new interface: DocsBuildStrategy This adds a new interface called DocsBuildStrategy. This strategy allows for different answers to the question: should the TechDocs backend perform a build of the given entity? The default implementation replicates the existing functionality, in that if the techdocs.builder config value is set to local, then we do trigger builds, and if the value is set to anything else, we don't trigger builds. However the strategy has access to the entity, and so more complex strategies are possible. This strategy is added as an optional parameter on the RouterOptions types, and defaults to the DefaultDocsBuildStrategy if unspecified, allowing for backwards compatibility. TODO: rename some of the config options, and reword errors to reflect the new interface Signed-off-by: Nikolas Skoufis --- .../src/service/DocsBuildStrategy.test.ts | 63 +++++++++++++++++++ .../src/service/DocsBuildStrategy.ts | 33 ++++++++++ .../src/service/router.test.ts | 30 +++++---- .../techdocs-backend/src/service/router.ts | 11 +++- 4 files changed, 124 insertions(+), 13 deletions(-) create mode 100644 plugins/techdocs-backend/src/service/DocsBuildStrategy.test.ts create mode 100644 plugins/techdocs-backend/src/service/DocsBuildStrategy.ts diff --git a/plugins/techdocs-backend/src/service/DocsBuildStrategy.test.ts b/plugins/techdocs-backend/src/service/DocsBuildStrategy.test.ts new file mode 100644 index 0000000000..71e5377a03 --- /dev/null +++ b/plugins/techdocs-backend/src/service/DocsBuildStrategy.test.ts @@ -0,0 +1,63 @@ +/* + * Copyright 2021 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 { DefaultDocsBuildStrategy } from './DocsBuildStrategy'; +import { ConfigReader } from '@backstage/config'; + +const MockedConfigReader = ConfigReader as jest.MockedClass< + typeof ConfigReader +>; + +jest.mock('@backstage/config'); + +describe('DefaultDocsBuildStrategy', () => { + const entity = { + apiVersion: 'backstage.io/v1alpha1', + kind: 'Component', + metadata: { + uid: '0', + name: 'test', + }, + }; + + const config = new ConfigReader({}); + + beforeEach(() => { + jest.resetAllMocks(); + }); + + describe('shouldBuild', () => { + it('should return true when techdocs.build is set to local', async () => { + const defaultDocsBuildStrategy = new DefaultDocsBuildStrategy(config); + + MockedConfigReader.prototype.getString.mockReturnValue('local'); + + const result = await defaultDocsBuildStrategy.shouldBuild(entity); + + expect(result).toBe(true); + }); + + it('should return false when techdocs.build is set to external', async () => { + const defaultDocsBuildStrategy = new DefaultDocsBuildStrategy(config); + + MockedConfigReader.prototype.getString.mockReturnValue('external'); + + const result = await defaultDocsBuildStrategy.shouldBuild(entity); + + expect(result).toBe(false); + }); + }); +}); diff --git a/plugins/techdocs-backend/src/service/DocsBuildStrategy.ts b/plugins/techdocs-backend/src/service/DocsBuildStrategy.ts new file mode 100644 index 0000000000..69b0474a85 --- /dev/null +++ b/plugins/techdocs-backend/src/service/DocsBuildStrategy.ts @@ -0,0 +1,33 @@ +/* + * 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 { Entity } from '@backstage/catalog-model'; +import { Config } from '@backstage/config'; + +export interface DocsBuildStrategy { + shouldBuild(entity: Entity): Promise; +} + +export class DefaultDocsBuildStrategy { + private readonly config: Config; + + constructor(config: Config) { + this.config = config; + } + + async shouldBuild(_: Entity): Promise { + return this.config.getString('techdocs.builder') === 'local'; + } +} diff --git a/plugins/techdocs-backend/src/service/router.test.ts b/plugins/techdocs-backend/src/service/router.test.ts index 43e9641efb..521024252c 100644 --- a/plugins/techdocs-backend/src/service/router.test.ts +++ b/plugins/techdocs-backend/src/service/router.test.ts @@ -38,6 +38,7 @@ import { RouterOptions, } from './router'; import { TechDocsCache } from '../cache'; +import { DocsBuildStrategy } from './DocsBuildStrategy'; jest.mock('@backstage/catalog-client'); jest.mock('@backstage/config'); @@ -120,6 +121,9 @@ describe('createRouter', () => { const cache: jest.Mocked = { getClient: jest.fn(), }; + const docsBuildStrategy: jest.Mocked = { + shouldBuild: jest.fn(), + }; const outOfTheBoxOptions = { preparers, generators, @@ -128,6 +132,7 @@ describe('createRouter', () => { logger: getVoidLogger(), discovery, cache, + docsBuildStrategy, }; const recommendedOptions = { publisher, @@ -135,6 +140,7 @@ describe('createRouter', () => { logger: getVoidLogger(), discovery, cache, + docsBuildStrategy, }; beforeEach(() => { @@ -181,10 +187,10 @@ describe('createRouter', () => { expect(response.status).toBe(404); }); - it('should not check for an update without local builder', async () => { + it('should not check for an update when shouldBuild returns false', async () => { const app = await createApp(outOfTheBoxOptions); - MockedConfigReader.prototype.getString.mockReturnValue('external'); + docsBuildStrategy.shouldBuild.mockResolvedValue(false); MockCachedEntityLoader.prototype.load.mockResolvedValue(entity); MockDocsSynchronizer.prototype.doCacheSync.mockImplementation( async ({ responseHandler }) => @@ -198,10 +204,10 @@ describe('createRouter', () => { expect(response.status).toBe(304); }); - it('should error if missing builder', async () => { + it('should error if build is required and is missing preparer', async () => { const app = await createApp(recommendedOptions); - MockedConfigReader.prototype.getString.mockReturnValue('local'); + docsBuildStrategy.shouldBuild.mockResolvedValue(true); MockCachedEntityLoader.prototype.load.mockResolvedValue(entity); const response = await request(app) @@ -219,7 +225,7 @@ describe('createRouter', () => { it('should execute synchronization', async () => { const app = await createApp(outOfTheBoxOptions); - MockedConfigReader.prototype.getString.mockReturnValue('local'); + docsBuildStrategy.shouldBuild.mockResolvedValue(true); MockCachedEntityLoader.prototype.load.mockResolvedValue(entity); MockDocsSynchronizer.prototype.doSync.mockImplementation( async ({ responseHandler }) => @@ -244,7 +250,7 @@ describe('createRouter', () => { it('should return on updated', async () => { const app = await createApp(outOfTheBoxOptions); - MockedConfigReader.prototype.getString.mockReturnValue('local'); + docsBuildStrategy.shouldBuild.mockResolvedValue(true); MockCachedEntityLoader.prototype.load.mockResolvedValue(entity); MockDocsSynchronizer.prototype.doSync.mockImplementation( async ({ responseHandler }) => { @@ -297,10 +303,10 @@ describe('createRouter', () => { expect(response.status).toBe(404); }); - it('should not check for an update without local builder', async () => { + it('should not check for an update when shouldBuild returns false', async () => { const app = await createApp(outOfTheBoxOptions); - MockedConfigReader.prototype.getString.mockReturnValue('external'); + docsBuildStrategy.shouldBuild.mockResolvedValue(false); MockCachedEntityLoader.prototype.load.mockResolvedValue(entity); MockDocsSynchronizer.prototype.doCacheSync.mockImplementation( async ({ responseHandler }) => @@ -322,10 +328,10 @@ data: {"updated":false} ); }); - it('should error if missing builder', async () => { + it('should error if build is required and is missing preparer', async () => { const app = await createApp(recommendedOptions); - MockedConfigReader.prototype.getString.mockReturnValue('local'); + docsBuildStrategy.shouldBuild.mockResolvedValue(true); MockCachedEntityLoader.prototype.load.mockResolvedValue(entity); const response = await request(app) @@ -348,7 +354,7 @@ data: "Invalid configuration. 'techdocs.builder' was set to 'local' but no 'prep it('should execute synchronization', async () => { const app = await createApp(outOfTheBoxOptions); - MockedConfigReader.prototype.getString.mockReturnValue('local'); + docsBuildStrategy.shouldBuild.mockResolvedValue(true); MockCachedEntityLoader.prototype.load.mockResolvedValue(entity); MockDocsSynchronizer.prototype.doSync.mockImplementation( async ({ responseHandler }) => @@ -376,7 +382,7 @@ data: "Invalid configuration. 'techdocs.builder' was set to 'local' but no 'prep it('should return an event-stream', async () => { const app = await createApp(outOfTheBoxOptions); - MockedConfigReader.prototype.getString.mockReturnValue('local'); + docsBuildStrategy.shouldBuild.mockResolvedValue(true); MockCachedEntityLoader.prototype.load.mockResolvedValue(entity); MockDocsSynchronizer.prototype.doSync.mockImplementation( async ({ responseHandler }) => { diff --git a/plugins/techdocs-backend/src/service/router.ts b/plugins/techdocs-backend/src/service/router.ts index 141ce93f79..96916bdc9b 100644 --- a/plugins/techdocs-backend/src/service/router.ts +++ b/plugins/techdocs-backend/src/service/router.ts @@ -35,6 +35,10 @@ import { ScmIntegrations } from '@backstage/integration'; import { DocsSynchronizer, DocsSynchronizerSyncOpts } from './DocsSynchronizer'; import { createCacheMiddleware, TechDocsCache } from '../cache'; import { CachedEntityLoader } from './CachedEntityLoader'; +import { + DefaultDocsBuildStrategy, + DocsBuildStrategy, +} from './DocsBuildStrategy'; /** * All of the required dependencies for running TechDocs in the "out-of-the-box" @@ -49,6 +53,7 @@ export type OutOfTheBoxDeploymentOptions = { database?: Knex; // TODO: Make database required when we're implementing database stuff. config: Config; cache: PluginCacheManager; + docsBuildStrategy?: DocsBuildStrategy; }; /** @@ -61,6 +66,7 @@ export type RecommendedDeploymentOptions = { discovery: PluginEndpointDiscovery; config: Config; cache: PluginCacheManager; + docsBuildStrategy?: DocsBuildStrategy; }; /** @@ -86,6 +92,8 @@ export async function createRouter( const router = Router(); const { publisher, config, logger, discovery } = options; const catalogClient = new CatalogClient({ discoveryApi: discovery }); + const docsBuildStrategy = + options.docsBuildStrategy ?? new DefaultDocsBuildStrategy(config); // Entities are cached to optimize the /static/docs request path, which can be called many times // when loading a single techdocs page. @@ -200,7 +208,8 @@ export async function createRouter( // techdocs-backend will only try to build documentation for an entity if techdocs.builder is set to 'local' // If set to 'external', it will assume that an external process (e.g. CI/CD pipeline // of the repository) is responsible for building and publishing documentation to the storage provider - if (config.getString('techdocs.builder') !== 'local') { + const shouldBuild = await docsBuildStrategy.shouldBuild(entity); + if (!shouldBuild) { // However, if caching is enabled, take the opportunity to check and // invalidate stale cache entries. if (cache) {