From 35952103d66db80859058082d094139436558ae6 Mon Sep 17 00:00:00 2001 From: Himanshu Mishra Date: Thu, 18 Feb 2021 13:38:45 +0100 Subject: [PATCH 01/13] TechDocs: AWS Publisher - add tests that fail on Windows --- packages/techdocs-common/__mocks__/aws-sdk.ts | 10 +++++++--- .../src/stages/publish/awsS3.test.ts | 17 ++++++++++++----- 2 files changed, 19 insertions(+), 8 deletions(-) diff --git a/packages/techdocs-common/__mocks__/aws-sdk.ts b/packages/techdocs-common/__mocks__/aws-sdk.ts index f0fb13c642..b956b4309a 100644 --- a/packages/techdocs-common/__mocks__/aws-sdk.ts +++ b/packages/techdocs-common/__mocks__/aws-sdk.ts @@ -41,7 +41,7 @@ export class S3 { } else { emitter.emit( 'error', - new Error(`The file ${Key} doest not exist !`), + new Error(`The file ${Key} does not exist !`), ); } emitter.emit('end'); @@ -56,7 +56,7 @@ export class S3 { if (fs.existsSync(Key)) { resolve(''); } else { - reject({ message: 'The object doest not exist !' }); + reject({ message: 'The object does not exist !' }); } }); } @@ -71,7 +71,11 @@ export class S3 { return { promise: () => new Promise((resolve, reject) => { - resolve(''); + if (!fs.existsSync(Key)) { + reject(''); + } else { + resolve(''); + } }), }; } diff --git a/packages/techdocs-common/src/stages/publish/awsS3.test.ts b/packages/techdocs-common/src/stages/publish/awsS3.test.ts index f8450a07a0..9376e74e92 100644 --- a/packages/techdocs-common/src/stages/publish/awsS3.test.ts +++ b/packages/techdocs-common/src/stages/publish/awsS3.test.ts @@ -127,9 +127,9 @@ describe('AwsS3Publish', () => { directory: wrongPathToGeneratedDirectory, }) .catch(error => - expect(error.message).toEqual( - expect.stringContaining( - 'Unable to upload file(s) to AWS S3. Error Failed to read template directory: ENOENT, no such file or directory', + expect(error).toEqual( + new Error( + `Unable to upload file(s) to AWS S3. Error Failed to read template directory: ENOENT, no such file or directory '${wrongPathToGeneratedDirectory}'`, ), ), ); @@ -205,12 +205,19 @@ describe('AwsS3Publish', () => { it('should return an error if the techdocs_metadata.json file is not present', async () => { const entityNameMock = createMockEntityName(); + const entity = createMockEntity(); + const entityRootDir = getEntityRootDir(entity); await publisher .fetchTechDocsMetadata(entityNameMock) .catch(error => - expect(error.message).toEqual( - expect.stringContaining('TechDocs metadata fetch'), + expect(error).toEqual( + new Error( + `TechDocs metadata fetch failed, The file ${path.join( + entityRootDir, + 'techdocs_metadata.json', + )} does not exist !`, + ), ), ); }); From 1966cc1802a2b993b3c32e2c863b47cc259e18a3 Mon Sep 17 00:00:00 2001 From: Himanshu Mishra Date: Thu, 18 Feb 2021 14:11:52 +0100 Subject: [PATCH 02/13] TechDocs: AWS Publisher - fix upload path to be OS agnostic --- packages/techdocs-common/__mocks__/aws-sdk.ts | 54 ++++++++++++------- .../src/stages/publish/awsS3.test.ts | 33 ++++++++---- .../src/stages/publish/awsS3.ts | 12 ++++- 3 files changed, 68 insertions(+), 31 deletions(-) diff --git a/packages/techdocs-common/__mocks__/aws-sdk.ts b/packages/techdocs-common/__mocks__/aws-sdk.ts index b956b4309a..b546494118 100644 --- a/packages/techdocs-common/__mocks__/aws-sdk.ts +++ b/packages/techdocs-common/__mocks__/aws-sdk.ts @@ -15,7 +15,28 @@ */ import type { S3 as S3Types } from 'aws-sdk'; import { EventEmitter } from 'events'; -import fs from 'fs'; +import fs from 'fs-extra'; +import os from 'os'; +import path from 'path'; + +const rootDir = os.platform() === 'win32' ? 'C:\\rootDir' : '/rootDir'; + +/** + * @param Key Contains either / or \ as file separator depending upon OS. + */ +const checkFileExists = async (Key: string): Promise => { + // Key will always have / as file separator irrespective of OS since S3 expects /. + // Normalize Key to OS specific path before checking if file exists. + const relativeFilePath = Key.split(path.posix.sep).join(path.sep); + const filePath = path.join(rootDir, Key); + + try { + await fs.access(filePath, fs.constants.F_OK); + return true; + } catch (err) { + return false; + } +}; export class S3 { private readonly options; @@ -26,22 +47,27 @@ export class S3 { headObject({ Key }: { Key: string }) { return { - promise: () => this.checkFileExists(Key), + promise: async () => { + if (!(await checkFileExists(Key))) { + throw new Error('File does not exist'); + } + }, }; } getObject({ Key }: { Key: string }) { + const filePath = path.join(rootDir, Key); return { - promise: () => this.checkFileExists(Key), + promise: () => checkFileExists(filePath), createReadStream: () => { const emitter = new EventEmitter(); process.nextTick(() => { - if (fs.existsSync(Key)) { - emitter.emit('data', Buffer.from(fs.readFileSync(Key))); + if (fs.existsSync(filePath)) { + emitter.emit('data', Buffer.from(fs.readFileSync(filePath))); } else { emitter.emit( 'error', - new Error(`The file ${Key} does not exist !`), + new Error(`The file ${filePath} does not exist !`), ); } emitter.emit('end'); @@ -51,16 +77,6 @@ export class S3 { }; } - checkFileExists(Key: string) { - return new Promise((resolve, reject) => { - if (fs.existsSync(Key)) { - resolve(''); - } else { - reject({ message: 'The object does not exist !' }); - } - }); - } - headBucket() { return new Promise(resolve => { resolve(''); @@ -70,9 +86,9 @@ export class S3 { upload({ Key }: { Key: string }) { return { promise: () => - new Promise((resolve, reject) => { - if (!fs.existsSync(Key)) { - reject(''); + new Promise(async (resolve, reject) => { + if (!(await checkFileExists(Key))) { + reject(`The file ${Key} does not exist`); } else { resolve(''); } diff --git a/packages/techdocs-common/src/stages/publish/awsS3.test.ts b/packages/techdocs-common/src/stages/publish/awsS3.test.ts index 9376e74e92..bf029dd7db 100644 --- a/packages/techdocs-common/src/stages/publish/awsS3.test.ts +++ b/packages/techdocs-common/src/stages/publish/awsS3.test.ts @@ -16,7 +16,8 @@ import type { Entity, EntityName } from '@backstage/catalog-model'; import { ConfigReader } from '@backstage/config'; import mockFs from 'mock-fs'; -import path from 'path'; +import * as os from 'os'; +import * as path from 'path'; import * as winston from 'winston'; import { AwsS3Publish } from './awsS3'; import { PublisherBase, TechDocsMetadata } from './types'; @@ -41,13 +42,15 @@ const createMockEntityName = (): EntityName => ({ namespace: 'test-namespace', }); +const rootDir = os.platform() === 'win32' ? 'C:\\rootDir' : '/rootDir'; + const getEntityRootDir = (entity: Entity) => { const { kind, metadata: { namespace, name }, } = entity; - const entityRootDir = path.join(namespace as string, kind, name); - return entityRootDir; + + return path.join(rootDir, namespace as string, kind, name); }; const logger = winston.createLogger(); @@ -57,6 +60,7 @@ jest.spyOn(logger, 'error').mockReturnValue(logger); let publisher: PublisherBase; beforeEach(() => { + mockFs.restore(); const mockConfig = new ConfigReader({ techdocs: { requestUrl: 'http://localhost:7000', @@ -78,6 +82,10 @@ beforeEach(() => { describe('AwsS3Publish', () => { describe('publish', () => { + afterEach(() => { + mockFs.restore(); + }); + it('should publish a directory', async () => { const entity = createMockEntity(); const entityRootDir = getEntityRootDir(entity); @@ -98,11 +106,11 @@ describe('AwsS3Publish', () => { directory: entityRootDir, }), ).toBeUndefined(); - mockFs.restore(); }); it('should fail to publish a directory', async () => { const wrongPathToGeneratedDirectory = path.join( + rootDir, 'wrong', 'path', 'to', @@ -126,13 +134,18 @@ describe('AwsS3Publish', () => { entity, directory: wrongPathToGeneratedDirectory, }) - .catch(error => - expect(error).toEqual( - new Error( - `Unable to upload file(s) to AWS S3. Error Failed to read template directory: ENOENT, no such file or directory '${wrongPathToGeneratedDirectory}'`, + .catch(error => { + expect(error.message).toEqual( + // Can not do exact error message match due to mockFs adding unexpected characters in the path when throwing the error + // Issue reported https://github.com/tschaub/mock-fs/issues/118 + expect.stringContaining( + `Unable to upload file(s) to AWS S3. Error Failed to read template directory: ENOENT, no such file or directory`, ), - ), - ); + ); + expect(error.message).toEqual( + expect.stringContaining(wrongPathToGeneratedDirectory), + ); + }); mockFs.restore(); }); }); diff --git a/packages/techdocs-common/src/stages/publish/awsS3.ts b/packages/techdocs-common/src/stages/publish/awsS3.ts index b0b29e8654..6145ec6365 100644 --- a/packages/techdocs-common/src/stages/publish/awsS3.ts +++ b/packages/techdocs-common/src/stages/publish/awsS3.ts @@ -140,9 +140,17 @@ export class AwsS3Publish implements PublisherBase { // Remove the absolute path prefix of the source directory // Path of all files to upload, relative to the root of the source directory // e.g. ['index.html', 'sub-page/index.html', 'assets/images/favicon.png'] - const relativeFilePath = filePath.replace(`${directory}/`, ''); + const relativeFilePath = path.relative(directory, filePath); + + // Convert destination file path to a POSIX path for uploading. + // S3 expects / as path separator and relativeFilePath will contain \\ on Windows. + // https://docs.aws.amazon.com/AmazonS3/latest/userguide/object-keys.html + const relativeFilePathPosix = relativeFilePath + .split(path.sep) + .join(path.posix.sep); + const entityRootDir = `${entity.metadata.namespace}/${entity.kind}/${entity.metadata.name}`; - const destination = `${entityRootDir}/${relativeFilePath}`; // S3 Bucket file relative path + const destination = `${entityRootDir}/${relativeFilePathPosix}`; // S3 Bucket file relative path const fileContent = await fs.readFile(filePath, 'utf8'); From 4b3500021c2311fe59c4d8b2a15e5872f8d0c7a0 Mon Sep 17 00:00:00 2001 From: Himanshu Mishra Date: Thu, 18 Feb 2021 20:55:35 +0100 Subject: [PATCH 03/13] TechDocs/GCS: Fix publisher to work with Windows file path --- .../__mocks__/@google-cloud/storage.ts | 29 ++++++++++++- packages/techdocs-common/__mocks__/aws-sdk.ts | 2 +- .../src/stages/publish/googleStorage.test.ts | 36 ++++++++++++---- .../src/stages/publish/googleStorage.ts | 42 +++++++++++-------- 4 files changed, 82 insertions(+), 27 deletions(-) diff --git a/packages/techdocs-common/__mocks__/@google-cloud/storage.ts b/packages/techdocs-common/__mocks__/@google-cloud/storage.ts index b84018c089..503ee397b4 100644 --- a/packages/techdocs-common/__mocks__/@google-cloud/storage.ts +++ b/packages/techdocs-common/__mocks__/@google-cloud/storage.ts @@ -13,10 +13,31 @@ * See the License for the specific language governing permissions and * limitations under the License. */ +import fs from 'fs-extra'; +import path from 'path'; + type storageOptions = { keyFilename?: string; }; +/** + * @param sourceFile contains either / or \ as file separator depending upon OS. + */ +const checkFileExists = async (sourceFile: string): Promise => { + // sourceFile will always have / as file separator irrespective of OS since S3 expects /. + // Normalize sourceFile to OS specific path before checking if file exists. + const filePath = sourceFile.split(path.posix.sep).join(path.sep); + + try { + await fs.access(filePath, fs.constants.F_OK); + return true; + } catch (err) { + console.log("File doesn't exist"); + console.log(filePath); + return false; + } +}; + class Bucket { private readonly bucketName; @@ -31,8 +52,12 @@ class Bucket { } upload(source: string, { destination }) { - return new Promise(resolve => { - resolve({ source, destination }); + return new Promise(async (resolve, reject) => { + if (await checkFileExists(source)) { + resolve({ source, destination }); + } else { + reject(`Source file ${source} does not exist.`); + } }); } } diff --git a/packages/techdocs-common/__mocks__/aws-sdk.ts b/packages/techdocs-common/__mocks__/aws-sdk.ts index b546494118..6957ea7430 100644 --- a/packages/techdocs-common/__mocks__/aws-sdk.ts +++ b/packages/techdocs-common/__mocks__/aws-sdk.ts @@ -22,7 +22,7 @@ import path from 'path'; const rootDir = os.platform() === 'win32' ? 'C:\\rootDir' : '/rootDir'; /** - * @param Key Contains either / or \ as file separator depending upon OS. + * @param Key contains either / or \ as file separator depending upon OS. */ const checkFileExists = async (Key: string): Promise => { // Key will always have / as file separator irrespective of OS since S3 expects /. diff --git a/packages/techdocs-common/src/stages/publish/googleStorage.test.ts b/packages/techdocs-common/src/stages/publish/googleStorage.test.ts index f8fb00647c..bf6822b82c 100644 --- a/packages/techdocs-common/src/stages/publish/googleStorage.test.ts +++ b/packages/techdocs-common/src/stages/publish/googleStorage.test.ts @@ -13,18 +13,22 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -import mockFs from 'mock-fs'; -import * as winston from 'winston'; +import { getVoidLogger } from '@backstage/backend-common'; +import { Entity } from '@backstage/catalog-model'; import { ConfigReader } from '@backstage/config'; +import mockFs from 'mock-fs'; +import os from 'os'; +import path from 'path'; import { GoogleGCSPublish } from './googleStorage'; import { PublisherBase } from './types'; -const createMockEntity = (annotations = {}) => { +const createMockEntity = (annotations = {}): Entity => { return { apiVersion: 'version', kind: 'TestKind', metadata: { name: 'test-component-name', + namespace: 'test-namespace', annotations: { ...annotations, }, @@ -32,12 +36,24 @@ const createMockEntity = (annotations = {}) => { }; }; -const logger = winston.createLogger(); +const rootDir = os.platform() === 'win32' ? 'C:\\rootDir' : '/rootDir'; + +const getEntityRootDir = (entity: Entity) => { + const { + kind, + metadata: { namespace, name }, + } = entity; + + return path.join(rootDir, namespace as string, kind, name); +}; + +const logger = getVoidLogger(); jest.spyOn(logger, 'info').mockReturnValue(logger); let publisher: PublisherBase; beforeEach(async () => { + mockFs.restore(); const mockConfig = new ConfigReader({ techdocs: { requestUrl: 'http://localhost:7000', @@ -55,9 +71,16 @@ beforeEach(async () => { }); describe('GoogleGCSPublish', () => { + afterEach(() => { + mockFs.restore(); + }); + it('should publish a directory', async () => { + const entity = createMockEntity(); + const entityRootDir = getEntityRootDir(entity); + mockFs({ - '/path/to/generatedDirectory': { + [entityRootDir]: { 'index.html': '', '404.html': '', assets: { @@ -66,11 +89,10 @@ describe('GoogleGCSPublish', () => { }, }); - const entity = createMockEntity(); expect( await publisher.publish({ entity, - directory: '/path/to/generatedDirectory', + directory: entityRootDir, }), ).toBeUndefined(); mockFs.restore(); diff --git a/packages/techdocs-common/src/stages/publish/googleStorage.ts b/packages/techdocs-common/src/stages/publish/googleStorage.ts index 8876007504..1041794161 100644 --- a/packages/techdocs-common/src/stages/publish/googleStorage.ts +++ b/packages/techdocs-common/src/stages/publish/googleStorage.ts @@ -13,20 +13,20 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -import path from 'path'; -import express from 'express'; -import { - Storage, - UploadResponse, - FileExistsResponse, -} from '@google-cloud/storage'; -import { Logger } from 'winston'; import { Entity, EntityName } from '@backstage/catalog-model'; import { Config } from '@backstage/config'; -import { getHeadersForFileExtension, getFileTreeRecursively } from './helpers'; -import { PublisherBase, PublishRequest, TechDocsMetadata } from './types'; +import { + FileExistsResponse, + Storage, + UploadResponse, +} from '@google-cloud/storage'; +import express from 'express'; import JSON5 from 'json5'; import createLimiter from 'p-limit'; +import path from 'path'; +import { Logger } from 'winston'; +import { getFileTreeRecursively, getHeadersForFileExtension } from './helpers'; +import { PublisherBase, PublishRequest, TechDocsMetadata } from './types'; export class GoogleGCSPublish implements PublisherBase { static async fromConfig( @@ -105,19 +105,27 @@ export class GoogleGCSPublish implements PublisherBase { const limiter = createLimiter(10); const uploadPromises: Array> = []; - allFilesToUpload.forEach(filePath => { + allFilesToUpload.forEach(sourceFilePath => { // Remove the absolute path prefix of the source directory // Path of all files to upload, relative to the root of the source directory // e.g. ['index.html', 'sub-page/index.html', 'assets/images/favicon.png'] - const relativeFilePath = filePath.replace(`${directory}/`, ''); + const relativeFilePath = path.relative(directory, sourceFilePath); + + // Convert destination file path to a POSIX path for uploading. + // GCS expects / as path separator and relativeFilePath will contain \\ on Windows. + // https://cloud.google.com/storage/docs/gsutil/addlhelp/HowSubdirectoriesWork + const relativeFilePathPosix = relativeFilePath + .split(path.sep) + .join(path.posix.sep); + const entityRootDir = `${entity.metadata.namespace}/${entity.kind}/${entity.metadata.name}`; - const destination = `${entityRootDir}/${relativeFilePath}`; // GCS Bucket file relative path + const destination = `${entityRootDir}/${relativeFilePathPosix}`; // GCS Bucket file relative path // Rate limit the concurrent execution of file uploads to batches of 10 (per publish) const uploadFile = limiter(() => - this.storageClient - .bucket(this.bucketName) - .upload(filePath, { destination }), + this.storageClient.bucket(this.bucketName).upload(sourceFilePath, { + destination, + }), ); uploadPromises.push(uploadFile); }); @@ -130,7 +138,7 @@ export class GoogleGCSPublish implements PublisherBase { resolve(undefined); }) .catch((err: Error) => { - const errorMessage = `Unable to upload file(s) to Google Cloud Storage. Error ${err.message}`; + const errorMessage = `Unable to upload file(s) to Google Cloud Storage. Error ${err}`; this.logger.error(errorMessage); reject(errorMessage); }); From 9ff646b6a89bd8aa118470800933d58f8c143fc8 Mon Sep 17 00:00:00 2001 From: Himanshu Mishra Date: Thu, 18 Feb 2021 21:33:53 +0100 Subject: [PATCH 04/13] TechDocs/GCS: Use async/await and add more test --- .../src/stages/publish/awsS3.test.ts | 35 ++++++------ .../src/stages/publish/awsS3.ts | 2 +- .../src/stages/publish/googleStorage.test.ts | 54 +++++++++++++++++-- .../src/stages/publish/googleStorage.ts | 27 +++++----- 4 files changed, 80 insertions(+), 38 deletions(-) diff --git a/packages/techdocs-common/src/stages/publish/awsS3.test.ts b/packages/techdocs-common/src/stages/publish/awsS3.test.ts index bf029dd7db..4f586119e5 100644 --- a/packages/techdocs-common/src/stages/publish/awsS3.test.ts +++ b/packages/techdocs-common/src/stages/publish/awsS3.test.ts @@ -82,11 +82,7 @@ beforeEach(() => { describe('AwsS3Publish', () => { describe('publish', () => { - afterEach(() => { - mockFs.restore(); - }); - - it('should publish a directory', async () => { + beforeEach(() => { const entity = createMockEntity(); const entityRootDir = getEntityRootDir(entity); @@ -99,6 +95,15 @@ describe('AwsS3Publish', () => { }, }, }); + }); + + afterEach(() => { + mockFs.restore(); + }); + + it('should publish a directory', async () => { + const entity = createMockEntity(); + const entityRootDir = getEntityRootDir(entity); expect( await publisher.publish({ @@ -116,18 +121,14 @@ describe('AwsS3Publish', () => { 'to', 'generatedDirectory', ); - const entity = createMockEntity(); - const entityRootDir = getEntityRootDir(entity); - mockFs({ - [entityRootDir]: { - 'index.html': '', - '404.html': '', - assets: { - 'main.css': '', - }, - }, - }); + const entity = createMockEntity(); + await expect( + publisher.publish({ + entity, + directory: wrongPathToGeneratedDirectory, + }), + ).rejects.toThrowError(); await publisher .publish({ @@ -139,7 +140,7 @@ describe('AwsS3Publish', () => { // Can not do exact error message match due to mockFs adding unexpected characters in the path when throwing the error // Issue reported https://github.com/tschaub/mock-fs/issues/118 expect.stringContaining( - `Unable to upload file(s) to AWS S3. Error Failed to read template directory: ENOENT, no such file or directory`, + `Unable to upload file(s) to AWS S3. Error: Failed to read template directory: ENOENT, no such file or directory`, ), ); expect(error.message).toEqual( diff --git a/packages/techdocs-common/src/stages/publish/awsS3.ts b/packages/techdocs-common/src/stages/publish/awsS3.ts index 6145ec6365..50f55ffe91 100644 --- a/packages/techdocs-common/src/stages/publish/awsS3.ts +++ b/packages/techdocs-common/src/stages/publish/awsS3.ts @@ -172,7 +172,7 @@ export class AwsS3Publish implements PublisherBase { ); return; } catch (e) { - const errorMessage = `Unable to upload file(s) to AWS S3. Error ${e.message}`; + const errorMessage = `Unable to upload file(s) to AWS S3. ${e}`; this.logger.error(errorMessage); throw new Error(errorMessage); } diff --git a/packages/techdocs-common/src/stages/publish/googleStorage.test.ts b/packages/techdocs-common/src/stages/publish/googleStorage.test.ts index bf6822b82c..9d4904b223 100644 --- a/packages/techdocs-common/src/stages/publish/googleStorage.test.ts +++ b/packages/techdocs-common/src/stages/publish/googleStorage.test.ts @@ -71,11 +71,7 @@ beforeEach(async () => { }); describe('GoogleGCSPublish', () => { - afterEach(() => { - mockFs.restore(); - }); - - it('should publish a directory', async () => { + beforeEach(() => { const entity = createMockEntity(); const entityRootDir = getEntityRootDir(entity); @@ -88,6 +84,15 @@ describe('GoogleGCSPublish', () => { }, }, }); + }); + + afterEach(() => { + mockFs.restore(); + }); + + it('should publish a directory', async () => { + const entity = createMockEntity(); + const entityRootDir = getEntityRootDir(entity); expect( await publisher.publish({ @@ -97,4 +102,43 @@ describe('GoogleGCSPublish', () => { ).toBeUndefined(); mockFs.restore(); }); + + it('should fail to publish a directory', async () => { + const wrongPathToGeneratedDirectory = path.join( + rootDir, + 'wrong', + 'path', + 'to', + 'generatedDirectory', + ); + + const entity = createMockEntity(); + + await expect( + publisher.publish({ + entity, + directory: wrongPathToGeneratedDirectory, + }), + ).rejects.toThrowError(); + + await publisher + .publish({ + entity, + directory: wrongPathToGeneratedDirectory, + }) + .catch(error => { + expect(error.message).toEqual( + // Can not do exact error message match due to mockFs adding unexpected characters in the path when throwing the error + // Issue reported https://github.com/tschaub/mock-fs/issues/118 + expect.stringContaining( + `Unable to upload file(s) to Google Cloud Storage. Error: Failed to read template directory: ENOENT, no such file or directory`, + ), + ); + expect(error.message).toEqual( + expect.stringContaining(wrongPathToGeneratedDirectory), + ); + }); + + mockFs.restore(); + }); }); diff --git a/packages/techdocs-common/src/stages/publish/googleStorage.ts b/packages/techdocs-common/src/stages/publish/googleStorage.ts index 1041794161..f45fb7eafb 100644 --- a/packages/techdocs-common/src/stages/publish/googleStorage.ts +++ b/packages/techdocs-common/src/stages/publish/googleStorage.ts @@ -97,8 +97,8 @@ 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 */ - publish({ entity, directory }: PublishRequest): Promise { - return new Promise(async (resolve, reject) => { + async publish({ entity, directory }: PublishRequest): Promise { + try { // Note: GCS 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); @@ -130,19 +130,16 @@ export class GoogleGCSPublish implements PublisherBase { uploadPromises.push(uploadFile); }); - Promise.all(uploadPromises) - .then(() => { - this.logger.info( - `Successfully uploaded all the generated files for Entity ${entity.metadata.name}. Total number of files: ${allFilesToUpload.length}`, - ); - resolve(undefined); - }) - .catch((err: Error) => { - const errorMessage = `Unable to upload file(s) to Google Cloud Storage. Error ${err}`; - this.logger.error(errorMessage); - reject(errorMessage); - }); - }); + await Promise.all(uploadPromises); + + this.logger.info( + `Successfully uploaded all the generated files for Entity ${entity.metadata.name}. Total number of files: ${allFilesToUpload.length}`, + ); + } catch (e) { + const errorMessage = `Unable to upload file(s) to Google Cloud Storage. ${e}`; + this.logger.error(errorMessage); + throw new Error(errorMessage); + } } fetchTechDocsMetadata(entityName: EntityName): Promise { From 6c36689039d5a952c7c8d9da7e6c5b9dfb989426 Mon Sep 17 00:00:00 2001 From: Himanshu Mishra Date: Thu, 18 Feb 2021 21:58:44 +0100 Subject: [PATCH 05/13] TechDocs/Azure: Fix publisher to work with Windows file paths --- .../stages/publish/azureBlobStorage.test.ts | 70 ++++++++----------- .../src/stages/publish/azureBlobStorage.ts | 44 +++++++----- 2 files changed, 57 insertions(+), 57 deletions(-) diff --git a/packages/techdocs-common/src/stages/publish/azureBlobStorage.test.ts b/packages/techdocs-common/src/stages/publish/azureBlobStorage.test.ts index 62c81df5c4..e43f9112f5 100644 --- a/packages/techdocs-common/src/stages/publish/azureBlobStorage.test.ts +++ b/packages/techdocs-common/src/stages/publish/azureBlobStorage.test.ts @@ -13,13 +13,12 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -import mockFs from 'mock-fs'; -import { ConfigReader } from '@backstage/config'; import { getVoidLogger } from '@backstage/backend-common'; +import type { Entity } from '@backstage/catalog-model'; +import { ConfigReader } from '@backstage/config'; +import mockFs from 'mock-fs'; import { AzureBlobStoragePublish } from './azureBlobStorage'; import { PublisherBase } from './types'; -import type { Entity } from '@backstage/catalog-model'; -import type { Logger } from 'winston'; const createMockEntity = (annotations = {}) => { return { @@ -52,34 +51,33 @@ function createLogger() { } let publisher: PublisherBase; - -describe('publishing with valid credentials', () => { - let logger: Logger; - - beforeEach(async () => { - const mockConfig = new ConfigReader({ - techdocs: { - requestUrl: 'http://localhost:7000', - publisher: { - type: 'azureBlobStorage', - azureBlobStorage: { - credentials: { - accountName: 'accountName', - accountKey: 'accountKey', - }, - containerName: 'containerName', +beforeEach(async () => { + mockFs.restore(); + const mockConfig = new ConfigReader({ + techdocs: { + requestUrl: 'http://localhost:7000', + publisher: { + type: 'azureBlobStorage', + azureBlobStorage: { + credentials: { + accountName: 'accountName', + accountKey: 'accountKey', }, + containerName: 'containerName', }, }, - }); - - logger = createLogger(); - - publisher = await AzureBlobStoragePublish.fromConfig(mockConfig, logger); + }, }); + publisher = await AzureBlobStoragePublish.fromConfig( + mockConfig, + createLogger(), + ); +}); + +describe('publishing with valid credentials', () => { describe('publish', () => { - it('should publish a directory', async () => { + beforeEach(() => { const entity = createMockEntity(); const entityRootDir = getEntityRootDir(entity); @@ -92,6 +90,11 @@ describe('publishing with valid credentials', () => { }, }, }); + }); + + it('should publish a directory', async () => { + const entity = createMockEntity(); + const entityRootDir = getEntityRootDir(entity); expect( await publisher.publish({ @@ -105,17 +108,6 @@ describe('publishing with valid credentials', () => { it('should fail to publish a directory', async () => { const wrongPathToGeneratedDirectory = 'wrong/path/to/generatedDirectory'; const entity = createMockEntity(); - const entityRootDir = getEntityRootDir(entity); - - mockFs({ - [entityRootDir]: { - 'index.html': '', - '404.html': '', - assets: { - 'main.css': '', - }, - }, - }); await publisher .publish({ @@ -124,7 +116,7 @@ describe('publishing with valid credentials', () => { }) .catch(error => expect(error.message).toContain( - 'Unable to upload file(s) to Azure Blob Storage. Failed to read template directory: ENOENT, no such file or directory', + 'Unable to upload file(s) to Azure Blob Storage. Error: Failed to read template directory: ENOENT, no such file or directory', ), ); mockFs.restore(); @@ -235,7 +227,7 @@ describe('error reporting', () => { expect(logger.error).toHaveBeenCalledWith( expect.stringContaining( - `Unable to upload file(s) to Azure Blob Storage. Upload failed for test-namespace/TestKind/test-component-name/index.html with status code 500`, + `Unable to upload file(s) to Azure Blob Storage. Error: Upload failed for test-namespace/TestKind/test-component-name/index.html with status code 500`, ), ); diff --git a/packages/techdocs-common/src/stages/publish/azureBlobStorage.ts b/packages/techdocs-common/src/stages/publish/azureBlobStorage.ts index 7000e9f28c..0d2e9871bf 100644 --- a/packages/techdocs-common/src/stages/publish/azureBlobStorage.ts +++ b/packages/techdocs-common/src/stages/publish/azureBlobStorage.ts @@ -13,20 +13,20 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -import platformPath from 'path'; -import express from 'express'; +import { DefaultAzureCredential } from '@azure/identity'; import { BlobServiceClient, StorageSharedKeyCredential, } from '@azure/storage-blob'; -import { DefaultAzureCredential } from '@azure/identity'; -import { Logger } from 'winston'; import { Entity, EntityName } from '@backstage/catalog-model'; import { Config } from '@backstage/config'; -import { getHeadersForFileExtension, getFileTreeRecursively } from './helpers'; -import { PublisherBase, PublishRequest, TechDocsMetadata } from './types'; -import limiterFactory from 'p-limit'; +import express from 'express'; import JSON5 from 'json5'; +import limiterFactory from 'p-limit'; +import { default as path, default as platformPath } from 'path'; +import { Logger } from 'winston'; +import { getFileTreeRecursively, getHeadersForFileExtension } from './helpers'; +import { PublisherBase, PublishRequest, TechDocsMetadata } from './types'; // The number of batches that may be ongoing at the same time. const BATCH_CONCURRENCY = 3; @@ -126,27 +126,35 @@ export class AzureBlobStoragePublish implements PublisherBase { // or start thrashing. const limiter = limiterFactory(BATCH_CONCURRENCY); - const promises = allFilesToUpload.map(filePath => { + const promises = allFilesToUpload.map(sourceFilePath => { // Remove the absolute path prefix of the source directory // Path of all files to upload, relative to the root of the source directory // e.g. ['index.html', 'sub-page/index.html', 'assets/images/favicon.png'] - const relativeFilePath = filePath.replace(`${directory}/`, ''); - const entityRootDir = `${entity.metadata.namespace}/${entity.kind}/${entity.metadata.name}`; - const destination = platformPath.normalize( - `${entityRootDir}/${relativeFilePath}`, - ); // Azure Blob Storage Container file relative path + const relativeFilePath = path.normalize( + path.relative(directory, sourceFilePath), + ); + // Convert destination file path to a POSIX path for uploading. + // Azure Blob Storage expects / as path separator and relativeFilePath will contain \\ on Windows. + // https://docs.microsoft.com/en-us/rest/api/storageservices/naming-and-referencing-containers--blobs--and-metadata#blob-names + const relativeFilePathPosix = relativeFilePath + .split(path.sep) + .join(path.posix.sep); + + // 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}`; // Azure Blob Storage Container file relative path return limiter(async () => { const response = await this.storageClient .getContainerClient(this.containerName) .getBlockBlobClient(destination) - .uploadFile(filePath); + .uploadFile(sourceFilePath); if (response._response.status >= 400) { return { ...response, error: new Error( - `Upload failed for ${filePath} with status code ${response._response.status}`, + `Upload failed for ${sourceFilePath} with status code ${response._response.status}`, ), }; } @@ -173,18 +181,18 @@ export class AzureBlobStoragePublish implements PublisherBase { ); } } catch (e) { - const errorMessage = `Unable to upload file(s) to Azure Blob Storage. ${e.message}`; + const errorMessage = `Unable to upload file(s) to Azure Blob Storage. ${e}`; this.logger.error(errorMessage); throw new Error(errorMessage); } } - private download(containerName: string, path: string): Promise { + private download(containerName: string, blobPath: string): Promise { return new Promise((resolve, reject) => { const fileStreamChunks: Array = []; this.storageClient .getContainerClient(containerName) - .getBlockBlobClient(path) + .getBlockBlobClient(blobPath) .download() .then(res => { const body = res.readableStreamBody; From 837ce612deca7d7d92c3b6e171876740e59f6a82 Mon Sep 17 00:00:00 2001 From: Himanshu Mishra Date: Thu, 18 Feb 2021 21:59:02 +0100 Subject: [PATCH 06/13] TechDocs/Publishers: Use path.normalize --- packages/techdocs-common/src/stages/publish/awsS3.ts | 1 + packages/techdocs-common/src/stages/publish/googleStorage.ts | 1 + 2 files changed, 2 insertions(+) diff --git a/packages/techdocs-common/src/stages/publish/awsS3.ts b/packages/techdocs-common/src/stages/publish/awsS3.ts index 50f55ffe91..96cce331ec 100644 --- a/packages/techdocs-common/src/stages/publish/awsS3.ts +++ b/packages/techdocs-common/src/stages/publish/awsS3.ts @@ -149,6 +149,7 @@ export class AwsS3Publish implements PublisherBase { .split(path.sep) .join(path.posix.sep); + // 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}`; // S3 Bucket file relative path diff --git a/packages/techdocs-common/src/stages/publish/googleStorage.ts b/packages/techdocs-common/src/stages/publish/googleStorage.ts index f45fb7eafb..e07defb237 100644 --- a/packages/techdocs-common/src/stages/publish/googleStorage.ts +++ b/packages/techdocs-common/src/stages/publish/googleStorage.ts @@ -118,6 +118,7 @@ export class GoogleGCSPublish implements PublisherBase { .split(path.sep) .join(path.posix.sep); + // 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}`; // GCS Bucket file relative path From b0fc3549e6dc7f25a1c44aad9bd03f04f19ca7f0 Mon Sep 17 00:00:00 2001 From: Himanshu Mishra Date: Thu, 18 Feb 2021 22:16:48 +0100 Subject: [PATCH 07/13] TechDocs/Azure: Update mock library tests to work with Windows --- .../__mocks__/@azure/storage-blob.ts | 24 +++++++++++++++-- .../__mocks__/@google-cloud/storage.ts | 2 -- .../src/stages/publish/awsS3.test.ts | 2 ++ .../stages/publish/azureBlobStorage.test.ts | 27 ++++++++++++++----- .../src/stages/publish/googleStorage.test.ts | 2 ++ 5 files changed, 47 insertions(+), 10 deletions(-) diff --git a/packages/techdocs-common/__mocks__/@azure/storage-blob.ts b/packages/techdocs-common/__mocks__/@azure/storage-blob.ts index 6633a71b2c..7157f9da60 100644 --- a/packages/techdocs-common/__mocks__/@azure/storage-blob.ts +++ b/packages/techdocs-common/__mocks__/@azure/storage-blob.ts @@ -13,11 +13,31 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -import fs from 'fs'; import type { BlobUploadCommonResponse, ContainerGetPropertiesResponse, } from '@azure/storage-blob'; +import fs from 'fs-extra'; +import os from 'os'; +import path from 'path'; + +const rootDir = os.platform() === 'win32' ? 'C:\\rootDir' : '/rootDir'; +/** + * @param sourceFile contains either / or \ as file separator depending upon OS. + */ +const checkFileExists = async (sourceFile: string): Promise => { + // sourceFile will always have / as file separator irrespective of OS since S3 expects /. + // Normalize sourceFile to OS specific path before checking if file exists. + const relativeFilePath = sourceFile.split(path.posix.sep).join(path.sep); + const filePath = path.join(rootDir, sourceFile); + + try { + await fs.access(filePath, fs.constants.F_OK); + return true; + } catch (err) { + return false; + } +}; export class BlockBlobClient { private readonly blobName; @@ -39,7 +59,7 @@ export class BlockBlobClient { } exists() { - return Promise.resolve(fs.existsSync(this.blobName)); + return checkFileExists(this.blobName); } } diff --git a/packages/techdocs-common/__mocks__/@google-cloud/storage.ts b/packages/techdocs-common/__mocks__/@google-cloud/storage.ts index 503ee397b4..9f19e8bf99 100644 --- a/packages/techdocs-common/__mocks__/@google-cloud/storage.ts +++ b/packages/techdocs-common/__mocks__/@google-cloud/storage.ts @@ -32,8 +32,6 @@ const checkFileExists = async (sourceFile: string): Promise => { await fs.access(filePath, fs.constants.F_OK); return true; } catch (err) { - console.log("File doesn't exist"); - console.log(filePath); return false; } }; diff --git a/packages/techdocs-common/src/stages/publish/awsS3.test.ts b/packages/techdocs-common/src/stages/publish/awsS3.test.ts index 4f586119e5..0c2a33dc03 100644 --- a/packages/techdocs-common/src/stages/publish/awsS3.test.ts +++ b/packages/techdocs-common/src/stages/publish/awsS3.test.ts @@ -22,6 +22,8 @@ import * as winston from 'winston'; import { AwsS3Publish } from './awsS3'; import { PublisherBase, TechDocsMetadata } from './types'; +// NOTE: /packages/techdocs-common/__mocks__ is being used to mock aws-sdk client library + const createMockEntity = (annotations = {}): Entity => { return { apiVersion: 'version', diff --git a/packages/techdocs-common/src/stages/publish/azureBlobStorage.test.ts b/packages/techdocs-common/src/stages/publish/azureBlobStorage.test.ts index e43f9112f5..dfb32b9ce8 100644 --- a/packages/techdocs-common/src/stages/publish/azureBlobStorage.test.ts +++ b/packages/techdocs-common/src/stages/publish/azureBlobStorage.test.ts @@ -17,9 +17,13 @@ import { getVoidLogger } from '@backstage/backend-common'; import type { Entity } from '@backstage/catalog-model'; import { ConfigReader } from '@backstage/config'; import mockFs from 'mock-fs'; +import os from 'os'; +import path from 'path'; import { AzureBlobStoragePublish } from './azureBlobStorage'; import { PublisherBase } from './types'; +// NOTE: /packages/techdocs-common/__mocks__ is being used to mock Azure client library + const createMockEntity = (annotations = {}) => { return { apiVersion: 'version', @@ -34,13 +38,14 @@ const createMockEntity = (annotations = {}) => { }; }; +const rootDir = os.platform() === 'win32' ? 'C:\\rootDir' : '/rootDir'; const getEntityRootDir = (entity: Entity) => { const { kind, metadata: { namespace, name }, } = entity; - const entityRootDir = `${namespace}/${kind}/${name}`; - return entityRootDir; + + return path.join(rootDir, namespace as string, kind, name); }; function createLogger() { @@ -106,7 +111,14 @@ describe('publishing with valid credentials', () => { }); it('should fail to publish a directory', async () => { - const wrongPathToGeneratedDirectory = 'wrong/path/to/generatedDirectory'; + const wrongPathToGeneratedDirectory = path.join( + rootDir, + 'wrong', + 'path', + 'to', + 'generatedDirectory', + ); + const entity = createMockEntity(); await publisher @@ -115,8 +127,8 @@ describe('publishing with valid credentials', () => { directory: wrongPathToGeneratedDirectory, }) .catch(error => - expect(error.message).toContain( - 'Unable to upload file(s) to Azure Blob Storage. Error: Failed to read template directory: ENOENT, no such file or directory', + expect(error.message).toBe( + `Unable to upload file(s) to Azure Blob Storage. Error: Failed to read template directory: ENOENT, no such file or directory '${wrongPathToGeneratedDirectory}'`, ), ); mockFs.restore(); @@ -227,7 +239,10 @@ describe('error reporting', () => { expect(logger.error).toHaveBeenCalledWith( expect.stringContaining( - `Unable to upload file(s) to Azure Blob Storage. Error: Upload failed for test-namespace/TestKind/test-component-name/index.html with status code 500`, + `Unable to upload file(s) to Azure Blob Storage. Error: Upload failed for ${path.join( + entityRootDir, + 'index.html', + )} with status code 500`, ), ); diff --git a/packages/techdocs-common/src/stages/publish/googleStorage.test.ts b/packages/techdocs-common/src/stages/publish/googleStorage.test.ts index 9d4904b223..1069b7b9a4 100644 --- a/packages/techdocs-common/src/stages/publish/googleStorage.test.ts +++ b/packages/techdocs-common/src/stages/publish/googleStorage.test.ts @@ -22,6 +22,8 @@ import path from 'path'; import { GoogleGCSPublish } from './googleStorage'; import { PublisherBase } from './types'; +// NOTE: /packages/techdocs-common/__mocks__ is being used to mock Google Cloud Storage client library + const createMockEntity = (annotations = {}): Entity => { return { apiVersion: 'version', From 0084248dc0c28db2dea0bccca14666cb6a2d4fd6 Mon Sep 17 00:00:00 2001 From: Himanshu Mishra Date: Thu, 18 Feb 2021 22:19:29 +0100 Subject: [PATCH 08/13] TechDocs/Azure: mockFs inserts unexpected characters in windows path --- .../src/stages/publish/azureBlobStorage.test.ts | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/packages/techdocs-common/src/stages/publish/azureBlobStorage.test.ts b/packages/techdocs-common/src/stages/publish/azureBlobStorage.test.ts index dfb32b9ce8..b88b58424d 100644 --- a/packages/techdocs-common/src/stages/publish/azureBlobStorage.test.ts +++ b/packages/techdocs-common/src/stages/publish/azureBlobStorage.test.ts @@ -126,11 +126,17 @@ describe('publishing with valid credentials', () => { entity, directory: wrongPathToGeneratedDirectory, }) - .catch(error => - expect(error.message).toBe( - `Unable to upload file(s) to Azure Blob Storage. Error: Failed to read template directory: ENOENT, no such file or directory '${wrongPathToGeneratedDirectory}'`, - ), - ); + .catch(error => { + // Can not do exact error message match due to mockFs adding unexpected characters in the path when throwing the error + // Issue reported https://github.com/tschaub/mock-fs/issues/118 + expect.stringContaining( + `Unable to upload file(s) to Azure Blob Storage. Error: Failed to read template directory: ENOENT, no such file or directory`, + ); + + expect(error.message).toEqual( + expect.stringContaining(wrongPathToGeneratedDirectory), + ); + }); mockFs.restore(); }); }); From 1e4ddd71da08717c1822ae5ec0ae5da5a70d3dbd Mon Sep 17 00:00:00 2001 From: Himanshu Mishra Date: Thu, 18 Feb 2021 22:21:38 +0100 Subject: [PATCH 09/13] TechDocs: Add changeset about publisher fix on windows --- .changeset/techdocs-wild-zoos-do.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/techdocs-wild-zoos-do.md diff --git a/.changeset/techdocs-wild-zoos-do.md b/.changeset/techdocs-wild-zoos-do.md new file mode 100644 index 0000000000..42a2a6f1c4 --- /dev/null +++ b/.changeset/techdocs-wild-zoos-do.md @@ -0,0 +1,5 @@ +--- +'@backstage/techdocs-common': patch +--- + +Fix AWS, GCS and Azure publisher to work on Windows. From b032111f37e8d09f59d0d084002e8dfb89a3754f Mon Sep 17 00:00:00 2001 From: Himanshu Mishra Date: Thu, 18 Feb 2021 22:32:43 +0100 Subject: [PATCH 10/13] TechDocs: Clarify differences in sourceFile Path among publishers --- packages/techdocs-common/__mocks__/@azure/storage-blob.ts | 5 +++-- packages/techdocs-common/__mocks__/@google-cloud/storage.ts | 4 ++-- packages/techdocs-common/__mocks__/aws-sdk.ts | 3 ++- packages/techdocs-common/src/stages/publish/awsS3.test.ts | 4 ++-- 4 files changed, 9 insertions(+), 7 deletions(-) diff --git a/packages/techdocs-common/__mocks__/@azure/storage-blob.ts b/packages/techdocs-common/__mocks__/@azure/storage-blob.ts index 7157f9da60..4fc1e4c374 100644 --- a/packages/techdocs-common/__mocks__/@azure/storage-blob.ts +++ b/packages/techdocs-common/__mocks__/@azure/storage-blob.ts @@ -23,10 +23,11 @@ import path from 'path'; const rootDir = os.platform() === 'win32' ? 'C:\\rootDir' : '/rootDir'; /** - * @param sourceFile contains either / or \ as file separator depending upon OS. + * @param sourceFile Relative path to entity root dir. Contains either / or \ as file separator + * depending upon the OS. */ const checkFileExists = async (sourceFile: string): Promise => { - // sourceFile will always have / as file separator irrespective of OS since S3 expects /. + // sourceFile will always have / as file separator irrespective of OS since Azure expects /. // Normalize sourceFile to OS specific path before checking if file exists. const relativeFilePath = sourceFile.split(path.posix.sep).join(path.sep); const filePath = path.join(rootDir, sourceFile); diff --git a/packages/techdocs-common/__mocks__/@google-cloud/storage.ts b/packages/techdocs-common/__mocks__/@google-cloud/storage.ts index 9f19e8bf99..648aceadb4 100644 --- a/packages/techdocs-common/__mocks__/@google-cloud/storage.ts +++ b/packages/techdocs-common/__mocks__/@google-cloud/storage.ts @@ -21,10 +21,10 @@ type storageOptions = { }; /** - * @param sourceFile contains either / or \ as file separator depending upon OS. + * @param sourceFile Absolute path. Contains either / or \ as file separator depending upon the OS. */ const checkFileExists = async (sourceFile: string): Promise => { - // sourceFile will always have / as file separator irrespective of OS since S3 expects /. + // sourceFile will always have / as file separator irrespective of OS since GCS expects /. // Normalize sourceFile to OS specific path before checking if file exists. const filePath = sourceFile.split(path.posix.sep).join(path.sep); diff --git a/packages/techdocs-common/__mocks__/aws-sdk.ts b/packages/techdocs-common/__mocks__/aws-sdk.ts index 6957ea7430..0a50b0e792 100644 --- a/packages/techdocs-common/__mocks__/aws-sdk.ts +++ b/packages/techdocs-common/__mocks__/aws-sdk.ts @@ -22,7 +22,8 @@ import path from 'path'; const rootDir = os.platform() === 'win32' ? 'C:\\rootDir' : '/rootDir'; /** - * @param Key contains either / or \ as file separator depending upon OS. + * @param Key Relative path to entity root dir. Contains either / or \ as file separator + * depending upon the OS. */ const checkFileExists = async (Key: string): Promise => { // Key will always have / as file separator irrespective of OS since S3 expects /. diff --git a/packages/techdocs-common/src/stages/publish/awsS3.test.ts b/packages/techdocs-common/src/stages/publish/awsS3.test.ts index 0c2a33dc03..0696ec5b1a 100644 --- a/packages/techdocs-common/src/stages/publish/awsS3.test.ts +++ b/packages/techdocs-common/src/stages/publish/awsS3.test.ts @@ -16,8 +16,8 @@ import type { Entity, EntityName } from '@backstage/catalog-model'; import { ConfigReader } from '@backstage/config'; import mockFs from 'mock-fs'; -import * as os from 'os'; -import * as path from 'path'; +import os from 'os'; +import path from 'path'; import * as winston from 'winston'; import { AwsS3Publish } from './awsS3'; import { PublisherBase, TechDocsMetadata } from './types'; From 15203089585f989f25b760638f818527918ce2fa Mon Sep 17 00:00:00 2001 From: Himanshu Mishra Date: Thu, 18 Feb 2021 23:19:33 +0100 Subject: [PATCH 11/13] TechDocs/GCS: Improve test coverage by testing other two methods --- .../__mocks__/@google-cloud/storage.ts | 42 ++++ packages/techdocs-common/__mocks__/aws-sdk.ts | 2 +- .../src/stages/publish/googleStorage.test.ts | 222 +++++++++++++----- .../src/stages/publish/googleStorage.ts | 6 +- 4 files changed, 205 insertions(+), 67 deletions(-) diff --git a/packages/techdocs-common/__mocks__/@google-cloud/storage.ts b/packages/techdocs-common/__mocks__/@google-cloud/storage.ts index 648aceadb4..264852d31c 100644 --- a/packages/techdocs-common/__mocks__/@google-cloud/storage.ts +++ b/packages/techdocs-common/__mocks__/@google-cloud/storage.ts @@ -13,13 +13,16 @@ * See the License for the specific language governing permissions and * limitations under the License. */ +import { EventEmitter } from 'events'; import fs from 'fs-extra'; +import os from 'os'; import path from 'path'; type storageOptions = { keyFilename?: string; }; +const rootDir = os.platform() === 'win32' ? 'C:\\rootDir' : '/rootDir'; /** * @param sourceFile Absolute path. Contains either / or \ as file separator depending upon the OS. */ @@ -36,6 +39,41 @@ const checkFileExists = async (sourceFile: string): Promise => { } }; +class GCSFile { + private readonly localFilePath: string; + + constructor(private readonly destinationFilePath: string) { + this.destinationFilePath = destinationFilePath; + this.localFilePath = path.join(rootDir, this.destinationFilePath); + } + + exists() { + return new Promise(async (resolve, reject) => { + if (await checkFileExists(this.localFilePath)) { + resolve([true]); + } else { + reject(); + } + }); + } + + createReadStream() { + const emitter = new EventEmitter(); + process.nextTick(() => { + if (fs.existsSync(this.localFilePath)) { + emitter.emit('data', Buffer.from(fs.readFileSync(this.localFilePath))); + emitter.emit('end'); + } else { + emitter.emit( + 'error', + new Error(`The file ${this.localFilePath} does not exist !`), + ); + } + }); + return emitter; + } +} + class Bucket { private readonly bucketName; @@ -58,6 +96,10 @@ class Bucket { } }); } + + file(destinationFilePath: string) { + return new GCSFile(destinationFilePath); + } } export class Storage { diff --git a/packages/techdocs-common/__mocks__/aws-sdk.ts b/packages/techdocs-common/__mocks__/aws-sdk.ts index 0a50b0e792..4717feda9b 100644 --- a/packages/techdocs-common/__mocks__/aws-sdk.ts +++ b/packages/techdocs-common/__mocks__/aws-sdk.ts @@ -65,13 +65,13 @@ export class S3 { process.nextTick(() => { if (fs.existsSync(filePath)) { emitter.emit('data', Buffer.from(fs.readFileSync(filePath))); + emitter.emit('end'); } else { emitter.emit( 'error', new Error(`The file ${filePath} does not exist !`), ); } - emitter.emit('end'); }); return emitter; }, diff --git a/packages/techdocs-common/src/stages/publish/googleStorage.test.ts b/packages/techdocs-common/src/stages/publish/googleStorage.test.ts index 1069b7b9a4..a8df9ccbc1 100644 --- a/packages/techdocs-common/src/stages/publish/googleStorage.test.ts +++ b/packages/techdocs-common/src/stages/publish/googleStorage.test.ts @@ -14,13 +14,13 @@ * limitations under the License. */ import { getVoidLogger } from '@backstage/backend-common'; -import { Entity } from '@backstage/catalog-model'; +import { Entity, EntityName } from '@backstage/catalog-model'; import { ConfigReader } from '@backstage/config'; import mockFs from 'mock-fs'; import os from 'os'; import path from 'path'; import { GoogleGCSPublish } from './googleStorage'; -import { PublisherBase } from './types'; +import { PublisherBase, TechDocsMetadata } from './types'; // NOTE: /packages/techdocs-common/__mocks__ is being used to mock Google Cloud Storage client library @@ -38,6 +38,12 @@ const createMockEntity = (annotations = {}): Entity => { }; }; +const createMockEntityName = (): EntityName => ({ + kind: 'TestKind', + name: 'test-component-name', + namespace: 'test-namespace', +}); + const rootDir = os.platform() === 'win32' ? 'C:\\rootDir' : '/rootDir'; const getEntityRootDir = (entity: Entity) => { @@ -73,74 +79,164 @@ beforeEach(async () => { }); describe('GoogleGCSPublish', () => { - beforeEach(() => { - const entity = createMockEntity(); - const entityRootDir = getEntityRootDir(entity); + describe('publish', () => { + beforeEach(() => { + const entity = createMockEntity(); + const entityRootDir = getEntityRootDir(entity); - mockFs({ - [entityRootDir]: { - 'index.html': '', - '404.html': '', - assets: { - 'main.css': '', + mockFs({ + [entityRootDir]: { + 'index.html': '', + '404.html': '', + assets: { + 'main.css': '', + }, }, - }, + }); + }); + + afterEach(() => { + mockFs.restore(); + }); + + it('should publish a directory', async () => { + const entity = createMockEntity(); + const entityRootDir = getEntityRootDir(entity); + + expect( + await publisher.publish({ + entity, + directory: entityRootDir, + }), + ).toBeUndefined(); + mockFs.restore(); + }); + + it('should fail to publish a directory', async () => { + const wrongPathToGeneratedDirectory = path.join( + rootDir, + 'wrong', + 'path', + 'to', + 'generatedDirectory', + ); + + const entity = createMockEntity(); + + await expect( + publisher.publish({ + entity, + directory: wrongPathToGeneratedDirectory, + }), + ).rejects.toThrowError(); + + await publisher + .publish({ + entity, + directory: wrongPathToGeneratedDirectory, + }) + .catch(error => { + expect(error.message).toEqual( + // Can not do exact error message match due to mockFs adding unexpected characters in the path when throwing the error + // Issue reported https://github.com/tschaub/mock-fs/issues/118 + expect.stringContaining( + `Unable to upload file(s) to Google Cloud Storage. Error: Failed to read template directory: ENOENT, no such file or directory`, + ), + ); + expect(error.message).toEqual( + expect.stringContaining(wrongPathToGeneratedDirectory), + ); + }); + + mockFs.restore(); }); }); - afterEach(() => { - mockFs.restore(); - }); + describe('hasDocsBeenGenerated', () => { + it('should return true if docs has been generated', async () => { + const entity = createMockEntity(); + const entityRootDir = getEntityRootDir(entity); - it('should publish a directory', async () => { - const entity = createMockEntity(); - const entityRootDir = getEntityRootDir(entity); - - expect( - await publisher.publish({ - entity, - directory: entityRootDir, - }), - ).toBeUndefined(); - mockFs.restore(); - }); - - it('should fail to publish a directory', async () => { - const wrongPathToGeneratedDirectory = path.join( - rootDir, - 'wrong', - 'path', - 'to', - 'generatedDirectory', - ); - - const entity = createMockEntity(); - - await expect( - publisher.publish({ - entity, - directory: wrongPathToGeneratedDirectory, - }), - ).rejects.toThrowError(); - - await publisher - .publish({ - entity, - directory: wrongPathToGeneratedDirectory, - }) - .catch(error => { - expect(error.message).toEqual( - // Can not do exact error message match due to mockFs adding unexpected characters in the path when throwing the error - // Issue reported https://github.com/tschaub/mock-fs/issues/118 - expect.stringContaining( - `Unable to upload file(s) to Google Cloud Storage. Error: Failed to read template directory: ENOENT, no such file or directory`, - ), - ); - expect(error.message).toEqual( - expect.stringContaining(wrongPathToGeneratedDirectory), - ); + mockFs({ + [entityRootDir]: { + 'index.html': 'file-content', + }, }); - mockFs.restore(); + expect(await publisher.hasDocsBeenGenerated(entity)).toBe(true); + mockFs.restore(); + }); + + it('should return false if docs has not been generated', async () => { + const entity = createMockEntity(); + + expect(await publisher.hasDocsBeenGenerated(entity)).toBe(false); + }); + }); + + describe('fetchTechDocsMetadata', () => { + beforeEach(() => { + mockFs.restore(); + }); + + it('should return tech docs metadata', async () => { + const entityNameMock = createMockEntityName(); + const entity = createMockEntity(); + const entityRootDir = getEntityRootDir(entity); + + mockFs({ + [entityRootDir]: { + 'techdocs_metadata.json': + '{"site_name": "backstage", "site_description": "site_content"}', + }, + }); + + const expectedMetadata: TechDocsMetadata = { + site_name: 'backstage', + site_description: 'site_content', + }; + expect( + await publisher.fetchTechDocsMetadata(entityNameMock), + ).toStrictEqual(expectedMetadata); + }); + + it('should return tech docs metadata when json encoded with single quotes', async () => { + const entityNameMock = createMockEntityName(); + const entity = createMockEntity(); + const entityRootDir = getEntityRootDir(entity); + + mockFs({ + [entityRootDir]: { + 'techdocs_metadata.json': + "{'site_name': 'backstage', 'site_description': 'site_content'}", + }, + }); + + const expectedMetadata: TechDocsMetadata = { + site_name: 'backstage', + site_description: 'site_content', + }; + expect( + await publisher.fetchTechDocsMetadata(entityNameMock), + ).toStrictEqual(expectedMetadata); + mockFs.restore(); + }); + + it('should return an error if the techdocs_metadata.json file is not present', async () => { + const entityNameMock = createMockEntityName(); + const entity = createMockEntity(); + const entityRootDir = getEntityRootDir(entity); + + await publisher + .fetchTechDocsMetadata(entityNameMock) + .catch(errorMessage => + expect(errorMessage).toEqual( + `The file ${path.join( + entityRootDir, + 'techdocs_metadata.json', + )} does not exist !`, + ), + ); + }); }); }); diff --git a/packages/techdocs-common/src/stages/publish/googleStorage.ts b/packages/techdocs-common/src/stages/publish/googleStorage.ts index e07defb237..12ecf45b2d 100644 --- a/packages/techdocs-common/src/stages/publish/googleStorage.ts +++ b/packages/techdocs-common/src/stages/publish/googleStorage.ts @@ -160,9 +160,9 @@ export class GoogleGCSPublish implements PublisherBase { fileStreamChunks.push(chunk); }) .on('end', () => { - const techdocsMetadataJson = Buffer.concat( - fileStreamChunks, - ).toString(); + const techdocsMetadataJson = Buffer.concat(fileStreamChunks).toString( + 'utf-8', + ); resolve(JSON5.parse(techdocsMetadataJson)); }); }); From 346a0feecf9436db9c36335e6ba10325e73fcef9 Mon Sep 17 00:00:00 2001 From: Himanshu Mishra Date: Fri, 19 Feb 2021 10:58:53 +0100 Subject: [PATCH 12/13] TechDocs: Fallback to ENTITY_DEFAULT_NAMESPACE when namespace is undefined --- packages/techdocs-common/src/stages/publish/awsS3.test.ts | 8 ++++++-- .../src/stages/publish/azureBlobStorage.test.ts | 4 ++-- .../src/stages/publish/googleStorage.test.ts | 8 ++++++-- 3 files changed, 14 insertions(+), 6 deletions(-) diff --git a/packages/techdocs-common/src/stages/publish/awsS3.test.ts b/packages/techdocs-common/src/stages/publish/awsS3.test.ts index 0696ec5b1a..b98f62e0f5 100644 --- a/packages/techdocs-common/src/stages/publish/awsS3.test.ts +++ b/packages/techdocs-common/src/stages/publish/awsS3.test.ts @@ -13,7 +13,11 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -import type { Entity, EntityName } from '@backstage/catalog-model'; +import { + Entity, + EntityName, + ENTITY_DEFAULT_NAMESPACE, +} from '@backstage/catalog-model'; import { ConfigReader } from '@backstage/config'; import mockFs from 'mock-fs'; import os from 'os'; @@ -52,7 +56,7 @@ const getEntityRootDir = (entity: Entity) => { metadata: { namespace, name }, } = entity; - return path.join(rootDir, namespace as string, kind, name); + return path.join(rootDir, namespace || ENTITY_DEFAULT_NAMESPACE, kind, name); }; const logger = winston.createLogger(); diff --git a/packages/techdocs-common/src/stages/publish/azureBlobStorage.test.ts b/packages/techdocs-common/src/stages/publish/azureBlobStorage.test.ts index b88b58424d..d8a112a403 100644 --- a/packages/techdocs-common/src/stages/publish/azureBlobStorage.test.ts +++ b/packages/techdocs-common/src/stages/publish/azureBlobStorage.test.ts @@ -14,7 +14,7 @@ * limitations under the License. */ import { getVoidLogger } from '@backstage/backend-common'; -import type { Entity } from '@backstage/catalog-model'; +import { Entity, ENTITY_DEFAULT_NAMESPACE } from '@backstage/catalog-model'; import { ConfigReader } from '@backstage/config'; import mockFs from 'mock-fs'; import os from 'os'; @@ -45,7 +45,7 @@ const getEntityRootDir = (entity: Entity) => { metadata: { namespace, name }, } = entity; - return path.join(rootDir, namespace as string, kind, name); + return path.join(rootDir, namespace || ENTITY_DEFAULT_NAMESPACE, kind, name); }; function createLogger() { diff --git a/packages/techdocs-common/src/stages/publish/googleStorage.test.ts b/packages/techdocs-common/src/stages/publish/googleStorage.test.ts index a8df9ccbc1..e0d56db8cc 100644 --- a/packages/techdocs-common/src/stages/publish/googleStorage.test.ts +++ b/packages/techdocs-common/src/stages/publish/googleStorage.test.ts @@ -14,7 +14,11 @@ * limitations under the License. */ import { getVoidLogger } from '@backstage/backend-common'; -import { Entity, EntityName } from '@backstage/catalog-model'; +import { + Entity, + EntityName, + ENTITY_DEFAULT_NAMESPACE, +} from '@backstage/catalog-model'; import { ConfigReader } from '@backstage/config'; import mockFs from 'mock-fs'; import os from 'os'; @@ -52,7 +56,7 @@ const getEntityRootDir = (entity: Entity) => { metadata: { namespace, name }, } = entity; - return path.join(rootDir, namespace as string, kind, name); + return path.join(rootDir, namespace || ENTITY_DEFAULT_NAMESPACE, kind, name); }; const logger = getVoidLogger(); From 5851bd5d2b01738c3dab7c0926a45636286be8f4 Mon Sep 17 00:00:00 2001 From: Himanshu Mishra Date: Fri, 19 Feb 2021 11:06:27 +0100 Subject: [PATCH 13/13] TechDocs: Use expect.assertions to make sure the error messages are checked --- packages/techdocs-common/src/stages/publish/awsS3.test.ts | 1 + .../techdocs-common/src/stages/publish/azureBlobStorage.test.ts | 1 + .../techdocs-common/src/stages/publish/googleStorage.test.ts | 2 ++ 3 files changed, 4 insertions(+) diff --git a/packages/techdocs-common/src/stages/publish/awsS3.test.ts b/packages/techdocs-common/src/stages/publish/awsS3.test.ts index b98f62e0f5..3d7bd337d2 100644 --- a/packages/techdocs-common/src/stages/publish/awsS3.test.ts +++ b/packages/techdocs-common/src/stages/publish/awsS3.test.ts @@ -120,6 +120,7 @@ describe('AwsS3Publish', () => { }); it('should fail to publish a directory', async () => { + expect.assertions(3); const wrongPathToGeneratedDirectory = path.join( rootDir, 'wrong', diff --git a/packages/techdocs-common/src/stages/publish/azureBlobStorage.test.ts b/packages/techdocs-common/src/stages/publish/azureBlobStorage.test.ts index d8a112a403..cf1ea60592 100644 --- a/packages/techdocs-common/src/stages/publish/azureBlobStorage.test.ts +++ b/packages/techdocs-common/src/stages/publish/azureBlobStorage.test.ts @@ -111,6 +111,7 @@ describe('publishing with valid credentials', () => { }); it('should fail to publish a directory', async () => { + expect.assertions(1); const wrongPathToGeneratedDirectory = path.join( rootDir, 'wrong', diff --git a/packages/techdocs-common/src/stages/publish/googleStorage.test.ts b/packages/techdocs-common/src/stages/publish/googleStorage.test.ts index e0d56db8cc..10da0d476a 100644 --- a/packages/techdocs-common/src/stages/publish/googleStorage.test.ts +++ b/packages/techdocs-common/src/stages/publish/googleStorage.test.ts @@ -117,6 +117,8 @@ describe('GoogleGCSPublish', () => { }); it('should fail to publish a directory', async () => { + expect.assertions(3); + const wrongPathToGeneratedDirectory = path.join( rootDir, 'wrong',