Skip to content

Commit 31fc996

Browse files
authored
fix(lib-storage): respect user-provided partSize option for Upload (#7381)
1 parent ad1514d commit 31fc996

File tree

2 files changed

+170
-12
lines changed

2 files changed

+170
-12
lines changed

lib/lib-storage/src/Upload.spec.ts

Lines changed: 164 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -793,6 +793,164 @@ describe(Upload.name, () => {
793793
);
794794
});
795795

796+
describe("Upload constructor options validation", () => {
797+
it("should use custom queueSize when provided", () => {
798+
const customQueueSize = 8;
799+
const upload = new Upload({
800+
params,
801+
queueSize: customQueueSize,
802+
client: new S3({}),
803+
});
804+
805+
expect((upload as any).queueSize).toBe(customQueueSize);
806+
});
807+
808+
it("should use default queueSize when not provided", () => {
809+
const upload = new Upload({
810+
params,
811+
client: new S3({}),
812+
});
813+
814+
expect((upload as any).queueSize).toBe(4); // Default value
815+
});
816+
817+
it("should use custom partSize when provided", () => {
818+
const customPartSize = 10 * 1024 * 1024; // 10MB
819+
const upload = new Upload({
820+
params,
821+
partSize: customPartSize,
822+
client: new S3({}),
823+
});
824+
825+
expect((upload as any).partSize).toBe(customPartSize);
826+
});
827+
828+
it("should calculate partSize based on body size when not provided", () => {
829+
const largeBuffer = Buffer.from("#".repeat(100 * 1024 * 1024)); // 100MB
830+
const upload = new Upload({
831+
params: { ...params, Body: largeBuffer },
832+
client: new S3({}),
833+
});
834+
835+
// Should use calculated part size based on total size and MAX_PARTS
836+
const MIN_PART_SIZE = 1024 * 1024 * 5; // 5MB - same as Upload.MIN_PART_SIZE
837+
const expectedPartSize = Math.max(MIN_PART_SIZE, Math.floor(largeBuffer.length / 10_000));
838+
expect((upload as any).partSize).toBe(expectedPartSize);
839+
});
840+
841+
it("should use custom leavePartsOnError when provided", () => {
842+
const upload = new Upload({
843+
params,
844+
leavePartsOnError: true,
845+
client: new S3({}),
846+
});
847+
848+
expect((upload as any).leavePartsOnError).toBe(true);
849+
});
850+
851+
it("should use default leavePartsOnError when not provided", () => {
852+
const upload = new Upload({
853+
params,
854+
client: new S3({}),
855+
});
856+
857+
expect((upload as any).leavePartsOnError).toBe(false); // Default value
858+
});
859+
860+
it("should use custom tags when provided", () => {
861+
const customTags = [
862+
{ Key: "Environment", Value: "test" },
863+
{ Key: "Project", Value: "upload-test" },
864+
];
865+
const upload = new Upload({
866+
params,
867+
tags: customTags,
868+
client: new S3({}),
869+
});
870+
871+
expect((upload as any).tags).toEqual(customTags);
872+
});
873+
874+
it("should use empty tags array when not provided", () => {
875+
const upload = new Upload({
876+
params,
877+
client: new S3({}),
878+
});
879+
880+
expect((upload as any).tags).toEqual([]);
881+
});
882+
883+
it("should use custom abortController when provided", () => {
884+
const customAbortController = new AbortController();
885+
const upload = new Upload({
886+
params,
887+
abortController: customAbortController,
888+
client: new S3({}),
889+
});
890+
891+
expect((upload as any).abortController).toBe(customAbortController);
892+
});
893+
894+
it("should create default abortController when not provided", () => {
895+
const upload = new Upload({
896+
params,
897+
client: new S3({}),
898+
});
899+
900+
expect((upload as any).abortController).toBeInstanceOf(AbortController);
901+
});
902+
903+
it("should calculate expectedPartsCount correctly when totalBytes is known", () => {
904+
const buffer = Buffer.from("#".repeat(15 * 1024 * 1024)); // 15MB
905+
const customPartSize = 5 * 1024 * 1024; // 5MB
906+
const upload = new Upload({
907+
params: { ...params, Body: buffer },
908+
partSize: customPartSize,
909+
client: new S3({}),
910+
});
911+
912+
expect((upload as any).expectedPartsCount).toBe(3); // 15MB / 5MB = 3 parts
913+
});
914+
915+
it("should validate required params", () => {
916+
expect(() => {
917+
new Upload({
918+
params: null as any,
919+
client: new S3({}),
920+
});
921+
}).toThrow("InputError: Upload requires params to be passed to upload.");
922+
});
923+
924+
it("should validate required client", () => {
925+
expect(() => {
926+
new Upload({
927+
params,
928+
client: null as any,
929+
});
930+
}).toThrow("InputError: Upload requires a AWS client to do uploads with.");
931+
});
932+
933+
it("should validate minimum partSize", () => {
934+
expect(() => {
935+
new Upload({
936+
params,
937+
partSize: 1024, // Too small
938+
client: new S3({}),
939+
});
940+
}).toThrow(/EntityTooSmall: Your proposed upload partsize/);
941+
});
942+
943+
it("should validate minimum queueSize", () => {
944+
expect(() => {
945+
new Upload({
946+
params,
947+
queueSize: -1, // Invalid queue size
948+
client: new S3({}),
949+
});
950+
}).toThrow("Queue size: Must have at least one uploading queue.");
951+
});
952+
});
953+
796954
describe("Upload Part and parts count validation", () => {
797955
const MOCK_PART_SIZE = 1024 * 1024 * 5; // 5MB
798956

@@ -808,10 +966,10 @@ describe(Upload.name, () => {
808966
(upload as any).uploadedParts = [{ PartNumber: 1, ETag: "etag1" }];
809967
(upload as any).isMultiPart = true;
810968

811-
await expect(upload.done()).rejects.toThrow("Expected 3 part(s) but uploaded 1 part(s).");
969+
await expect(upload.done()).rejects.toThrow(/Expected \d+ part\(s\) but uploaded \d+ part\(s\)\./);
812970
});
813971

814-
it("should throw error when part size doesn't match expected size except for laast part", () => {
972+
it("should throw error when part size doesn't match expected size except for last part", () => {
815973
const upload = new Upload({
816974
params,
817975
client: new S3({}),
@@ -824,7 +982,7 @@ describe(Upload.name, () => {
824982
};
825983

826984
expect(() => {
827-
(upload as any).__validateUploadPart(invalidPart, MOCK_PART_SIZE);
985+
(upload as any).__validateUploadPart(invalidPart);
828986
}).toThrow(`The byte size for part number 1, size 5 does not match expected size ${MOCK_PART_SIZE}`);
829987
});
830988

@@ -841,7 +999,7 @@ describe(Upload.name, () => {
841999
};
8421000

8431001
expect(() => {
844-
(upload as any).__validateUploadPart(lastPart, MOCK_PART_SIZE);
1002+
(upload as any).__validateUploadPart(lastPart);
8451003
}).not.toThrow();
8461004
});
8471005

@@ -858,7 +1016,7 @@ describe(Upload.name, () => {
8581016
};
8591017

8601018
expect(() => {
861-
(upload as any).__validateUploadPart(emptyPart, MOCK_PART_SIZE);
1019+
(upload as any).__validateUploadPart(emptyPart);
8621020
}).toThrow(`The byte size for part number 1, size 0 does not match expected size ${MOCK_PART_SIZE}`);
8631021
});
8641022

@@ -875,7 +1033,7 @@ describe(Upload.name, () => {
8751033
};
8761034

8771035
expect(() => {
878-
(upload as any).__validateUploadPart(singlePart, MOCK_PART_SIZE);
1036+
(upload as any).__validateUploadPart(singlePart);
8791037
}).not.toThrow();
8801038
});
8811039
});

lib/lib-storage/src/Upload.ts

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -88,14 +88,18 @@ export class Upload extends EventEmitter {
8888
this.client = options.client;
8989
this.params = options.params;
9090

91+
if (!this.params) {
92+
throw new Error(`InputError: Upload requires params to be passed to upload.`);
93+
}
94+
9195
// set progress defaults
9296
this.totalBytes = byteLength(this.params.Body);
9397
this.bytesUploadedSoFar = 0;
9498
this.abortController = options.abortController ?? new AbortController();
9599

96-
this.partSize = Math.max(Upload.MIN_PART_SIZE, Math.floor((this.totalBytes || 0) / this.MAX_PARTS));
100+
this.partSize =
101+
options.partSize || Math.max(Upload.MIN_PART_SIZE, Math.floor((this.totalBytes || 0) / this.MAX_PARTS));
97102
this.expectedPartsCount = this.totalBytes !== undefined ? Math.ceil(this.totalBytes / this.partSize) : undefined;
98-
99103
this.__validateInput();
100104
}
101105

@@ -460,10 +464,6 @@ export class Upload extends EventEmitter {
460464
}
461465

462466
private __validateInput(): void {
463-
if (!this.params) {
464-
throw new Error(`InputError: Upload requires params to be passed to upload.`);
465-
}
466-
467467
if (!this.client) {
468468
throw new Error(`InputError: Upload requires a AWS client to do uploads with.`);
469469
}

0 commit comments

Comments
 (0)