From 057af3cac0961788501af151c409727c4a8e04e7 Mon Sep 17 00:00:00 2001 From: Himanshu Mishra Date: Sat, 6 Feb 2021 11:44:51 +0100 Subject: [PATCH] TechDocs: Implement etag based caching for git preparer Signed-off-by: Himanshu Mishra --- .../src/stages/prepare/commonGit.ts | 36 +++++++-- .../src/DocsBuilder/builder.ts | 76 ++++--------------- .../techdocs-backend/src/service/router.ts | 1 - 3 files changed, 41 insertions(+), 72 deletions(-) diff --git a/packages/techdocs-common/src/stages/prepare/commonGit.ts b/packages/techdocs-common/src/stages/prepare/commonGit.ts index 0445da8187..4f889d981c 100644 --- a/packages/techdocs-common/src/stages/prepare/commonGit.ts +++ b/packages/techdocs-common/src/stages/prepare/commonGit.ts @@ -13,12 +13,17 @@ * See the License for the specific language governing permissions and * limitations under the License. */ +import { NotModifiedError } from '@backstage/backend-common'; import { Entity } from '@backstage/catalog-model'; import { Config } from '@backstage/config'; import parseGitUrl from 'git-url-parse'; import path from 'path'; import { Logger } from 'winston'; -import { checkoutGitRepository, parseReferenceAnnotation } from '../../helpers'; +import { + checkoutGitRepository, + getLastCommitTimestamp, + parseReferenceAnnotation, +} from '../../helpers'; import { PreparerBase, PreparerResponse } from './types'; export class CommonGitPreparer implements PreparerBase { @@ -30,29 +35,44 @@ export class CommonGitPreparer implements PreparerBase { this.logger = logger; } - async prepare(entity: Entity): Promise { + async prepare( + entity: Entity, + options?: { etag?: string }, + ): Promise { const { target } = parseReferenceAnnotation( 'backstage.io/techdocs-ref', entity, ); try { + // Update repository or do a fresh clone. const repoPath = await checkoutGitRepository( target, this.config, this.logger, ); + + // Check if etag has changed for cache invalidation. + const etag = await getLastCommitTimestamp( + target, + this.config, + this.logger, + ); + if (options?.etag === etag.toString()) { + throw new NotModifiedError(); + } + const parsedGitLocation = parseGitUrl(target); - - // TODO: Return git commit sha - const etag = ''; - return { preparedDir: path.join(repoPath, parsedGitLocation.filepath), - etag, + etag: etag.toString(), }; } catch (error) { - this.logger.debug(`Repo checkout failed with error ${error.message}`); + if (error instanceof NotModifiedError) { + this.logger.debug(`Cache is valid for etag ${options?.etag}`); + } else { + this.logger.debug(`Repo checkout failed with error ${error.message}`); + } throw error; } } diff --git a/plugins/techdocs-backend/src/DocsBuilder/builder.ts b/plugins/techdocs-backend/src/DocsBuilder/builder.ts index 768c4d3a5f..8cf278dc87 100644 --- a/plugins/techdocs-backend/src/DocsBuilder/builder.ts +++ b/plugins/techdocs-backend/src/DocsBuilder/builder.ts @@ -14,15 +14,14 @@ * limitations under the License. */ import { Entity } from '@backstage/catalog-model'; -import { Config } from '@backstage/config'; import { GeneratorBase, GeneratorBuilder, - getLastCommitTimestamp, getLocationForEntity, PreparerBase, PreparerBuilder, PublisherBase, + UrlPreparer, } from '@backstage/techdocs-common'; import Docker from 'dockerode'; import fs from 'fs-extra'; @@ -44,7 +43,6 @@ type DocsBuilderArguments = { entity: Entity; logger: Logger; dockerClient: Docker; - config: Config; }; export class DocsBuilder { @@ -54,7 +52,6 @@ export class DocsBuilder { private entity: Entity; private logger: Logger; private dockerClient: Docker; - private config: Config; constructor({ preparers, @@ -63,7 +60,6 @@ export class DocsBuilder { entity, logger, dockerClient, - config, }: DocsBuilderArguments) { this.preparer = preparers.get(entity); this.generator = generators.get(entity); @@ -71,7 +67,6 @@ export class DocsBuilder { this.entity = entity; this.logger = logger; this.dockerClient = dockerClient; - this.config = config; } public async build(): Promise { @@ -127,14 +122,18 @@ export class DocsBuilder { 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}`); + // Can not remove prepared directory in case of git preparer since the local git repository + // is used to get etag on subsequent requests. + if (this.preparer instanceof UrlPreparer) { + 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)}`); @@ -155,53 +154,4 @@ export class DocsBuilder { // Store the latest build etag for the entity new BuildMetadataStorage(this.entity.metadata.uid).setEtag(etag); } - - public async docsUpToDate() { - if (!this.entity.metadata.uid) { - throw new Error( - 'Trying to build documentation for entity not in service catalog', - ); - } - - const buildMetadataStorage = new BuildMetadataStorage( - this.entity.metadata.uid, - ); - const { type, target } = getLocationForEntity(this.entity); - - // Unless docs are stored locally - const nonAgeCheckTypes = ['dir', 'file', 'url']; - if (!nonAgeCheckTypes.includes(type)) { - const lastCommit = await getLastCommitTimestamp( - target, - this.config, - this.logger, - ); - const storageTimeStamp = buildMetadataStorage.getTimestamp(); - - // Check if documentation source is newer than what we have - if (storageTimeStamp && storageTimeStamp >= lastCommit) { - this.logger.debug( - `Docs for entity ${getEntityId(this.entity)} is up to date.`, - ); - return true; - } - } - - // Cache downloaded source files for 30 minutes. - // TODO: When urlReader/readTree supports some way to get latest commit timestamp, - // it should be used to invalidate cache. - if (type === 'url') { - const builtAt = buildMetadataStorage.getTimestamp(); - const now = Date.now(); - - if (builtAt > now - 1800000) { - return true; - } - } - - this.logger.debug( - `Docs for entity ${getEntityId(this.entity)} was outdated.`, - ); - return false; - } } diff --git a/plugins/techdocs-backend/src/service/router.ts b/plugins/techdocs-backend/src/service/router.ts index a379e47bfb..b2053926ca 100644 --- a/plugins/techdocs-backend/src/service/router.ts +++ b/plugins/techdocs-backend/src/service/router.ts @@ -141,7 +141,6 @@ export async function createRouter({ dockerClient, logger, entity, - config, }); switch (publisherType) { case 'local':