From 9b98328e99bfd17abf0d34a77af54c839cba2143 Mon Sep 17 00:00:00 2001 From: Dominik Henneke Date: Tue, 6 Jul 2021 18:38:40 +0200 Subject: [PATCH] Move the synchronization code into a dedicated class and still support non-eventstream responses Signed-off-by: Dominik Henneke --- .changeset/techdocs-loud-spies-attack.md | 6 +- plugins/techdocs-backend/src/index.ts | 2 +- .../src/service/DocsSynchronizer.test.ts | 250 ++++++++++ .../src/service/DocsSynchronizer.ts | 154 +++++++ .../src/service/router.test.ts | 428 +++++++++++------- .../techdocs-backend/src/service/router.ts | 135 +++--- 6 files changed, 732 insertions(+), 243 deletions(-) create mode 100644 plugins/techdocs-backend/src/service/DocsSynchronizer.test.ts create mode 100644 plugins/techdocs-backend/src/service/DocsSynchronizer.ts diff --git a/.changeset/techdocs-loud-spies-attack.md b/.changeset/techdocs-loud-spies-attack.md index ea80c73116..50fbe82fbc 100644 --- a/.changeset/techdocs-loud-spies-attack.md +++ b/.changeset/techdocs-loud-spies-attack.md @@ -1,8 +1,8 @@ --- -'@backstage/plugin-techdocs': minor -'@backstage/plugin-techdocs-backend': minor +'@backstage/plugin-techdocs': patch +'@backstage/plugin-techdocs-backend': patch --- -Rewrite the `/sync/:namespace/:kind/:name` to return an event-stream. +Rewrite the `/sync/:namespace/:kind/:name` endpoint to support an event-stream as response. This change allows the sync process to take longer than a normal HTTP timeout. The stream also emits log events, so the caller can follow the build process in the frontend. diff --git a/plugins/techdocs-backend/src/index.ts b/plugins/techdocs-backend/src/index.ts index afbf04aba9..6d1e551629 100644 --- a/plugins/techdocs-backend/src/index.ts +++ b/plugins/techdocs-backend/src/index.ts @@ -14,5 +14,5 @@ * limitations under the License. */ -export * from './service/router'; +export { createRouter } from './service/router'; export * from '@backstage/techdocs-common'; diff --git a/plugins/techdocs-backend/src/service/DocsSynchronizer.test.ts b/plugins/techdocs-backend/src/service/DocsSynchronizer.test.ts new file mode 100644 index 0000000000..ba411e538f --- /dev/null +++ b/plugins/techdocs-backend/src/service/DocsSynchronizer.test.ts @@ -0,0 +1,250 @@ +/* + * 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 { + getVoidLogger, + PluginEndpointDiscovery, +} from '@backstage/backend-common'; +import { CatalogApi } from '@backstage/catalog-client'; +import { ConfigReader } from '@backstage/config'; +import { NotFoundError } from '@backstage/errors'; +import { + GeneratorBuilder, + PreparerBuilder, + PublisherBase, +} from '@backstage/techdocs-common'; +import { DocsBuilder, shouldCheckForUpdate } from '../DocsBuilder'; +import { DocsSynchronizer, DocsSynchronizerSyncOpts } from './DocsSynchronizer'; + +jest.mock('@backstage/config'); +jest.mock('../DocsBuilder'); + +const MockedConfigReader = ConfigReader as jest.MockedClass< + typeof ConfigReader +>; +const MockedDocsBuilder = DocsBuilder as jest.MockedClass; + +describe('DocsSynchronizer', () => { + const preparers: jest.Mocked = { + register: jest.fn(), + get: jest.fn(), + }; + const generators: jest.Mocked = { + register: jest.fn(), + get: jest.fn(), + }; + const publisher: jest.Mocked = { + docsRouter: jest.fn(), + fetchTechDocsMetadata: jest.fn(), + getReadiness: jest.fn(), + hasDocsBeenGenerated: jest.fn(), + publish: jest.fn(), + }; + const discovery: jest.Mocked = { + getBaseUrl: jest.fn(), + getExternalBaseUrl: jest.fn(), + }; + const catalogClient: jest.Mocked = { + getLocationById: jest.fn(), + getEntityByName: jest.fn(), + getEntities: jest.fn(), + addLocation: jest.fn(), + removeLocationById: jest.fn(), + getOriginLocationByEntity: jest.fn(), + getLocationByEntity: jest.fn(), + removeEntityByUid: jest.fn(), + }; + + let docsSynchronizer: DocsSynchronizer; + const mockResponseHandler: jest.Mocked = { + log: jest.fn(), + finish: jest.fn(), + error: jest.fn(), + }; + + beforeEach(async () => { + publisher.docsRouter.mockReturnValue(() => {}); + discovery.getBaseUrl.mockImplementation(async type => { + return `http://backstage.local/api/${type}`; + }); + + docsSynchronizer = new DocsSynchronizer({ + preparers, + generators, + publisher, + catalogClient, + config: new ConfigReader({}), + logger: getVoidLogger(), + }); + }); + + afterEach(() => { + jest.resetAllMocks(); + }); + + describe('doSync', () => { + it('should execute an update', async () => { + (shouldCheckForUpdate as jest.Mock).mockReturnValue(true); + MockedConfigReader.prototype.getString.mockReturnValue('local'); + + const entity = { + apiVersion: 'backstage.io/v1alpha1', + kind: 'Component', + metadata: { + uid: '0', + name: 'test', + namespace: 'default', + annotations: { + 'sda.se/release-notes-location': + 'github-releases:https://github.com/backstage/backstage', + }, + }, + }; + + catalogClient.getEntityByName.mockResolvedValue(entity); + + MockedDocsBuilder.prototype.build.mockImplementation(async () => { + // extract the logStream from the constructor call + const logStream = MockedDocsBuilder.mock.calls[0][0].logStream; + + logStream?.write('Some log'); + logStream?.write('Another log'); + + return true; + }); + + publisher.hasDocsBeenGenerated.mockResolvedValue(true); + + await docsSynchronizer.doSync(() => mockResponseHandler, { + kind: 'Component', + namespace: 'default', + name: 'test', + token: undefined, + }); + + expect(catalogClient.getEntityByName).toBeCalledWith( + { + kind: 'Component', + namespace: 'default', + name: 'test', + }, + { token: undefined }, + ); + + expect(mockResponseHandler.log).toBeCalledTimes(2); + expect(mockResponseHandler.log).toBeCalledWith('Some log'); + expect(mockResponseHandler.log).toBeCalledWith('Another log'); + + expect(mockResponseHandler.finish).toBeCalledWith({ updated: true }); + + expect(mockResponseHandler.error).toBeCalledTimes(0); + + expect(shouldCheckForUpdate).toBeCalledTimes(1); + expect(MockedConfigReader.prototype.getString).toBeCalledTimes(1); + expect(DocsBuilder.prototype.build).toBeCalledTimes(1); + }); + + it('should not check for an update too often', async () => { + (shouldCheckForUpdate as jest.Mock).mockReturnValue(false); + + const entity = { + apiVersion: 'backstage.io/v1alpha1', + kind: 'Component', + metadata: { + uid: '0', + name: 'test', + namespace: 'default', + annotations: { + 'sda.se/release-notes-location': + 'github-releases:https://github.com/backstage/backstage', + }, + }, + }; + + catalogClient.getEntityByName.mockResolvedValue(entity); + + await docsSynchronizer.doSync(() => mockResponseHandler, { + kind: 'Component', + namespace: 'default', + name: 'test', + token: undefined, + }); + + expect(mockResponseHandler.finish).toBeCalledWith({ updated: false }); + + expect(mockResponseHandler.log).toBeCalledTimes(0); + expect(mockResponseHandler.error).toBeCalledTimes(0); + + expect(shouldCheckForUpdate).toBeCalledTimes(1); + expect(MockedConfigReader.prototype.getString).toBeCalledTimes(0); + expect(DocsBuilder.prototype.build).toBeCalledTimes(0); + }); + + it('should not check for an update without local builder', async () => { + (shouldCheckForUpdate as jest.Mock).mockReturnValue(true); + MockedConfigReader.prototype.getString.mockReturnValue('external'); + + const entity = { + apiVersion: 'backstage.io/v1alpha1', + kind: 'Component', + metadata: { + uid: '0', + name: 'test', + namespace: 'default', + annotations: { + 'sda.se/release-notes-location': + 'github-releases:https://github.com/backstage/backstage', + }, + }, + }; + + catalogClient.getEntityByName.mockResolvedValue(entity); + + await docsSynchronizer.doSync(() => mockResponseHandler, { + kind: 'Component', + namespace: 'default', + name: 'test', + token: undefined, + }); + + expect(mockResponseHandler.finish).toBeCalledWith({ updated: false }); + + expect(mockResponseHandler.log).toBeCalledTimes(0); + expect(mockResponseHandler.error).toBeCalledTimes(0); + + expect(shouldCheckForUpdate).toBeCalledTimes(1); + expect(MockedConfigReader.prototype.getString).toBeCalledTimes(1); + expect(DocsBuilder.prototype.build).toBeCalledTimes(0); + }); + + it('rejects when entity is not found', async () => { + catalogClient.getEntityByName.mockResolvedValue(undefined); + + await expect( + docsSynchronizer.doSync(() => mockResponseHandler, { + kind: 'Component', + namespace: 'default', + name: 'test', + token: undefined, + }), + ).rejects.toThrowError(NotFoundError); + + expect(mockResponseHandler.finish).toBeCalledTimes(0); + expect(mockResponseHandler.log).toBeCalledTimes(0); + expect(mockResponseHandler.error).toBeCalledTimes(0); + }); + }); +}); diff --git a/plugins/techdocs-backend/src/service/DocsSynchronizer.ts b/plugins/techdocs-backend/src/service/DocsSynchronizer.ts new file mode 100644 index 0000000000..f4ffa1149d --- /dev/null +++ b/plugins/techdocs-backend/src/service/DocsSynchronizer.ts @@ -0,0 +1,154 @@ +/* + * 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 { CatalogApi } from '@backstage/catalog-client'; +import { Config } from '@backstage/config'; +import { NotFoundError } from '@backstage/errors'; +import { + GeneratorBuilder, + PreparerBuilder, + PublisherBase, +} from '@backstage/techdocs-common'; +import { PassThrough } from 'stream'; +import { Logger } from 'winston'; +import { DocsBuilder, shouldCheckForUpdate } from '../DocsBuilder'; + +export type DocsSynchronizerSyncOpts = { + log: (message: string) => void; + error: (e: Error) => void; + finish: (result: { updated: boolean }) => void; +}; + +export class DocsSynchronizer { + private readonly preparers: PreparerBuilder; + private readonly generators: GeneratorBuilder; + private readonly publisher: PublisherBase; + private readonly logger: Logger; + private readonly config: Config; + private readonly catalogClient: CatalogApi; + + constructor({ + preparers, + generators, + publisher, + logger, + config, + catalogClient, + }: { + preparers: PreparerBuilder; + generators: GeneratorBuilder; + publisher: PublisherBase; + logger: Logger; + config: Config; + catalogClient: CatalogApi; + }) { + this.catalogClient = catalogClient; + this.config = config; + this.logger = logger; + this.publisher = publisher; + this.generators = generators; + this.preparers = preparers; + } + + async doSync( + initResponseHandler: () => DocsSynchronizerSyncOpts, + { + kind, + namespace, + name, + token, + }: { + kind: string; + namespace: string; + name: string; + token: string | undefined; + }, + ) { + const entity = await this.catalogClient.getEntityByName( + { kind, namespace, name }, + { token }, + ); + + if (!entity?.metadata?.uid) { + throw new NotFoundError('Entity metadata UID missing'); + } + + // open the event-stream + const { log, error, finish } = initResponseHandler(); + // create an in-memory stream to forward logs to the event-stream + const logStream = new PassThrough(); + logStream.on('data', async data => { + log(data.toString().trim()); + }); + + // check if the last update check was too recent + if (!shouldCheckForUpdate(entity.metadata.uid)) { + finish({ updated: false }); + return; + } + + // 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 (this.config.getString('techdocs.builder') !== 'local') { + finish({ updated: false }); + return; + } + + const docsBuilder = new DocsBuilder({ + preparers: this.preparers, + generators: this.generators, + publisher: this.publisher, + logger: this.logger, + entity, + config: this.config, + logStream, + }); + + let foundDocs = false; + + const updated = await docsBuilder.build(); + + if (!updated) { + finish({ updated: false }); + return; + } + + // With a maximum of ~5 seconds wait, check if the files got published and if docs will be fetched + // on the user's page. If not, respond with a message asking them to check back later. + // The delay here is to make sure GCS/AWS/etc. registers newly uploaded files which is usually <1 second + for (let attempt = 0; attempt < 5; attempt++) { + if (await this.publisher.hasDocsBeenGenerated(entity)) { + foundDocs = true; + break; + } + await new Promise(r => setTimeout(r, 1000)); + } + if (!foundDocs) { + this.logger.error( + 'Published files are taking longer to show up in storage. Something went wrong.', + ); + error( + new NotFoundError( + 'Sorry! It took too long for the generated docs to show up in storage. Check back later.', + ), + ); + return; + } + + finish({ updated: true }); + } +} diff --git a/plugins/techdocs-backend/src/service/router.test.ts b/plugins/techdocs-backend/src/service/router.test.ts index a3e17589ae..6216d515f7 100644 --- a/plugins/techdocs-backend/src/service/router.test.ts +++ b/plugins/techdocs-backend/src/service/router.test.ts @@ -20,34 +20,24 @@ import { PluginEndpointDiscovery, } from '@backstage/backend-common'; import { ConfigReader } from '@backstage/config'; +import { NotFoundError, NotModifiedError } from '@backstage/errors'; import { GeneratorBuilder, PreparerBuilder, PublisherBase, } from '@backstage/techdocs-common'; -import express from 'express'; -import { rest } from 'msw'; -import { setupServer } from 'msw/node'; +import express, { Response } from 'express'; import request from 'supertest'; -import { DocsBuilder, shouldCheckForUpdate } from '../DocsBuilder'; -import { createRouter } from './router'; +import { DocsSynchronizer, DocsSynchronizerSyncOpts } from './DocsSynchronizer'; +import { createEventStream, createHttpResponse, createRouter } from './router'; -jest.mock('@backstage/config'); -jest.mock('../DocsBuilder'); +jest.mock('./DocsSynchronizer'); -const MockedConfigReader = ConfigReader as jest.MockedClass< - typeof ConfigReader +const MockDocsSynchronizer = DocsSynchronizer as jest.MockedClass< + typeof DocsSynchronizer >; -const MockedDocsBuilder = DocsBuilder as jest.MockedClass; - -const server = setupServer(); describe('createRouter', () => { - // the calls from supertest should not be handled by msw so we only warn onUnhandledRequest - beforeAll(() => server.listen({ onUnhandledRequest: 'warn' })); - afterAll(() => server.close()); - afterEach(() => server.resetHandlers()); - const preparers: jest.Mocked = { register: jest.fn(), get: jest.fn(), @@ -95,53 +85,124 @@ describe('createRouter', () => { }); describe('GET /sync/:namespace/:kind/:name', () => { - it('should execute an update', async () => { - (shouldCheckForUpdate as jest.Mock).mockReturnValue(true); - MockedConfigReader.prototype.getString.mockReturnValue('local'); + describe('accept application/json', () => { + it('should execute synchronization', async () => { + MockDocsSynchronizer.prototype.doSync.mockImplementation( + async handler => handler().finish({ updated: true }), + ); - const entity = { - apiVersion: 'backstage.io/v1alpha1', - kind: 'Component', - metadata: { - uid: '0', - name: 'test', - namespace: 'default', - annotations: { - 'sda.se/release-notes-location': - 'github-releases:https://github.com/backstage/backstage', + await request(app).get('/sync/default/Component/test').send(); + + expect(MockDocsSynchronizer.prototype.doSync).toBeCalledTimes(1); + expect(MockDocsSynchronizer.prototype.doSync).toBeCalledWith( + expect.any(Function), + { + kind: 'Component', + name: 'test', + namespace: 'default', + token: undefined, }, - }, - }; - - server.use( - rest.get( - 'http://backstage.local/api/catalog/entities/by-name/Component/default/test', - (_req, res, ctx) => { - return res(ctx.json(entity)); - }, - ), - ); - - MockedDocsBuilder.prototype.build.mockImplementation(async () => { - // extract the logStream from the constructor call - const logStream = MockedDocsBuilder.mock.calls[0][0].logStream; - - logStream?.write('Some log'); - logStream?.write('Another log'); - - return true; + ); }); - publisher.hasDocsBeenGenerated.mockResolvedValue(true); + it('should return on updated', async () => { + MockDocsSynchronizer.prototype.doSync.mockImplementation( + async handler => { + const { log, finish } = handler(); - const response = await request(app) - .get('/sync/default/Component/test') - .send(); + log('Some log'); - expect(response.status).toBe(200); - expect(response.get('content-type')).toBe('text/event-stream'); - expect(response.text).toEqual( - `event: log + finish({ updated: true }); + }, + ); + + const response = await request(app) + .get('/sync/default/Component/test') + .send(); + + expect(response.status).toBe(201); + expect(response.get('content-type')).toMatch(/application\/json/); + expect(response.text).toEqual( + '{"message":"Docs updated or did not need updating"}', + ); + }); + + it('should return error', async () => { + MockDocsSynchronizer.prototype.doSync.mockImplementation( + async handler => { + const { log, error } = handler(); + + log('Some log'); + + error(new Error('Some Error')); + }, + ); + + const response = await request(app) + .get('/sync/default/Component/test') + .send(); + + expect(response.status).toBe(500); + expect(response.text).toMatch(/Some Error/); + }); + + it('should return not found', async () => { + MockDocsSynchronizer.prototype.doSync.mockRejectedValue( + new NotFoundError(), + ); + + const response = await request(app) + .get('/sync/default/Component/test') + .send(); + + expect(response.status).toBe(404); + }); + }); + + describe('accept text/event-stream', () => { + it('should execute synchronization', async () => { + MockDocsSynchronizer.prototype.doSync.mockImplementation( + async handler => handler().finish({ updated: true }), + ); + + await request(app) + .get('/sync/default/Component/test') + .set('accept', 'text/event-stream') + .send(); + + expect(MockDocsSynchronizer.prototype.doSync).toBeCalledTimes(1); + expect(MockDocsSynchronizer.prototype.doSync).toBeCalledWith( + expect.any(Function), + { + kind: 'Component', + name: 'test', + namespace: 'default', + token: undefined, + }, + ); + }); + + it('should return an event-stream', async () => { + MockDocsSynchronizer.prototype.doSync.mockImplementation( + async handler => { + const { log, finish } = handler(); + + log('Some log'); + log('Another log'); + + finish({ updated: true }); + }, + ); + + const response = await request(app) + .get('/sync/default/Component/test') + .set('accept', 'text/event-stream') + .send(); + + expect(response.status).toBe(200); + expect(response.get('content-type')).toBe('text/event-stream'); + expect(response.text).toEqual( + `event: log data: "Some log" event: log @@ -151,117 +212,170 @@ event: finish data: {"updated":true} `, - ); + ); + }); - expect(shouldCheckForUpdate).toBeCalledTimes(1); - expect(MockedConfigReader.prototype.getString).toBeCalledTimes(1); - expect(DocsBuilder.prototype.build).toBeCalledTimes(1); - }); + it('should return error', async () => { + MockDocsSynchronizer.prototype.doSync.mockImplementation( + async handler => { + const { log, error } = handler(); - it('should not check for an update too often', async () => { - (shouldCheckForUpdate as jest.Mock).mockReturnValue(false); + log('Some log'); - const entity = { - apiVersion: 'backstage.io/v1alpha1', - kind: 'Component', - metadata: { - uid: '0', - name: 'test', - namespace: 'default', - annotations: { - 'sda.se/release-notes-location': - 'github-releases:https://github.com/backstage/backstage', + error(new Error('Some Error')); }, - }, - }; + ); - server.use( - rest.get( - 'http://backstage.local/api/catalog/entities/by-name/Component/default/test', - (_req, res, ctx) => { - return res(ctx.json(entity)); - }, - ), - ); + const response = await request(app) + .get('/sync/default/Component/test') + .set('accept', 'text/event-stream') + .send(); - const response = await request(app) - .get('/sync/default/Component/test') - .send(); + expect(response.status).toBe(200); + expect(response.get('content-type')).toBe('text/event-stream'); + expect(response.text).toEqual( + `event: log +data: "Some log" - expect(response.status).toBe(200); - expect(response.get('content-type')).toBe('text/event-stream'); - expect(response.text).toEqual( - `event: finish -data: {"updated":false} +event: error +data: "Some Error" `, - ); + ); + }); - expect(shouldCheckForUpdate).toBeCalledTimes(1); - expect(MockedConfigReader.prototype.getString).toBeCalledTimes(0); - expect(DocsBuilder.prototype.build).toBeCalledTimes(0); - }); + it('should return not found', async () => { + MockDocsSynchronizer.prototype.doSync.mockRejectedValue( + new NotFoundError(), + ); - it('should not check for an update without local builder', async () => { - (shouldCheckForUpdate as jest.Mock).mockReturnValue(true); - MockedConfigReader.prototype.getString.mockReturnValue('external'); + const response = await request(app) + .get('/sync/default/Component/test') + .set('accept', 'text/event-stream') + .send(); - const entity = { - apiVersion: 'backstage.io/v1alpha1', - kind: 'Component', - metadata: { - uid: '0', - name: 'test', - namespace: 'default', - annotations: { - 'sda.se/release-notes-location': - 'github-releases:https://github.com/backstage/backstage', - }, - }, - }; - - server.use( - rest.get( - 'http://backstage.local/api/catalog/entities/by-name/Component/default/test', - (_req, res, ctx) => { - return res(ctx.json(entity)); - }, - ), - ); - - const response = await request(app) - .get('/sync/default/Component/test') - .send(); - - expect(response.status).toBe(200); - expect(response.get('content-type')).toBe('text/event-stream'); - expect(response.text).toEqual( - `event: finish -data: {"updated":false} - -`, - ); - - expect(shouldCheckForUpdate).toBeCalledTimes(1); - expect(MockedConfigReader.prototype.getString).toBeCalledTimes(1); - expect(DocsBuilder.prototype.build).toBeCalledTimes(0); - }); - - it('rejects when entity is not found', async () => { - server.use( - rest.get( - 'http://backstage.local/api/catalog/entities/by-name/Component/default/test', - (_req, res, ctx) => { - return res(ctx.status(404)); - }, - ), - ); - - const response = await request(app) - .get('/sync/default/Component/test') - .send(); - - expect(response.status).toBe(404); + expect(response.status).toBe(404); + }); }); }); }); + +describe('createEventStream', () => { + const res: jest.Mocked = { + writeHead: jest.fn(), + write: jest.fn(), + end: jest.fn(), + } as any; + + let handlers: DocsSynchronizerSyncOpts; + + beforeEach(() => { + handlers = createEventStream(res); + }); + afterEach(() => { + jest.resetAllMocks(); + }); + + it('should return correct event stream', async () => { + // called in beforeEach + + expect(res.writeHead).toBeCalledTimes(1); + expect(res.writeHead).toBeCalledWith(200, { + 'Cache-Control': 'no-cache', + Connection: 'keep-alive', + 'Content-Type': 'text/event-stream', + }); + }); + + it('should flush after write if defined', async () => { + res.flush = jest.fn(); + + handlers.log('A Message'); + + expect(res.write).toBeCalledTimes(1); + expect(res.write).toBeCalledWith(`event: log +data: "A Message" + +`); + expect(res.flush).toBeCalledTimes(1); + }); + + it('should write log', async () => { + handlers.log('A Message'); + + expect(res.write).toBeCalledTimes(1); + expect(res.write).toBeCalledWith(`event: log +data: "A Message" + +`); + expect(res.end).toBeCalledTimes(0); + }); + + it('should write error and end the connection', async () => { + handlers.error(new Error('Some Error')); + + expect(res.write).toBeCalledTimes(1); + expect(res.write).toBeCalledWith(`event: error +data: "Some Error" + +`); + expect(res.end).toBeCalledTimes(1); + }); + + it('should finish and end the connection', async () => { + handlers.finish({ updated: true }); + + expect(res.write).toBeCalledTimes(1); + expect(res.write).toBeCalledWith(`event: finish +data: {"updated":true} + +`); + + expect(res.end).toBeCalledTimes(1); + }); +}); + +describe('createHttpResponse', () => { + const res: jest.Mocked = { + status: jest.fn(), + json: jest.fn(), + } as any; + + let handlers: DocsSynchronizerSyncOpts; + + beforeEach(() => { + res.status.mockImplementation(() => res); + handlers = createHttpResponse(res); + }); + afterEach(() => { + jest.resetAllMocks(); + }); + + it('should return CREATED if updated', async () => { + handlers.finish({ updated: true }); + + expect(res.status).toBeCalledTimes(1); + expect(res.status).toBeCalledWith(201); + + expect(res.json).toBeCalledTimes(1); + expect(res.json).toBeCalledWith({ + message: 'Docs updated or did not need updating', + }); + }); + + it('should return NOT_MODIFIED if not updated', async () => { + expect(() => handlers.finish({ updated: false })).toThrowError( + NotModifiedError, + ); + }); + + it('should throw custom error', async () => { + expect(() => handlers.error(new Error('Some Error'))).toThrowError( + /Some Error/, + ); + }); + + it('should ignore logs', async () => { + expect(() => handlers.log('Some Message')).not.toThrow(); + }); +}); diff --git a/plugins/techdocs-backend/src/service/router.ts b/plugins/techdocs-backend/src/service/router.ts index 08c5301f20..8b189cff16 100644 --- a/plugins/techdocs-backend/src/service/router.ts +++ b/plugins/techdocs-backend/src/service/router.ts @@ -17,7 +17,7 @@ import { PluginEndpointDiscovery } from '@backstage/backend-common'; import { CatalogClient } from '@backstage/catalog-client'; import { Entity, stringifyEntityRef } from '@backstage/catalog-model'; import { Config } from '@backstage/config'; -import { NotFoundError } from '@backstage/errors'; +import { NotFoundError, NotModifiedError } from '@backstage/errors'; import { GeneratorBuilder, getLocationForEntity, @@ -28,9 +28,8 @@ import fetch from 'cross-fetch'; import express, { Response } from 'express'; import Router from 'express-promise-router'; import { Knex } from 'knex'; -import { PassThrough } from 'stream'; import { Logger } from 'winston'; -import { DocsBuilder, shouldCheckForUpdate } from '../DocsBuilder'; +import { DocsSynchronizer, DocsSynchronizerSyncOpts } from './DocsSynchronizer'; type RouterOptions = { preparers: PreparerBuilder; @@ -52,6 +51,14 @@ export async function createRouter({ }: RouterOptions): Promise { const router = Router(); const catalogClient = new CatalogClient({ discoveryApi: discovery }); + const docsSynchronizer = new DocsSynchronizer({ + preparers: preparers, + generators: generators, + publisher: publisher, + logger: logger, + config: config, + catalogClient: catalogClient, + }); router.get('/metadata/techdocs/:namespace/:kind/:name', async (req, res) => { const { kind, namespace, name } = req.params; @@ -117,80 +124,21 @@ export async function createRouter({ const { kind, namespace, name } = req.params; const token = getBearerToken(req.headers.authorization); - const entity = await catalogClient.getEntityByName( - { kind, namespace, name }, - { token }, + await docsSynchronizer.doSync( + () => { + if (req.header('accept') !== 'text/event-stream') { + return createHttpResponse(res); + } + + return createEventStream(res); + }, + { + kind, + namespace, + name, + token, + }, ); - - if (!entity?.metadata?.uid) { - throw new NotFoundError('Entity metadata UID missing'); - } - - // open the event-stream - const { log, error, finish } = createEventStream(res); - - // create an in-memory stream to forward logs to the event-stream - const logStream = new PassThrough(); - logStream.on('data', async data => { - log(data.toString().trim()); - }); - - // check if the last update check was too recent - if (!shouldCheckForUpdate(entity.metadata.uid)) { - finish({ updated: false }); - return; - } - - // 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') { - finish({ updated: false }); - return; - } - - const docsBuilder = new DocsBuilder({ - preparers, - generators, - publisher, - logger, - entity, - config, - logStream, - }); - - let foundDocs = false; - - const updated = await docsBuilder.build(); - - if (!updated) { - finish({ updated: false }); - return; - } - - // With a maximum of ~5 seconds wait, check if the files got published and if docs will be fetched - // on the user's page. If not, respond with a message asking them to check back later. - // The delay here is to make sure GCS/AWS/etc. registers newly uploaded files which is usually <1 second - for (let attempt = 0; attempt < 5; attempt++) { - if (await publisher.hasDocsBeenGenerated(entity)) { - foundDocs = true; - break; - } - await new Promise(r => setTimeout(r, 1000)); - } - if (!foundDocs) { - logger.error( - 'Published files are taking longer to show up in storage. Something went wrong.', - ); - error( - new NotFoundError( - 'Sorry! It took too long for the generated docs to show up in storage. Check back later.', - ), - ); - return; - } - - finish({ updated: true }); }); // Route middleware which serves files from the storage set in the publisher. @@ -210,13 +158,9 @@ function getBearerToken(header?: string): string | undefined { * @returns A tuple of callbacks to emit messages. A call to 'error' or 'finish' * will close the event-stream. */ -function createEventStream( +export function createEventStream( res: Response, -): { - log: (message: string) => void; - error: (e: Error) => void; - finish: (result: { updated: boolean }) => void; -} { +): DocsSynchronizerSyncOpts { // Mandatory headers and http status to keep connection open res.writeHead(200, { Connection: 'keep-alive', @@ -255,3 +199,30 @@ function createEventStream( }, }; } + +/** + * Create a HTTP response. This is used for the legacy non-event-stream implementation of the sync endpoint. + * + * @param res the response to write the event-stream to + * @returns A tuple of callbacks to emit messages. A call to 'error' or 'finish' + * will close the event-stream. + */ +export function createHttpResponse( + res: Response, +): DocsSynchronizerSyncOpts { + return { + log: () => {}, + error: e => { + throw e; + }, + finish: ({ updated }) => { + if (!updated) { + throw new NotModifiedError(); + } + + res + .status(201) + .json({ message: 'Docs updated or did not need updating' }); + }, + }; +}