Move the synchronization code into a dedicated class and still support non-eventstream responses

Signed-off-by: Dominik Henneke <dominik.henneke@sda-se.com>

.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.

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';

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<typeof DocsBuilder>;
+
+describe('DocsSynchronizer', () => {
+  const preparers: jest.Mocked<PreparerBuilder> = {
+    register: jest.fn(),
+    get: jest.fn(),
+  };
+  const generators: jest.Mocked<GeneratorBuilder> = {
+    register: jest.fn(),
+    get: jest.fn(),
+  };
+  const publisher: jest.Mocked<PublisherBase> = {
+    docsRouter: jest.fn(),
+    fetchTechDocsMetadata: jest.fn(),
+    getReadiness: jest.fn(),
+    hasDocsBeenGenerated: jest.fn(),
+    publish: jest.fn(),
+  };
+  const discovery: jest.Mocked<PluginEndpointDiscovery> = {
+    getBaseUrl: jest.fn(),
+    getExternalBaseUrl: jest.fn(),
+  };
+  const catalogClient: jest.Mocked<CatalogApi> = {
+    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<DocsSynchronizerSyncOpts> = {
+    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);
+    });
+  });
+});

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 });
+  }
+}

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<typeof DocsBuilder>;
-
-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<PreparerBuilder> = {
     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<Response> = {
+    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<Response> = {
+    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();
+  });
+});

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<express.Router> {
   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 <log, error, finish> callbacks to emit messages. A call to 'error' or 'finish'
  *          will close the event-stream.
  */
-function createEventStream(
+export function createEventStream(
   res: Response<any, any>,
-): {
-  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 <log, error, finish> callbacks to emit messages. A call to 'error' or 'finish'
+ *          will close the event-stream.
+ */
+export function createHttpResponse(
+  res: Response<any, any>,
+): 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' });
+    },
+  };
+}