From b8fc937011a3336daef6fee0da86889c4915d525 Mon Sep 17 00:00:00 2001 From: Michael Bromley Date: Wed, 15 Apr 2020 12:46:43 +0200 Subject: [PATCH] feat(core): Implement asset binary deletion Relates to #306 --- .../asset-storage-strategy.ts | 20 +++++++++++++++---- .../no-asset-storage-strategy.ts | 4 ++++ .../src/service/services/asset.service.ts | 12 ++++++++--- .../config/testing-asset-storage-strategy.ts | 4 ++++ 4 files changed, 33 insertions(+), 7 deletions(-) diff --git a/packages/core/src/config/asset-storage-strategy/asset-storage-strategy.ts b/packages/core/src/config/asset-storage-strategy/asset-storage-strategy.ts index 736ca1a670..c1144cc4b0 100644 --- a/packages/core/src/config/asset-storage-strategy/asset-storage-strategy.ts +++ b/packages/core/src/config/asset-storage-strategy/asset-storage-strategy.ts @@ -10,36 +10,48 @@ import { Stream } from 'stream'; */ export interface AssetStorageStrategy { /** + * @description * Writes a buffer to the store and returns a unique identifier for that * file such as a file path or a URL. */ writeFileFromBuffer(fileName: string, data: Buffer): Promise; /** + * @description * Writes a readable stream to the store and returns a unique identifier for that * file such as a file path or a URL. */ writeFileFromStream(fileName: string, data: Stream): Promise; /** - * Reads a file based on an identifier which was generated by the writeFile - * method, and returns the file in binary form. + * @description + * Reads a file based on an identifier which was generated by the a writeFile + * method, and returns the as a Buffer. */ readFileToBuffer(identifier: string): Promise; /** - * Reads a file based on an identifier which was generated by the writeFile - * method, and returns the file in binary form. + * @description + * Reads a file based on an identifier which was generated by the a writeFile + * method, and returns the file as a Stream. */ readFileToStream(identifier: string): Promise; /** + * @description + * Deletes a file from the storage. + */ + deleteFile(identifier: string): Promise; + + /** + * @description * Check whether a file with the given name already exists. Used to avoid * naming conflicts before saving the file. */ fileExists(fileName: string): Promise; /** + * @description * Convert an identifier as generated by the writeFile... methods into an absolute * url (if it is not already in that form). If no conversion step is needed * (i.e. the identifier is already an absolute url) then this method diff --git a/packages/core/src/config/asset-storage-strategy/no-asset-storage-strategy.ts b/packages/core/src/config/asset-storage-strategy/no-asset-storage-strategy.ts index 90fc391386..b6a72b9740 100644 --- a/packages/core/src/config/asset-storage-strategy/no-asset-storage-strategy.ts +++ b/packages/core/src/config/asset-storage-strategy/no-asset-storage-strategy.ts @@ -27,6 +27,10 @@ export class NoAssetStorageStrategy implements AssetStorageStrategy { throw new InternalServerError(errorMessage); } + deleteFile(identifier: string): Promise { + throw new InternalServerError(errorMessage); + } + toAbsoluteUrl(request: Request, identifier: string): string { throw new InternalServerError(errorMessage); } diff --git a/packages/core/src/service/services/asset.service.ts b/packages/core/src/service/services/asset.service.ts index ba54f5d8e5..f9374d7afd 100644 --- a/packages/core/src/service/services/asset.service.ts +++ b/packages/core/src/service/services/asset.service.ts @@ -103,7 +103,7 @@ export class AssetService { }); assets = (entityWithAssets && entityWithAssets.assets) || []; } - return assets.sort((a, b) => a.position - b.position).map((a) => a.asset); + return assets.sort((a, b) => a.position - b.position).map(a => a.asset); } async updateFeaturedAsset(entity: T, input: EntityAssetInput): Promise { @@ -133,7 +133,7 @@ export class AssetService { if (assetIds && assetIds.length) { const assets = await this.connection.getRepository(Asset).findByIds(assetIds); const sortedAssets = assetIds - .map((id) => assets.find((a) => idsAreEqual(a.id, id))) + .map(id => assets.find(a => idsAreEqual(a.id, id))) .filter(notNullOrUndefined); await this.removeExistingOrderableAssets(entity); entity.assets = await this.createOrderableAssets(entity, sortedAssets); @@ -185,6 +185,12 @@ export class AssetService { // after deletion (the .remove() method sets it to undefined) const deletedAsset = new Asset(asset); await this.connection.getRepository(Asset).remove(asset); + try { + await this.configService.assetOptions.assetStorageStrategy.deleteFile(asset.source); + await this.configService.assetOptions.assetStorageStrategy.deleteFile(asset.preview); + } catch (e) { + Logger.error(`error.could-not-delete-asset-file`, undefined, e.stack); + } this.eventBus.publish(new AssetEvent(ctx, deletedAsset, 'deleted')); return { result: DeletionResult.DELETED, @@ -303,7 +309,7 @@ export class AssetService { private getOrderableAssetType(entity: EntityWithAssets): Type { const assetRelation = this.connection .getRepository(entity.constructor) - .metadata.relations.find((r) => r.propertyName === 'assets'); + .metadata.relations.find(r => r.propertyName === 'assets'); if (!assetRelation || typeof assetRelation.type === 'string') { throw new InternalServerError('error.could-not-find-matching-orderable-asset'); } diff --git a/packages/testing/src/config/testing-asset-storage-strategy.ts b/packages/testing/src/config/testing-asset-storage-strategy.ts index 1369edf5c6..af80c7f86d 100644 --- a/packages/testing/src/config/testing-asset-storage-strategy.ts +++ b/packages/testing/src/config/testing-asset-storage-strategy.ts @@ -43,4 +43,8 @@ export class TestingAssetStorageStrategy implements AssetStorageStrategy { fileExists(fileName: string): Promise { return Promise.resolve(false); } + + deleteFile(identifier: string): Promise { + return Promise.resolve(); + } }