Skip to content

Commit 45d5730

Browse files
authored
fix(lib-storage): use input parameter object size information if available (#7385)
* fix(lib-storage): use input parameter object size information if available # Conflicts: # lib/lib-storage/src/Upload.ts * fix(lib-storage): use only ContentLength * fix: import ordering * fix: byteLengthSource * fix: add error message
1 parent 99ff240 commit 45d5730

14 files changed

+302
-53
lines changed

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

Lines changed: 38 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,12 @@
1+
import { EventEmitter, Readable } from "stream";
12
import { afterAll, afterEach, beforeEach, describe, expect, test as it, vi } from "vitest";
23

4+
import { Progress, Upload } from "./index";
5+
36
/* eslint-disable no-var */
47
var hostname = "s3.region.amazonaws.com";
58
var port: number | undefined;
69

7-
import { EventEmitter, Readable } from "stream";
8-
910
vi.mock("@aws-sdk/client-s3", async () => {
1011
const sendMock = vi.fn().mockImplementation(async (x) => x);
1112
const endpointMock = vi.fn().mockImplementation(() => ({
@@ -73,12 +74,20 @@ import {
7374
} from "@aws-sdk/client-s3";
7475
import { AbortController } from "@smithy/abort-controller";
7576

76-
import { Progress, Upload } from "./index";
77-
7877
const DEFAULT_PART_SIZE = 1024 * 1024 * 5;
7978

79+
type Expose = {
80+
totalBytes: number | undefined;
81+
};
82+
type VisibleForTesting = Omit<Upload, keyof Expose> & Expose;
83+
8084
describe(Upload.name, () => {
81-
const s3MockInstance = new S3Client();
85+
const s3MockInstance = new S3Client({
86+
credentials: {
87+
accessKeyId: "UNIT",
88+
secretAccessKey: "UNIT",
89+
},
90+
});
8291

8392
beforeEach(() => {
8493
vi.clearAllMocks();
@@ -107,6 +116,29 @@ describe(Upload.name, () => {
107116
Body: "this-is-a-sample-payload",
108117
};
109118

119+
it("uses the input parameters for object length if provided", async () => {
120+
let upload = new Upload({
121+
params: {
122+
Bucket: "",
123+
Key: "",
124+
Body: Buffer.from("a".repeat(256)),
125+
ContentLength: 6 * 1024 * 1024,
126+
},
127+
client: s3MockInstance,
128+
}) as unknown as VisibleForTesting;
129+
expect(upload.totalBytes).toEqual(6 * 1024 * 1024);
130+
131+
upload = new Upload({
132+
params: {
133+
Bucket: "",
134+
Key: "",
135+
Body: Buffer.from("a".repeat(256)),
136+
},
137+
client: s3MockInstance,
138+
}) as unknown as VisibleForTesting;
139+
expect(upload.totalBytes).toEqual(256);
140+
});
141+
110142
it("correctly exposes the event emitter API", () => {
111143
const upload = new Upload({
112144
params,
@@ -937,7 +969,7 @@ describe(Upload.name, () => {
937969
partSize: 1024, // Too small
938970
client: new S3({}),
939971
});
940-
}).toThrow(/EntityTooSmall: Your proposed upload partsize/);
972+
}).toThrow(/EntityTooSmall: Your proposed upload part size/);
941973
});
942974

943975
it("should validate minimum queueSize", () => {

lib/lib-storage/src/Upload.ts

Lines changed: 24 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3,15 +3,18 @@ import {
33
ChecksumAlgorithm,
44
CompletedPart,
55
CompleteMultipartUploadCommand,
6+
CompleteMultipartUploadCommandInput,
67
CompleteMultipartUploadCommandOutput,
78
CreateMultipartUploadCommand,
9+
CreateMultipartUploadCommandInput,
810
CreateMultipartUploadCommandOutput,
911
PutObjectCommand,
1012
PutObjectCommandInput,
1113
PutObjectTaggingCommand,
1214
S3Client,
1315
Tag,
1416
UploadPartCommand,
17+
UploadPartCommandInput,
1518
} from "@aws-sdk/client-s3";
1619
import { AbortController } from "@smithy/abort-controller";
1720
import {
@@ -24,7 +27,8 @@ import { extendedEncodeURIComponent } from "@smithy/smithy-client";
2427
import type { AbortController as IAbortController, AbortSignal as IAbortSignal, Endpoint } from "@smithy/types";
2528
import { EventEmitter } from "events";
2629

27-
import { byteLength } from "./bytelength";
30+
import { byteLength } from "./byteLength";
31+
import { BYTE_LENGTH_SOURCE, byteLengthSource } from "./byteLengthSource";
2832
import { getChunk } from "./chunker";
2933
import { BodyDataTypes, Options, Progress } from "./types";
3034

@@ -52,10 +56,12 @@ export class Upload extends EventEmitter {
5256
private readonly tags: Tag[] = [];
5357

5458
private readonly client: S3Client;
55-
private readonly params: PutObjectCommandInput;
59+
private readonly params: PutObjectCommandInput &
60+
Partial<CreateMultipartUploadCommandInput & UploadPartCommandInput & CompleteMultipartUploadCommandInput>;
5661

5762
// used for reporting progress.
5863
private totalBytes?: number;
64+
private readonly totalBytesSource?: BYTE_LENGTH_SOURCE;
5965
private bytesUploadedSoFar: number;
6066

6167
// used in the upload.
@@ -93,13 +99,18 @@ export class Upload extends EventEmitter {
9399
}
94100

95101
// set progress defaults
96-
this.totalBytes = byteLength(this.params.Body);
102+
this.totalBytes = this.params.ContentLength ?? byteLength(this.params.Body);
103+
this.totalBytesSource = byteLengthSource(this.params.Body, this.params.ContentLength);
97104
this.bytesUploadedSoFar = 0;
98105
this.abortController = options.abortController ?? new AbortController();
99106

100107
this.partSize =
101108
options.partSize || Math.max(Upload.MIN_PART_SIZE, Math.floor((this.totalBytes || 0) / this.MAX_PARTS));
102-
this.expectedPartsCount = this.totalBytes !== undefined ? Math.ceil(this.totalBytes / this.partSize) : undefined;
109+
110+
if (this.totalBytes !== undefined) {
111+
this.expectedPartsCount = Math.ceil(this.totalBytes / this.partSize);
112+
}
113+
103114
this.__validateInput();
104115
}
105116

@@ -373,9 +384,14 @@ export class Upload extends EventEmitter {
373384

374385
let result;
375386
if (this.isMultiPart) {
376-
const { expectedPartsCount, uploadedParts } = this;
377-
if (expectedPartsCount !== undefined && uploadedParts.length !== expectedPartsCount) {
378-
throw new Error(`Expected ${expectedPartsCount} part(s) but uploaded ${uploadedParts.length} part(s).`);
387+
const { expectedPartsCount, uploadedParts, totalBytes, totalBytesSource } = this;
388+
if (totalBytes !== undefined && expectedPartsCount !== undefined && uploadedParts.length !== expectedPartsCount) {
389+
throw new Error(`Expected ${expectedPartsCount} part(s) but uploaded ${uploadedParts.length} part(s).
390+
The expected part count is based on the byte-count of the input.params.Body,
391+
which was read from ${totalBytesSource} and is ${totalBytes}.
392+
If this is not correct, provide an override value by setting a number
393+
to input.params.ContentLength in bytes.
394+
`);
379395
}
380396

381397
this.uploadedParts.sort((a, b) => a.PartNumber! - b.PartNumber!);
@@ -470,7 +486,7 @@ export class Upload extends EventEmitter {
470486

471487
if (this.partSize < Upload.MIN_PART_SIZE) {
472488
throw new Error(
473-
`EntityTooSmall: Your proposed upload partsize [${this.partSize}] is smaller than the minimum allowed size [${Upload.MIN_PART_SIZE}] (5MB)`
489+
`EntityTooSmall: Your proposed upload part size [${this.partSize}] is smaller than the minimum allowed size [${Upload.MIN_PART_SIZE}] (5MB)`
474490
);
475491
}
476492

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
import { describe, expect, it, vi } from "vitest";
2+
3+
import { BYTE_LENGTH_SOURCE, byteLengthSource } from "./byteLengthSource";
4+
import { runtimeConfig } from "./runtimeConfig";
5+
6+
vi.mock("./runtimeConfig", () => ({
7+
runtimeConfig: {
8+
lstatSync: vi.fn(),
9+
},
10+
}));
11+
12+
describe("byteLengthSource", () => {
13+
it("should return CONTENT_LENGTH when override is provided", () => {
14+
expect(byteLengthSource({}, 100)).toBe(BYTE_LENGTH_SOURCE.CONTENT_LENGTH);
15+
});
16+
17+
it("should return EMPTY_INPUT for null input", () => {
18+
expect(byteLengthSource(null)).toBe(BYTE_LENGTH_SOURCE.EMPTY_INPUT);
19+
});
20+
21+
it("should return EMPTY_INPUT for undefined input", () => {
22+
expect(byteLengthSource(undefined)).toBe(BYTE_LENGTH_SOURCE.EMPTY_INPUT);
23+
});
24+
25+
it("should return STRING_LENGTH for string input", () => {
26+
expect(byteLengthSource("test")).toBe(BYTE_LENGTH_SOURCE.STRING_LENGTH);
27+
});
28+
29+
it("should return TYPED_ARRAY for input with byteLength", () => {
30+
const input = new Uint8Array(10);
31+
expect(byteLengthSource(input)).toBe(BYTE_LENGTH_SOURCE.TYPED_ARRAY);
32+
});
33+
34+
it("should return LENGTH for input with length property", () => {
35+
const input = { length: 10 };
36+
expect(byteLengthSource(input)).toBe(BYTE_LENGTH_SOURCE.LENGTH);
37+
});
38+
39+
it("should return SIZE for input with size property", () => {
40+
const input = { size: 10 };
41+
expect(byteLengthSource(input)).toBe(BYTE_LENGTH_SOURCE.SIZE);
42+
});
43+
44+
it("should return START_END_DIFF for input with start and end properties", () => {
45+
const input = { start: 0, end: 10 };
46+
expect(byteLengthSource(input)).toBe(BYTE_LENGTH_SOURCE.START_END_DIFF);
47+
});
48+
49+
it("should return LSTAT for input with path that exists", () => {
50+
const input = { path: "/test/path" };
51+
vi.mocked(runtimeConfig.lstatSync).mockReturnValue({ size: 100 } as any);
52+
53+
expect(byteLengthSource(input)).toBe(BYTE_LENGTH_SOURCE.LSTAT);
54+
expect(runtimeConfig.lstatSync).toHaveBeenCalledWith("/test/path");
55+
});
56+
57+
it("should return undefined for input with path that throws error", () => {
58+
const input = { path: "/test/path" };
59+
vi.mocked(runtimeConfig.lstatSync).mockImplementation(() => {
60+
throw new Error("File not found");
61+
});
62+
63+
expect(byteLengthSource(input)).toBeUndefined();
64+
});
65+
66+
it("should return undefined for input with no matching properties", () => {
67+
const input = { foo: "bar" };
68+
expect(byteLengthSource(input)).toBeUndefined();
69+
});
70+
});

lib/lib-storage/src/byteLength.ts

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
import { Buffer } from "buffer"; // do not remove this import: Node.js buffer or buffer NPM module for browser.
2+
3+
import { runtimeConfig } from "./runtimeConfig";
4+
5+
/**
6+
* Clients use util-body-length-[node|browser] instead.
7+
* @internal
8+
* @param input - to examine.
9+
* @returns byte count of input or undefined if indeterminable.
10+
*/
11+
export const byteLength = (input: any): number | undefined => {
12+
if (input == null) {
13+
return 0;
14+
}
15+
16+
if (typeof input === "string") {
17+
return Buffer.byteLength(input);
18+
}
19+
20+
if (typeof input.byteLength === "number") {
21+
// Uint8Array, ArrayBuffer, Buffer, and ArrayBufferView
22+
return input.byteLength;
23+
} else if (typeof input.length === "number") {
24+
// todo: unclear in what cases this is a valid byte count.
25+
return input.length;
26+
} else if (typeof input.size === "number") {
27+
// todo: unclear in what cases this is a valid byte count.
28+
return input.size;
29+
} else if (typeof input.start === "number" && typeof input.end === "number") {
30+
// file read stream with range.
31+
return input.end + 1 - input.start;
32+
} else if (typeof input.path === "string") {
33+
// file read stream with path.
34+
try {
35+
return runtimeConfig.lstatSync(input.path).size;
36+
} catch (error) {
37+
return undefined;
38+
}
39+
}
40+
return undefined;
41+
};
Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
import { runtimeConfig } from "./runtimeConfig";
2+
3+
/**
4+
* @internal
5+
*/
6+
export enum BYTE_LENGTH_SOURCE {
7+
EMPTY_INPUT = "a null or undefined Body",
8+
CONTENT_LENGTH = "the ContentLength property of the params set by the caller",
9+
STRING_LENGTH = "the encoded byte length of the Body string",
10+
TYPED_ARRAY = "the byteLength of a typed byte array such as Uint8Array",
11+
LENGTH = "the value of Body.length",
12+
SIZE = "the value of Body.size",
13+
START_END_DIFF = "the numeric difference between Body.start and Body.end",
14+
LSTAT = "the size of the file given by Body.path on disk as reported by lstatSync",
15+
}
16+
17+
/**
18+
* The returned value should complete the sentence, "The byte count of the data was determined by ...".
19+
* @internal
20+
* @param input - to examine.
21+
* @param override - manually specified value.
22+
* @returns source of byte count information.
23+
*/
24+
export const byteLengthSource = (input: any, override?: number): BYTE_LENGTH_SOURCE | undefined => {
25+
if (override != null) {
26+
return BYTE_LENGTH_SOURCE.CONTENT_LENGTH;
27+
}
28+
29+
if (input == null) {
30+
return BYTE_LENGTH_SOURCE.EMPTY_INPUT;
31+
}
32+
33+
if (typeof input === "string") {
34+
return BYTE_LENGTH_SOURCE.STRING_LENGTH;
35+
}
36+
37+
if (typeof input.byteLength === "number") {
38+
return BYTE_LENGTH_SOURCE.TYPED_ARRAY;
39+
} else if (typeof input.length === "number") {
40+
return BYTE_LENGTH_SOURCE.LENGTH;
41+
} else if (typeof input.size === "number") {
42+
return BYTE_LENGTH_SOURCE.SIZE;
43+
} else if (typeof input.start === "number" && typeof input.end === "number") {
44+
return BYTE_LENGTH_SOURCE.START_END_DIFF;
45+
} else if (typeof input.path === "string") {
46+
try {
47+
runtimeConfig.lstatSync(input.path).size;
48+
return BYTE_LENGTH_SOURCE.LSTAT;
49+
} catch (error) {
50+
return undefined;
51+
}
52+
}
53+
return undefined;
54+
};

lib/lib-storage/src/bytelength.ts

Lines changed: 0 additions & 26 deletions
This file was deleted.

0 commit comments

Comments
 (0)