From 97a9a7365fdc8d1165e73e8051b89822e3e33f4e Mon Sep 17 00:00:00 2001 From: Himanshu Mishra Date: Sat, 6 Feb 2021 11:06:38 +0100 Subject: [PATCH] TechDocs: Implement etag based caching for url preparer Signed-off-by: Himanshu Mishra --- packages/techdocs-common/src/helpers.test.ts | 5 +- packages/techdocs-common/src/helpers.ts | 26 ++++--- .../src/stages/prepare/types.ts | 4 +- .../techdocs-common/src/stages/prepare/url.ts | 29 ++++---- .../src/stages/publish/local.ts | 2 +- .../src/DocsBuilder/BuildMetadataStorage.ts | 10 +-- .../src/DocsBuilder/builder.ts | 69 +++++++++++++++---- 7 files changed, 99 insertions(+), 46 deletions(-) diff --git a/packages/techdocs-common/src/helpers.test.ts b/packages/techdocs-common/src/helpers.test.ts index eba1386b11..6520f0f8e4 100644 --- a/packages/techdocs-common/src/helpers.test.ts +++ b/packages/techdocs-common/src/helpers.test.ts @@ -139,7 +139,7 @@ describe('getDocFilesFromRepository', () => { archive: async () => { return Readable.from(''); }, - etag: '', + etag: 'etag123abc', }; } @@ -156,6 +156,7 @@ describe('getDocFilesFromRepository', () => { mockEntityWithAnnotation, ); - expect(output).toBe('/tmp/testfolder'); + expect(output.preparedDir).toBe('/tmp/testfolder'); + expect(output.etag).toBe('etag123abc'); }); }); diff --git a/packages/techdocs-common/src/helpers.ts b/packages/techdocs-common/src/helpers.ts index f70b0f3840..bfbbd5b379 100644 --- a/packages/techdocs-common/src/helpers.ts +++ b/packages/techdocs-common/src/helpers.ts @@ -14,17 +14,17 @@ * limitations under the License. */ -import os from 'os'; -import path from 'path'; -import parseGitUrl from 'git-url-parse'; -import fs from 'fs-extra'; -import { InputError, UrlReader, Git } from '@backstage/backend-common'; +import { Git, InputError, UrlReader } from '@backstage/backend-common'; import { Entity } from '@backstage/catalog-model'; import { Config } from '@backstage/config'; +import fs from 'fs-extra'; +import parseGitUrl from 'git-url-parse'; +import os from 'os'; +import path from 'path'; +import { Logger } from 'winston'; import { getDefaultBranch } from './default-branch'; import { getGitRepoType, getTokenForGitRepo } from './git-auth'; -import { RemoteProtocol } from './stages/prepare/types'; -import { Logger } from 'winston'; +import { PreparerResponse, RemoteProtocol } from './stages/prepare/types'; export type ParsedLocationAnnotation = { type: RemoteProtocol; @@ -216,13 +216,19 @@ export const getLastCommitTimestamp = async ( export const getDocFilesFromRepository = async ( reader: UrlReader, entity: Entity, -): Promise => { + opts?: { etag?: string }, +): Promise => { const { target } = parseReferenceAnnotation( 'backstage.io/techdocs-ref', entity, ); - const response = await reader.readTree(target); + // readTree will throw NotModifiedError if etag has not changed. + const readTreeResponse = await reader.readTree(target, { etag: opts?.etag }); + const preparedDir = await readTreeResponse.dir(); - return await response.dir(); + return { + preparedDir, + etag: readTreeResponse.etag, + }; }; diff --git a/packages/techdocs-common/src/stages/prepare/types.ts b/packages/techdocs-common/src/stages/prepare/types.ts index c8ea9c611a..96510e8eec 100644 --- a/packages/techdocs-common/src/stages/prepare/types.ts +++ b/packages/techdocs-common/src/stages/prepare/types.ts @@ -33,13 +33,13 @@ export type PreparerBase = { * with contents from the location in temporary storage and return the path. * * @param entity The entity from the Service Catalog - * @param opts.etag (Optional) If etag is provider, it will be used to check if the target has + * @param options.etag (Optional) If etag is provider, it will be used to check if the target has * updated since the last build. * @throws {NotModifiedError} when the prepared directory has not been changed since the last build. */ prepare( entity: Entity, - opts?: { logger: Logger; etag?: string }, + options?: { logger?: Logger; etag?: string }, ): Promise; }; diff --git a/packages/techdocs-common/src/stages/prepare/url.ts b/packages/techdocs-common/src/stages/prepare/url.ts index c10822dace..60f176ffb4 100644 --- a/packages/techdocs-common/src/stages/prepare/url.ts +++ b/packages/techdocs-common/src/stages/prepare/url.ts @@ -13,7 +13,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -import { UrlReader } from '@backstage/backend-common'; +import { NotModifiedError, UrlReader } from '@backstage/backend-common'; import { Entity } from '@backstage/catalog-model'; import { Logger } from 'winston'; import { getDocFilesFromRepository } from '../../helpers'; @@ -28,19 +28,24 @@ export class UrlPreparer implements PreparerBase { this.reader = reader; } - async prepare(entity: Entity): Promise { + async prepare( + entity: Entity, + options?: { etag?: string }, + ): Promise { try { - const preparedDir = await getDocFilesFromRepository(this.reader, entity); - // TODO: This will be actual etag from URL Reader. - const etag = ''; - return { - preparedDir, - etag, - }; + return await getDocFilesFromRepository(this.reader, entity, { + etag: options?.etag, + }); } catch (error) { - this.logger.debug( - `Unable to fetch files for building docs ${error.message}`, - ); + // NotModifiedError means that etag based cache is still valid. + if (error instanceof NotModifiedError) { + this.logger.debug(`Cache is valid for etag ${options?.etag}`); + } else { + this.logger.debug( + `Unable to fetch files for building docs ${error.message}`, + ); + } + throw error; } } diff --git a/packages/techdocs-common/src/stages/publish/local.ts b/packages/techdocs-common/src/stages/publish/local.ts index e349a2119a..ac09331dcd 100644 --- a/packages/techdocs-common/src/stages/publish/local.ts +++ b/packages/techdocs-common/src/stages/publish/local.ts @@ -88,7 +88,7 @@ export class LocalPublish implements PublisherBase { ); reject(err); } - + this.logger.info(`Published site stored at ${publishDir}`); this.discovery .getBaseUrl('techdocs') .then(techdocsApiUrl => { diff --git a/plugins/techdocs-backend/src/DocsBuilder/BuildMetadataStorage.ts b/plugins/techdocs-backend/src/DocsBuilder/BuildMetadataStorage.ts index eecb161074..a1bb918fae 100644 --- a/plugins/techdocs-backend/src/DocsBuilder/BuildMetadataStorage.ts +++ b/plugins/techdocs-backend/src/DocsBuilder/BuildMetadataStorage.ts @@ -14,8 +14,8 @@ * limitations under the License. */ type buildInfo = { - // uid: timestamp - [key: string]: number; + // Entity uid: etag + [key: string]: string; }; // TODO: Build info should be part of TechDocs storage, inside `techdocs_metadata.json` @@ -39,11 +39,11 @@ export class BuildMetadataStorage { this.builds = builds; } - storeBuildTimestamp() { - this.builds[this.entityUid] = Date.now(); + setEtag(etag: string): void { + this.builds[this.entityUid] = etag; } - getTimestamp() { + getEtag(): string | undefined { return this.builds[this.entityUid]; } } diff --git a/plugins/techdocs-backend/src/DocsBuilder/builder.ts b/plugins/techdocs-backend/src/DocsBuilder/builder.ts index 7a3cbef1e9..768c4d3a5f 100644 --- a/plugins/techdocs-backend/src/DocsBuilder/builder.ts +++ b/plugins/techdocs-backend/src/DocsBuilder/builder.ts @@ -74,14 +74,40 @@ export class DocsBuilder { this.config = config; } - public async build() { - this.logger.info(`Running preparer on entity ${getEntityId(this.entity)}`); - // TODO: This will throw an error if no further build is required, site is cached. - // TODO: Use etag from here and store in memory. - const { preparedDir } = await this.preparer.prepare(this.entity); + public async build(): Promise { + if (!this.entity.metadata.uid) { + throw new Error( + 'Trying to build documentation for entity not in service catalog', + ); + } - const parsedLocationAnnotation = getLocationForEntity(this.entity); + // Use the in-memory storage for setting and getting etag for this entity. + const buildMetadataStorage = new BuildMetadataStorage( + this.entity.metadata.uid, + ); + // TODO: As of now, this happens on each and every request to TechDocs. + // In a high traffic environment, this will cause a lot of requests to the source code provider. + // After Async build is implemented https://github.com/backstage/backstage/issues/3717, + // make sure to limit checking for cache invalidation to once per minute or so. + let preparedDir: string; + let etag: string; + try { + const preparerResponse = await this.preparer.prepare(this.entity, { + etag: buildMetadataStorage.getEtag(), + }); + + preparedDir = preparerResponse.preparedDir; + etag = preparerResponse.etag; + } catch (NotModifiedError) { + // No need to prepare anymore since cache is valid. + return; + } + + this.logger.info( + `TechDocs prepare step completed for entity ${getEntityId(this.entity)}.`, + ); + this.logger.debug(`Prepared files temporarily stored at ${preparedDir}`); this.logger.info(`Running generator on entity ${getEntityId(this.entity)}`); // Create a temporary directory to store the generated files in. const tmpdirPath = os.tmpdir(); @@ -91,6 +117,7 @@ export class DocsBuilder { path.join(tmpdirResolvedPath, 'techdocs-tmp-'), ); + const parsedLocationAnnotation = getLocationForEntity(this.entity); await this.generator.run({ inputDir: preparedDir, outputDir, @@ -98,21 +125,35 @@ export class DocsBuilder { parsedLocationAnnotation, }); + this.logger.debug(`Generated files temporarily stored at ${outputDir}`); + // Remove Prepared directory + this.logger.debug( + `Removing prepared directory ${preparedDir} since the site has been generated.`, + ); + try { + // Not a blocker hence no need to await this. + fs.remove(preparedDir); + } catch (error) { + this.logger.debug(`Error removing prepared directory ${error.message}`); + } + this.logger.info(`Running publisher on entity ${getEntityId(this.entity)}`); await this.publisher.publish({ entity: this.entity, directory: outputDir, }); - - // TODO: Remove the generated directory once published. - - if (!this.entity.metadata.uid) { - throw new Error( - 'Trying to build documentation for entity not in service catalog', - ); + this.logger.debug( + `Removing generated directory ${outputDir} since the site has been published`, + ); + try { + // Not a blocker hence no need to await this. + fs.remove(outputDir); + } catch (error) { + this.logger.debug(`Error removing generated directory ${error.message}`); } - new BuildMetadataStorage(this.entity.metadata.uid).storeBuildTimestamp(); + // Store the latest build etag for the entity + new BuildMetadataStorage(this.entity.metadata.uid).setEtag(etag); } public async docsUpToDate() {