From 66944ec7bf5641320bb3fc9a9cf3ea75754c8bd5 Mon Sep 17 00:00:00 2001
From: Himanshu Mishra <himanshu@orkohunter.net>
Date: Sat, 6 Feb 2021 09:52:01 +0100
Subject: [PATCH 01/10] TechDocs: Preparers will take and return an etag for
 cache invalidation

---
 docs/features/techdocs/getting-started.md     | 14 ++++---
 .../src/stages/prepare/commonGit.test.ts      | 14 +++----
 .../src/stages/prepare/commonGit.ts           | 19 ++++++----
 .../src/stages/prepare/dir.test.ts            | 15 ++++----
 .../techdocs-common/src/stages/prepare/dir.ts | 20 ++++++----
 .../src/stages/prepare/types.ts               | 30 +++++++++++++--
 .../techdocs-common/src/stages/prepare/url.ts | 16 +++++---
 .../src/DocsBuilder/BuildMetadataStorage.ts   |  5 +++
 .../src/DocsBuilder/builder.ts                | 24 ++++++------
 .../techdocs-backend/src/service/router.ts    | 38 +++++++++----------
 10 files changed, 120 insertions(+), 75 deletions(-)

diff --git a/docs/features/techdocs/getting-started.md b/docs/features/techdocs/getting-started.md
index e0b323857d..efed896fd0 100644
--- a/docs/features/techdocs/getting-started.md
+++ b/docs/features/techdocs/getting-started.md
@@ -157,15 +157,17 @@ techdocs:
   builder: 'local'
 ```
 
-Set `techdocs.builder` to `'local'` if you want your TechDocs Backend to be
-responsible for generating documentation sites. If set to `'external'`,
-Backstage will assume that the sites are being generated on each entity's CI/CD
-pipeline, and are being stored in a storage somewhere.
+Note that we recommend generating docs on CI/CD instead. Read more in the
+"Basic" and "Recommended" sections of the
+[TechDocs Architecture](architecture.md). But if you want to get started quickly
+set `techdocs.builder` to `'local'` so that TechDocs Backend is responsible for
+generating documentation sites. If set to `'external'`, Backstage will assume
+that the sites are being generated on each entity's CI/CD pipeline, and are
+being stored in a storage somewhere.
 
 When `techdocs.builder` is set to `'external'`, TechDocs becomes more or less a
 read-only experience where it serves static files from a storage containing all
-the generated documentation. Read more in the "Basic" and "Recommended" sections
-of the [TechDocs Architecture](architecture.md).
+the generated documentation.
 
 ### Choosing storage (publisher)
 
diff --git a/packages/techdocs-common/src/stages/prepare/commonGit.test.ts b/packages/techdocs-common/src/stages/prepare/commonGit.test.ts
index 704cd23daa..b715b27407 100644
--- a/packages/techdocs-common/src/stages/prepare/commonGit.test.ts
+++ b/packages/techdocs-common/src/stages/prepare/commonGit.test.ts
@@ -16,8 +16,8 @@
 
 import { getVoidLogger } from '@backstage/backend-common';
 import { ConfigReader } from '@backstage/config';
-import { CommonGitPreparer } from './commonGit';
 import { checkoutGitRepository } from '../../helpers';
+import { CommonGitPreparer } from './commonGit';
 
 function normalizePath(path: string) {
   return path
@@ -57,9 +57,9 @@ describe('commonGit preparer', () => {
         'github:https://github.com/backstage/backstage/blob/master/plugins/techdocs-backend/examples/documented-component',
     });
 
-    const tempDocsPath = await preparer.prepare(mockEntity);
+    const { preparedDir } = await preparer.prepare(mockEntity);
     expect(checkoutGitRepository).toHaveBeenCalledTimes(1);
-    expect(normalizePath(tempDocsPath)).toEqual(
+    expect(normalizePath(preparedDir)).toEqual(
       '/tmp/backstage-repo/org/name/branch/plugins/techdocs-backend/examples/documented-component',
     );
   });
@@ -72,9 +72,9 @@ describe('commonGit preparer', () => {
         'gitlab:https://gitlab.com/xesjkeee/go-logger/blob/master/catalog-info.yaml',
     });
 
-    const tempDocsPath = await preparer.prepare(mockEntity);
+    const { preparedDir } = await preparer.prepare(mockEntity);
     expect(checkoutGitRepository).toHaveBeenCalledTimes(2);
-    expect(normalizePath(tempDocsPath)).toEqual(
+    expect(normalizePath(preparedDir)).toEqual(
       '/tmp/backstage-repo/org/name/branch/catalog-info.yaml',
     );
   });
@@ -87,9 +87,9 @@ describe('commonGit preparer', () => {
         'azure/api:https://dev.azure.com/backstage-org/backstage-project/_git/template-repo?path=%2Ftemplate.yaml',
     });
 
-    const tempDocsPath = await preparer.prepare(mockEntity);
+    const { preparedDir } = await preparer.prepare(mockEntity);
     expect(checkoutGitRepository).toHaveBeenCalledTimes(3);
-    expect(normalizePath(tempDocsPath)).toEqual(
+    expect(normalizePath(preparedDir)).toEqual(
       '/tmp/backstage-repo/org/name/branch/template.yaml',
     );
   });
diff --git a/packages/techdocs-common/src/stages/prepare/commonGit.ts b/packages/techdocs-common/src/stages/prepare/commonGit.ts
index 7eac07d76f..0445da8187 100644
--- a/packages/techdocs-common/src/stages/prepare/commonGit.ts
+++ b/packages/techdocs-common/src/stages/prepare/commonGit.ts
@@ -13,14 +13,13 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-import path from 'path';
-import parseGitUrl from 'git-url-parse';
 import { Entity } from '@backstage/catalog-model';
 import { Config } from '@backstage/config';
-import { PreparerBase } from './types';
-import { parseReferenceAnnotation, checkoutGitRepository } from '../../helpers';
-
+import parseGitUrl from 'git-url-parse';
+import path from 'path';
 import { Logger } from 'winston';
+import { checkoutGitRepository, parseReferenceAnnotation } from '../../helpers';
+import { PreparerBase, PreparerResponse } from './types';
 
 export class CommonGitPreparer implements PreparerBase {
   private readonly config: Config;
@@ -31,7 +30,7 @@ export class CommonGitPreparer implements PreparerBase {
     this.logger = logger;
   }
 
-  async prepare(entity: Entity): Promise<string> {
+  async prepare(entity: Entity): Promise<PreparerResponse> {
     const { target } = parseReferenceAnnotation(
       'backstage.io/techdocs-ref',
       entity,
@@ -45,7 +44,13 @@ export class CommonGitPreparer implements PreparerBase {
       );
       const parsedGitLocation = parseGitUrl(target);
 
-      return path.join(repoPath, parsedGitLocation.filepath);
+      // TODO: Return git commit sha
+      const etag = '';
+
+      return {
+        preparedDir: path.join(repoPath, parsedGitLocation.filepath),
+        etag,
+      };
     } catch (error) {
       this.logger.debug(`Repo checkout failed with error ${error.message}`);
       throw error;
diff --git a/packages/techdocs-common/src/stages/prepare/dir.test.ts b/packages/techdocs-common/src/stages/prepare/dir.test.ts
index cd8d358afe..3538175622 100644
--- a/packages/techdocs-common/src/stages/prepare/dir.test.ts
+++ b/packages/techdocs-common/src/stages/prepare/dir.test.ts
@@ -15,8 +15,8 @@
  */
 import { getVoidLogger, UrlReader } from '@backstage/backend-common';
 import { ConfigReader } from '@backstage/config';
-import { DirectoryPreparer } from './dir';
 import { checkoutGitRepository } from '../../helpers';
+import { DirectoryPreparer } from './dir';
 
 function normalizePath(path: string) {
   return path
@@ -66,9 +66,8 @@ describe('directory preparer', () => {
       'backstage.io/techdocs-ref': 'dir:./our-documentation',
     });
 
-    expect(normalizePath(await directoryPreparer.prepare(mockEntity))).toEqual(
-      '/directory/our-documentation',
-    );
+    const { preparedDir } = await directoryPreparer.prepare(mockEntity);
+    expect(normalizePath(preparedDir)).toEqual('/directory/our-documentation');
   });
 
   it('should merge managed-by-location and techdocs-ref when techdocs-ref is absolute', async () => {
@@ -84,9 +83,8 @@ describe('directory preparer', () => {
       'backstage.io/techdocs-ref': 'dir:/our-documentation/techdocs',
     });
 
-    expect(normalizePath(await directoryPreparer.prepare(mockEntity))).toEqual(
-      '/our-documentation/techdocs',
-    );
+    const { preparedDir } = await directoryPreparer.prepare(mockEntity);
+    expect(normalizePath(preparedDir)).toEqual('/our-documentation/techdocs');
   });
 
   it('should merge managed-by-location and techdocs-ref when managed-by-location is a git repository', async () => {
@@ -102,7 +100,8 @@ describe('directory preparer', () => {
       'backstage.io/techdocs-ref': 'dir:./docs',
     });
 
-    expect(normalizePath(await directoryPreparer.prepare(mockEntity))).toEqual(
+    const { preparedDir } = await directoryPreparer.prepare(mockEntity);
+    expect(normalizePath(preparedDir)).toEqual(
       '/tmp/backstage-repo/org/name/branch/docs',
     );
     expect(checkoutGitRepository).toHaveBeenCalledTimes(1);
diff --git a/packages/techdocs-common/src/stages/prepare/dir.ts b/packages/techdocs-common/src/stages/prepare/dir.ts
index ab277fda9a..e59a3f02b6 100644
--- a/packages/techdocs-common/src/stages/prepare/dir.ts
+++ b/packages/techdocs-common/src/stages/prepare/dir.ts
@@ -13,14 +13,14 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-import { PreparerBase } from './types';
+import { InputError, UrlReader } from '@backstage/backend-common';
 import { Entity } from '@backstage/catalog-model';
 import { Config } from '@backstage/config';
-import path from 'path';
-import { parseReferenceAnnotation, checkoutGitRepository } from '../../helpers';
-import { UrlReader, InputError } from '@backstage/backend-common';
 import parseGitUrl from 'git-url-parse';
+import path from 'path';
 import { Logger } from 'winston';
+import { checkoutGitRepository, parseReferenceAnnotation } from '../../helpers';
+import { PreparerBase, PreparerResponse } from './types';
 
 export class DirectoryPreparer implements PreparerBase {
   constructor(
@@ -68,7 +68,7 @@ export class DirectoryPreparer implements PreparerBase {
     }
   }
 
-  async prepare(entity: Entity): Promise<string> {
+  async prepare(entity: Entity): Promise<PreparerResponse> {
     const { target } = parseReferenceAnnotation(
       'backstage.io/techdocs-ref',
       entity,
@@ -78,8 +78,12 @@ export class DirectoryPreparer implements PreparerBase {
       entity,
     );
 
-    return new Promise(resolve => {
-      resolve(path.resolve(managedByLocationDirectory, target));
-    });
+    // TODO: etag will be returned as a commit sha from resolveManagedByLocationToDir.
+    const etag = '';
+
+    return {
+      preparedDir: path.resolve(managedByLocationDirectory, target),
+      etag,
+    };
   }
 }
diff --git a/packages/techdocs-common/src/stages/prepare/types.ts b/packages/techdocs-common/src/stages/prepare/types.ts
index 97e75306c3..c8ea9c611a 100644
--- a/packages/techdocs-common/src/stages/prepare/types.ts
+++ b/packages/techdocs-common/src/stages/prepare/types.ts
@@ -16,13 +16,31 @@
 import type { Entity } from '@backstage/catalog-model';
 import { Logger } from 'winston';
 
+export type PreparerResponse = {
+  /**
+   * The path to directory where the tree is downloaded.
+   */
+  preparedDir: string;
+  /**
+   * A unique identifer of the tree blob, usually the commit SHA or etag from the target.
+   */
+  etag: string;
+};
+
 export type PreparerBase = {
   /**
    * Given an Entity definition from the Service Catalog, go and prepare a directory
-   * with contents from the location in temporary storage and return the path
+   * 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
+   * 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 }): Promise<string>;
+  prepare(
+    entity: Entity,
+    opts?: { logger: Logger; etag?: string },
+  ): Promise<PreparerResponse>;
 };
 
 export type PreparerBuilder = {
@@ -30,10 +48,14 @@ export type PreparerBuilder = {
   get(entity: Entity): PreparerBase;
 };
 
+/**
+ * Everything except `url` will be deprecated.
+ * Read more https://github.com/backstage/backstage/issues/4409
+ */
 export type RemoteProtocol =
+  | 'url'
   | 'dir'
   | 'github'
   | 'gitlab'
   | 'file'
-  | 'azure/api'
-  | 'url';
+  | 'azure/api';
diff --git a/packages/techdocs-common/src/stages/prepare/url.ts b/packages/techdocs-common/src/stages/prepare/url.ts
index b938c6b5bc..c10822dace 100644
--- a/packages/techdocs-common/src/stages/prepare/url.ts
+++ b/packages/techdocs-common/src/stages/prepare/url.ts
@@ -13,11 +13,11 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-import { Logger } from 'winston';
-import { Entity } from '@backstage/catalog-model';
 import { UrlReader } from '@backstage/backend-common';
-import { PreparerBase } from './types';
+import { Entity } from '@backstage/catalog-model';
+import { Logger } from 'winston';
 import { getDocFilesFromRepository } from '../../helpers';
+import { PreparerBase, PreparerResponse } from './types';
 
 export class UrlPreparer implements PreparerBase {
   private readonly logger: Logger;
@@ -28,9 +28,15 @@ export class UrlPreparer implements PreparerBase {
     this.reader = reader;
   }
 
-  async prepare(entity: Entity): Promise<string> {
+  async prepare(entity: Entity): Promise<PreparerResponse> {
     try {
-      return getDocFilesFromRepository(this.reader, entity);
+      const preparedDir = await getDocFilesFromRepository(this.reader, entity);
+      // TODO: This will be actual etag from URL Reader.
+      const etag = '';
+      return {
+        preparedDir,
+        etag,
+      };
     } catch (error) {
       this.logger.debug(
         `Unable to fetch files for building docs ${error.message}`,
diff --git a/plugins/techdocs-backend/src/DocsBuilder/BuildMetadataStorage.ts b/plugins/techdocs-backend/src/DocsBuilder/BuildMetadataStorage.ts
index d6a19764d9..eecb161074 100644
--- a/plugins/techdocs-backend/src/DocsBuilder/BuildMetadataStorage.ts
+++ b/plugins/techdocs-backend/src/DocsBuilder/BuildMetadataStorage.ts
@@ -18,6 +18,11 @@ type buildInfo = {
   [key: string]: number;
 };
 
+// TODO: Build info should be part of TechDocs storage, inside `techdocs_metadata.json`
+// instead of in-memory storage of the Backstage instance.
+// In case of multi-region Backstage deployments, or even using multiple Kubernetes pods,
+// if each instance creates its separate build info in-memory, it will result in duplicate
+// builds per instance. Also if the pod restarts, all the sites will have to be re-built.
 const builds = {} as buildInfo;
 
 /**
diff --git a/plugins/techdocs-backend/src/DocsBuilder/builder.ts b/plugins/techdocs-backend/src/DocsBuilder/builder.ts
index 33d3120c47..7a3cbef1e9 100644
--- a/plugins/techdocs-backend/src/DocsBuilder/builder.ts
+++ b/plugins/techdocs-backend/src/DocsBuilder/builder.ts
@@ -13,22 +13,22 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-import fs from 'fs-extra';
-import os from 'os';
-import path from 'path';
-import Docker from 'dockerode';
-import { Logger } from 'winston';
 import { Entity } from '@backstage/catalog-model';
 import { Config } from '@backstage/config';
 import {
+  GeneratorBase,
+  GeneratorBuilder,
+  getLastCommitTimestamp,
+  getLocationForEntity,
+  PreparerBase,
   PreparerBuilder,
   PublisherBase,
-  GeneratorBuilder,
-  PreparerBase,
-  GeneratorBase,
-  getLocationForEntity,
-  getLastCommitTimestamp,
 } from '@backstage/techdocs-common';
+import Docker from 'dockerode';
+import fs from 'fs-extra';
+import os from 'os';
+import path from 'path';
+import { Logger } from 'winston';
 import { BuildMetadataStorage } from '.';
 
 const getEntityId = (entity: Entity) => {
@@ -76,7 +76,9 @@ export class DocsBuilder {
 
   public async build() {
     this.logger.info(`Running preparer on entity ${getEntityId(this.entity)}`);
-    const preparedDir = await this.preparer.prepare(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);
 
     const parsedLocationAnnotation = getLocationForEntity(this.entity);
 
diff --git a/plugins/techdocs-backend/src/service/router.ts b/plugins/techdocs-backend/src/service/router.ts
index fe7b52ab52..a379e47bfb 100644
--- a/plugins/techdocs-backend/src/service/router.ts
+++ b/plugins/techdocs-backend/src/service/router.ts
@@ -13,23 +13,23 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-import { Logger } from 'winston';
-import Router from 'express-promise-router';
-import express from 'express';
-import Knex from 'knex';
-import fetch from 'cross-fetch';
-import { Config } from '@backstage/config';
-import Docker from 'dockerode';
-import {
-  GeneratorBuilder,
-  PreparerBuilder,
-  PublisherBase,
-  getLocationForEntity,
-} from '@backstage/techdocs-common';
 import { PluginEndpointDiscovery } from '@backstage/backend-common';
 import { Entity } from '@backstage/catalog-model';
-import { getEntityNameFromUrlPath } from './helpers';
+import { Config } from '@backstage/config';
+import {
+  GeneratorBuilder,
+  getLocationForEntity,
+  PreparerBuilder,
+  PublisherBase,
+} from '@backstage/techdocs-common';
+import fetch from 'cross-fetch';
+import Docker from 'dockerode';
+import express from 'express';
+import Router from 'express-promise-router';
+import Knex from 'knex';
+import { Logger } from 'winston';
 import { DocsBuilder } from '../DocsBuilder';
+import { getEntityNameFromUrlPath } from './helpers';
 
 type RouterOptions = {
   preparers: PreparerBuilder;
@@ -145,9 +145,7 @@ export async function createRouter({
       });
       switch (publisherType) {
         case 'local':
-          if (!(await docsBuilder.docsUpToDate())) {
-            await docsBuilder.build();
-          }
+          await docsBuilder.build();
           break;
         case 'awsS3':
         case 'azureBlobStorage':
@@ -186,9 +184,11 @@ export async function createRouter({
               'Found pre-generated docs for this entity. Serving them.',
             );
             // TODO: re-trigger build for cache invalidation.
-            // Compare the date modified of the requested file on storage and compare it against
-            // the last modified or last commit timestamp in the repository.
+            // Add build info in techdocs_metadata.json and compare it against
+            // the eTag/commit in the repository.
             // Without this, docs will not be re-built once they have been generated.
+            // Although it is unconventional that anyone will face this issue - because
+            // if you have an external storage, you should be using CI/CD to build and publish docs.
           }
           break;
         default:

From 97a9a7365fdc8d1165e73e8051b89822e3e33f4e Mon Sep 17 00:00:00 2001
From: Himanshu Mishra <himanshu@orkohunter.net>
Date: Sat, 6 Feb 2021 11:06:38 +0100
Subject: [PATCH 02/10] TechDocs: Implement etag based caching for url preparer

Signed-off-by: Himanshu Mishra <himanshu@orkohunter.net>
---
 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<any> => {
+  opts?: { etag?: string },
+): Promise<PreparerResponse> => {
   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<PreparerResponse>;
 };
 
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<PreparerResponse> {
+  async prepare(
+    entity: Entity,
+    options?: { etag?: string },
+  ): Promise<PreparerResponse> {
     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<void> {
+    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() {

From 057af3cac0961788501af151c409727c4a8e04e7 Mon Sep 17 00:00:00 2001
From: Himanshu Mishra <himanshu@orkohunter.net>
Date: Sat, 6 Feb 2021 11:44:51 +0100
Subject: [PATCH 03/10] TechDocs: Implement etag based caching for git preparer

Signed-off-by: Himanshu Mishra <himanshu@orkohunter.net>
---
 .../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<PreparerResponse> {
+  async prepare(
+    entity: Entity,
+    options?: { etag?: string },
+  ): Promise<PreparerResponse> {
     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<void> {
@@ -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':

From 50fcdc48b4a6e8b5b076c1496eb6318670e1a7e5 Mon Sep 17 00:00:00 2001
From: Himanshu Mishra <himanshu@orkohunter.net>
Date: Sat, 6 Feb 2021 11:56:49 +0100
Subject: [PATCH 04/10] TechDocs: Warn when using legacy git preparer and dir
 preparer in backstage.io/techdocs-ref

Context: https://github.com/backstage/backstage/issues/4409
---
 .../src/stages/prepare/commonGit.ts           |  6 ++++++
 .../techdocs-common/src/stages/prepare/dir.ts |  6 ++++++
 .../src/DocsBuilder/builder.ts                | 20 +++++++++++++++----
 3 files changed, 28 insertions(+), 4 deletions(-)

diff --git a/packages/techdocs-common/src/stages/prepare/commonGit.ts b/packages/techdocs-common/src/stages/prepare/commonGit.ts
index 4f889d981c..73a074b30c 100644
--- a/packages/techdocs-common/src/stages/prepare/commonGit.ts
+++ b/packages/techdocs-common/src/stages/prepare/commonGit.ts
@@ -39,6 +39,12 @@ export class CommonGitPreparer implements PreparerBase {
     entity: Entity,
     options?: { etag?: string },
   ): Promise<PreparerResponse> {
+    this.logger.warn(
+      'You are using the legacy git preparer in TechDocs which will be removed in near future (30 days). ' +
+        'Migrate to URL reader by updating `backstage.io/techdocs-ref` annotation in `catalog-info.yaml` ' +
+        'to be prefixed with `url:`. Read the migration guide and benefits at https://github.com/backstage/backstage/issues/4409 ',
+    );
+
     const { target } = parseReferenceAnnotation(
       'backstage.io/techdocs-ref',
       entity,
diff --git a/packages/techdocs-common/src/stages/prepare/dir.ts b/packages/techdocs-common/src/stages/prepare/dir.ts
index e59a3f02b6..ab3d22f515 100644
--- a/packages/techdocs-common/src/stages/prepare/dir.ts
+++ b/packages/techdocs-common/src/stages/prepare/dir.ts
@@ -69,6 +69,12 @@ export class DirectoryPreparer implements PreparerBase {
   }
 
   async prepare(entity: Entity): Promise<PreparerResponse> {
+    this.logger.warn(
+      'You are using the legacy dir preparer in TechDocs which will be removed in near future (30 days). ' +
+        'Migrate to URL reader by updating `backstage.io/techdocs-ref` annotation in `catalog-info.yaml` ' +
+        'to be prefixed with `url:`. Read the migration guide and benefits at https://github.com/backstage/backstage/issues/4409 ',
+    );
+
     const { target } = parseReferenceAnnotation(
       'backstage.io/techdocs-ref',
       entity,
diff --git a/plugins/techdocs-backend/src/DocsBuilder/builder.ts b/plugins/techdocs-backend/src/DocsBuilder/builder.ts
index 8cf278dc87..2ce7464a55 100644
--- a/plugins/techdocs-backend/src/DocsBuilder/builder.ts
+++ b/plugins/techdocs-backend/src/DocsBuilder/builder.ts
@@ -76,6 +76,10 @@ export class DocsBuilder {
       );
     }
 
+    /**
+     * Prepare and cache check
+     */
+
     // Use the in-memory storage for setting and getting etag for this entity.
     const buildMetadataStorage = new BuildMetadataStorage(
       this.entity.metadata.uid,
@@ -103,6 +107,11 @@ export class DocsBuilder {
       `TechDocs prepare step completed for entity ${getEntityId(this.entity)}.`,
     );
     this.logger.debug(`Prepared files temporarily stored at ${preparedDir}`);
+
+    /**
+     * Generate
+     */
+
     this.logger.info(`Running generator on entity ${getEntityId(this.entity)}`);
     // Create a temporary directory to store the generated files in.
     const tmpdirPath = os.tmpdir();
@@ -111,7 +120,6 @@ export class DocsBuilder {
     const outputDir = await fs.mkdtemp(
       path.join(tmpdirResolvedPath, 'techdocs-tmp-'),
     );
-
     const parsedLocationAnnotation = getLocationForEntity(this.entity);
     await this.generator.run({
       inputDir: preparedDir,
@@ -121,9 +129,9 @@ export class DocsBuilder {
     });
 
     this.logger.debug(`Generated files temporarily stored at ${outputDir}`);
-    // Remove Prepared directory
-    // Can not remove prepared directory in case of git preparer since the local git repository
-    // is used to get etag on subsequent requests.
+    // Remove Prepared directory since it is no longer needed.
+    // Caveat: 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.`,
@@ -136,6 +144,10 @@ export class DocsBuilder {
       }
     }
 
+    /**
+     * Publish
+     */
+
     this.logger.info(`Running publisher on entity ${getEntityId(this.entity)}`);
     await this.publisher.publish({
       entity: this.entity,

From f8a4a30fb0f2344c4ef4823268abc0a45af2d928 Mon Sep 17 00:00:00 2001
From: Himanshu Mishra <himanshu@orkohunter.net>
Date: Sat, 6 Feb 2021 12:01:50 +0100
Subject: [PATCH 05/10] TechDocs: Update tests for proper url preparer caching

---
 .../src/DocsBuilder/BuildMetadataStorage.test.ts          | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/plugins/techdocs-backend/src/DocsBuilder/BuildMetadataStorage.test.ts b/plugins/techdocs-backend/src/DocsBuilder/BuildMetadataStorage.test.ts
index 87de9d1d02..54ea860fb9 100644
--- a/plugins/techdocs-backend/src/DocsBuilder/BuildMetadataStorage.test.ts
+++ b/plugins/techdocs-backend/src/DocsBuilder/BuildMetadataStorage.test.ts
@@ -17,11 +17,11 @@ import { BuildMetadataStorage } from './BuildMetadataStorage';
 
 describe('BuildMetadataStorage', () => {
   it('should return build timestamp', () => {
-    const newMetadataStorage = new BuildMetadataStorage('123abc');
-    newMetadataStorage.storeBuildTimestamp();
+    const newMetadataStorage = new BuildMetadataStorage('entityID123abc');
+    newMetadataStorage.setEtag('etag123abc');
 
-    const timestamp = newMetadataStorage.getTimestamp();
+    const timestamp = newMetadataStorage.getEtag();
 
-    expect(timestamp).toBeLessThanOrEqual(Date.now());
+    expect(timestamp).toBe('etag123abc');
   });
 });

From df130db75969fde54803727b2e33a7c5bc6a8abd Mon Sep 17 00:00:00 2001
From: Himanshu Mishra <himanshu@orkohunter.net>
Date: Sat, 6 Feb 2021 12:13:02 +0100
Subject: [PATCH 06/10] TechDocs: Deduplicate git clone for cache check

Signed-off-by: Himanshu Mishra <himanshu@orkohunter.net>
---
 packages/techdocs-common/src/helpers.ts                  | 9 +--------
 .../techdocs-common/src/stages/prepare/commonGit.test.ts | 1 +
 packages/techdocs-common/src/stages/prepare/commonGit.ts | 9 ++-------
 packages/techdocs-common/src/stages/prepare/dir.ts       | 2 +-
 4 files changed, 5 insertions(+), 16 deletions(-)

diff --git a/packages/techdocs-common/src/helpers.ts b/packages/techdocs-common/src/helpers.ts
index bfbbd5b379..3146689a15 100644
--- a/packages/techdocs-common/src/helpers.ts
+++ b/packages/techdocs-common/src/helpers.ts
@@ -196,16 +196,9 @@ export const checkoutGitRepository = async (
 };
 
 export const getLastCommitTimestamp = async (
-  repositoryUrl: string,
-  config: Config,
+  repositoryLocation: string,
   logger: Logger,
 ): Promise<number> => {
-  const repositoryLocation = await checkoutGitRepository(
-    repositoryUrl,
-    config,
-    logger,
-  );
-
   const git = Git.fromAuth({ logger });
   const sha = await git.resolveRef({ dir: repositoryLocation, ref: 'HEAD' });
   const commit = await git.readCommit({ dir: repositoryLocation, sha });
diff --git a/packages/techdocs-common/src/stages/prepare/commonGit.test.ts b/packages/techdocs-common/src/stages/prepare/commonGit.test.ts
index b715b27407..dcc06313bd 100644
--- a/packages/techdocs-common/src/stages/prepare/commonGit.test.ts
+++ b/packages/techdocs-common/src/stages/prepare/commonGit.test.ts
@@ -29,6 +29,7 @@ function normalizePath(path: string) {
 jest.mock('../../helpers', () => ({
   ...jest.requireActual<{}>('../../helpers'),
   checkoutGitRepository: jest.fn(() => '/tmp/backstage-repo/org/name/branch'),
+  getLastCommitTimestamp: jest.fn(() => 12345678),
 }));
 
 const createMockEntity = (annotations = {}) => {
diff --git a/packages/techdocs-common/src/stages/prepare/commonGit.ts b/packages/techdocs-common/src/stages/prepare/commonGit.ts
index 73a074b30c..46b96675f3 100644
--- a/packages/techdocs-common/src/stages/prepare/commonGit.ts
+++ b/packages/techdocs-common/src/stages/prepare/commonGit.ts
@@ -40,7 +40,7 @@ export class CommonGitPreparer implements PreparerBase {
     options?: { etag?: string },
   ): Promise<PreparerResponse> {
     this.logger.warn(
-      'You are using the legacy git preparer in TechDocs which will be removed in near future (30 days). ' +
+      'You are using the legacy git preparer in TechDocs which will be removed in near future (March 2021). ' +
         'Migrate to URL reader by updating `backstage.io/techdocs-ref` annotation in `catalog-info.yaml` ' +
         'to be prefixed with `url:`. Read the migration guide and benefits at https://github.com/backstage/backstage/issues/4409 ',
     );
@@ -57,13 +57,8 @@ export class CommonGitPreparer implements PreparerBase {
         this.config,
         this.logger,
       );
-
       // Check if etag has changed for cache invalidation.
-      const etag = await getLastCommitTimestamp(
-        target,
-        this.config,
-        this.logger,
-      );
+      const etag = await getLastCommitTimestamp(repoPath, this.logger);
       if (options?.etag === etag.toString()) {
         throw new NotModifiedError();
       }
diff --git a/packages/techdocs-common/src/stages/prepare/dir.ts b/packages/techdocs-common/src/stages/prepare/dir.ts
index ab3d22f515..545297189f 100644
--- a/packages/techdocs-common/src/stages/prepare/dir.ts
+++ b/packages/techdocs-common/src/stages/prepare/dir.ts
@@ -70,7 +70,7 @@ export class DirectoryPreparer implements PreparerBase {
 
   async prepare(entity: Entity): Promise<PreparerResponse> {
     this.logger.warn(
-      'You are using the legacy dir preparer in TechDocs which will be removed in near future (30 days). ' +
+      'You are using the legacy dir preparer in TechDocs which will be removed in near future (March 2021). ' +
         'Migrate to URL reader by updating `backstage.io/techdocs-ref` annotation in `catalog-info.yaml` ' +
         'to be prefixed with `url:`. Read the migration guide and benefits at https://github.com/backstage/backstage/issues/4409 ',
     );

From 08142b256804055448a40672bad5d14faccde8e3 Mon Sep 17 00:00:00 2001
From: Himanshu Mishra <himanshu@orkohunter.net>
Date: Sat, 6 Feb 2021 12:19:38 +0100
Subject: [PATCH 07/10] TechDocs: Add changesets for deprecation and proper
 caching

---
 .changeset/techdocs-green-singers-jump.md      | 6 ++++++
 .changeset/techdocs-new-needles-arrive.md      | 8 ++++++++
 plugins/techdocs-backend/src/service/router.ts | 2 +-
 3 files changed, 15 insertions(+), 1 deletion(-)
 create mode 100644 .changeset/techdocs-green-singers-jump.md
 create mode 100644 .changeset/techdocs-new-needles-arrive.md

diff --git a/.changeset/techdocs-green-singers-jump.md b/.changeset/techdocs-green-singers-jump.md
new file mode 100644
index 0000000000..79d1e82a81
--- /dev/null
+++ b/.changeset/techdocs-green-singers-jump.md
@@ -0,0 +1,6 @@
+---
+'@backstage/techdocs-common': minor
+'@backstage/plugin-techdocs-backend': minor
+---
+
+URL Preparer will now use proper etag based caching introduced in https://github.com/backstage/backstage/pull/4120. Previously, builds used to be cached for 30 minutes.
diff --git a/.changeset/techdocs-new-needles-arrive.md b/.changeset/techdocs-new-needles-arrive.md
new file mode 100644
index 0000000000..c00df772ad
--- /dev/null
+++ b/.changeset/techdocs-new-needles-arrive.md
@@ -0,0 +1,8 @@
+---
+'@backstage/techdocs-common': patch
+'@backstage/plugin-techdocs-backend': patch
+---
+
+TechDocs will throw warning in backend logs when legacy git preparer or dir preparer is used to preparer docs. Migrate to URL Preparer by updating `backstage.io/techdocs-ref` annotation to be prefixed with `url:`.
+Detailed docs are here https://backstage.io/docs/features/techdocs/how-to-guides#how-to-use-url-reader-in-techdocs-prepare-step
+See benefits and reason for doing so https://github.com/backstage/backstage/issues/4409
diff --git a/plugins/techdocs-backend/src/service/router.ts b/plugins/techdocs-backend/src/service/router.ts
index b2053926ca..ef032e9f8b 100644
--- a/plugins/techdocs-backend/src/service/router.ts
+++ b/plugins/techdocs-backend/src/service/router.ts
@@ -184,7 +184,7 @@ export async function createRouter({
             );
             // TODO: re-trigger build for cache invalidation.
             // Add build info in techdocs_metadata.json and compare it against
-            // the eTag/commit in the repository.
+            // the etag/commit in the repository.
             // Without this, docs will not be re-built once they have been generated.
             // Although it is unconventional that anyone will face this issue - because
             // if you have an external storage, you should be using CI/CD to build and publish docs.

From 4ed4d44ccaaf2571877b23de22238c8722027d18 Mon Sep 17 00:00:00 2001
From: Himanshu Mishra <himanshu@orkohunter.net>
Date: Sat, 6 Feb 2021 13:55:03 +0100
Subject: [PATCH 08/10] TechDocs: Implement caching for dir preparer

---
 .../src/stages/prepare/dir.test.ts            |  1 +
 .../techdocs-common/src/stages/prepare/dir.ts | 59 +++++++++++++------
 2 files changed, 43 insertions(+), 17 deletions(-)

diff --git a/packages/techdocs-common/src/stages/prepare/dir.test.ts b/packages/techdocs-common/src/stages/prepare/dir.test.ts
index 3538175622..67fa0dd21f 100644
--- a/packages/techdocs-common/src/stages/prepare/dir.test.ts
+++ b/packages/techdocs-common/src/stages/prepare/dir.test.ts
@@ -28,6 +28,7 @@ function normalizePath(path: string) {
 jest.mock('../../helpers', () => ({
   ...jest.requireActual<{}>('../../helpers'),
   checkoutGitRepository: jest.fn(() => '/tmp/backstage-repo/org/name/branch/'),
+  getLastCommitTimestamp: jest.fn(() => 12345678),
 }));
 
 const logger = getVoidLogger();
diff --git a/packages/techdocs-common/src/stages/prepare/dir.ts b/packages/techdocs-common/src/stages/prepare/dir.ts
index 545297189f..08832938df 100644
--- a/packages/techdocs-common/src/stages/prepare/dir.ts
+++ b/packages/techdocs-common/src/stages/prepare/dir.ts
@@ -13,13 +13,21 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-import { InputError, UrlReader } from '@backstage/backend-common';
+import {
+  InputError,
+  NotModifiedError,
+  UrlReader,
+} 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 DirectoryPreparer implements PreparerBase {
@@ -33,7 +41,10 @@ export class DirectoryPreparer implements PreparerBase {
     this.reader = reader;
   }
 
-  private async resolveManagedByLocationToDir(entity: Entity) {
+  private async resolveManagedByLocationToDir(
+    entity: Entity,
+    options?: { etag?: string },
+  ): Promise<PreparerResponse> {
     const { type, target } = parseReferenceAnnotation(
       'backstage.io/managed-by-location',
       entity,
@@ -44,8 +55,14 @@ export class DirectoryPreparer implements PreparerBase {
     );
     switch (type) {
       case 'url': {
-        const response = await this.reader.readTree(target);
-        return await response.dir();
+        const response = await this.reader.readTree(target, {
+          etag: options?.etag,
+        });
+        const preparedDir = await response.dir();
+        return {
+          preparedDir,
+          etag: response.etag,
+        };
       }
       case 'github':
       case 'gitlab':
@@ -57,12 +74,24 @@ export class DirectoryPreparer implements PreparerBase {
           this.logger,
         );
 
-        return path.dirname(
-          path.join(repoLocation, parsedGitLocation.filepath),
-        );
+        // Check if etag has changed for cache invalidation.
+        const etag = await getLastCommitTimestamp(repoLocation, this.logger);
+        if (options?.etag === etag.toString()) {
+          throw new NotModifiedError();
+        }
+        return {
+          preparedDir: path.dirname(
+            path.join(repoLocation, parsedGitLocation.filepath),
+          ),
+          etag: etag.toString(),
+        };
       }
       case 'file':
-        return path.dirname(target);
+        return {
+          preparedDir: path.dirname(target),
+          // Instead of supporting caching on local sources, use techdocs-cli for local development and debugging.
+          etag: '',
+        };
       default:
         throw new InputError(`Unable to resolve location type ${type}`);
     }
@@ -80,16 +109,12 @@ export class DirectoryPreparer implements PreparerBase {
       entity,
     );
 
-    const managedByLocationDirectory = await this.resolveManagedByLocationToDir(
-      entity,
-    );
-
-    // TODO: etag will be returned as a commit sha from resolveManagedByLocationToDir.
-    const etag = '';
+    // This will throw NotModified error if etag has not changed.
+    const response = await this.resolveManagedByLocationToDir(entity);
 
     return {
-      preparedDir: path.resolve(managedByLocationDirectory, target),
-      etag,
+      preparedDir: path.resolve(response.preparedDir, target),
+      etag: response.etag,
     };
   }
 }

From b73b0fe9792cd4b09f89ead00517cf6df92ab0fc Mon Sep 17 00:00:00 2001
From: Himanshu Mishra <himanshu@orkohunter.net>
Date: Wed, 10 Feb 2021 12:21:14 +0100
Subject: [PATCH 09/10] techdocs: don't swallow errors other than
 NotModifiedError

---
 plugins/techdocs-backend/src/DocsBuilder/builder.ts | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/plugins/techdocs-backend/src/DocsBuilder/builder.ts b/plugins/techdocs-backend/src/DocsBuilder/builder.ts
index 2ce7464a55..8c54a2b1e4 100644
--- a/plugins/techdocs-backend/src/DocsBuilder/builder.ts
+++ b/plugins/techdocs-backend/src/DocsBuilder/builder.ts
@@ -13,6 +13,7 @@
  * 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 {
   GeneratorBase,
@@ -98,9 +99,12 @@ export class DocsBuilder {
 
       preparedDir = preparerResponse.preparedDir;
       etag = preparerResponse.etag;
-    } catch (NotModifiedError) {
-      // No need to prepare anymore since cache is valid.
-      return;
+    } catch (err) {
+      if (err instanceof NotModifiedError) {
+        // No need to prepare anymore since cache is valid.
+        return;
+      }
+      throw new Error(err.message);
     }
 
     this.logger.info(

From 9e6e4b89a86a544f559995f82c2f2dbe7684eb5c Mon Sep 17 00:00:00 2001
From: Himanshu Mishra <himanshu@orkohunter.net>
Date: Wed, 10 Feb 2021 13:14:52 +0100
Subject: [PATCH 10/10] techdocs: meaningful logs when readTree starts

---
 packages/techdocs-common/src/helpers.ts            | 5 ++++-
 packages/techdocs-common/src/stages/prepare/url.ts | 1 +
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/packages/techdocs-common/src/helpers.ts b/packages/techdocs-common/src/helpers.ts
index 3146689a15..beff0b2a6e 100644
--- a/packages/techdocs-common/src/helpers.ts
+++ b/packages/techdocs-common/src/helpers.ts
@@ -209,17 +209,20 @@ export const getLastCommitTimestamp = async (
 export const getDocFilesFromRepository = async (
   reader: UrlReader,
   entity: Entity,
-  opts?: { etag?: string },
+  opts?: { etag?: string; logger?: Logger },
 ): Promise<PreparerResponse> => {
   const { target } = parseReferenceAnnotation(
     'backstage.io/techdocs-ref',
     entity,
   );
 
+  opts?.logger?.info(`Reading files from ${target}`);
   // readTree will throw NotModifiedError if etag has not changed.
   const readTreeResponse = await reader.readTree(target, { etag: opts?.etag });
   const preparedDir = await readTreeResponse.dir();
 
+  opts?.logger?.info(`Tree downloaded and stored at ${preparedDir}`);
+
   return {
     preparedDir,
     etag: readTreeResponse.etag,
diff --git a/packages/techdocs-common/src/stages/prepare/url.ts b/packages/techdocs-common/src/stages/prepare/url.ts
index 60f176ffb4..9e4715a4ee 100644
--- a/packages/techdocs-common/src/stages/prepare/url.ts
+++ b/packages/techdocs-common/src/stages/prepare/url.ts
@@ -35,6 +35,7 @@ export class UrlPreparer implements PreparerBase {
     try {
       return await getDocFilesFromRepository(this.reader, entity, {
         etag: options?.etag,
+        logger: this.logger,
       });
     } catch (error) {
       // NotModifiedError means that etag based cache is still valid.