From e9e86c0d5448f33b1bbc48f3102936f009753e72 Mon Sep 17 00:00:00 2001 From: Pavel Denisjuk Date: Wed, 18 Sep 2024 13:27:23 +0200 Subject: [PATCH] fix(api-file-manager): use precalculated asset size and add logging --- .../src/assetDelivery/assetDeliveryConfig.ts | 8 ++- .../src/assetDelivery/s3/S3OutputStrategy.ts | 15 +++-- .../src/assetDelivery/s3/SharpTransform.ts | 66 +++++++++++++++---- .../src/delivery/AssetDelivery/Asset.ts | 17 +++-- .../FilesAssetRequestResolver.ts | 16 +++-- .../TransformationAssetProcessor.ts | 1 + .../src/delivery/setupAssetDelivery.ts | 1 + 7 files changed, 94 insertions(+), 30 deletions(-) diff --git a/packages/api-file-manager-s3/src/assetDelivery/assetDeliveryConfig.ts b/packages/api-file-manager-s3/src/assetDelivery/assetDeliveryConfig.ts index a2cafb0c368..7847b083597 100644 --- a/packages/api-file-manager-s3/src/assetDelivery/assetDeliveryConfig.ts +++ b/packages/api-file-manager-s3/src/assetDelivery/assetDeliveryConfig.ts @@ -10,6 +10,7 @@ import { SharpTransform } from "~/assetDelivery/s3/SharpTransform"; export type AssetDeliveryParams = Parameters[0] & { imageResizeWidths?: number[]; presignedUrlTtl?: number; + assetStreamingMaxSize?: number; }; export const assetDeliveryConfig = (params: AssetDeliveryParams) => { @@ -19,6 +20,11 @@ export const assetDeliveryConfig = (params: AssetDeliveryParams) => { const { presignedUrlTtl = 900, imageResizeWidths = [100, 300, 500, 750, 1000, 1500, 2500], + /** + * Even though Lambda's response payload limit is 6,291,556 bytes, we leave some room for the response envelope. + * We had situations where a 4.7MB file would cause the payload to go over the limit, so let's be on the safe side. + */ + assetStreamingMaxSize = 4718592, ...baseParams } = params; @@ -35,7 +41,7 @@ export const assetDeliveryConfig = (params: AssetDeliveryParams) => { }); config.decorateAssetOutputStrategy(() => { - return new S3OutputStrategy(s3, bucket, presignedUrlTtl); + return new S3OutputStrategy(s3, bucket, presignedUrlTtl, assetStreamingMaxSize); }); config.decorateAssetTransformationStrategy(() => { diff --git a/packages/api-file-manager-s3/src/assetDelivery/s3/S3OutputStrategy.ts b/packages/api-file-manager-s3/src/assetDelivery/s3/S3OutputStrategy.ts index ea574101aed..894cfd33f85 100644 --- a/packages/api-file-manager-s3/src/assetDelivery/s3/S3OutputStrategy.ts +++ b/packages/api-file-manager-s3/src/assetDelivery/s3/S3OutputStrategy.ts @@ -3,8 +3,6 @@ import { GetObjectCommand, getSignedUrl, S3 } from "@webiny/aws-sdk/client-s3"; import { S3RedirectAssetReply } from "~/assetDelivery/s3/S3RedirectAssetReply"; import { S3StreamAssetReply } from "~/assetDelivery/s3/S3StreamAssetReply"; -const MAX_RETURN_CONTENT_LENGTH = 4915200; // ~4.8 MB - /** * This strategy outputs an asset taking into account the size of the asset contents. * If the asset is larger than 5MB, a presigned URL will be generated, and a redirect will happen. @@ -13,21 +11,30 @@ export class S3OutputStrategy implements AssetOutputStrategy { private readonly s3: S3; private readonly bucket: string; private readonly presignedUrlTtl: number; + private readonly assetStreamingMaxSize: number; - constructor(s3: S3, bucket: string, presignedUrlTtl: number) { + constructor(s3: S3, bucket: string, presignedUrlTtl: number, assetStreamingMaxSize: number) { + this.assetStreamingMaxSize = assetStreamingMaxSize; this.presignedUrlTtl = presignedUrlTtl; this.s3 = s3; this.bucket = bucket; } async output(asset: Asset): Promise { - if ((await asset.getSize()) > MAX_RETURN_CONTENT_LENGTH) { + if (asset.getSize() > this.assetStreamingMaxSize) { + console.log( + `Asset size is greater than ${this.assetStreamingMaxSize}; redirecting to a presigned S3 URL.` + ); + return new S3RedirectAssetReply( await this.getPresignedUrl(asset), this.presignedUrlTtl ); } + console.log( + `Asset size is smaller than ${this.assetStreamingMaxSize}; streaming directly from Lambda function.` + ); return new S3StreamAssetReply(asset); } diff --git a/packages/api-file-manager-s3/src/assetDelivery/s3/SharpTransform.ts b/packages/api-file-manager-s3/src/assetDelivery/s3/SharpTransform.ts index cb616787810..39eba04f637 100644 --- a/packages/api-file-manager-s3/src/assetDelivery/s3/SharpTransform.ts +++ b/packages/api-file-manager-s3/src/assetDelivery/s3/SharpTransform.ts @@ -26,6 +26,9 @@ export class SharpTransform implements AssetTransformationStrategy { async transform(assetRequest: AssetRequest, asset: Asset): Promise { if (!utils.SUPPORTED_TRANSFORMABLE_IMAGES.includes(asset.getExtension())) { + console.log( + `Transformations/optimizations of ${asset.getContentType()} assets are not supported. Skipping.` + ); return asset; } @@ -45,6 +48,7 @@ export class SharpTransform implements AssetTransformationStrategy { } private async transformAsset(asset: Asset, options: Omit) { + console.log("Transform asset", options); if (options.width) { const { s3, bucket } = this.params; @@ -63,7 +67,15 @@ export class SharpTransform implements AssetTransformationStrategy { const buffer = Buffer.from(await Body.transformToByteArray()); - asset.setContentsReader(new CallableContentsReader(() => buffer)); + const newAsset = asset.withProps({ size: buffer.length }); + newAsset.setContentsReader(new CallableContentsReader(() => buffer)); + + console.log(`Return a previously transformed asset`, { + key: transformedAssetKey, + size: newAsset.getSize() + }); + + return newAsset; } catch (e) { const optimizedImage = await this.optimizeAsset(asset); @@ -73,22 +85,33 @@ export class SharpTransform implements AssetTransformationStrategy { /** * `width` is the only transformation we currently support. */ + console.log(`Resize the asset (width: ${width})`); const buffer = await optimizedImage.getContents(); - const transformedBuffer = sharp(buffer, { animated: this.isAssetAnimated(asset) }) + const transformedBuffer = await sharp(buffer, { + animated: this.isAssetAnimated(asset) + }) .resize({ width }) .toBuffer(); /** * Transformations are applied to the optimized image. */ - asset.setContentsReader(new CallableContentsReader(() => transformedBuffer)); + const newAsset = asset.withProps({ size: transformedBuffer.length }); + newAsset.setContentsReader(new CallableContentsReader(() => transformedBuffer)); await s3.putObject({ Bucket: bucket, Key: transformedAssetKey, - ContentType: asset.getContentType(), - Body: await asset.getContents() + ContentType: newAsset.getContentType(), + Body: await newAsset.getContents() }); + + console.log(`Return the resized asset`, { + key: transformedAssetKey, + size: newAsset.getSize() + }); + + return newAsset; } } @@ -98,6 +121,13 @@ export class SharpTransform implements AssetTransformationStrategy { private async optimizeAsset(asset: Asset) { const { s3, bucket } = this.params; + console.log("Optimize asset", { + id: asset.getId(), + key: asset.getKey(), + size: asset.getSize(), + type: asset.getContentType() + }); + const assetKey = new AssetKeyGenerator(asset); const optimizedAssetKey = assetKey.getOptimizedImageKey(); @@ -111,10 +141,16 @@ export class SharpTransform implements AssetTransformationStrategy { throw new Error(`Missing image body!`); } + console.log("Return a previously optimized asset", optimizedAssetKey); + const buffer = Buffer.from(await Body.transformToByteArray()); - asset.setContentsReader(new CallableContentsReader(() => buffer)); + const newAsset = asset.withProps({ size: buffer.length }); + newAsset.setContentsReader(new CallableContentsReader(() => buffer)); + + return newAsset; } catch (e) { + console.log("Create an optimized version of the original asset", asset.getKey()); // If not found, create an optimized version of the original asset. const buffer = await asset.getContents(); @@ -127,23 +163,26 @@ export class SharpTransform implements AssetTransformationStrategy { const optimization = optimizationMap[asset.getContentType()]; if (!optimization) { - console.log(`no optimizations defined for ${asset.getContentType()}`); + console.log(`No optimizations defined for ${asset.getContentType()}`); return asset; } - const optimizedBuffer = optimization(buffer).toBuffer(); + const optimizedBuffer = await optimization(buffer).toBuffer(); + + console.log("Optimized asset size", optimizedBuffer.length); - asset.setContentsReader(new CallableContentsReader(() => optimizedBuffer)); + const newAsset = asset.withProps({ size: optimizedBuffer.length }); + newAsset.setContentsReader(new CallableContentsReader(() => optimizedBuffer)); await s3.putObject({ Bucket: bucket, Key: optimizedAssetKey, - ContentType: asset.getContentType(), - Body: await asset.getContents() + ContentType: newAsset.getContentType(), + Body: await newAsset.getContents() }); - } - return asset; + return newAsset; + } } private isAssetAnimated(asset: Asset) { @@ -160,6 +199,7 @@ export class SharpTransform implements AssetTransformationStrategy { private optimizeJpeg(buffer: Buffer) { return sharp(buffer) .resize({ width: 2560, withoutEnlargement: true, fit: "inside" }) + .withMetadata() .toFormat("jpeg", { quality: 90 }); } } diff --git a/packages/api-file-manager/src/delivery/AssetDelivery/Asset.ts b/packages/api-file-manager/src/delivery/AssetDelivery/Asset.ts index ed209ee8470..44a2daf379a 100644 --- a/packages/api-file-manager/src/delivery/AssetDelivery/Asset.ts +++ b/packages/api-file-manager/src/delivery/AssetDelivery/Asset.ts @@ -21,10 +21,14 @@ export class Asset { } clone() { - const clonedAsset = new Asset(structuredClone(this.props)); - clonedAsset.outputStrategy = this.outputStrategy; - clonedAsset.contentsReader = this.contentsReader; - return clonedAsset; + return this.withProps(structuredClone(this.props)); + } + + withProps(props: Partial) { + const newAsset = new Asset({ ...this.props, ...props }); + newAsset.contentsReader = this.contentsReader; + newAsset.outputStrategy = this.outputStrategy; + return newAsset; } getId() { @@ -39,9 +43,8 @@ export class Asset { getKey() { return this.props.key; } - async getSize() { - const buffer = await this.getContents(); - return buffer.length; + getSize() { + return this.props.size; } getContentType() { return this.props.contentType; diff --git a/packages/api-file-manager/src/delivery/AssetDelivery/FilesAssetRequestResolver.ts b/packages/api-file-manager/src/delivery/AssetDelivery/FilesAssetRequestResolver.ts index b0b17f62685..5ef161179f8 100644 --- a/packages/api-file-manager/src/delivery/AssetDelivery/FilesAssetRequestResolver.ts +++ b/packages/api-file-manager/src/delivery/AssetDelivery/FilesAssetRequestResolver.ts @@ -1,6 +1,6 @@ import { Request } from "@webiny/handler/types"; import { AssetRequestResolver } from "./abstractions/AssetRequestResolver"; -import { AssetRequest } from "./AssetRequest"; +import { AssetRequest, AssetRequestOptions } from "./AssetRequest"; export class FilesAssetRequestResolver implements AssetRequestResolver { async resolve(request: Request): Promise { @@ -15,15 +15,21 @@ export class FilesAssetRequestResolver implements AssetRequestResolver { // Example: { '*': '/files/65722cb5c7824a0008d05963/image-48.jpg' }, const path = params["*"]; + const options: AssetRequestOptions = { + ...query, + original: "original" in query + }; + + if (query.width) { + options.width = parseInt(query.width); + } + return new AssetRequest({ key: decodeURI(path).replace("/files/", ""), context: { url: request.url }, - options: { - ...query, - width: query.width ? parseInt(query.width) : undefined - } + options }); } } diff --git a/packages/api-file-manager/src/delivery/AssetDelivery/transformation/TransformationAssetProcessor.ts b/packages/api-file-manager/src/delivery/AssetDelivery/transformation/TransformationAssetProcessor.ts index fbd330330a7..39d17f83d52 100644 --- a/packages/api-file-manager/src/delivery/AssetDelivery/transformation/TransformationAssetProcessor.ts +++ b/packages/api-file-manager/src/delivery/AssetDelivery/transformation/TransformationAssetProcessor.ts @@ -12,6 +12,7 @@ export class TransformationAssetProcessor implements AssetProcessor { // If the `original` image was requested, we skip all transformations. if (original) { + console.log("Skip transformations; original asset was requested."); return asset; } diff --git a/packages/api-file-manager/src/delivery/setupAssetDelivery.ts b/packages/api-file-manager/src/delivery/setupAssetDelivery.ts index 9a50159a9be..6e09b6394c9 100644 --- a/packages/api-file-manager/src/delivery/setupAssetDelivery.ts +++ b/packages/api-file-manager/src/delivery/setupAssetDelivery.ts @@ -146,6 +146,7 @@ export const setupAssetDelivery = (params: AssetDeliveryParams) => { ); // Get reply object (runs the output strategy under the hood). + console.log(`Output asset (size: ${processedAsset.getSize()} bytes).`); return outputAsset(reply, processedAsset); }, { override: true }