From 28c14f540ed0dfd02125087973586bac161b2812 Mon Sep 17 00:00:00 2001 From: smilkuri Date: Fri, 26 Sep 2025 21:51:15 +0000 Subject: [PATCH 1/2] fix(lib-storage): respect user-provided partSize option --- lib/lib-storage/src/Upload.spec.ts | 17 +++++++++++++++++ lib/lib-storage/src/Upload.ts | 3 ++- 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/lib/lib-storage/src/Upload.spec.ts b/lib/lib-storage/src/Upload.spec.ts index 383549a58edf..3085df433c03 100644 --- a/lib/lib-storage/src/Upload.spec.ts +++ b/lib/lib-storage/src/Upload.spec.ts @@ -862,6 +862,23 @@ describe(Upload.name, () => { }).toThrow(`The byte size for part number 1, size 0 does not match expected size ${MOCK_PART_SIZE}`); }); + it("should use the user-specified partSize when provided, taking precedence over calculated partSize", () => { + const bigBuffer = Buffer.alloc(10); + const customPartSize = 50 * 1024 * 1024; // 50 MB + + const upload = new Upload({ + params: { + Key: "big-file", + Bucket: "bucket", + Body: bigBuffer, + }, + partSize: customPartSize, + client: new S3({}), + }); + + expect((upload as any).partSize).toBe(customPartSize); + }); + it("should skip validation for single-part uploads", () => { const upload = new Upload({ params, diff --git a/lib/lib-storage/src/Upload.ts b/lib/lib-storage/src/Upload.ts index 99098f80c8d8..760a66e1a040 100644 --- a/lib/lib-storage/src/Upload.ts +++ b/lib/lib-storage/src/Upload.ts @@ -93,7 +93,8 @@ export class Upload extends EventEmitter { this.bytesUploadedSoFar = 0; this.abortController = options.abortController ?? new AbortController(); - this.partSize = Math.max(Upload.MIN_PART_SIZE, Math.floor((this.totalBytes || 0) / this.MAX_PARTS)); + this.partSize = + options.partSize || Math.max(Upload.MIN_PART_SIZE, Math.floor((this.totalBytes || 0) / this.MAX_PARTS)); this.expectedPartsCount = this.totalBytes !== undefined ? Math.ceil(this.totalBytes / this.partSize) : undefined; this.__validateInput(); From d955c2161163df985ae9972ac083fb704eb938d4 Mon Sep 17 00:00:00 2001 From: smilkuri Date: Mon, 29 Sep 2025 15:42:59 +0000 Subject: [PATCH 2/2] fix(lib-storage): add comprehensive unit tests --- lib/lib-storage/src/Upload.spec.ts | 187 +++++++++++++++++++++++++---- lib/lib-storage/src/Upload.ts | 9 +- 2 files changed, 168 insertions(+), 28 deletions(-) diff --git a/lib/lib-storage/src/Upload.spec.ts b/lib/lib-storage/src/Upload.spec.ts index 3085df433c03..15c9b9a2f4ce 100644 --- a/lib/lib-storage/src/Upload.spec.ts +++ b/lib/lib-storage/src/Upload.spec.ts @@ -793,6 +793,164 @@ describe(Upload.name, () => { ); }); + describe("Upload constructor options validation", () => { + it("should use custom queueSize when provided", () => { + const customQueueSize = 8; + const upload = new Upload({ + params, + queueSize: customQueueSize, + client: new S3({}), + }); + + expect((upload as any).queueSize).toBe(customQueueSize); + }); + + it("should use default queueSize when not provided", () => { + const upload = new Upload({ + params, + client: new S3({}), + }); + + expect((upload as any).queueSize).toBe(4); // Default value + }); + + it("should use custom partSize when provided", () => { + const customPartSize = 10 * 1024 * 1024; // 10MB + const upload = new Upload({ + params, + partSize: customPartSize, + client: new S3({}), + }); + + expect((upload as any).partSize).toBe(customPartSize); + }); + + it("should calculate partSize based on body size when not provided", () => { + const largeBuffer = Buffer.from("#".repeat(100 * 1024 * 1024)); // 100MB + const upload = new Upload({ + params: { ...params, Body: largeBuffer }, + client: new S3({}), + }); + + // Should use calculated part size based on total size and MAX_PARTS + const MIN_PART_SIZE = 1024 * 1024 * 5; // 5MB - same as Upload.MIN_PART_SIZE + const expectedPartSize = Math.max(MIN_PART_SIZE, Math.floor(largeBuffer.length / 10_000)); + expect((upload as any).partSize).toBe(expectedPartSize); + }); + + it("should use custom leavePartsOnError when provided", () => { + const upload = new Upload({ + params, + leavePartsOnError: true, + client: new S3({}), + }); + + expect((upload as any).leavePartsOnError).toBe(true); + }); + + it("should use default leavePartsOnError when not provided", () => { + const upload = new Upload({ + params, + client: new S3({}), + }); + + expect((upload as any).leavePartsOnError).toBe(false); // Default value + }); + + it("should use custom tags when provided", () => { + const customTags = [ + { Key: "Environment", Value: "test" }, + { Key: "Project", Value: "upload-test" }, + ]; + const upload = new Upload({ + params, + tags: customTags, + client: new S3({}), + }); + + expect((upload as any).tags).toEqual(customTags); + }); + + it("should use empty tags array when not provided", () => { + const upload = new Upload({ + params, + client: new S3({}), + }); + + expect((upload as any).tags).toEqual([]); + }); + + it("should use custom abortController when provided", () => { + const customAbortController = new AbortController(); + const upload = new Upload({ + params, + abortController: customAbortController, + client: new S3({}), + }); + + expect((upload as any).abortController).toBe(customAbortController); + }); + + it("should create default abortController when not provided", () => { + const upload = new Upload({ + params, + client: new S3({}), + }); + + expect((upload as any).abortController).toBeInstanceOf(AbortController); + }); + + it("should calculate expectedPartsCount correctly when totalBytes is known", () => { + const buffer = Buffer.from("#".repeat(15 * 1024 * 1024)); // 15MB + const customPartSize = 5 * 1024 * 1024; // 5MB + const upload = new Upload({ + params: { ...params, Body: buffer }, + partSize: customPartSize, + client: new S3({}), + }); + + expect((upload as any).expectedPartsCount).toBe(3); // 15MB / 5MB = 3 parts + }); + + it("should validate required params", () => { + expect(() => { + new Upload({ + params: null as any, + client: new S3({}), + }); + }).toThrow("InputError: Upload requires params to be passed to upload."); + }); + + it("should validate required client", () => { + expect(() => { + new Upload({ + params, + client: null as any, + }); + }).toThrow("InputError: Upload requires a AWS client to do uploads with."); + }); + + it("should validate minimum partSize", () => { + expect(() => { + new Upload({ + params, + partSize: 1024, // Too small + client: new S3({}), + }); + }).toThrow(/EntityTooSmall: Your proposed upload partsize/); + }); + + it("should validate minimum queueSize", () => { + expect(() => { + new Upload({ + params, + queueSize: -1, // Invalid queue size + client: new S3({}), + }); + }).toThrow("Queue size: Must have at least one uploading queue."); + }); + }); + describe("Upload Part and parts count validation", () => { const MOCK_PART_SIZE = 1024 * 1024 * 5; // 5MB @@ -808,10 +966,10 @@ describe(Upload.name, () => { (upload as any).uploadedParts = [{ PartNumber: 1, ETag: "etag1" }]; (upload as any).isMultiPart = true; - await expect(upload.done()).rejects.toThrow("Expected 3 part(s) but uploaded 1 part(s)."); + await expect(upload.done()).rejects.toThrow(/Expected \d+ part\(s\) but uploaded \d+ part\(s\)\./); }); - it("should throw error when part size doesn't match expected size except for laast part", () => { + it("should throw error when part size doesn't match expected size except for last part", () => { const upload = new Upload({ params, client: new S3({}), @@ -824,7 +982,7 @@ describe(Upload.name, () => { }; expect(() => { - (upload as any).__validateUploadPart(invalidPart, MOCK_PART_SIZE); + (upload as any).__validateUploadPart(invalidPart); }).toThrow(`The byte size for part number 1, size 5 does not match expected size ${MOCK_PART_SIZE}`); }); @@ -841,7 +999,7 @@ describe(Upload.name, () => { }; expect(() => { - (upload as any).__validateUploadPart(lastPart, MOCK_PART_SIZE); + (upload as any).__validateUploadPart(lastPart); }).not.toThrow(); }); @@ -858,27 +1016,10 @@ describe(Upload.name, () => { }; expect(() => { - (upload as any).__validateUploadPart(emptyPart, MOCK_PART_SIZE); + (upload as any).__validateUploadPart(emptyPart); }).toThrow(`The byte size for part number 1, size 0 does not match expected size ${MOCK_PART_SIZE}`); }); - it("should use the user-specified partSize when provided, taking precedence over calculated partSize", () => { - const bigBuffer = Buffer.alloc(10); - const customPartSize = 50 * 1024 * 1024; // 50 MB - - const upload = new Upload({ - params: { - Key: "big-file", - Bucket: "bucket", - Body: bigBuffer, - }, - partSize: customPartSize, - client: new S3({}), - }); - - expect((upload as any).partSize).toBe(customPartSize); - }); - it("should skip validation for single-part uploads", () => { const upload = new Upload({ params, @@ -892,7 +1033,7 @@ describe(Upload.name, () => { }; expect(() => { - (upload as any).__validateUploadPart(singlePart, MOCK_PART_SIZE); + (upload as any).__validateUploadPart(singlePart); }).not.toThrow(); }); }); diff --git a/lib/lib-storage/src/Upload.ts b/lib/lib-storage/src/Upload.ts index 760a66e1a040..cbc847d15c33 100644 --- a/lib/lib-storage/src/Upload.ts +++ b/lib/lib-storage/src/Upload.ts @@ -88,6 +88,10 @@ export class Upload extends EventEmitter { this.client = options.client; this.params = options.params; + if (!this.params) { + throw new Error(`InputError: Upload requires params to be passed to upload.`); + } + // set progress defaults this.totalBytes = byteLength(this.params.Body); this.bytesUploadedSoFar = 0; @@ -96,7 +100,6 @@ export class Upload extends EventEmitter { this.partSize = options.partSize || Math.max(Upload.MIN_PART_SIZE, Math.floor((this.totalBytes || 0) / this.MAX_PARTS)); this.expectedPartsCount = this.totalBytes !== undefined ? Math.ceil(this.totalBytes / this.partSize) : undefined; - this.__validateInput(); } @@ -461,10 +464,6 @@ export class Upload extends EventEmitter { } private __validateInput(): void { - if (!this.params) { - throw new Error(`InputError: Upload requires params to be passed to upload.`); - } - if (!this.client) { throw new Error(`InputError: Upload requires a AWS client to do uploads with.`); }