From d5699813f79e9ca142b0c79f4af20528e8c70e35 Mon Sep 17 00:00:00 2001 From: Eric Peterson Date: Sat, 3 Jul 2021 13:53:58 +0200 Subject: [PATCH 01/27] Proposed architecture. Signed-off-by: Eric Peterson --- .../architecture-recommended.drawio.svg | 225 +++++++++++++----- 1 file changed, 168 insertions(+), 57 deletions(-) diff --git a/docs/assets/techdocs/architecture-recommended.drawio.svg b/docs/assets/techdocs/architecture-recommended.drawio.svg index e3af4b6b5f..12892cfad7 100644 --- a/docs/assets/techdocs/architecture-recommended.drawio.svg +++ b/docs/assets/techdocs/architecture-recommended.drawio.svg @@ -1,4 +1,4 @@ - + @@ -7,7 +7,7 @@ - + @@ -67,8 +67,8 @@ - - + + @@ -105,7 +105,7 @@ - + @@ -124,7 +124,7 @@ - + @@ -232,34 +232,17 @@
- TechDocs plugin + TechDocs Plugin
- TechDocs plugin + TechDocs Plugin - - - - -
-
-
- TechDocs Backend plugin -
-
-
- - - TechDocs Backend plu... - - - - + @@ -268,21 +251,23 @@
- Request TechDocs site + + Request TechDocs Site +
- Request TechDocs site + Request TechDocs Site - + -
+
Fetch files to render @@ -290,7 +275,7 @@
- + Fetch files to render @@ -302,15 +287,15 @@ - + - + -
+
Source code hosting @@ -323,28 +308,9 @@ - - - - - -
-
-
- Caching -
- (Optional) -
-
-
- - - Caching... - - - - - + + + @@ -379,10 +345,155 @@ + + + + +
+
+
+ Cache Store +
+ (Optional) +
+
+
+ + + Cache Store... + + + + + + + +
+
+
+ Read/Write +
+ Objects +
+
+
+ + + Read/Write... + + + + + + + + + + + +
+
+
+ + Invalidate Objects (Optional) + +
+
+
+ + + Invalidate Objects (Optional) + + + + + + + + + + +
+
+
+ + + Memcache + + +
+
+
+ + + Memcache + + + + + + + + +
+
+
+ + TechDocs Service + +
+
+
+ + + TechDocs Service + + + + + + + +
+
+
+ + Cache Middleware +
+ (Optional) + +
+
+
+ + + Cache Middleware... + + + + + + + + + +
+
+
+ TechDocs Backend Plugin +
+
+
+ + + TechDocs Backend Plugin + + + - + Viewer does not support full SVG 1.1 From 3e443e8e31913e6de83568d5df2be5517c902287 Mon Sep 17 00:00:00 2001 From: Eric Peterson Date: Sat, 3 Jul 2021 20:03:24 +0200 Subject: [PATCH 02/27] Express middleware for reading/writing data to cache. Signed-off-by: Eric Peterson --- .../src/cache/TechDocsCache.ts | 87 +++++++++++ .../src/cache/cacheMiddleware.ts | 139 ++++++++++++++++++ plugins/techdocs-backend/src/cache/index.ts | 17 +++ 3 files changed, 243 insertions(+) create mode 100644 plugins/techdocs-backend/src/cache/TechDocsCache.ts create mode 100644 plugins/techdocs-backend/src/cache/cacheMiddleware.ts create mode 100644 plugins/techdocs-backend/src/cache/index.ts diff --git a/plugins/techdocs-backend/src/cache/TechDocsCache.ts b/plugins/techdocs-backend/src/cache/TechDocsCache.ts new file mode 100644 index 0000000000..83497608cf --- /dev/null +++ b/plugins/techdocs-backend/src/cache/TechDocsCache.ts @@ -0,0 +1,87 @@ +/* + * 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 { CacheClient } from '@backstage/backend-common'; +import { Logger } from 'winston'; + +export class CacheInvalidationError extends Error { + public readonly rejections: PromiseRejectedResult[]; + + constructor(rejections: PromiseRejectedResult[]) { + super(); + this.rejections = rejections; + } +} + +export class TechDocsCache { + protected readonly cache: CacheClient; + protected readonly logger: Logger; + + constructor({ cache, logger }: { cache: CacheClient; logger: Logger }) { + this.cache = cache; + this.logger = logger; + } + + async get(path: string): Promise { + try { + // Promise.race ensures we don't hang the client for long if the cache is + // temporarily unreachable. + const response = (await Promise.race([ + this.cache.get(path), + new Promise(cancelAfter => setTimeout(cancelAfter, 1000)), + ])) as string | undefined; + + if (response !== undefined) { + this.logger.debug(`Cache hit: ${path}`); + return Buffer.from(response, 'base64'); + } + + this.logger.debug(`Cache miss: ${path}`); + return response; + } catch (e) { + this.logger.warn(`Error getting cache entry ${path}: ${e.message}`); + this.logger.debug(e.stack); + return undefined; + } + } + + async set(path: string, data: Buffer): Promise { + this.logger.debug(`Writing cache entry for ${path}`); + this.cache + .set(path, data.toString('base64')) + .catch(e => this.logger.error('write error', e)); + } + + async invalidate(path: string): Promise { + return this.cache.delete(path); + } + + async invalidateMultiple( + paths: string[], + ): Promise[]> { + const settled = await Promise.allSettled( + paths.map(path => this.cache.delete(path)), + ); + const rejected = settled.filter( + s => s.status === 'rejected', + ) as PromiseRejectedResult[]; + + if (rejected.length) { + throw new CacheInvalidationError(rejected); + } + + return settled; + } +} diff --git a/plugins/techdocs-backend/src/cache/cacheMiddleware.ts b/plugins/techdocs-backend/src/cache/cacheMiddleware.ts new file mode 100644 index 0000000000..2abbb24cf3 --- /dev/null +++ b/plugins/techdocs-backend/src/cache/cacheMiddleware.ts @@ -0,0 +1,139 @@ +/* + * 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 { Router, Request, json } from 'express'; +import router from 'express-promise-router'; +import { Logger } from 'winston'; +import { TechDocsCache } from '.'; +import { CacheInvalidationError } from './TechDocsCache'; + +type CacheClearRequestParams = { + objects: string[]; +}; + +type CacheClearRequest = Request< + any, + unknown, + CacheClearRequestParams, + unknown +>; + +type CacheMiddlewareOptions = { + cache: TechDocsCache; + logger: Logger; +}; + +type ErrorCallback = (err?: Error) => void; + +export const createCacheMiddleware = ({ + cache, + logger, +}: CacheMiddlewareOptions): Router => { + const cacheMiddleware = router(); + + // And endpoint for handling cache invalidation external to the Backstage + // Backend (e.g. from the TechDocs CLI). + cacheMiddleware.use(json()); + cacheMiddleware.post( + '/cache/invalidate', + async (req: CacheClearRequest, res) => { + if (req.body?.objects?.length) { + logger.debug( + `Clearing ${req.body.objects.length} cache entries: (eg: ${req.body.objects[0]})`, + ); + + try { + const invalidated = await cache.invalidateMultiple(req.body.objects); + logger.debug( + `Successfully invalidated ${invalidated.length} cache entries`, + ); + res.status(204).send(); + } catch (e) { + if (e instanceof CacheInvalidationError) { + const uniqueReasons = [ + ...new Set(e.rejections.map(r => r.reason.message)), + ].join(', '); + logger.warn( + `Problem invalidating ${e.rejections.length} entries: ${uniqueReasons}`, + ); + } + res.status(500).send(); + } + } else { + res.status(400).send(); + } + }, + ); + + // Middleware that, through socket monkey patching, captures responses as + // they're sent over /static/docs/* and caches them. Subsequent requests are + // loaded from cache. Cache key is the object's path (after `/static/docs/`). + cacheMiddleware.use(async (req, res, next) => { + const socket = res.socket; + const isCacheable = req.path.includes('/static/docs/'); + + // Continue early if this is non-cacheable, or there's no socket. + if (!isCacheable || !socket) { + next(); + return; + } + + // Make concrete references to these things. + const reqPath = decodeURI(req.path.match(/\/static\/docs\/(.*)$/)![1]); + const realEnd = socket.end.bind(socket); + const realWrite = socket.write.bind(socket); + let writeToCache = true; + const chunks: Buffer[] = []; + + // Monkey-patch the response's socket to keep track of chunks as they are + // written over the wire. + socket.write = ( + data, + encoding?: BufferEncoding | ErrorCallback, + callback?: ErrorCallback, + ) => { + chunks.push(Buffer.from(data)); + if (typeof encoding === 'function') { + return realWrite(data, encoding); + } + return realWrite(data, encoding, callback); + }; + + // When a socket is closed, if there were no errors and the data written + // over the socket should be cached, cache it as a base64-encoded string! + socket.on('close', hadError => { + if (writeToCache && !hadError) { + cache.set(reqPath, Buffer.concat(chunks)); + } + }); + + // Attempt to retrieve data from the cache. + const cached = await cache.get(reqPath); + + // If there is a cache hit, write it out on the socket, ensure we don't re- + // cache the data, and prevent going back to canonical storage by never + // calling next(). + if (cached) { + writeToCache = false; + realEnd(cached); + return; + } + + // No data retrieved from cache: allow retrieval from canonical storage. + next(); + }); + + return cacheMiddleware; +}; diff --git a/plugins/techdocs-backend/src/cache/index.ts b/plugins/techdocs-backend/src/cache/index.ts new file mode 100644 index 0000000000..751d8e8625 --- /dev/null +++ b/plugins/techdocs-backend/src/cache/index.ts @@ -0,0 +1,17 @@ +/* + * 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. + */ +export { createCacheMiddleware } from './cacheMiddleware'; +export { TechDocsCache } from './TechDocsCache'; From 0a44eb7d5297bd61a7f224505f4711b60e7a87a2 Mon Sep 17 00:00:00 2001 From: Eric Peterson Date: Sat, 24 Jul 2021 17:00:22 +0200 Subject: [PATCH 03/27] Initial implementation of TechDocs cache in backend plugin Signed-off-by: Eric Peterson --- plugins/techdocs-backend/config.d.ts | 17 ++++++++++++++++ .../src/DocsBuilder/builder.ts | 12 ++++++++++- .../techdocs-backend/src/service/router.ts | 20 ++++++++++++++++++- 3 files changed, 47 insertions(+), 2 deletions(-) diff --git a/plugins/techdocs-backend/config.d.ts b/plugins/techdocs-backend/config.d.ts index 9e563137bd..1767120a35 100644 --- a/plugins/techdocs-backend/config.d.ts +++ b/plugins/techdocs-backend/config.d.ts @@ -226,6 +226,23 @@ export interface Config { }; }; + /** + * @example http://localhost:7007/api/techdocs + * Techdocs cache information + */ + cache?: { + /** + * The cache time-to-live for TechDocs sites (in milliseconds). Set this + * to a non-zero value to cache TechDocs sites and assets as they are + * read from storage. + * + * Note: you must also configure `backend.cache` appropriately as well, + * and to pass a PluginCacheManager instance to TechDocs Backend's + * createRouter method in your backend. + */ + ttl: number; + }; + /** * @example http://localhost:7007/api/techdocs * @visibility frontend diff --git a/plugins/techdocs-backend/src/DocsBuilder/builder.ts b/plugins/techdocs-backend/src/DocsBuilder/builder.ts index 821b3478a0..4843acf2c2 100644 --- a/plugins/techdocs-backend/src/DocsBuilder/builder.ts +++ b/plugins/techdocs-backend/src/DocsBuilder/builder.ts @@ -36,6 +36,7 @@ import path from 'path'; import { Writable } from 'stream'; import { Logger } from 'winston'; import { BuildMetadataStorage } from './BuildMetadataStorage'; +import { TechDocsCache } from '../cache'; type DocsBuilderArguments = { preparers: PreparerBuilder; @@ -46,6 +47,7 @@ type DocsBuilderArguments = { config: Config; scmIntegrations: ScmIntegrationRegistry; logStream?: Writable; + cache?: TechDocsCache; }; export class DocsBuilder { @@ -57,6 +59,7 @@ export class DocsBuilder { private config: Config; private scmIntegrations: ScmIntegrationRegistry; private logStream: Writable | undefined; + private cache?: TechDocsCache; constructor({ preparers, @@ -67,6 +70,7 @@ export class DocsBuilder { config, scmIntegrations, logStream, + cache, }: DocsBuilderArguments) { this.preparer = preparers.get(entity); this.generator = generators.get(entity); @@ -76,6 +80,7 @@ export class DocsBuilder { this.config = config; this.scmIntegrations = scmIntegrations; this.logStream = logStream; + this.cache = cache; } /** @@ -210,11 +215,16 @@ export class DocsBuilder { )}`, ); - await this.publisher.publish({ + const published = await this.publisher.publish({ entity: this.entity, directory: outputDir, }); + // Invalidate the cache for any published objects. + if (this.cache && published?.objects?.length) { + await this.cache.invalidateMultiple(published.objects); + } + try { // Not a blocker hence no need to await this. fs.remove(outputDir); diff --git a/plugins/techdocs-backend/src/service/router.ts b/plugins/techdocs-backend/src/service/router.ts index d53a2faed4..07c6887ef7 100644 --- a/plugins/techdocs-backend/src/service/router.ts +++ b/plugins/techdocs-backend/src/service/router.ts @@ -13,7 +13,10 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -import { PluginEndpointDiscovery } from '@backstage/backend-common'; +import { + PluginEndpointDiscovery, + PluginCacheManager, +} from '@backstage/backend-common'; import { CatalogClient } from '@backstage/catalog-client'; import { Entity, stringifyEntityRef } from '@backstage/catalog-model'; import { Config } from '@backstage/config'; @@ -31,6 +34,7 @@ import { Knex } from 'knex'; import { Logger } from 'winston'; import { ScmIntegrations } from '@backstage/integration'; import { DocsSynchronizer, DocsSynchronizerSyncOpts } from './DocsSynchronizer'; +import { createCacheMiddleware, TechDocsCache } from '../cache'; /** * All of the required dependencies for running TechDocs in the "out-of-the-box" @@ -44,6 +48,7 @@ type OutOfTheBoxDeploymentOptions = { discovery: PluginEndpointDiscovery; database?: Knex; // TODO: Make database required when we're implementing database stuff. config: Config; + cache?: PluginCacheManager; }; /** @@ -88,6 +93,14 @@ export async function createRouter( scmIntegrations, }); + // Set up a cache client if configured. + let cache: TechDocsCache | undefined; + const defaultTtl = config.getOptionalNumber('techdocs.cache.ttl'); + if (isOutOfTheBoxOption(options) && options.cache && defaultTtl) { + const cacheClient = options.cache.getClient({ defaultTtl }); + cache = new TechDocsCache({ cache: cacheClient, logger }); + } + router.get('/metadata/techdocs/:namespace/:kind/:name', async (req, res) => { const { kind, namespace, name } = req.params; const entityName = { kind, namespace, name }; @@ -199,6 +212,11 @@ export async function createRouter( ); }); + // If a cache manager was provided, attach the cache middleware. + if (cache) { + router.use(createCacheMiddleware({ logger, cache })); + } + // Route middleware which serves files from the storage set in the publisher. router.use('/static/docs', publisher.docsRouter()); From 8b438c77176d2df962a85abbca1f98a096d02092 Mon Sep 17 00:00:00 2001 From: Eric Peterson Date: Fri, 12 Nov 2021 10:43:05 +0100 Subject: [PATCH 04/27] Update all Publishers so that they resolve a list of objects on successful publish. Signed-off-by: Eric Peterson --- .../src/stages/publish/awsS3.test.ts | 8 +++++- .../src/stages/publish/awsS3.ts | 9 ++++++- .../stages/publish/azureBlobStorage.test.ts | 8 +++++- .../src/stages/publish/azureBlobStorage.ts | 23 +++++++++++------ .../src/stages/publish/googleStorage.test.ts | 8 +++++- .../src/stages/publish/googleStorage.ts | 25 ++++++++++++------- .../src/stages/publish/local.ts | 25 +++++++++++-------- .../src/stages/publish/openStackSwift.test.ts | 8 +++++- .../src/stages/publish/openStackSwift.ts | 11 ++++++-- .../src/stages/publish/types.ts | 6 +++++ 10 files changed, 97 insertions(+), 34 deletions(-) diff --git a/packages/techdocs-common/src/stages/publish/awsS3.test.ts b/packages/techdocs-common/src/stages/publish/awsS3.test.ts index 1f49668d35..abae5fda44 100644 --- a/packages/techdocs-common/src/stages/publish/awsS3.test.ts +++ b/packages/techdocs-common/src/stages/publish/awsS3.test.ts @@ -171,7 +171,13 @@ describe('AwsS3Publish', () => { bucketRootPath: 'backstage-data/techdocs', legacyUseCaseSensitiveTripletPaths: true, }); - expect(await publisher.publish({ entity, directory })).toBeUndefined(); + expect(await publisher.publish({ entity, directory })).toMatchObject({ + objects: expect.arrayContaining([ + 'test-namespace/TestKind/test-component-name/404.html', + `test-namespace/TestKind/test-component-name/index.html`, + `test-namespace/TestKind/test-component-name/assets/main.css`, + ]), + }); }); it('should publish a directory when sse is specified', async () => { diff --git a/packages/techdocs-common/src/stages/publish/awsS3.ts b/packages/techdocs-common/src/stages/publish/awsS3.ts index abfa8ddd24..ed76edc0cb 100644 --- a/packages/techdocs-common/src/stages/publish/awsS3.ts +++ b/packages/techdocs-common/src/stages/publish/awsS3.ts @@ -39,6 +39,7 @@ import { import { PublisherBase, PublishRequest, + PublishResponse, ReadinessResponse, TechDocsMetadata, } from './types'; @@ -214,7 +215,11 @@ export class AwsS3Publish implements PublisherBase { * Upload all the files from the generated `directory` to the S3 bucket. * Directory structure used in the bucket is - entityNamespace/entityKind/entityName/index.html */ - async publish({ entity, directory }: PublishRequest): Promise { + async publish({ + entity, + directory, + }: PublishRequest): Promise { + const objects: string[] = []; const useLegacyPathCasing = this.legacyPathCasing; const bucketRootPath = this.bucketRootPath; const sse = this.sse; @@ -263,6 +268,7 @@ export class AwsS3Publish implements PublisherBase { ...(sse && { ServerSideEncryption: sse }), } as aws.S3.PutObjectRequest; + objects.push(params.Key); return this.storageClient.upload(params).promise(); }, absoluteFilesToUpload, @@ -311,6 +317,7 @@ export class AwsS3Publish implements PublisherBase { const errorMessage = `Unable to delete file(s) from AWS S3. ${error}`; this.logger.error(errorMessage); } + return { objects }; } async fetchTechDocsMetadata( diff --git a/packages/techdocs-common/src/stages/publish/azureBlobStorage.test.ts b/packages/techdocs-common/src/stages/publish/azureBlobStorage.test.ts index a2503bd7b2..0e261f7f86 100644 --- a/packages/techdocs-common/src/stages/publish/azureBlobStorage.test.ts +++ b/packages/techdocs-common/src/stages/publish/azureBlobStorage.test.ts @@ -161,7 +161,13 @@ describe('AzureBlobStoragePublish', () => { const publisher = createPublisherFromConfig({ legacyUseCaseSensitiveTripletPaths: true, }); - expect(await publisher.publish({ entity, directory })).toBeUndefined(); + expect(await publisher.publish({ entity, directory })).toMatchObject({ + objects: expect.arrayContaining([ + 'test-namespace/TestKind/test-component-name/404.html', + `test-namespace/TestKind/test-component-name/index.html`, + `test-namespace/TestKind/test-component-name/assets/main.css`, + ]), + }); }); it('should fail to publish a directory', async () => { diff --git a/packages/techdocs-common/src/stages/publish/azureBlobStorage.ts b/packages/techdocs-common/src/stages/publish/azureBlobStorage.ts index 7a58d92095..b082079be2 100644 --- a/packages/techdocs-common/src/stages/publish/azureBlobStorage.ts +++ b/packages/techdocs-common/src/stages/publish/azureBlobStorage.ts @@ -39,6 +39,7 @@ import { import { PublisherBase, PublishRequest, + PublishResponse, ReadinessResponse, TechDocsMetadata, } from './types'; @@ -156,7 +157,11 @@ export class AzureBlobStoragePublish implements PublisherBase { * Upload all the files from the generated `directory` to the Azure Blob Storage container. * Directory structure used in the container is - entityNamespace/entityKind/entityName/index.html */ - async publish({ entity, directory }: PublishRequest): Promise { + async publish({ + entity, + directory, + }: PublishRequest): Promise { + const objects: string[] = []; const useLegacyPathCasing = this.legacyPathCasing; // First, try to retrieve a list of all individual files currently existing @@ -194,14 +199,14 @@ export class AzureBlobStoragePublish implements PublisherBase { const relativeFilePath = path.normalize( path.relative(directory, absoluteFilePath), ); + const remotePath = getCloudPathForLocalPath( + entity, + relativeFilePath, + useLegacyPathCasing, + ); + objects.push(remotePath); const response = await container - .getBlockBlobClient( - getCloudPathForLocalPath( - entity, - relativeFilePath, - useLegacyPathCasing, - ), - ) + .getBlockBlobClient(remotePath) .uploadFile(absoluteFilePath); if (response._response.status >= 400) { @@ -264,6 +269,8 @@ export class AzureBlobStoragePublish implements PublisherBase { const errorMessage = `Unable to delete file(s) from Azure. ${error}`; this.logger.error(errorMessage); } + + return { objects }; } private download(containerName: string, blobPath: string): Promise { diff --git a/packages/techdocs-common/src/stages/publish/googleStorage.test.ts b/packages/techdocs-common/src/stages/publish/googleStorage.test.ts index a52fd0f0bd..ea97d81269 100644 --- a/packages/techdocs-common/src/stages/publish/googleStorage.test.ts +++ b/packages/techdocs-common/src/stages/publish/googleStorage.test.ts @@ -164,7 +164,13 @@ describe('GoogleGCSPublish', () => { bucketRootPath: 'backstage-data/techdocs', legacyUseCaseSensitiveTripletPaths: true, }); - expect(await publisher.publish({ entity, directory })).toBeUndefined(); + expect(await publisher.publish({ entity, directory })).toMatchObject({ + objects: expect.arrayContaining([ + 'test-namespace/TestKind/test-component-name/404.html', + `test-namespace/TestKind/test-component-name/index.html`, + `test-namespace/TestKind/test-component-name/assets/main.css`, + ]), + }); }); it('should fail to publish a directory', async () => { diff --git a/packages/techdocs-common/src/stages/publish/googleStorage.ts b/packages/techdocs-common/src/stages/publish/googleStorage.ts index f072412c20..ba70c36e27 100644 --- a/packages/techdocs-common/src/stages/publish/googleStorage.ts +++ b/packages/techdocs-common/src/stages/publish/googleStorage.ts @@ -36,6 +36,7 @@ import { MigrateWriteStream } from './migrations'; import { PublisherBase, PublishRequest, + PublishResponse, ReadinessResponse, TechDocsMetadata, } from './types'; @@ -145,7 +146,11 @@ export class GoogleGCSPublish implements PublisherBase { * Upload all the files from the generated `directory` to the GCS bucket. * Directory structure used in the bucket is - entityNamespace/entityKind/entityName/index.html */ - async publish({ entity, directory }: PublishRequest): Promise { + async publish({ + entity, + directory, + }: PublishRequest): Promise { + const objects: string[] = []; const useLegacyPathCasing = this.legacyPathCasing; const bucket = this.storageClient.bucket(this.bucketName); const bucketRootPath = this.bucketRootPath; @@ -178,14 +183,14 @@ export class GoogleGCSPublish implements PublisherBase { await bulkStorageOperation( async absoluteFilePath => { const relativeFilePath = path.relative(directory, absoluteFilePath); - return await bucket.upload(absoluteFilePath, { - destination: getCloudPathForLocalPath( - entity, - relativeFilePath, - useLegacyPathCasing, - bucketRootPath, - ), - }); + const destination = getCloudPathForLocalPath( + entity, + relativeFilePath, + useLegacyPathCasing, + bucketRootPath, + ); + objects.push(destination); + return await bucket.upload(absoluteFilePath, { destination }); }, absoluteFilesToUpload, { concurrencyLimit: 10 }, @@ -228,6 +233,8 @@ export class GoogleGCSPublish implements PublisherBase { const errorMessage = `Unable to delete file(s) from Google Cloud Storage. ${error}`; this.logger.error(errorMessage); } + + return { objects }; } fetchTechDocsMetadata(entityName: EntityName): Promise { diff --git a/packages/techdocs-common/src/stages/publish/local.ts b/packages/techdocs-common/src/stages/publish/local.ts index 70b4eb3ff2..67a4a22914 100644 --- a/packages/techdocs-common/src/stages/publish/local.ts +++ b/packages/techdocs-common/src/stages/publish/local.ts @@ -113,7 +113,7 @@ export class LocalPublish implements PublisherBase { } return new Promise((resolve, reject) => { - fs.copy(directory, publishDir, err => { + fs.copy(directory, publishDir, async err => { if (err) { this.logger.debug( `Failed to copy docs from ${directory} to ${publishDir}`, @@ -121,16 +121,21 @@ export class LocalPublish implements PublisherBase { reject(err); } this.logger.info(`Published site stored at ${publishDir}`); - this.discovery - .getBaseUrl('techdocs') - .then(techdocsApiUrl => { - resolve({ - remoteUrl: `${techdocsApiUrl}/static/docs/${entity.metadata.name}`, - }); - }) - .catch(reason => { - reject(reason); + + try { + const techdocsApiUrl = await this.discovery.getBaseUrl('techdocs'); + const objects = (await getFileTreeRecursively(publishDir)).map( + abs => { + return abs.split(`${staticDocsDir}/`)[1]; + }, + ); + resolve({ + remoteUrl: `${techdocsApiUrl}/static/docs/${entity.metadata.name}`, + objects, }); + } catch (reason) { + reject(reason); + } }); }); } diff --git a/packages/techdocs-common/src/stages/publish/openStackSwift.test.ts b/packages/techdocs-common/src/stages/publish/openStackSwift.test.ts index c2236fb880..b8ad43aac3 100644 --- a/packages/techdocs-common/src/stages/publish/openStackSwift.test.ts +++ b/packages/techdocs-common/src/stages/publish/openStackSwift.test.ts @@ -170,7 +170,13 @@ describe('OpenStackSwiftPublish', () => { entity, directory: entityRootDir, }), - ).toBeUndefined(); + ).toMatchObject({ + objects: expect.arrayContaining([ + 'test-namespace/TestKind/test-component-name/404.html', + `test-namespace/TestKind/test-component-name/index.html`, + `test-namespace/TestKind/test-component-name/assets/main.css`, + ]), + }); }); it('should fail to publish a directory', async () => { diff --git a/packages/techdocs-common/src/stages/publish/openStackSwift.ts b/packages/techdocs-common/src/stages/publish/openStackSwift.ts index 7b623933e3..62b40f9a76 100644 --- a/packages/techdocs-common/src/stages/publish/openStackSwift.ts +++ b/packages/techdocs-common/src/stages/publish/openStackSwift.ts @@ -32,6 +32,7 @@ import { import { PublisherBase, PublishRequest, + PublishResponse, ReadinessResponse, TechDocsMetadata, } from './types'; @@ -139,8 +140,13 @@ export class OpenStackSwiftPublish implements PublisherBase { * Upload all the files from the generated `directory` to the OpenStack Swift container. * Directory structure used in the bucket is - entityNamespace/entityKind/entityName/index.html */ - async publish({ entity, directory }: PublishRequest): Promise { + async publish({ + entity, + directory, + }: PublishRequest): Promise { try { + const objects: string[] = []; + // Note: OpenStack Swift manages creation of parent directories if they do not exist. // So collecting path of only the files is good enough. const allFilesToUpload = await getFileTreeRecursively(directory); @@ -161,6 +167,7 @@ export class OpenStackSwiftPublish implements PublisherBase { // The / delimiter is intentional since it represents the cloud storage and not the local file system. const entityRootDir = `${entity.metadata.namespace}/${entity.kind}/${entity.metadata.name}`; const destination = `${entityRootDir}/${relativeFilePathPosix}`; // Swift container file relative path + objects.push(destination); // Rate limit the concurrent execution of file uploads to batches of 10 (per publish) const uploadFile = limiter(async () => { @@ -178,7 +185,7 @@ export class OpenStackSwiftPublish implements PublisherBase { this.logger.info( `Successfully uploaded all the generated files for Entity ${entity.metadata.name}. Total number of files: ${allFilesToUpload.length}`, ); - return; + return { objects }; } catch (e) { const errorMessage = `Unable to upload file(s) to OpenStack Swift. ${e}`; this.logger.error(errorMessage); diff --git a/packages/techdocs-common/src/stages/publish/types.ts b/packages/techdocs-common/src/stages/publish/types.ts index 229c853427..f68cb9cc8f 100644 --- a/packages/techdocs-common/src/stages/publish/types.ts +++ b/packages/techdocs-common/src/stages/publish/types.ts @@ -35,6 +35,12 @@ export type PublishRequest = { /* `remoteUrl` is the URL which serves files from the local publisher's static directory. */ export type PublishResponse = { remoteUrl?: string; + /** + * The list of objects (specifically their paths) that were published. + * Objects should not have a preceding slash, and should match how one would + * load the object over the `/static/docs/` TechDocs Backend Plugin endpoint. + */ + objects?: string[]; } | void; /** From 5fe0c2ce7879b961bd1fe2209f4603b2c8b23b87 Mon Sep 17 00:00:00 2001 From: Eric Peterson Date: Sat, 3 Jul 2021 20:07:13 +0200 Subject: [PATCH 05/27] Wire up example backend to use in-memory cache. Signed-off-by: Eric Peterson --- app-config.yaml | 3 ++- packages/backend/src/plugins/techdocs.ts | 2 ++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/app-config.yaml b/app-config.yaml index 913a72dbab..881e97aa5f 100644 --- a/app-config.yaml +++ b/app-config.yaml @@ -112,7 +112,8 @@ techdocs: # pullImage: true # or false to disable automatic pulling of image (e.g. if custom docker login is required) publisher: type: 'local' # Alternatives - 'googleGcs' or 'awsS3' or 'azureBlobStorage' or 'openStackSwift'. Read documentation for using alternatives. - + cache: + ttl: 60000 # 1 minute cache for demonstration purposes. You may wish to set this higher in production. sentry: organization: my-company diff --git a/packages/backend/src/plugins/techdocs.ts b/packages/backend/src/plugins/techdocs.ts index eb1e0502db..c32bbccbb0 100644 --- a/packages/backend/src/plugins/techdocs.ts +++ b/packages/backend/src/plugins/techdocs.ts @@ -29,6 +29,7 @@ export default async function createPlugin({ config, discovery, reader, + cache, }: PluginEnvironment): Promise { // Preparers are responsible for fetching source files for documentation. const preparers = await Preparers.fromConfig(config, { @@ -64,5 +65,6 @@ export default async function createPlugin({ logger, config, discovery, + cache, }); } From 790f02c898df9e053a302097d0992f58cb96d05a Mon Sep 17 00:00:00 2001 From: Eric Peterson Date: Tue, 6 Jul 2021 10:14:00 +0200 Subject: [PATCH 06/27] Improve types/documentation. Signed-off-by: Eric Peterson --- packages/techdocs-common/src/stages/publish/types.ts | 12 +++++++++--- plugins/techdocs-backend/src/DocsBuilder/builder.ts | 2 +- .../techdocs-backend/src/cache/cacheMiddleware.ts | 2 +- 3 files changed, 11 insertions(+), 5 deletions(-) diff --git a/packages/techdocs-common/src/stages/publish/types.ts b/packages/techdocs-common/src/stages/publish/types.ts index f68cb9cc8f..ab497067b3 100644 --- a/packages/techdocs-common/src/stages/publish/types.ts +++ b/packages/techdocs-common/src/stages/publish/types.ts @@ -32,13 +32,19 @@ export type PublishRequest = { directory: string; }; -/* `remoteUrl` is the URL which serves files from the local publisher's static directory. */ +/** + * Response containing metadata about where files were published and what may + * have been published or updated. + */ export type PublishResponse = { + /** + * The URL which serves files from the local publisher's static directory. + */ remoteUrl?: string; /** * The list of objects (specifically their paths) that were published. - * Objects should not have a preceding slash, and should match how one would - * load the object over the `/static/docs/` TechDocs Backend Plugin endpoint. + * Objects do not have a preceding slash, and match how one would load the + * object over the `/static/docs/*` TechDocs Backend Plugin endpoint. */ objects?: string[]; } | void; diff --git a/plugins/techdocs-backend/src/DocsBuilder/builder.ts b/plugins/techdocs-backend/src/DocsBuilder/builder.ts index 4843acf2c2..1d725bd69e 100644 --- a/plugins/techdocs-backend/src/DocsBuilder/builder.ts +++ b/plugins/techdocs-backend/src/DocsBuilder/builder.ts @@ -221,7 +221,7 @@ export class DocsBuilder { }); // Invalidate the cache for any published objects. - if (this.cache && published?.objects?.length) { + if (this.cache && published && published?.objects?.length) { await this.cache.invalidateMultiple(published.objects); } diff --git a/plugins/techdocs-backend/src/cache/cacheMiddleware.ts b/plugins/techdocs-backend/src/cache/cacheMiddleware.ts index 2abbb24cf3..2650b39296 100644 --- a/plugins/techdocs-backend/src/cache/cacheMiddleware.ts +++ b/plugins/techdocs-backend/src/cache/cacheMiddleware.ts @@ -100,7 +100,7 @@ export const createCacheMiddleware = ({ // Monkey-patch the response's socket to keep track of chunks as they are // written over the wire. socket.write = ( - data, + data: string | Uint8Array, encoding?: BufferEncoding | ErrorCallback, callback?: ErrorCallback, ) => { From 0a5278b92afcad9cb009c5e8e5f5760f5d791f2a Mon Sep 17 00:00:00 2001 From: Eric Peterson Date: Fri, 12 Nov 2021 10:46:53 +0100 Subject: [PATCH 07/27] Update Backstage.io docs with config/recs Signed-off-by: Eric Peterson --- .../architecture-recommended.drawio.svg | 20 +++++++++---------- docs/features/techdocs/architecture.md | 14 ++++++------- docs/features/techdocs/configuration.md | 9 +++++++++ 3 files changed, 26 insertions(+), 17 deletions(-) diff --git a/docs/assets/techdocs/architecture-recommended.drawio.svg b/docs/assets/techdocs/architecture-recommended.drawio.svg index 12892cfad7..8ddc9dc35a 100644 --- a/docs/assets/techdocs/architecture-recommended.drawio.svg +++ b/docs/assets/techdocs/architecture-recommended.drawio.svg @@ -1,4 +1,4 @@ - + @@ -383,15 +383,15 @@ - - - - - + + + + + -
+
@@ -401,7 +401,7 @@
- + Invalidate Objects (Optional) @@ -471,8 +471,8 @@ - - + + diff --git a/docs/features/techdocs/architecture.md b/docs/features/techdocs/architecture.md index 8d25b4047f..23af83d09b 100644 --- a/docs/features/techdocs/architecture.md +++ b/docs/features/techdocs/architecture.md @@ -40,7 +40,7 @@ storage system (e.g. AWS S3, GCS or Azure Blob Storage). Read more in ## Recommended deployment -This is how we recommend deploying TechDocs in production environment. +This is how we recommend deploying TechDocs in a production environment. TechDocs Architecture diagram @@ -58,12 +58,12 @@ Similar to how it is done in the Basic setup, the TechDocs Reader requests your configured storage solution for the necessary files and returns them to TechDocs Reader. -Note about caching: We have noticed internally that some storage providers can -be quite slow, which is why we are recommending a cache that sits between the -TechDocs Reader and the Storage. - -_Feel free to suggest better ideas to us in #docs-like-code channel in Discord -or via a GitHub issue._ +Depending on your chosen cloud storage provider and its real-world proximity to +your backend server, there may be a comparably high amount of latency when +loading TechDocs sites using this deployment approach. If you encounter this, +you can optionally configure the `techdocs-backend` to cache responses in a +cache store +[supported by Backstage](../../overview/architecture-overview.md#cache). ### Security consideration diff --git a/docs/features/techdocs/configuration.md b/docs/features/techdocs/configuration.md index 4148749a62..3234cdc034 100644 --- a/docs/features/techdocs/configuration.md +++ b/docs/features/techdocs/configuration.md @@ -135,6 +135,15 @@ techdocs: # the old, case-sensitive entity triplet behavior. legacyUseCaseSensitiveTripletPaths: false + # techdocs.cache is optional, and is only recommended when you've configured + # an external techdocs.publisher.type above. Also requires backend.cache to + # be configured with a valid cache store. + cache: + # Represents the number of milliseconds a statically built asset should + # stay cached. Cache invalidation is handled automatically if you publish + # to storage using the techdocs-cli, allowing long TTLs (e.g. 1 month/year) + ttl: 3600000 + # (Optional and Legacy) TechDocs makes API calls to techdocs-backend using this URL. e.g. get docs of an entity, get metadata, etc. # You don't have to specify this anymore. From 12ce46b229ad440eac9c73647b06816d1536bc5a Mon Sep 17 00:00:00 2001 From: Eric Peterson Date: Sat, 24 Jul 2021 17:04:24 +0200 Subject: [PATCH 08/27] Update TechDocs Backend API Report Signed-off-by: Eric Peterson --- plugins/techdocs-backend/api-report.md | 1 + 1 file changed, 1 insertion(+) diff --git a/plugins/techdocs-backend/api-report.md b/plugins/techdocs-backend/api-report.md index f9be757e6a..9736555687 100644 --- a/plugins/techdocs-backend/api-report.md +++ b/plugins/techdocs-backend/api-report.md @@ -10,6 +10,7 @@ import express from 'express'; import { GeneratorBuilder } from '@backstage/techdocs-common'; import { Knex } from 'knex'; import { Logger as Logger_2 } from 'winston'; +import { PluginCacheManager } from '@backstage/backend-common'; import { PluginEndpointDiscovery } from '@backstage/backend-common'; import { PreparerBuilder } from '@backstage/techdocs-common'; import { PublisherBase } from '@backstage/techdocs-common'; From d48aa5ca1c1669f8764ca45683e4b2e72505aa78 Mon Sep 17 00:00:00 2001 From: Eric Peterson Date: Tue, 6 Jul 2021 11:34:04 +0200 Subject: [PATCH 09/27] Update create-app so that cache manager is passed through to router. Signed-off-by: Eric Peterson --- .../default-app/packages/backend/src/plugins/techdocs.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/create-app/templates/default-app/packages/backend/src/plugins/techdocs.ts b/packages/create-app/templates/default-app/packages/backend/src/plugins/techdocs.ts index 906d86d4a2..054c64db65 100644 --- a/packages/create-app/templates/default-app/packages/backend/src/plugins/techdocs.ts +++ b/packages/create-app/templates/default-app/packages/backend/src/plugins/techdocs.ts @@ -14,6 +14,7 @@ export default async function createPlugin({ config, discovery, reader, + cache, }: PluginEnvironment): Promise { // Preparers are responsible for fetching source files for documentation. const preparers = await Preparers.fromConfig(config, { @@ -49,5 +50,6 @@ export default async function createPlugin({ logger, config, discovery, + cache, }); } From 1bada775a9e15f6407d3bf0afcd779b9fc429dea Mon Sep 17 00:00:00 2001 From: Eric Peterson Date: Tue, 6 Jul 2021 13:13:46 +0200 Subject: [PATCH 10/27] Changesets for affected packages. Signed-off-by: Eric Peterson --- .changeset/techdocs-satisfied-you-blinked.md | 7 ++++ .changeset/the-renegade-feeling.md | 42 ++++++++++++++++++++ 2 files changed, 49 insertions(+) create mode 100644 .changeset/techdocs-satisfied-you-blinked.md create mode 100644 .changeset/the-renegade-feeling.md diff --git a/.changeset/techdocs-satisfied-you-blinked.md b/.changeset/techdocs-satisfied-you-blinked.md new file mode 100644 index 0000000000..4cbc9d4f3e --- /dev/null +++ b/.changeset/techdocs-satisfied-you-blinked.md @@ -0,0 +1,7 @@ +--- +'@backstage/techdocs-common': minor +'@backstage/plugin-techdocs-backend': minor +--- + +Added the ability for the TechDocs Backend to (optionally) leverage a cache +store to improve performance when reading files from a cloud storage provider. diff --git a/.changeset/the-renegade-feeling.md b/.changeset/the-renegade-feeling.md new file mode 100644 index 0000000000..c9a4b1965e --- /dev/null +++ b/.changeset/the-renegade-feeling.md @@ -0,0 +1,42 @@ +--- +'@backstage/create-app': patch +--- + +TechDocs Backend may now (optionally) leverage a cache store to improve +performance when reading content from a cloud storage provider. + +To apply this change to an existing app, pass the cache manager from the plugin +environment to the `createRouter` function in your backend: + +```diff +// packages/backend/src/plugins/techdocs.ts + +export default async function createPlugin({ + logger, + config, + discovery, + reader, ++ cache, +}: PluginEnvironment): Promise { + + // ... + + return await createRouter({ + preparers, + generators, + publisher, + logger, + config, + discovery, ++ cache, + }); +``` + +If your `PluginEnvironment` does not include a cache manager, be sure you've +applied [the cache management change][cm-change] to your backend as well. + +[Additional configuration][td-rec-arch] is required if you wish to enable +caching in TechDocs. + +[cm-change]: https://github.com/backstage/backstage/blob/master/packages/create-app/CHANGELOG.md#patch-changes-6 +[td-rec-arch]: https://backstage.io/docs/features/techdocs/architecture#recommended-deployment From 693db1da5487cc1c5d37ccfa495a4994eb3f30de Mon Sep 17 00:00:00 2001 From: Eric Peterson Date: Sat, 24 Jul 2021 17:45:48 +0200 Subject: [PATCH 11/27] Review feedback. Signed-off-by: Eric Peterson --- app-config.yaml | 2 +- .../src/stages/publish/local.ts | 47 +++++++++---------- .../src/cache/cacheMiddleware.ts | 2 +- 3 files changed, 24 insertions(+), 27 deletions(-) diff --git a/app-config.yaml b/app-config.yaml index 881e97aa5f..bde037dfe3 100644 --- a/app-config.yaml +++ b/app-config.yaml @@ -113,7 +113,7 @@ techdocs: publisher: type: 'local' # Alternatives - 'googleGcs' or 'awsS3' or 'azureBlobStorage' or 'openStackSwift'. Read documentation for using alternatives. cache: - ttl: 60000 # 1 minute cache for demonstration purposes. You may wish to set this higher in production. + ttl: 3600000 # 1 hour cache for demonstration purposes. You may wish to set this higher in production. sentry: organization: my-company diff --git a/packages/techdocs-common/src/stages/publish/local.ts b/packages/techdocs-common/src/stages/publish/local.ts index 67a4a22914..91a9b57a9e 100644 --- a/packages/techdocs-common/src/stages/publish/local.ts +++ b/packages/techdocs-common/src/stages/publish/local.ts @@ -98,7 +98,10 @@ export class LocalPublish implements PublisherBase { }; } - publish({ entity, directory }: PublishRequest): Promise { + async publish({ + entity, + directory, + }: PublishRequest): Promise { const entityNamespace = entity.metadata.namespace ?? 'default'; const publishDir = this.staticEntityPathJoin( @@ -112,32 +115,26 @@ export class LocalPublish implements PublisherBase { fs.mkdirSync(publishDir, { recursive: true }); } - return new Promise((resolve, reject) => { - fs.copy(directory, publishDir, async err => { - if (err) { - this.logger.debug( - `Failed to copy docs from ${directory} to ${publishDir}`, - ); - reject(err); - } - this.logger.info(`Published site stored at ${publishDir}`); + try { + await fs.copy(directory, publishDir); + this.logger.info(`Published site stored at ${publishDir}`); + } catch (error) { + this.logger.debug( + `Failed to copy docs from ${directory} to ${publishDir}`, + ); + throw error; + } - try { - const techdocsApiUrl = await this.discovery.getBaseUrl('techdocs'); - const objects = (await getFileTreeRecursively(publishDir)).map( - abs => { - return abs.split(`${staticDocsDir}/`)[1]; - }, - ); - resolve({ - remoteUrl: `${techdocsApiUrl}/static/docs/${entity.metadata.name}`, - objects, - }); - } catch (reason) { - reject(reason); - } - }); + // Generate publish response. + const techdocsApiUrl = await this.discovery.getBaseUrl('techdocs'); + const objects = (await getFileTreeRecursively(publishDir)).map(abs => { + return abs.split(`${staticDocsDir}/`)[1]; }); + + return { + remoteUrl: `${techdocsApiUrl}/static/docs/${entity.metadata.name}`, + objects, + }; } async fetchTechDocsMetadata( diff --git a/plugins/techdocs-backend/src/cache/cacheMiddleware.ts b/plugins/techdocs-backend/src/cache/cacheMiddleware.ts index 2650b39296..0c06052ba4 100644 --- a/plugins/techdocs-backend/src/cache/cacheMiddleware.ts +++ b/plugins/techdocs-backend/src/cache/cacheMiddleware.ts @@ -112,7 +112,7 @@ export const createCacheMiddleware = ({ }; // When a socket is closed, if there were no errors and the data written - // over the socket should be cached, cache it as a base64-encoded string! + // over the socket should be cached, cache it! socket.on('close', hadError => { if (writeToCache && !hadError) { cache.set(reqPath, Buffer.concat(chunks)); From a16dce0433c161dad036aec5a85dfeaad13e8fa6 Mon Sep 17 00:00:00 2001 From: Eric Peterson Date: Sat, 24 Jul 2021 18:46:28 +0200 Subject: [PATCH 12/27] Tests for TechDocsCache class Signed-off-by: Eric Peterson --- .../src/cache/TechDocsCache.test.ts | 147 ++++++++++++++++++ 1 file changed, 147 insertions(+) create mode 100644 plugins/techdocs-backend/src/cache/TechDocsCache.test.ts diff --git a/plugins/techdocs-backend/src/cache/TechDocsCache.test.ts b/plugins/techdocs-backend/src/cache/TechDocsCache.test.ts new file mode 100644 index 0000000000..e4308b7c3b --- /dev/null +++ b/plugins/techdocs-backend/src/cache/TechDocsCache.test.ts @@ -0,0 +1,147 @@ +/* + * 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 { CacheClient, getVoidLogger } from '@backstage/backend-common'; +import { CacheInvalidationError, TechDocsCache } from './TechDocsCache'; + +const cached = (str: string): string => { + return Buffer.from(str).toString('base64'); +}; + +describe('TechDocsCache', () => { + let CacheUnderTest: TechDocsCache; + let MockClient: jest.Mocked; + + beforeEach(() => { + MockClient = { + get: jest.fn(), + set: jest.fn(), + delete: jest.fn(), + }; + CacheUnderTest = new TechDocsCache({ + cache: MockClient, + logger: getVoidLogger(), + }); + }); + + describe('get', () => { + it('returns undefined if no response', async () => { + const expectedPath = 'some/index.html'; + MockClient.get.mockResolvedValueOnce(undefined); + + const actual = await CacheUnderTest.get(expectedPath); + expect(MockClient.get).toHaveBeenCalledWith(expectedPath); + expect(actual).toBe(undefined); + }); + + it('returns undefined if cache get throws', async () => { + const expectedPath = 'some/index.html'; + MockClient.get.mockRejectedValueOnce(new Error()); + + const actual = await CacheUnderTest.get(expectedPath); + expect(actual).toBe(undefined); + }); + + it('returns undefined if no response after 1s', async () => { + const expectedPath = 'some/index.html'; + MockClient.get.mockImplementationOnce(() => { + return new Promise(resolve => { + setTimeout(() => resolve(cached('value')), 1500); + }); + }); + const actual = await CacheUnderTest.get(expectedPath); + expect(actual).toBe(undefined); + }); + + it('returns data if cache get returns it', async () => { + const expectedPath = 'some/index.html'; + MockClient.get.mockResolvedValueOnce(cached('expected value')); + + const actual = await CacheUnderTest.get(expectedPath); + expect(actual?.toString()).toBe('expected value'); + }); + }); + + describe('set', () => { + it('sets a base64-encoded string', async () => { + const expectedPath = 'some/index.html'; + MockClient.set.mockResolvedValueOnce(undefined); + + await CacheUnderTest.set(expectedPath, Buffer.from('some data')); + expect(MockClient.set).toHaveBeenCalledWith( + expectedPath, + cached('some data'), + ); + }); + + it('does not throw if client throws', () => { + MockClient.set.mockRejectedValueOnce(new Error()); + expect(() => CacheUnderTest.set('i.html', Buffer.from(''))).not.toThrow(); + }); + }); + + describe('invalidate', () => { + it('calls delete on client', async () => { + const expectedPath = 'some/index.html'; + MockClient.delete.mockResolvedValueOnce(undefined); + + await CacheUnderTest.invalidate(expectedPath); + expect(MockClient.delete).toHaveBeenCalledWith(expectedPath); + }); + }); + + describe('invalidateMultiple', () => { + it('calls delete once per given path', async () => { + const expectedPaths = ['one/index.html', 'two/index.html']; + MockClient.delete.mockResolvedValue(undefined); + + await CacheUnderTest.invalidateMultiple(expectedPaths); + expect(MockClient.delete).toHaveBeenNthCalledWith(1, expectedPaths[0]); + expect(MockClient.delete).toHaveBeenNthCalledWith(2, expectedPaths[1]); + }); + + it('returns an array of as many paths provided', async () => { + const expectedPaths = ['one/index.html', 'two/index.html']; + MockClient.delete.mockResolvedValue(undefined); + + const actual = await CacheUnderTest.invalidateMultiple(expectedPaths); + expect(actual.length).toBe(2); + }); + + it('calls delete on all paths even if the first rejects', async () => { + const expectedPaths = ['one/index.html', 'two/index.html']; + MockClient.delete.mockRejectedValueOnce(new Error()); + MockClient.delete.mockResolvedValueOnce(undefined); + + await expect( + CacheUnderTest.invalidateMultiple(expectedPaths), + ).rejects.toThrowError(CacheInvalidationError); + expect(MockClient.delete).toHaveBeenCalledTimes(2); + }); + + it('rejects with invalidations error response', async () => { + const expectedPaths = ['one/index.html', 'two/index.html']; + MockClient.delete.mockResolvedValueOnce(undefined); + MockClient.delete.mockRejectedValueOnce(new Error()); + + await expect( + CacheUnderTest.invalidateMultiple.bind(CacheUnderTest, expectedPaths), + ).rejects.toThrow( + expect.objectContaining({ rejections: expect.arrayContaining([]) }), + ); + }); + }); +}); From 74ccd66e26ab36612173ca4a1bb3914bfcdfca41 Mon Sep 17 00:00:00 2001 From: Eric Peterson Date: Sat, 24 Jul 2021 22:27:18 +0200 Subject: [PATCH 13/27] Add tests for cache middleware. Do not cache non-200 responses. Signed-off-by: Eric Peterson --- .../src/cache/cacheMiddleware.test.ts | 142 ++++++++++++++++++ .../src/cache/cacheMiddleware.ts | 6 +- 2 files changed, 146 insertions(+), 2 deletions(-) create mode 100644 plugins/techdocs-backend/src/cache/cacheMiddleware.test.ts diff --git a/plugins/techdocs-backend/src/cache/cacheMiddleware.test.ts b/plugins/techdocs-backend/src/cache/cacheMiddleware.test.ts new file mode 100644 index 0000000000..5c591bdda4 --- /dev/null +++ b/plugins/techdocs-backend/src/cache/cacheMiddleware.test.ts @@ -0,0 +1,142 @@ +/* + * 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 } from '@backstage/backend-common'; +import express from 'express'; +import request from 'supertest'; +import { createCacheMiddleware, TechDocsCache } from '.'; + +/** + * Mocks cached HTTP response. + */ +const getMockHttpResponseFor = (content: string): Buffer => { + return Buffer.concat([ + Buffer.from(`HTTP/1.1 200 OK +Content-Type: text/plain; charset=utf-8 +Accept-Ranges: bytes +Cache-Control: public, max-age=0 +Last-Modified: Sat, 1 Jul 2021 12:00:00 GMT +Date: Sat, 1 Jul 2021 12:00:00 GMT +Connection: close +Content-Length: ${content.length}\n\n`), + Buffer.from(content), + ]); +}; + +/** + * Wait for the socket to close. Works because, above, we set connection: close + */ +const waitForSocketClose = () => new Promise(resolve => setTimeout(resolve, 0)); + +describe('createCacheMiddleware', () => { + let cache: jest.Mocked; + let app: express.Express; + + beforeEach(async () => { + cache = ({ + get: jest.fn().mockResolvedValue(undefined), + set: jest.fn().mockResolvedValue(undefined), + invalidate: jest.fn().mockResolvedValue(undefined), + invalidateMultiple: jest.fn().mockResolvedValue(undefined), + } as unknown) as jest.Mocked; + const router = await createCacheMiddleware({ + logger: getVoidLogger(), + cache, + }); + app = express().use(router); + app.use((req, res, next) => { + // By default, send cacheable content. + if (req.path !== '/api/static/docs/error.png') { + res.send('default-response'); + } else { + next(new Error()); + } + }); + }); + + describe('invalidate', () => { + it('responds with 400 when no objects are provided', async () => { + const response = await request(app).post('/cache/invalidate'); + expect(response.status).toBe(400); + }); + + it('responds with 500 if invalidation throws', async () => { + cache.invalidateMultiple.mockRejectedValueOnce(new Error()); + const response = await request(app) + .post('/cache/invalidate') + .set('Content-Type', 'application/json') + .send({ + objects: ['one/index.html', 'two/index.html'], + }); + + expect(response.status).toBe(500); + }); + + it('responds with 204 if invalidation succeeds', async () => { + const expectedObjects = ['one/index.html', 'two/index.html']; + cache.invalidateMultiple.mockResolvedValue([]); + await request(app) + .post('/cache/invalidate') + .set('Content-Type', 'application/json') + .send({ + objects: expectedObjects, + }) + .expect(204); + + expect(cache.invalidateMultiple).toHaveBeenCalledWith(expectedObjects); + }); + }); + + describe('middleware', () => { + it('does not apply to non-static/docs paths', async () => { + await request(app) + .get('/api/static/not-docs') + .expect(200, 'default-response'); + + expect(cache.set).not.toHaveBeenCalled(); + }); + + it('responds with cached response', async () => { + cache.get.mockResolvedValueOnce(getMockHttpResponseFor('xyz')); + + await request(app).get('/api/static/docs/foo.html').expect(200, 'xyz'); + + await waitForSocketClose(); + expect(cache.set).not.toHaveBeenCalled(); + }); + + it('sets cache when content is cacheable', async () => { + const expectedPath = 'default/api/xyz/index.html'; + await request(app) + .get(`/api/static/docs/${expectedPath}`) + .expect(200, 'default-response'); + + await waitForSocketClose(); + expect(cache.set).toHaveBeenCalled(); + + const [actualPath, actualBuffer] = (cache.set as jest.Mock).mock.calls[0]; + expect(actualPath).toBe(expectedPath); + expect(actualBuffer.toString()).toContain('default-response'); + }); + + it('does not set cache on error', async () => { + await request(app).get('/api/static/docs/error.png').expect(500); + + await waitForSocketClose(); + expect(cache.set).not.toHaveBeenCalled(); + }); + }); +}); diff --git a/plugins/techdocs-backend/src/cache/cacheMiddleware.ts b/plugins/techdocs-backend/src/cache/cacheMiddleware.ts index 0c06052ba4..c25d002181 100644 --- a/plugins/techdocs-backend/src/cache/cacheMiddleware.ts +++ b/plugins/techdocs-backend/src/cache/cacheMiddleware.ts @@ -114,8 +114,10 @@ export const createCacheMiddleware = ({ // When a socket is closed, if there were no errors and the data written // over the socket should be cached, cache it! socket.on('close', hadError => { - if (writeToCache && !hadError) { - cache.set(reqPath, Buffer.concat(chunks)); + const content = Buffer.concat(chunks); + const head = content.toString('utf8', 0, 12); + if (writeToCache && !hadError && head === 'HTTP/1.1 200') { + cache.set(reqPath, content); } }); From d011fd61ea16aacfcbd32716adc093117ea1c03d Mon Sep 17 00:00:00 2001 From: Eric Peterson Date: Sat, 24 Jul 2021 23:07:33 +0200 Subject: [PATCH 14/27] Ensure DocsSynchronizer has access to cache client. Signed-off-by: Eric Peterson --- .../techdocs-backend/src/DocsBuilder/builder.ts | 3 +++ .../src/service/DocsSynchronizer.test.ts | 8 ++++++++ .../src/service/DocsSynchronizer.ts | 6 ++++++ plugins/techdocs-backend/src/service/router.ts | 16 +++++++++------- 4 files changed, 26 insertions(+), 7 deletions(-) diff --git a/plugins/techdocs-backend/src/DocsBuilder/builder.ts b/plugins/techdocs-backend/src/DocsBuilder/builder.ts index 1d725bd69e..66eb52c1aa 100644 --- a/plugins/techdocs-backend/src/DocsBuilder/builder.ts +++ b/plugins/techdocs-backend/src/DocsBuilder/builder.ts @@ -222,6 +222,9 @@ export class DocsBuilder { // Invalidate the cache for any published objects. if (this.cache && published && published?.objects?.length) { + this.logger.debug( + `Invalidating ${published.objects.length} cache objects`, + ); await this.cache.invalidateMultiple(published.objects); } diff --git a/plugins/techdocs-backend/src/service/DocsSynchronizer.test.ts b/plugins/techdocs-backend/src/service/DocsSynchronizer.test.ts index ac60f3da3a..c579f62fb8 100644 --- a/plugins/techdocs-backend/src/service/DocsSynchronizer.test.ts +++ b/plugins/techdocs-backend/src/service/DocsSynchronizer.test.ts @@ -25,6 +25,7 @@ import { PreparerBuilder, PublisherBase, } from '@backstage/techdocs-common'; +import { TechDocsCache } from '../cache'; import { DocsBuilder, shouldCheckForUpdate } from '../DocsBuilder'; import { DocsSynchronizer, DocsSynchronizerSyncOpts } from './DocsSynchronizer'; @@ -52,6 +53,12 @@ describe('DocsSynchronizer', () => { getBaseUrl: jest.fn(), getExternalBaseUrl: jest.fn(), }; + const cache: jest.Mocked = ({ + get: jest.fn(), + set: jest.fn(), + invalidate: jest.fn(), + invalidateMultiple: jest.fn(), + } as unknown) as jest.Mocked; let docsSynchronizer: DocsSynchronizer; const mockResponseHandler: jest.Mocked = { @@ -71,6 +78,7 @@ describe('DocsSynchronizer', () => { config: new ConfigReader({}), logger: getVoidLogger(), scmIntegrations: ScmIntegrations.fromConfig(new ConfigReader({})), + cache, }); }); diff --git a/plugins/techdocs-backend/src/service/DocsSynchronizer.ts b/plugins/techdocs-backend/src/service/DocsSynchronizer.ts index 6f977dcb47..d76420d936 100644 --- a/plugins/techdocs-backend/src/service/DocsSynchronizer.ts +++ b/plugins/techdocs-backend/src/service/DocsSynchronizer.ts @@ -25,6 +25,7 @@ import { } from '@backstage/techdocs-common'; import { PassThrough } from 'stream'; import * as winston from 'winston'; +import { TechDocsCache } from '../cache'; import { DocsBuilder, shouldCheckForUpdate } from '../DocsBuilder'; export type DocsSynchronizerSyncOpts = { @@ -38,22 +39,26 @@ export class DocsSynchronizer { private readonly logger: winston.Logger; private readonly config: Config; private readonly scmIntegrations: ScmIntegrationRegistry; + private readonly cache: TechDocsCache | undefined; constructor({ publisher, logger, config, scmIntegrations, + cache, }: { publisher: PublisherBase; logger: winston.Logger; config: Config; scmIntegrations: ScmIntegrationRegistry; + cache: TechDocsCache | undefined; }) { this.config = config; this.logger = logger; this.publisher = publisher; this.scmIntegrations = scmIntegrations; + this.cache = cache; } async doSync({ @@ -104,6 +109,7 @@ export class DocsSynchronizer { config: this.config, scmIntegrations: this.scmIntegrations, logStream, + cache: this.cache, }); const updated = await docsBuilder.build(); diff --git a/plugins/techdocs-backend/src/service/router.ts b/plugins/techdocs-backend/src/service/router.ts index 07c6887ef7..96c5aaf172 100644 --- a/plugins/techdocs-backend/src/service/router.ts +++ b/plugins/techdocs-backend/src/service/router.ts @@ -85,13 +85,6 @@ export async function createRouter( const router = Router(); const { publisher, config, logger, discovery } = options; const catalogClient = new CatalogClient({ discoveryApi: discovery }); - const scmIntegrations = ScmIntegrations.fromConfig(config); - const docsSynchronizer = new DocsSynchronizer({ - publisher, - logger, - config, - scmIntegrations, - }); // Set up a cache client if configured. let cache: TechDocsCache | undefined; @@ -101,6 +94,15 @@ export async function createRouter( cache = new TechDocsCache({ cache: cacheClient, logger }); } + const scmIntegrations = ScmIntegrations.fromConfig(config); + const docsSynchronizer = new DocsSynchronizer({ + publisher, + logger, + config, + scmIntegrations, + cache, + }); + router.get('/metadata/techdocs/:namespace/:kind/:name', async (req, res) => { const { kind, namespace, name } = req.params; const entityName = { kind, namespace, name }; From e99606d3e66c16a1db6ba299f7908c6fc19efb38 Mon Sep 17 00:00:00 2001 From: Eric Peterson Date: Fri, 12 Nov 2021 10:49:50 +0100 Subject: [PATCH 15/27] Add all generated files to techdocs_metadata.json Signed-off-by: Eric Peterson --- .../src/stages/generate/helpers.test.ts | 28 +++++++++++++------ .../src/stages/generate/helpers.ts | 24 ++++++++++++++-- .../src/stages/generate/techdocs.ts | 6 ++-- .../src/stages/publish/types.ts | 2 ++ 4 files changed, 47 insertions(+), 13 deletions(-) diff --git a/packages/techdocs-common/src/stages/generate/helpers.test.ts b/packages/techdocs-common/src/stages/generate/helpers.test.ts index 9e01c3e2ad..9323ab4f18 100644 --- a/packages/techdocs-common/src/stages/generate/helpers.test.ts +++ b/packages/techdocs-common/src/stages/generate/helpers.test.ts @@ -23,7 +23,7 @@ import os from 'os'; import path, { resolve as resolvePath } from 'path'; import { ParsedLocationAnnotation } from '../../helpers'; import { - addBuildTimestampMetadata, + createOrUpdateMetadata, getGeneratorKey, getMkdocsYml, getRepoUrlFromLocationAnnotation, @@ -369,13 +369,15 @@ describe('helpers', () => { }); describe('addBuildTimestampMetadata', () => { + const mockFiles = { + 'invalid_techdocs_metadata.json': 'dsds', + 'techdocs_metadata.json': '{"site_name": "Tech Docs"}', + }; + beforeEach(() => { mockFs.restore(); mockFs({ - [rootDir]: { - 'invalid_techdocs_metadata.json': 'dsds', - 'techdocs_metadata.json': '{"site_name": "Tech Docs"}', - }, + [rootDir]: mockFiles, }); }); @@ -385,7 +387,7 @@ describe('helpers', () => { it('should create the file if it does not exist', async () => { const filePath = path.join(rootDir, 'wrong_techdocs_metadata.json'); - await addBuildTimestampMetadata(filePath, mockLogger); + await createOrUpdateMetadata(filePath, mockLogger); // Check if the file exists await expect( @@ -397,18 +399,28 @@ describe('helpers', () => { const filePath = path.join(rootDir, 'invalid_techdocs_metadata.json'); await expect( - addBuildTimestampMetadata(filePath, mockLogger), + createOrUpdateMetadata(filePath, mockLogger), ).rejects.toThrowError('Unexpected token d in JSON at position 0'); }); it('should add build timestamp to the metadata json', async () => { const filePath = path.join(rootDir, 'techdocs_metadata.json'); - await addBuildTimestampMetadata(filePath, mockLogger); + await createOrUpdateMetadata(filePath, mockLogger); const json = await fs.readJson(filePath); expect(json.build_timestamp).toBeLessThanOrEqual(Date.now()); }); + + it('should add list of files to the metadata json', async () => { + const filePath = path.join(rootDir, 'techdocs_metadata.json'); + + await createOrUpdateMetadata(filePath, mockLogger); + + const json = await fs.readJson(filePath); + expect(json.files[0]).toEqual(Object.keys(mockFiles)[0]); + expect(json.files[1]).toEqual(Object.keys(mockFiles)[1]); + }); }); describe('storeEtagMetadata', () => { diff --git a/packages/techdocs-common/src/stages/generate/helpers.ts b/packages/techdocs-common/src/stages/generate/helpers.ts index f77b557076..f96d1209b6 100644 --- a/packages/techdocs-common/src/stages/generate/helpers.ts +++ b/packages/techdocs-common/src/stages/generate/helpers.ts @@ -27,6 +27,7 @@ import { PassThrough, Writable } from 'stream'; import { Logger } from 'winston'; import { ParsedLocationAnnotation } from '../../helpers'; import { SupportedGeneratorKey } from './types'; +import { getFileTreeRecursively } from '../publish/helpers'; // TODO: Implement proper support for more generators. export function getGeneratorKey(entity: Entity): SupportedGeneratorKey { @@ -345,14 +346,20 @@ export const patchIndexPreBuild = async ({ }; /** - * Update the techdocs_metadata.json to add a new build timestamp metadata. Create the .json file if it doesn't exist. + * Create or update the techdocs_metadata.json. Values initialized/updated are: + * - The build_timestamp (now) + * - The list of files generated * * @param {string} techdocsMetadataPath File path to techdocs_metadata.json */ -export const addBuildTimestampMetadata = async ( +export const createOrUpdateMetadata = async ( techdocsMetadataPath: string, logger: Logger, ): Promise => { + const techdocsMetadataDir = techdocsMetadataPath + .split(path.sep) + .slice(0, -1) + .join(path.sep); // check if file exists, create if it does not. try { await fs.access(techdocsMetadataPath, fs.constants.F_OK); @@ -372,6 +379,19 @@ export const addBuildTimestampMetadata = async ( } json.build_timestamp = Date.now(); + + // Get and write generated files to the metadata JSON. Each file string is in + // a form appropriate for invalidating the associated object from cache. + try { + json.files = (await getFileTreeRecursively(techdocsMetadataDir)).map(file => + file.replace(`${techdocsMetadataDir}/`, ''), + ); + } catch (err) { + assertError(err); + json.files = []; + logger.warn(`Unable to add files list to metadata: ${err.message}`); + } + await fs.writeJson(techdocsMetadataPath, json); return; }; diff --git a/packages/techdocs-common/src/stages/generate/techdocs.ts b/packages/techdocs-common/src/stages/generate/techdocs.ts index 728cab5a81..8681736815 100644 --- a/packages/techdocs-common/src/stages/generate/techdocs.ts +++ b/packages/techdocs-common/src/stages/generate/techdocs.ts @@ -23,7 +23,7 @@ import { ScmIntegrations, } from '@backstage/integration'; import { - addBuildTimestampMetadata, + createOrUpdateMetadata, getMkdocsYml, patchIndexPreBuild, patchMkdocsYmlPreBuild, @@ -164,9 +164,9 @@ export class TechdocsGenerator implements GeneratorBase { * Post Generate steps */ - // Add build timestamp to techdocs_metadata.json + // Add build timestamp and files to techdocs_metadata.json // Creates techdocs_metadata.json if file does not exist. - await addBuildTimestampMetadata( + await createOrUpdateMetadata( path.join(outputDir, 'techdocs_metadata.json'), childLogger, ); diff --git a/packages/techdocs-common/src/stages/publish/types.ts b/packages/techdocs-common/src/stages/publish/types.ts index ab497067b3..1fb301340b 100644 --- a/packages/techdocs-common/src/stages/publish/types.ts +++ b/packages/techdocs-common/src/stages/publish/types.ts @@ -65,6 +65,8 @@ export type TechDocsMetadata = { site_name: string; site_description: string; etag: string; + build_timestamp: number; + files?: string[]; }; export type MigrateRequest = { From a3909d2c2fc67d12451cc8f3c800ac97fac5c695 Mon Sep 17 00:00:00 2001 From: Eric Peterson Date: Sat, 7 Aug 2021 20:33:49 +0200 Subject: [PATCH 16/27] Invalidate stale cache entries on read for external builder config when cache is enabled. Signed-off-by: Eric Peterson --- .../src/service/DocsSynchronizer.test.ts | 91 +++++++++++++++++++ .../src/service/DocsSynchronizer.ts | 74 ++++++++++++++- .../techdocs-backend/src/service/router.ts | 11 +++ 3 files changed, 174 insertions(+), 2 deletions(-) diff --git a/plugins/techdocs-backend/src/service/DocsSynchronizer.test.ts b/plugins/techdocs-backend/src/service/DocsSynchronizer.test.ts index c579f62fb8..4089b08ef4 100644 --- a/plugins/techdocs-backend/src/service/DocsSynchronizer.test.ts +++ b/plugins/techdocs-backend/src/service/DocsSynchronizer.test.ts @@ -31,6 +31,19 @@ import { DocsSynchronizer, DocsSynchronizerSyncOpts } from './DocsSynchronizer'; jest.mock('../DocsBuilder'); +jest.mock('cross-fetch', () => ({ + __esModule: true, + default: async () => { + return { + json: async () => { + return { + build_timestamp: 123, + }; + }, + }; + }, +})); + const MockedDocsBuilder = DocsBuilder as jest.MockedClass; describe('DocsSynchronizer', () => { @@ -201,4 +214,82 @@ describe('DocsSynchronizer', () => { expect(mockResponseHandler.error).toBeCalledWith(error); }); }); + + describe('doCacheSync', () => { + const entity = { + apiVersion: 'backstage.io/v1alpha1', + kind: 'Component', + metadata: { + uid: '0', + name: 'test', + namespace: 'default', + }, + }; + + it('should not check metadata too often', async () => { + (shouldCheckForUpdate as jest.Mock).mockReturnValue(false); + + await docsSynchronizer.doCacheSync({ + responseHandler: mockResponseHandler, + discovery, + token: undefined, + entity, + }); + + expect(mockResponseHandler.finish).toBeCalledWith({ updated: false }); + expect(shouldCheckForUpdate).toBeCalledTimes(1); + }); + + it('should do nothing if source/cached metadata matches', async () => { + (shouldCheckForUpdate as jest.Mock).mockReturnValue(true); + (publisher.fetchTechDocsMetadata as jest.Mock).mockResolvedValue({ + build_timestamp: 123, + }); + + await docsSynchronizer.doCacheSync({ + responseHandler: mockResponseHandler, + discovery, + token: undefined, + entity, + }); + + expect(mockResponseHandler.finish).toBeCalledWith({ updated: false }); + }); + + it('should invalidate expected files when source/cached metadata differ', async () => { + (shouldCheckForUpdate as jest.Mock).mockReturnValue(true); + (publisher.fetchTechDocsMetadata as jest.Mock).mockResolvedValue({ + build_timestamp: 456, + files: ['index.html'], + }); + + await docsSynchronizer.doCacheSync({ + responseHandler: mockResponseHandler, + discovery, + token: undefined, + entity, + }); + + expect(mockResponseHandler.finish).toBeCalledWith({ updated: true }); + expect(cache.invalidateMultiple).toHaveBeenCalledWith([ + 'default/Component/test/index.html', + ]); + }); + + it('should gracefully handle errors', async () => { + (shouldCheckForUpdate as jest.Mock).mockReturnValue(true); + (publisher.fetchTechDocsMetadata as jest.Mock).mockRejectedValue( + new Error(), + ); + + await docsSynchronizer.doCacheSync({ + responseHandler: mockResponseHandler, + discovery, + token: undefined, + entity, + }); + + expect(mockResponseHandler.finish).toBeCalledWith({ updated: false }); + }); + }); }); diff --git a/plugins/techdocs-backend/src/service/DocsSynchronizer.ts b/plugins/techdocs-backend/src/service/DocsSynchronizer.ts index d76420d936..c6d198c41d 100644 --- a/plugins/techdocs-backend/src/service/DocsSynchronizer.ts +++ b/plugins/techdocs-backend/src/service/DocsSynchronizer.ts @@ -14,7 +14,8 @@ * limitations under the License. */ -import { Entity } from '@backstage/catalog-model'; +import { PluginEndpointDiscovery } from '@backstage/backend-common'; +import { Entity, ENTITY_DEFAULT_NAMESPACE } from '@backstage/catalog-model'; import { Config } from '@backstage/config'; import { assertError, NotFoundError } from '@backstage/errors'; import { ScmIntegrationRegistry } from '@backstage/integration'; @@ -23,10 +24,15 @@ import { PreparerBuilder, PublisherBase, } from '@backstage/techdocs-common'; +import fetch from 'cross-fetch'; import { PassThrough } from 'stream'; import * as winston from 'winston'; import { TechDocsCache } from '../cache'; -import { DocsBuilder, shouldCheckForUpdate } from '../DocsBuilder'; +import { + BuildMetadataStorage, + DocsBuilder, + shouldCheckForUpdate, +} from '../DocsBuilder'; export type DocsSynchronizerSyncOpts = { log: (message: string) => void; @@ -151,4 +157,68 @@ export class DocsSynchronizer { finish({ updated: true }); } + + async doCacheSync({ + responseHandler: { finish }, + discovery, + token, + entity, + }: { + responseHandler: DocsSynchronizerSyncOpts; + discovery: PluginEndpointDiscovery; + token: string | undefined; + entity: Entity; + }) { + // Check if the last update check was too recent. + if (!shouldCheckForUpdate(entity.metadata.uid!) || !this.cache) { + finish({ updated: false }); + return; + } + + // Fetch techdocs_metadata.json from the publisher and from cache. + const baseUrl = await discovery.getBaseUrl('techdocs'); + const namespace = entity.metadata?.namespace || ENTITY_DEFAULT_NAMESPACE; + const kind = entity.kind; + const name = entity.metadata.name; + const entityTripletPath = `${namespace}/${kind}/${name}`; + try { + const [sourceMetadata, cachedMetadata] = await Promise.all([ + this.publisher.fetchTechDocsMetadata({ namespace, kind, name }), + fetch( + `${baseUrl}/static/docs/${entityTripletPath}/techdocs_metadata.json`, + { + headers: token ? { Authorization: `Bearer ${token}` } : {}, + }, + ).then( + f => + f.json().catch(() => undefined) as ReturnType< + PublisherBase['fetchTechDocsMetadata'] + >, + ), + ]); + + // If build timestamps differ, merge their files[] lists and invalidate all objects. + if (sourceMetadata.build_timestamp !== cachedMetadata.build_timestamp) { + const files = [ + ...new Set([ + ...(sourceMetadata.files || []), + ...(cachedMetadata.files || []), + ]), + ].map(f => `${entityTripletPath}/${f}`); + await this.cache.invalidateMultiple(files); + finish({ updated: true }); + } else { + finish({ updated: false }); + } + } catch (e) { + // In case of error, log and allow the user to go about their business. + this.logger.error( + `Error syncing cache for ${entityTripletPath}: ${e.message}`, + ); + finish({ updated: false }); + } finally { + // Update the last check time for the entity + new BuildMetadataStorage(entity.metadata.uid!).setLastUpdated(); + } + } } diff --git a/plugins/techdocs-backend/src/service/router.ts b/plugins/techdocs-backend/src/service/router.ts index 96c5aaf172..ba3fc6ea87 100644 --- a/plugins/techdocs-backend/src/service/router.ts +++ b/plugins/techdocs-backend/src/service/router.ts @@ -190,6 +190,17 @@ export async function createRouter( // 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') { + // However, if caching is enabled, take the opportunity to check and + // invalidate stale cache entries. + if (cache) { + await docsSynchronizer.doCacheSync({ + responseHandler, + discovery, + token, + entity, + }); + return; + } responseHandler.finish({ updated: false }); return; } From 8cf4684a4eae472abb9d060958c56a9b5fe02ac3 Mon Sep 17 00:00:00 2001 From: Eric Peterson Date: Sat, 7 Aug 2021 20:34:10 +0200 Subject: [PATCH 17/27] Remove cache invalidation endpoint from middleware. Signed-off-by: Eric Peterson --- .../src/cache/cacheMiddleware.test.ts | 33 ------------- .../src/cache/cacheMiddleware.ts | 49 +------------------ 2 files changed, 1 insertion(+), 81 deletions(-) diff --git a/plugins/techdocs-backend/src/cache/cacheMiddleware.test.ts b/plugins/techdocs-backend/src/cache/cacheMiddleware.test.ts index 5c591bdda4..4bd680e8c7 100644 --- a/plugins/techdocs-backend/src/cache/cacheMiddleware.test.ts +++ b/plugins/techdocs-backend/src/cache/cacheMiddleware.test.ts @@ -67,39 +67,6 @@ describe('createCacheMiddleware', () => { }); }); - describe('invalidate', () => { - it('responds with 400 when no objects are provided', async () => { - const response = await request(app).post('/cache/invalidate'); - expect(response.status).toBe(400); - }); - - it('responds with 500 if invalidation throws', async () => { - cache.invalidateMultiple.mockRejectedValueOnce(new Error()); - const response = await request(app) - .post('/cache/invalidate') - .set('Content-Type', 'application/json') - .send({ - objects: ['one/index.html', 'two/index.html'], - }); - - expect(response.status).toBe(500); - }); - - it('responds with 204 if invalidation succeeds', async () => { - const expectedObjects = ['one/index.html', 'two/index.html']; - cache.invalidateMultiple.mockResolvedValue([]); - await request(app) - .post('/cache/invalidate') - .set('Content-Type', 'application/json') - .send({ - objects: expectedObjects, - }) - .expect(204); - - expect(cache.invalidateMultiple).toHaveBeenCalledWith(expectedObjects); - }); - }); - describe('middleware', () => { it('does not apply to non-static/docs paths', async () => { await request(app) diff --git a/plugins/techdocs-backend/src/cache/cacheMiddleware.ts b/plugins/techdocs-backend/src/cache/cacheMiddleware.ts index c25d002181..66471b0189 100644 --- a/plugins/techdocs-backend/src/cache/cacheMiddleware.ts +++ b/plugins/techdocs-backend/src/cache/cacheMiddleware.ts @@ -13,22 +13,10 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -import { Router, Request, json } from 'express'; +import { Router } from 'express'; import router from 'express-promise-router'; import { Logger } from 'winston'; import { TechDocsCache } from '.'; -import { CacheInvalidationError } from './TechDocsCache'; - -type CacheClearRequestParams = { - objects: string[]; -}; - -type CacheClearRequest = Request< - any, - unknown, - CacheClearRequestParams, - unknown ->; type CacheMiddlewareOptions = { cache: TechDocsCache; @@ -39,44 +27,9 @@ type ErrorCallback = (err?: Error) => void; export const createCacheMiddleware = ({ cache, - logger, }: CacheMiddlewareOptions): Router => { const cacheMiddleware = router(); - // And endpoint for handling cache invalidation external to the Backstage - // Backend (e.g. from the TechDocs CLI). - cacheMiddleware.use(json()); - cacheMiddleware.post( - '/cache/invalidate', - async (req: CacheClearRequest, res) => { - if (req.body?.objects?.length) { - logger.debug( - `Clearing ${req.body.objects.length} cache entries: (eg: ${req.body.objects[0]})`, - ); - - try { - const invalidated = await cache.invalidateMultiple(req.body.objects); - logger.debug( - `Successfully invalidated ${invalidated.length} cache entries`, - ); - res.status(204).send(); - } catch (e) { - if (e instanceof CacheInvalidationError) { - const uniqueReasons = [ - ...new Set(e.rejections.map(r => r.reason.message)), - ].join(', '); - logger.warn( - `Problem invalidating ${e.rejections.length} entries: ${uniqueReasons}`, - ); - } - res.status(500).send(); - } - } else { - res.status(400).send(); - } - }, - ); - // Middleware that, through socket monkey patching, captures responses as // they're sent over /static/docs/* and caches them. Subsequent requests are // loaded from cache. Cache key is the object's path (after `/static/docs/`). From 6e618bef06ca2c4150540b6f75914b00c38ab9a3 Mon Sep 17 00:00:00 2001 From: Eric Peterson Date: Fri, 12 Nov 2021 11:00:33 +0100 Subject: [PATCH 18/27] Fix up types/tests/report. Signed-off-by: Eric Peterson --- packages/techdocs-common/api-report.md | 2 ++ packages/techdocs-common/src/stages/publish/awsS3.test.ts | 1 + .../src/stages/publish/azureBlobStorage.test.ts | 1 + .../src/stages/publish/googleStorage.test.ts | 1 + .../src/stages/publish/openStackSwift.test.ts | 6 ++++-- 5 files changed, 9 insertions(+), 2 deletions(-) diff --git a/packages/techdocs-common/api-report.md b/packages/techdocs-common/api-report.md index 61371525e5..b2f9e40d0a 100644 --- a/packages/techdocs-common/api-report.md +++ b/packages/techdocs-common/api-report.md @@ -287,6 +287,8 @@ export type TechDocsMetadata = { site_name: string; site_description: string; etag: string; + build_timestamp: number; + files?: string[]; }; // Warning: (ae-missing-release-tag) "transformDirLocation" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal) diff --git a/packages/techdocs-common/src/stages/publish/awsS3.test.ts b/packages/techdocs-common/src/stages/publish/awsS3.test.ts index abae5fda44..37c2e06283 100644 --- a/packages/techdocs-common/src/stages/publish/awsS3.test.ts +++ b/packages/techdocs-common/src/stages/publish/awsS3.test.ts @@ -95,6 +95,7 @@ describe('AwsS3Publish', () => { site_name: 'backstage', site_description: 'site_content', etag: 'etag', + build_timestamp: 612741599, }; const directory = getEntityRootDir(entity); diff --git a/packages/techdocs-common/src/stages/publish/azureBlobStorage.test.ts b/packages/techdocs-common/src/stages/publish/azureBlobStorage.test.ts index 0e261f7f86..0ac00988a8 100644 --- a/packages/techdocs-common/src/stages/publish/azureBlobStorage.test.ts +++ b/packages/techdocs-common/src/stages/publish/azureBlobStorage.test.ts @@ -89,6 +89,7 @@ describe('AzureBlobStoragePublish', () => { site_name: 'backstage', site_description: 'site_content', etag: 'etag', + build_timestamp: 612741599, }; const directory = getEntityRootDir(entity); diff --git a/packages/techdocs-common/src/stages/publish/googleStorage.test.ts b/packages/techdocs-common/src/stages/publish/googleStorage.test.ts index ea97d81269..61c152c90d 100644 --- a/packages/techdocs-common/src/stages/publish/googleStorage.test.ts +++ b/packages/techdocs-common/src/stages/publish/googleStorage.test.ts @@ -88,6 +88,7 @@ describe('GoogleGCSPublish', () => { site_name: 'backstage', site_description: 'site_content', etag: 'etag', + build_timestamp: 612741599, }; const directory = getEntityRootDir(entity); diff --git a/packages/techdocs-common/src/stages/publish/openStackSwift.test.ts b/packages/techdocs-common/src/stages/publish/openStackSwift.test.ts index b8ad43aac3..ef06e26cfa 100644 --- a/packages/techdocs-common/src/stages/publish/openStackSwift.test.ts +++ b/packages/techdocs-common/src/stages/publish/openStackSwift.test.ts @@ -247,7 +247,7 @@ describe('OpenStackSwiftPublish', () => { mockFs({ [entityRootDir]: { 'techdocs_metadata.json': - '{"site_name": "backstage", "site_description": "site_content", "etag": "etag"}', + '{"site_name": "backstage", "site_description": "site_content", "etag": "etag", "build_timestamp": 612741599}', }, }); @@ -255,6 +255,7 @@ describe('OpenStackSwiftPublish', () => { site_name: 'backstage', site_description: 'site_content', etag: 'etag', + build_timestamp: 612741599, }; expect( await publisher.fetchTechDocsMetadata(entityNameMock), @@ -269,7 +270,7 @@ describe('OpenStackSwiftPublish', () => { mockFs({ [entityRootDir]: { - 'techdocs_metadata.json': `{'site_name': 'backstage', 'site_description': 'site_content', 'etag': 'etag'}`, + 'techdocs_metadata.json': `{'site_name': 'backstage', 'site_description': 'site_content', 'etag': 'etag', 'build_timestamp': 612741599}`, }, }); @@ -277,6 +278,7 @@ describe('OpenStackSwiftPublish', () => { site_name: 'backstage', site_description: 'site_content', etag: 'etag', + build_timestamp: 612741599, }; expect( await publisher.fetchTechDocsMetadata(entityNameMock), From 12157e8f5b2163931fc9eda45e34f1732aba9314 Mon Sep 17 00:00:00 2001 From: Eric Peterson Date: Fri, 12 Nov 2021 12:36:03 +0100 Subject: [PATCH 19/27] Fix up tests after rebase. Signed-off-by: Eric Peterson --- app-config.yaml | 3 +- .../src/stages/publish/awsS3.test.ts | 30 +++++++++++++++---- .../stages/publish/azureBlobStorage.test.ts | 14 ++++++--- .../src/stages/publish/googleStorage.test.ts | 30 +++++++++++++++---- 4 files changed, 59 insertions(+), 18 deletions(-) diff --git a/app-config.yaml b/app-config.yaml index bde037dfe3..913a72dbab 100644 --- a/app-config.yaml +++ b/app-config.yaml @@ -112,8 +112,7 @@ techdocs: # pullImage: true # or false to disable automatic pulling of image (e.g. if custom docker login is required) publisher: type: 'local' # Alternatives - 'googleGcs' or 'awsS3' or 'azureBlobStorage' or 'openStackSwift'. Read documentation for using alternatives. - cache: - ttl: 3600000 # 1 hour cache for demonstration purposes. You may wish to set this higher in production. + sentry: organization: my-company diff --git a/packages/techdocs-common/src/stages/publish/awsS3.test.ts b/packages/techdocs-common/src/stages/publish/awsS3.test.ts index 37c2e06283..591131907b 100644 --- a/packages/techdocs-common/src/stages/publish/awsS3.test.ts +++ b/packages/techdocs-common/src/stages/publish/awsS3.test.ts @@ -150,21 +150,39 @@ describe('AwsS3Publish', () => { describe('publish', () => { it('should publish a directory', async () => { const publisher = createPublisherFromConfig(); - expect(await publisher.publish({ entity, directory })).toBeUndefined(); + expect(await publisher.publish({ entity, directory })).toMatchObject({ + objects: expect.arrayContaining([ + 'default/component/backstage/404.html', + `default/component/backstage/index.html`, + `default/component/backstage/assets/main.css`, + ]), + }); }); it('should publish a directory as well when legacy casing is used', async () => { const publisher = createPublisherFromConfig({ legacyUseCaseSensitiveTripletPaths: true, }); - expect(await publisher.publish({ entity, directory })).toBeUndefined(); + expect(await publisher.publish({ entity, directory })).toMatchObject({ + objects: expect.arrayContaining([ + 'default/Component/backstage/404.html', + `default/Component/backstage/index.html`, + `default/Component/backstage/assets/main.css`, + ]), + }); }); it('should publish a directory when root path is specified', async () => { const publisher = createPublisherFromConfig({ bucketRootPath: 'backstage-data/techdocs', }); - expect(await publisher.publish({ entity, directory })).toBeUndefined(); + expect(await publisher.publish({ entity, directory })).toMatchObject({ + objects: expect.arrayContaining([ + 'backstage-data/techdocs/default/component/backstage/404.html', + `backstage-data/techdocs/default/component/backstage/index.html`, + `backstage-data/techdocs/default/component/backstage/assets/main.css`, + ]), + }); }); it('should publish a directory when root path is specified and legacy casing is used', async () => { @@ -174,9 +192,9 @@ describe('AwsS3Publish', () => { }); expect(await publisher.publish({ entity, directory })).toMatchObject({ objects: expect.arrayContaining([ - 'test-namespace/TestKind/test-component-name/404.html', - `test-namespace/TestKind/test-component-name/index.html`, - `test-namespace/TestKind/test-component-name/assets/main.css`, + 'backstage-data/techdocs/default/Component/backstage/404.html', + `backstage-data/techdocs/default/Component/backstage/index.html`, + `backstage-data/techdocs/default/Component/backstage/assets/main.css`, ]), }); }); diff --git a/packages/techdocs-common/src/stages/publish/azureBlobStorage.test.ts b/packages/techdocs-common/src/stages/publish/azureBlobStorage.test.ts index 0ac00988a8..e6fd45eed7 100644 --- a/packages/techdocs-common/src/stages/publish/azureBlobStorage.test.ts +++ b/packages/techdocs-common/src/stages/publish/azureBlobStorage.test.ts @@ -155,7 +155,13 @@ describe('AzureBlobStoragePublish', () => { describe('publish', () => { it('should publish a directory', async () => { const publisher = createPublisherFromConfig(); - expect(await publisher.publish({ entity, directory })).toBeUndefined(); + expect(await publisher.publish({ entity, directory })).toMatchObject({ + objects: expect.arrayContaining([ + 'default/component/backstage/404.html', + `default/component/backstage/index.html`, + `default/component/backstage/assets/main.css`, + ]), + }); }); it('should publish a directory as well when legacy casing is used', async () => { @@ -164,9 +170,9 @@ describe('AzureBlobStoragePublish', () => { }); expect(await publisher.publish({ entity, directory })).toMatchObject({ objects: expect.arrayContaining([ - 'test-namespace/TestKind/test-component-name/404.html', - `test-namespace/TestKind/test-component-name/index.html`, - `test-namespace/TestKind/test-component-name/assets/main.css`, + 'default/Component/backstage/404.html', + `default/Component/backstage/index.html`, + `default/Component/backstage/assets/main.css`, ]), }); }); diff --git a/packages/techdocs-common/src/stages/publish/googleStorage.test.ts b/packages/techdocs-common/src/stages/publish/googleStorage.test.ts index 61c152c90d..ed35f2e445 100644 --- a/packages/techdocs-common/src/stages/publish/googleStorage.test.ts +++ b/packages/techdocs-common/src/stages/publish/googleStorage.test.ts @@ -143,21 +143,39 @@ describe('GoogleGCSPublish', () => { describe('publish', () => { it('should publish a directory', async () => { const publisher = createPublisherFromConfig(); - expect(await publisher.publish({ entity, directory })).toBeUndefined(); + expect(await publisher.publish({ entity, directory })).toMatchObject({ + objects: expect.arrayContaining([ + 'default/component/backstage/404.html', + `default/component/backstage/index.html`, + `default/component/backstage/assets/main.css`, + ]), + }); }); it('should publish a directory as well when legacy casing is used', async () => { const publisher = createPublisherFromConfig({ legacyUseCaseSensitiveTripletPaths: true, }); - expect(await publisher.publish({ entity, directory })).toBeUndefined(); + expect(await publisher.publish({ entity, directory })).toMatchObject({ + objects: expect.arrayContaining([ + 'default/Component/backstage/404.html', + `default/Component/backstage/index.html`, + `default/Component/backstage/assets/main.css`, + ]), + }); }); it('should publish a directory when root path is specified', async () => { const publisher = createPublisherFromConfig({ bucketRootPath: 'backstage-data/techdocs', }); - expect(await publisher.publish({ entity, directory })).toBeUndefined(); + expect(await publisher.publish({ entity, directory })).toMatchObject({ + objects: expect.arrayContaining([ + 'backstage-data/techdocs/default/component/backstage/404.html', + `backstage-data/techdocs/default/component/backstage/index.html`, + `backstage-data/techdocs/default/component/backstage/assets/main.css`, + ]), + }); }); it('should publish a directory when root path is specified and legacy casing is used', async () => { @@ -167,9 +185,9 @@ describe('GoogleGCSPublish', () => { }); expect(await publisher.publish({ entity, directory })).toMatchObject({ objects: expect.arrayContaining([ - 'test-namespace/TestKind/test-component-name/404.html', - `test-namespace/TestKind/test-component-name/index.html`, - `test-namespace/TestKind/test-component-name/assets/main.css`, + 'backstage-data/techdocs/default/Component/backstage/404.html', + `backstage-data/techdocs/default/Component/backstage/index.html`, + `backstage-data/techdocs/default/Component/backstage/assets/main.css`, ]), }); }); From da0867f9526f7361848dd4b9ab1d57a187732da1 Mon Sep 17 00:00:00 2001 From: Eric Peterson Date: Fri, 12 Nov 2021 12:58:17 +0100 Subject: [PATCH 20/27] Account for legacy path casing in cache invalidation. Signed-off-by: Eric Peterson --- .../src/cache/cacheMiddleware.test.ts | 4 +-- .../src/service/DocsSynchronizer.test.ts | 34 +++++++++++++++++-- .../src/service/DocsSynchronizer.ts | 10 +++++- 3 files changed, 43 insertions(+), 5 deletions(-) diff --git a/plugins/techdocs-backend/src/cache/cacheMiddleware.test.ts b/plugins/techdocs-backend/src/cache/cacheMiddleware.test.ts index 4bd680e8c7..91e4b84222 100644 --- a/plugins/techdocs-backend/src/cache/cacheMiddleware.test.ts +++ b/plugins/techdocs-backend/src/cache/cacheMiddleware.test.ts @@ -46,12 +46,12 @@ describe('createCacheMiddleware', () => { let app: express.Express; beforeEach(async () => { - cache = ({ + cache = { get: jest.fn().mockResolvedValue(undefined), set: jest.fn().mockResolvedValue(undefined), invalidate: jest.fn().mockResolvedValue(undefined), invalidateMultiple: jest.fn().mockResolvedValue(undefined), - } as unknown) as jest.Mocked; + } as unknown as jest.Mocked; const router = await createCacheMiddleware({ logger: getVoidLogger(), cache, diff --git a/plugins/techdocs-backend/src/service/DocsSynchronizer.test.ts b/plugins/techdocs-backend/src/service/DocsSynchronizer.test.ts index 4089b08ef4..1eb8c8f56f 100644 --- a/plugins/techdocs-backend/src/service/DocsSynchronizer.test.ts +++ b/plugins/techdocs-backend/src/service/DocsSynchronizer.test.ts @@ -66,12 +66,12 @@ describe('DocsSynchronizer', () => { getBaseUrl: jest.fn(), getExternalBaseUrl: jest.fn(), }; - const cache: jest.Mocked = ({ + const cache: jest.Mocked = { get: jest.fn(), set: jest.fn(), invalidate: jest.fn(), invalidateMultiple: jest.fn(), - } as unknown) as jest.Mocked; + } as unknown as jest.Mocked; let docsSynchronizer: DocsSynchronizer; const mockResponseHandler: jest.Mocked = { @@ -270,6 +270,36 @@ describe('DocsSynchronizer', () => { entity, }); + expect(mockResponseHandler.finish).toBeCalledWith({ updated: true }); + expect(cache.invalidateMultiple).toHaveBeenCalledWith([ + 'default/component/test/index.html', + ]); + }); + + it('should invalidate expected files when source/cached metadata differ with legacy casing', async () => { + (shouldCheckForUpdate as jest.Mock).mockReturnValue(true); + (publisher.fetchTechDocsMetadata as jest.Mock).mockResolvedValue({ + build_timestamp: 456, + files: ['index.html'], + }); + + const docsSynchronizerWithLegacy = new DocsSynchronizer({ + publisher, + config: new ConfigReader({ + techdocs: { legacyUseCaseSensitiveTripletPaths: true }, + }), + logger: getVoidLogger(), + scmIntegrations: ScmIntegrations.fromConfig(new ConfigReader({})), + cache, + }); + + await docsSynchronizerWithLegacy.doCacheSync({ + responseHandler: mockResponseHandler, + discovery, + token: undefined, + entity, + }); + expect(mockResponseHandler.finish).toBeCalledWith({ updated: true }); expect(cache.invalidateMultiple).toHaveBeenCalledWith([ 'default/Component/test/index.html', diff --git a/plugins/techdocs-backend/src/service/DocsSynchronizer.ts b/plugins/techdocs-backend/src/service/DocsSynchronizer.ts index c6d198c41d..c7d5bf109a 100644 --- a/plugins/techdocs-backend/src/service/DocsSynchronizer.ts +++ b/plugins/techdocs-backend/src/service/DocsSynchronizer.ts @@ -180,7 +180,14 @@ export class DocsSynchronizer { const namespace = entity.metadata?.namespace || ENTITY_DEFAULT_NAMESPACE; const kind = entity.kind; const name = entity.metadata.name; - const entityTripletPath = `${namespace}/${kind}/${name}`; + const legacyPathCasing = + this.config.getOptionalBoolean( + 'techdocs.legacyUseCaseSensitiveTripletPaths', + ) || false; + const tripletPath = `${namespace}/${kind}/${name}`; + const entityTripletPath = `${ + legacyPathCasing ? tripletPath : tripletPath.toLocaleLowerCase() + }`; try { const [sourceMetadata, cachedMetadata] = await Promise.all([ this.publisher.fetchTechDocsMetadata({ namespace, kind, name }), @@ -211,6 +218,7 @@ export class DocsSynchronizer { finish({ updated: false }); } } catch (e) { + assertError(e); // In case of error, log and allow the user to go about their business. this.logger.error( `Error syncing cache for ${entityTripletPath}: ${e.message}`, From ef04b09f75058cce47e0b35eebef533e82772b2d Mon Sep 17 00:00:00 2001 From: Eric Peterson Date: Fri, 12 Nov 2021 15:01:37 +0100 Subject: [PATCH 21/27] Make cache readTimeout configurable. Signed-off-by: Eric Peterson --- docs/features/techdocs/configuration.md | 6 +++++ plugins/techdocs-backend/config.d.ts | 9 +++++++ .../src/cache/TechDocsCache.test.ts | 27 +++++++++++++++++-- .../src/cache/TechDocsCache.ts | 26 ++++++++++++++++-- .../techdocs-backend/src/service/router.ts | 2 +- 5 files changed, 65 insertions(+), 5 deletions(-) diff --git a/docs/features/techdocs/configuration.md b/docs/features/techdocs/configuration.md index 3234cdc034..983a8dd3e7 100644 --- a/docs/features/techdocs/configuration.md +++ b/docs/features/techdocs/configuration.md @@ -144,6 +144,12 @@ techdocs: # to storage using the techdocs-cli, allowing long TTLs (e.g. 1 month/year) ttl: 3600000 + # (Optional) The time (in milliseconds) that the TechDocs backend will wait + # for a cache service to respond before continuing on as though the cached + # object was not found (e.g. when the cache sercice is unavailable). The + # default value is 1000 + readTimeout: 500 + # (Optional and Legacy) TechDocs makes API calls to techdocs-backend using this URL. e.g. get docs of an entity, get metadata, etc. # You don't have to specify this anymore. diff --git a/plugins/techdocs-backend/config.d.ts b/plugins/techdocs-backend/config.d.ts index 1767120a35..8d531e0536 100644 --- a/plugins/techdocs-backend/config.d.ts +++ b/plugins/techdocs-backend/config.d.ts @@ -241,6 +241,15 @@ export interface Config { * createRouter method in your backend. */ ttl: number; + + /** + * The time (in milliseconds) that the TechDocs backend will wait for + * a cache service to respond before continuing on as though the cached + * object was not found (e.g. when the cache sercice is unavailable). + * + * Defaults to 1000 milliseconds. + */ + readTimeout?: number; }; /** diff --git a/plugins/techdocs-backend/src/cache/TechDocsCache.test.ts b/plugins/techdocs-backend/src/cache/TechDocsCache.test.ts index e4308b7c3b..17056fc442 100644 --- a/plugins/techdocs-backend/src/cache/TechDocsCache.test.ts +++ b/plugins/techdocs-backend/src/cache/TechDocsCache.test.ts @@ -15,6 +15,7 @@ */ import { CacheClient, getVoidLogger } from '@backstage/backend-common'; +import { ConfigReader } from '@backstage/config'; import { CacheInvalidationError, TechDocsCache } from './TechDocsCache'; const cached = (str: string): string => { @@ -31,7 +32,7 @@ describe('TechDocsCache', () => { set: jest.fn(), delete: jest.fn(), }; - CacheUnderTest = new TechDocsCache({ + CacheUnderTest = TechDocsCache.fromConfig(new ConfigReader({}), { cache: MockClient, logger: getVoidLogger(), }); @@ -55,7 +56,7 @@ describe('TechDocsCache', () => { expect(actual).toBe(undefined); }); - it('returns undefined if no response after 1s', async () => { + it('returns undefined if no response after 1s by default', async () => { const expectedPath = 'some/index.html'; MockClient.get.mockImplementationOnce(() => { return new Promise(resolve => { @@ -66,6 +67,28 @@ describe('TechDocsCache', () => { expect(actual).toBe(undefined); }); + it('returns undefined if no response after configured readTimeout', async () => { + const expectedPath = 'some/index.html'; + MockClient.get.mockImplementationOnce(() => { + return new Promise(resolve => { + setTimeout(() => resolve(cached('value')), 20); + }); + }); + + CacheUnderTest = TechDocsCache.fromConfig( + new ConfigReader({ + techdocs: { cache: { readTimeout: 10 } }, + }), + { + cache: MockClient, + logger: getVoidLogger(), + }, + ); + + const actual = await CacheUnderTest.get(expectedPath); + expect(actual).toBe(undefined); + }); + it('returns data if cache get returns it', async () => { const expectedPath = 'some/index.html'; MockClient.get.mockResolvedValueOnce(cached('expected value')); diff --git a/plugins/techdocs-backend/src/cache/TechDocsCache.ts b/plugins/techdocs-backend/src/cache/TechDocsCache.ts index 83497608cf..b12142dda5 100644 --- a/plugins/techdocs-backend/src/cache/TechDocsCache.ts +++ b/plugins/techdocs-backend/src/cache/TechDocsCache.ts @@ -14,6 +14,8 @@ * limitations under the License. */ import { CacheClient } from '@backstage/backend-common'; +import { assertError } from '@backstage/errors'; +import { Config } from '@backstage/config'; import { Logger } from 'winston'; export class CacheInvalidationError extends Error { @@ -28,10 +30,29 @@ export class CacheInvalidationError extends Error { export class TechDocsCache { protected readonly cache: CacheClient; protected readonly logger: Logger; + protected readonly readTimeout: number; - constructor({ cache, logger }: { cache: CacheClient; logger: Logger }) { + private constructor({ + cache, + logger, + readTimeout, + }: { + cache: CacheClient; + logger: Logger; + readTimeout: number; + }) { this.cache = cache; this.logger = logger; + this.readTimeout = readTimeout; + } + + static fromConfig( + config: Config, + { cache, logger }: { cache: CacheClient; logger: Logger }, + ) { + const readTimeout = + config.getOptionalNumber('techdocs.cache.readTimeout') || 1000; + return new TechDocsCache({ cache, logger, readTimeout }); } async get(path: string): Promise { @@ -40,7 +61,7 @@ export class TechDocsCache { // temporarily unreachable. const response = (await Promise.race([ this.cache.get(path), - new Promise(cancelAfter => setTimeout(cancelAfter, 1000)), + new Promise(cancelAfter => setTimeout(cancelAfter, this.readTimeout)), ])) as string | undefined; if (response !== undefined) { @@ -51,6 +72,7 @@ export class TechDocsCache { this.logger.debug(`Cache miss: ${path}`); return response; } catch (e) { + assertError(e); this.logger.warn(`Error getting cache entry ${path}: ${e.message}`); this.logger.debug(e.stack); return undefined; diff --git a/plugins/techdocs-backend/src/service/router.ts b/plugins/techdocs-backend/src/service/router.ts index ba3fc6ea87..8f48342683 100644 --- a/plugins/techdocs-backend/src/service/router.ts +++ b/plugins/techdocs-backend/src/service/router.ts @@ -91,7 +91,7 @@ export async function createRouter( const defaultTtl = config.getOptionalNumber('techdocs.cache.ttl'); if (isOutOfTheBoxOption(options) && options.cache && defaultTtl) { const cacheClient = options.cache.getClient({ defaultTtl }); - cache = new TechDocsCache({ cache: cacheClient, logger }); + cache = TechDocsCache.fromConfig(config, { cache: cacheClient, logger }); } const scmIntegrations = ScmIntegrations.fromConfig(config); From e4adc768a5a4c7ea2c5d4ab987dad3d400cb4f5a Mon Sep 17 00:00:00 2001 From: Eric Peterson Date: Fri, 12 Nov 2021 15:07:39 +0100 Subject: [PATCH 22/27] Update architecture diagram. Signed-off-by: Eric Peterson --- .../architecture-recommended.drawio.svg | 25 +------------------ 1 file changed, 1 insertion(+), 24 deletions(-) diff --git a/docs/assets/techdocs/architecture-recommended.drawio.svg b/docs/assets/techdocs/architecture-recommended.drawio.svg index 8ddc9dc35a..1768b5dc86 100644 --- a/docs/assets/techdocs/architecture-recommended.drawio.svg +++ b/docs/assets/techdocs/architecture-recommended.drawio.svg @@ -1,4 +1,4 @@ - + @@ -383,29 +383,6 @@ - - - - - - - - -
-
-
- - Invalidate Objects (Optional) - -
-
-
- - - Invalidate Objects (Optional) - - - From e0930f578e0c40a898b516bd1e5c63f78d13dce3 Mon Sep 17 00:00:00 2001 From: Eric Peterson Date: Fri, 12 Nov 2021 17:18:06 +0100 Subject: [PATCH 23/27] en-US lowercase locale Signed-off-by: Eric Peterson --- plugins/techdocs-backend/src/service/DocsSynchronizer.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/techdocs-backend/src/service/DocsSynchronizer.ts b/plugins/techdocs-backend/src/service/DocsSynchronizer.ts index c7d5bf109a..4d407ab54a 100644 --- a/plugins/techdocs-backend/src/service/DocsSynchronizer.ts +++ b/plugins/techdocs-backend/src/service/DocsSynchronizer.ts @@ -186,7 +186,7 @@ export class DocsSynchronizer { ) || false; const tripletPath = `${namespace}/${kind}/${name}`; const entityTripletPath = `${ - legacyPathCasing ? tripletPath : tripletPath.toLocaleLowerCase() + legacyPathCasing ? tripletPath : tripletPath.toLocaleLowerCase('en-US') }`; try { const [sourceMetadata, cachedMetadata] = await Promise.all([ From 6e9e82b99de658f610553df6913da60fc1154468 Mon Sep 17 00:00:00 2001 From: Eric Peterson Date: Sun, 28 Nov 2021 13:33:11 -0700 Subject: [PATCH 24/27] Review feedback. Signed-off-by: Eric Peterson --- .../techdocs-common/src/stages/publish/local.ts | 4 +++- .../src/cache/TechDocsCache.test.ts | 4 +--- .../techdocs-backend/src/cache/TechDocsCache.ts | 16 ++++++---------- .../src/cache/cacheMiddleware.ts | 4 ++-- 4 files changed, 12 insertions(+), 16 deletions(-) diff --git a/packages/techdocs-common/src/stages/publish/local.ts b/packages/techdocs-common/src/stages/publish/local.ts index 91a9b57a9e..fc64776e4c 100644 --- a/packages/techdocs-common/src/stages/publish/local.ts +++ b/packages/techdocs-common/src/stages/publish/local.ts @@ -132,7 +132,9 @@ export class LocalPublish implements PublisherBase { }); return { - remoteUrl: `${techdocsApiUrl}/static/docs/${entity.metadata.name}`, + remoteUrl: `${techdocsApiUrl}/static/docs/${encodeURIComponent( + entity.metadata.name, + )}`, objects, }; } diff --git a/plugins/techdocs-backend/src/cache/TechDocsCache.test.ts b/plugins/techdocs-backend/src/cache/TechDocsCache.test.ts index 17056fc442..17613e1b58 100644 --- a/plugins/techdocs-backend/src/cache/TechDocsCache.test.ts +++ b/plugins/techdocs-backend/src/cache/TechDocsCache.test.ts @@ -162,9 +162,7 @@ describe('TechDocsCache', () => { await expect( CacheUnderTest.invalidateMultiple.bind(CacheUnderTest, expectedPaths), - ).rejects.toThrow( - expect.objectContaining({ rejections: expect.arrayContaining([]) }), - ); + ).rejects.toThrow(CacheInvalidationError); }); }); }); diff --git a/plugins/techdocs-backend/src/cache/TechDocsCache.ts b/plugins/techdocs-backend/src/cache/TechDocsCache.ts index b12142dda5..147aed0a18 100644 --- a/plugins/techdocs-backend/src/cache/TechDocsCache.ts +++ b/plugins/techdocs-backend/src/cache/TechDocsCache.ts @@ -14,18 +14,11 @@ * limitations under the License. */ import { CacheClient } from '@backstage/backend-common'; -import { assertError } from '@backstage/errors'; +import { assertError, CustomErrorBase } from '@backstage/errors'; import { Config } from '@backstage/config'; import { Logger } from 'winston'; -export class CacheInvalidationError extends Error { - public readonly rejections: PromiseRejectedResult[]; - - constructor(rejections: PromiseRejectedResult[]) { - super(); - this.rejections = rejections; - } -} +export class CacheInvalidationError extends CustomErrorBase {} export class TechDocsCache { protected readonly cache: CacheClient; @@ -101,7 +94,10 @@ export class TechDocsCache { ) as PromiseRejectedResult[]; if (rejected.length) { - throw new CacheInvalidationError(rejected); + throw new CacheInvalidationError( + 'TechDocs cache invalidation error', + rejected, + ); } return settled; diff --git a/plugins/techdocs-backend/src/cache/cacheMiddleware.ts b/plugins/techdocs-backend/src/cache/cacheMiddleware.ts index 66471b0189..9e545c235c 100644 --- a/plugins/techdocs-backend/src/cache/cacheMiddleware.ts +++ b/plugins/techdocs-backend/src/cache/cacheMiddleware.ts @@ -16,7 +16,7 @@ import { Router } from 'express'; import router from 'express-promise-router'; import { Logger } from 'winston'; -import { TechDocsCache } from '.'; +import { TechDocsCache } from './TechDocsCache'; type CacheMiddlewareOptions = { cache: TechDocsCache; @@ -69,7 +69,7 @@ export const createCacheMiddleware = ({ socket.on('close', hadError => { const content = Buffer.concat(chunks); const head = content.toString('utf8', 0, 12); - if (writeToCache && !hadError && head === 'HTTP/1.1 200') { + if (writeToCache && !hadError && head.match(/HTTP\/\d\.\d 200/)) { cache.set(reqPath, content); } }); From dcabc18f7edf7aca99f63c1e76c82f18d53f5e54 Mon Sep 17 00:00:00 2001 From: Eric Peterson Date: Sun, 28 Nov 2021 13:44:21 -0700 Subject: [PATCH 25/27] Fix tests after rebase. Signed-off-by: Eric Peterson --- packages/techdocs-common/src/stages/publish/awsS3.test.ts | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/packages/techdocs-common/src/stages/publish/awsS3.test.ts b/packages/techdocs-common/src/stages/publish/awsS3.test.ts index 591131907b..3e4c4c705b 100644 --- a/packages/techdocs-common/src/stages/publish/awsS3.test.ts +++ b/packages/techdocs-common/src/stages/publish/awsS3.test.ts @@ -203,7 +203,13 @@ describe('AwsS3Publish', () => { const publisher = createPublisherFromConfig({ sse: 'aws:kms', }); - expect(await publisher.publish({ entity, directory })).toBeUndefined(); + expect(await publisher.publish({ entity, directory })).toMatchObject({ + objects: expect.arrayContaining([ + 'default/component/backstage/404.html', + 'default/component/backstage/index.html', + 'default/component/backstage/assets/main.css', + ]), + }); }); it('should fail to publish a directory', async () => { From ef600b3c8cfb84cb02fdfed6bb767b42257f96da Mon Sep 17 00:00:00 2001 From: Eric Peterson Date: Sun, 5 Dec 2021 16:50:09 -0600 Subject: [PATCH 26/27] Review feedback. Signed-off-by: Eric Peterson --- packages/techdocs-common/src/stages/publish/local.ts | 10 ++++++---- plugins/techdocs-backend/package.json | 1 + plugins/techdocs-backend/src/cache/TechDocsCache.ts | 4 ++-- .../techdocs-backend/src/cache/cacheMiddleware.test.ts | 10 +++++----- plugins/techdocs-backend/src/cache/cacheMiddleware.ts | 6 +++--- 5 files changed, 17 insertions(+), 14 deletions(-) diff --git a/packages/techdocs-common/src/stages/publish/local.ts b/packages/techdocs-common/src/stages/publish/local.ts index fc64776e4c..9c146d2935 100644 --- a/packages/techdocs-common/src/stages/publish/local.ts +++ b/packages/techdocs-common/src/stages/publish/local.ts @@ -127,15 +127,17 @@ export class LocalPublish implements PublisherBase { // Generate publish response. const techdocsApiUrl = await this.discovery.getBaseUrl('techdocs'); - const objects = (await getFileTreeRecursively(publishDir)).map(abs => { - return abs.split(`${staticDocsDir}/`)[1]; - }); + const publishedFilePaths = (await getFileTreeRecursively(publishDir)).map( + abs => { + return abs.split(`${staticDocsDir}/`)[1]; + }, + ); return { remoteUrl: `${techdocsApiUrl}/static/docs/${encodeURIComponent( entity.metadata.name, )}`, - objects, + objects: publishedFilePaths, }; } diff --git a/plugins/techdocs-backend/package.json b/plugins/techdocs-backend/package.json index ffad4bc4c1..6f3495737a 100644 --- a/plugins/techdocs-backend/package.json +++ b/plugins/techdocs-backend/package.json @@ -40,6 +40,7 @@ "@backstage/search-common": "^0.2.1", "@backstage/techdocs-common": "^0.10.8", "@types/express": "^4.17.6", + "cross-fetch": "^3.0.6", "dockerode": "^3.3.1", "express": "^4.17.1", "express-promise-router": "^4.1.0", diff --git a/plugins/techdocs-backend/src/cache/TechDocsCache.ts b/plugins/techdocs-backend/src/cache/TechDocsCache.ts index 147aed0a18..807d6ca87b 100644 --- a/plugins/techdocs-backend/src/cache/TechDocsCache.ts +++ b/plugins/techdocs-backend/src/cache/TechDocsCache.ts @@ -43,8 +43,8 @@ export class TechDocsCache { config: Config, { cache, logger }: { cache: CacheClient; logger: Logger }, ) { - const readTimeout = - config.getOptionalNumber('techdocs.cache.readTimeout') || 1000; + const timeout = config.getOptionalNumber('techdocs.cache.readTimeout'); + const readTimeout = timeout === undefined ? 1000 : timeout; return new TechDocsCache({ cache, logger, readTimeout }); } diff --git a/plugins/techdocs-backend/src/cache/cacheMiddleware.test.ts b/plugins/techdocs-backend/src/cache/cacheMiddleware.test.ts index 91e4b84222..312674a5bf 100644 --- a/plugins/techdocs-backend/src/cache/cacheMiddleware.test.ts +++ b/plugins/techdocs-backend/src/cache/cacheMiddleware.test.ts @@ -59,7 +59,7 @@ describe('createCacheMiddleware', () => { app = express().use(router); app.use((req, res, next) => { // By default, send cacheable content. - if (req.path !== '/api/static/docs/error.png') { + if (req.path !== '/static/docs/error.png') { res.send('default-response'); } else { next(new Error()); @@ -70,7 +70,7 @@ describe('createCacheMiddleware', () => { describe('middleware', () => { it('does not apply to non-static/docs paths', async () => { await request(app) - .get('/api/static/not-docs') + .get('/static/not-docs') .expect(200, 'default-response'); expect(cache.set).not.toHaveBeenCalled(); @@ -79,7 +79,7 @@ describe('createCacheMiddleware', () => { it('responds with cached response', async () => { cache.get.mockResolvedValueOnce(getMockHttpResponseFor('xyz')); - await request(app).get('/api/static/docs/foo.html').expect(200, 'xyz'); + await request(app).get('/static/docs/foo.html').expect(200, 'xyz'); await waitForSocketClose(); expect(cache.set).not.toHaveBeenCalled(); @@ -88,7 +88,7 @@ describe('createCacheMiddleware', () => { it('sets cache when content is cacheable', async () => { const expectedPath = 'default/api/xyz/index.html'; await request(app) - .get(`/api/static/docs/${expectedPath}`) + .get(`/static/docs/${expectedPath}`) .expect(200, 'default-response'); await waitForSocketClose(); @@ -100,7 +100,7 @@ describe('createCacheMiddleware', () => { }); it('does not set cache on error', async () => { - await request(app).get('/api/static/docs/error.png').expect(500); + await request(app).get('/static/docs/error.png').expect(500); await waitForSocketClose(); expect(cache.set).not.toHaveBeenCalled(); diff --git a/plugins/techdocs-backend/src/cache/cacheMiddleware.ts b/plugins/techdocs-backend/src/cache/cacheMiddleware.ts index 9e545c235c..cb59681304 100644 --- a/plugins/techdocs-backend/src/cache/cacheMiddleware.ts +++ b/plugins/techdocs-backend/src/cache/cacheMiddleware.ts @@ -35,7 +35,7 @@ export const createCacheMiddleware = ({ // loaded from cache. Cache key is the object's path (after `/static/docs/`). cacheMiddleware.use(async (req, res, next) => { const socket = res.socket; - const isCacheable = req.path.includes('/static/docs/'); + const isCacheable = req.path.startsWith('/static/docs/'); // Continue early if this is non-cacheable, or there's no socket. if (!isCacheable || !socket) { @@ -66,11 +66,11 @@ export const createCacheMiddleware = ({ // When a socket is closed, if there were no errors and the data written // over the socket should be cached, cache it! - socket.on('close', hadError => { + socket.on('close', async hadError => { const content = Buffer.concat(chunks); const head = content.toString('utf8', 0, 12); if (writeToCache && !hadError && head.match(/HTTP\/\d\.\d 200/)) { - cache.set(reqPath, content); + await cache.set(reqPath, content); } }); From e3e6e9a86d2d654d1378e52adbf2f3ec03d00c2a Mon Sep 17 00:00:00 2001 From: Eric Peterson Date: Sun, 5 Dec 2021 17:05:07 -0600 Subject: [PATCH 27/27] One last docs update to clarify new architecture. Signed-off-by: Eric Peterson --- docs/features/techdocs/configuration.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/docs/features/techdocs/configuration.md b/docs/features/techdocs/configuration.md index 983a8dd3e7..6317ae365b 100644 --- a/docs/features/techdocs/configuration.md +++ b/docs/features/techdocs/configuration.md @@ -140,8 +140,9 @@ techdocs: # be configured with a valid cache store. cache: # Represents the number of milliseconds a statically built asset should - # stay cached. Cache invalidation is handled automatically if you publish - # to storage using the techdocs-cli, allowing long TTLs (e.g. 1 month/year) + # stay cached. Cache invalidation is handled automatically by the frontend, + # which compares the build times in cached metadata vs. canonical storage, + # allowing long TTLs (e.g. 1 month/year) ttl: 3600000 # (Optional) The time (in milliseconds) that the TechDocs backend will wait