diff --git a/packages/techdocs-common/__mocks__/@azure/storage-blob.ts b/packages/techdocs-common/__mocks__/@azure/storage-blob.ts index 56574dedf7..f7303ce1cb 100644 --- a/packages/techdocs-common/__mocks__/@azure/storage-blob.ts +++ b/packages/techdocs-common/__mocks__/@azure/storage-blob.ts @@ -126,9 +126,21 @@ export class ContainerClient { return new BlockBlobClient(blobName); } - listBlobsFlat() { + listBlobsFlat({ prefix }: { prefix: string }) { + if ( + this.containerName === 'delete_stale_files_success' || + this.containerName === 'delete_stale_files_error' + ) { + return [{ name: `${prefix}stale_file.png` }]; + } return []; } + + deleteBlob() { + if (this.containerName === 'delete_stale_files_error') { + throw new Error('Message'); + } + } } class ContainerClientFailGetProperties extends ContainerClient { diff --git a/packages/techdocs-common/src/stages/publish/azureBlobStorage.test.ts b/packages/techdocs-common/src/stages/publish/azureBlobStorage.test.ts index 8c31153201..2e17b43b62 100644 --- a/packages/techdocs-common/src/stages/publish/azureBlobStorage.test.ts +++ b/packages/techdocs-common/src/stages/publish/azureBlobStorage.test.ts @@ -63,13 +63,18 @@ const getEntityRootDir = (entity: Entity) => { }; const logger = getVoidLogger(); -jest.spyOn(logger, 'info').mockReturnValue(logger); -const loggerSpy = jest.spyOn(logger, 'error').mockReturnValue(logger); +const loggerInfoSpy = jest.spyOn(logger, 'info'); +const loggerErrorSpy = jest.spyOn(logger, 'error'); -let publisher: PublisherBase; -beforeEach(async () => { - loggerSpy.mockClear(); - mockFs.restore(); +const createPublisherMock = ({ + accountName = 'accountName', + containerName = 'containerName', +}: + | { + accountName?: string; + containerName?: string; + } + | undefined = {}) => { const mockConfig = new ConfigReader({ techdocs: { requestUrl: 'http://localhost:7000', @@ -77,16 +82,24 @@ beforeEach(async () => { type: 'azureBlobStorage', azureBlobStorage: { credentials: { - accountName: 'accountName', + accountName, accountKey: 'accountKey', }, - containerName: 'containerName', + containerName, }, }, }, }); - publisher = AzureBlobStoragePublish.fromConfig(mockConfig, logger); + return AzureBlobStoragePublish.fromConfig(mockConfig, logger); +}; + +let publisher: PublisherBase; +beforeEach(async () => { + loggerInfoSpy.mockClear(); + loggerErrorSpy.mockClear(); + mockFs.restore(); + publisher = createPublisherMock(); }); describe('publishing with valid credentials', () => { @@ -98,32 +111,15 @@ describe('publishing with valid credentials', () => { }); it('should reject incorrect config', async () => { - const mockConfig = new ConfigReader({ - techdocs: { - requestUrl: 'http://localhost:7000', - publisher: { - type: 'azureBlobStorage', - azureBlobStorage: { - credentials: { - accountName: 'accountName', - accountKey: 'accountKey', - }, - containerName: 'bad_container', - }, - }, - }, + const errorPublisher = createPublisherMock({ + containerName: 'bad_container', }); - const errorPublisher = await AzureBlobStoragePublish.fromConfig( - mockConfig, - logger, - ); - expect(await errorPublisher.getReadiness()).toEqual({ isAvailable: false, }); - expect(logger.error).toHaveBeenCalledWith( + expect(loggerErrorSpy).toHaveBeenCalledWith( expect.stringContaining( `Could not retrieve metadata about the Azure Blob Storage container bad_container.`, ), @@ -147,6 +143,10 @@ describe('publishing with valid credentials', () => { }); }); + afterEach(() => { + mockFs.restore(); + }); + it('should publish a directory', async () => { const entity = createMockEntity(); const entityRootDir = getEntityRootDir(entity); @@ -157,7 +157,6 @@ describe('publishing with valid credentials', () => { directory: entityRootDir, }), ).toBeUndefined(); - mockFs.restore(); }); it('should fail to publish a directory', async () => { @@ -182,29 +181,13 @@ describe('publishing with valid credentials', () => { await expect(fails).rejects.toThrow( new RegExp(wrongPathToGeneratedDirectory), ); - - mockFs.restore(); }); it('reports an error when bad account credentials', async () => { - const mockConfig = new ConfigReader({ - techdocs: { - requestUrl: 'http://localhost:7000', - publisher: { - type: 'azureBlobStorage', - azureBlobStorage: { - credentials: { - accountName: 'failupload', - accountKey: 'accountKey', - }, - containerName: 'containerName', - }, - }, - }, + publisher = createPublisherMock({ + accountName: 'failupload', }); - publisher = await AzureBlobStoragePublish.fromConfig(mockConfig, logger); - const entity = createMockEntity(); const entityRootDir = getEntityRootDir(entity); @@ -226,7 +209,7 @@ describe('publishing with valid credentials', () => { expect(error.message).toContain(`Unable to upload file(s) to Azure`); - expect(logger.error).toHaveBeenCalledWith( + expect(loggerErrorSpy).toHaveBeenCalledWith( expect.stringContaining( `Unable to upload file(s) to Azure. Error: Upload failed for ${path.join( entityRootDir, @@ -234,8 +217,28 @@ describe('publishing with valid credentials', () => { )} with status code 500`, ), ); + }); - mockFs.restore(); + it('should delete stale files after upload', async () => { + const entity = createMockEntity(); + const directory = getEntityRootDir(entity); + const containerName = 'delete_stale_files_success'; + publisher = createPublisherMock({ containerName }); + await publisher.publish({ entity, directory }); + expect(loggerInfoSpy).toHaveBeenLastCalledWith( + `Successfully deleted stale files for Entity ${entity.metadata.name}. Total number of files: 1`, + ); + }); + + it('should log error when the stale files deletion fails', async () => { + const entity = createMockEntity(); + const directory = getEntityRootDir(entity); + const containerName = 'delete_stale_files_error'; + publisher = createPublisherMock({ containerName }); + await publisher.publish({ entity, directory }); + expect(loggerErrorSpy).toHaveBeenLastCalledWith( + 'Unable to delete file(s) from Azure. Error: Message', + ); }); }); diff --git a/packages/techdocs-common/src/stages/publish/azureBlobStorage.ts b/packages/techdocs-common/src/stages/publish/azureBlobStorage.ts index c7b86ed39e..f49b317ecf 100644 --- a/packages/techdocs-common/src/stages/publish/azureBlobStorage.ts +++ b/packages/techdocs-common/src/stages/publish/azureBlobStorage.ts @@ -143,7 +143,9 @@ export class AzureBlobStoragePublish implements PublisherBase { let container: ContainerClient; try { container = this.storageClient.getContainerClient(this.containerName); - for await (const blob of container.listBlobsFlat()) { + for await (const blob of container.listBlobsFlat({ + prefix: remoteFolder, + })) { existingFiles.push(blob.name); } } catch (e) { @@ -163,7 +165,6 @@ export class AzureBlobStoragePublish implements PublisherBase { const failedOperations: Error[] = []; await bulkStorageOperation( async absoluteFilePath => { - // const relativeFilePath = path.relative(directory, absoluteFilePath); const relativeFilePath = path.normalize( path.relative(directory, absoluteFilePath), ); @@ -215,11 +216,7 @@ export class AzureBlobStoragePublish implements PublisherBase { ), ); - const staleFiles = getStaleFiles( - relativeFilesToUpload, - existingFiles, - remoteFolder, - ); + const staleFiles = getStaleFiles(relativeFilesToUpload, existingFiles); await bulkStorageOperation( async relativeFilePath => { @@ -233,7 +230,7 @@ export class AzureBlobStoragePublish implements PublisherBase { `Successfully deleted stale files for Entity ${entity.metadata.name}. Total number of files: ${staleFiles.length}`, ); } catch (error) { - const errorMessage = `Unable to delete file(s) from AWS S3. ${error}`; + const errorMessage = `Unable to delete file(s) from Azure. ${error}`; this.logger.error(errorMessage); } } diff --git a/packages/techdocs-common/src/stages/publish/helpers.ts b/packages/techdocs-common/src/stages/publish/helpers.ts index 2aabcb3259..6f6eb7dfa5 100644 --- a/packages/techdocs-common/src/stages/publish/helpers.ts +++ b/packages/techdocs-common/src/stages/publish/helpers.ts @@ -119,15 +119,8 @@ export const lowerCaseEntityTripletInStoragePath = ( export const getStaleFiles = ( newFiles: string[], oldFiles: string[], - remoteFolder?: string, ): string[] => { - let filteredFiles = [...oldFiles]; - if (remoteFolder) { - filteredFiles = filteredFiles.filter(filePath => - filePath.match(remoteFolder), - ); - } - const staleFiles = new Set(filteredFiles); + const staleFiles = new Set(oldFiles); newFiles.forEach(newFile => { staleFiles.delete(newFile); });