Skip to content

Commit 7d58598

Browse files
authored
feat(storage): upload with fullbody checksum (#14383)
1 parent fd56414 commit 7d58598

File tree

18 files changed

+224
-440
lines changed

18 files changed

+224
-440
lines changed

packages/storage/__tests__/providers/s3/apis/internal/uploadData/multipartHandlers.test.ts

Lines changed: 131 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -75,11 +75,7 @@ const getZeroDelayTimeout = () =>
7575

7676
const mockCalculateContentCRC32Mock = () => {
7777
mockCalculateContentCRC32.mockReset();
78-
mockCalculateContentCRC32.mockResolvedValue({
79-
checksumArrayBuffer: new ArrayBuffer(0),
80-
checksum: 'mockChecksum',
81-
seed: 0,
82-
});
78+
mockCalculateContentCRC32.mockResolvedValue('mockChecksum');
8379
};
8480
const mockCalculateContentCRC32Reset = () => {
8581
mockCalculateContentCRC32.mockReset();
@@ -276,14 +272,25 @@ describe('getMultipartUploadHandlers with key', () => {
276272
'file',
277273
new File([getBlob(8 * MB)], 'someName'),
278274
['JCnBsQ==', 'HELzGQ=='],
275+
'/YBlgg==',
276+
],
277+
['blob', getBlob(8 * MB), ['JCnBsQ==', 'HELzGQ=='], '/YBlgg=='],
278+
['string', 'Ü'.repeat(4 * MB), ['DL735w==', 'Akga7g=='], 'dPB9Lw=='],
279+
[
280+
'arrayBuffer',
281+
new ArrayBuffer(8 * MB),
282+
['yTuzdQ==', 'eXJPxg=='],
283+
'GtK8RQ==',
284+
],
285+
[
286+
'arrayBufferView',
287+
new Uint8Array(8 * MB),
288+
['yTuzdQ==', 'eXJPxg=='],
289+
'GtK8RQ==',
279290
],
280-
['blob', getBlob(8 * MB), ['JCnBsQ==', 'HELzGQ==']],
281-
['string', 'Ü'.repeat(4 * MB), ['DL735w==', 'Akga7g==']],
282-
['arrayBuffer', new ArrayBuffer(8 * MB), ['yTuzdQ==', 'eXJPxg==']],
283-
['arrayBufferView', new Uint8Array(8 * MB), ['yTuzdQ==', 'eXJPxg==']],
284291
])(
285292
`should create crc32 for %s type body`,
286-
async (_, twoPartsPayload, expectedCrc32) => {
293+
async (_, twoPartsPayload, expectedCrc32, finalCrc32) => {
287294
mockMultipartUploadSuccess();
288295
const { multipartUploadJob } = getMultipartUploadHandlers(
289296
{
@@ -298,17 +305,12 @@ describe('getMultipartUploadHandlers with key', () => {
298305
await multipartUploadJob();
299306

300307
/**
301-
* final crc32 calculation calls calculateContentCRC32 3 times
302-
* 1 time for each of the 2 parts
303-
* 1 time to combine the resulting hash for each of the two parts
304-
*
305-
* uploading each part calls calculateContentCRC32 1 time each
306-
*
307-
* 1 time for optionsHash
308-
*
309-
* these steps results in 6 calls in total
308+
* `calculateContentCRC32` is called 4 times with Full-body checksum:
309+
* * 1 time when calculating final crc32 for the whole object to be uploaded.
310+
* * 1 time when calculating optionsHash.
311+
* * 1 time each when uploading part calls, 2 calls in total.
310312
*/
311-
expect(calculateContentCRC32).toHaveBeenCalledTimes(6);
313+
expect(calculateContentCRC32).toHaveBeenCalledTimes(4);
312314
expect(calculateContentMd5).not.toHaveBeenCalled();
313315
expect(mockUploadPart).toHaveBeenCalledTimes(2);
314316
expect(mockUploadPart).toHaveBeenCalledWith(
@@ -319,6 +321,13 @@ describe('getMultipartUploadHandlers with key', () => {
319321
expect.anything(),
320322
expect.objectContaining({ ChecksumCRC32: expectedCrc32[1] }),
321323
);
324+
expect(mockCompleteMultipartUpload).toHaveBeenCalledWith(
325+
expect.anything(),
326+
expect.objectContaining({
327+
ChecksumCRC32: finalCrc32,
328+
ChecksumType: 'FULL_OBJECT',
329+
}),
330+
);
322331
},
323332
);
324333

@@ -389,7 +398,7 @@ describe('getMultipartUploadHandlers with key', () => {
389398
file.size,
390399
);
391400
await multipartUploadJob();
392-
expect(file.slice).toHaveBeenCalledTimes(10_000 * 2); // S3 limit of parts count double for crc32 calculations
401+
expect(file.slice).toHaveBeenCalledTimes(10_000);
393402
expect(mockCreateMultipartUpload).toHaveBeenCalledTimes(1);
394403
expect(mockUploadPart).toHaveBeenCalledTimes(10_000);
395404
expect(mockCompleteMultipartUpload).toHaveBeenCalledTimes(1);
@@ -565,6 +574,7 @@ describe('getMultipartUploadHandlers with key', () => {
565574
bucket,
566575
key: defaultKey,
567576
finalCrc32: 'mock-crc32',
577+
lastTouched: Date.now() - 2 * 60 * 1000, // 2 mins ago
568578
},
569579
}),
570580
);
@@ -651,7 +661,7 @@ describe('getMultipartUploadHandlers with key', () => {
651661
await multipartUploadJob();
652662
expect(mockCalculateContentCRC32).toHaveBeenNthCalledWith(
653663
1,
654-
JSON.stringify(serializableOptions),
664+
JSON.stringify({ ...serializableOptions, checksumType: 'FULL_OBJECT' }),
655665
);
656666
});
657667

@@ -734,14 +744,46 @@ describe('getMultipartUploadHandlers with key', () => {
734744
]);
735745
});
736746

747+
it('should clean any outdated in-progress uploads', async () => {
748+
mockDefaultStorage.getItem.mockResolvedValue(
749+
JSON.stringify({
750+
'other-outdated-update': {
751+
uploadId: '000',
752+
bucket,
753+
key: defaultKey,
754+
lastTouched: Date.now() - 2 * 60 * 60 * 1000, // 2 hours ago
755+
},
756+
}),
757+
);
758+
mockMultipartUploadSuccess();
759+
mockListParts.mockResolvedValueOnce({ Parts: [], $metadata: {} });
760+
const size = 8 * MB;
761+
const { multipartUploadJob } = getMultipartUploadHandlers(
762+
{
763+
key: defaultKey,
764+
data: new File([new ArrayBuffer(size)], 'someName'),
765+
options: {
766+
resumableUploadsCache: mockDefaultStorage,
767+
},
768+
},
769+
size,
770+
);
771+
await multipartUploadJob();
772+
// 1 for evicting outdated cached uploads;
773+
// 1 for caching upload task;
774+
// 1 for remove cache after upload is completed
775+
expect(mockDefaultStorage.setItem).toHaveBeenCalledTimes(3);
776+
expect(mockDefaultStorage.setItem.mock.calls[0][1]).toEqual('{}');
777+
});
778+
737779
it('should send listParts request if the upload task is cached', async () => {
738780
mockDefaultStorage.getItem.mockResolvedValue(
739781
JSON.stringify({
740782
[defaultCacheKey]: {
741783
uploadId: 'uploadId',
742784
bucket,
743785
key: defaultKey,
744-
lastModified: Date.now(),
786+
lastTouched: Date.now(),
745787
},
746788
}),
747789
);
@@ -960,6 +1002,7 @@ describe('getMultipartUploadHandlers with key', () => {
9601002
uploadId: 'uploadId',
9611003
bucket,
9621004
key: defaultKey,
1005+
lastTouched: Date.now(),
9631006
},
9641007
}),
9651008
);
@@ -1090,14 +1133,25 @@ describe('getMultipartUploadHandlers with path', () => {
10901133
'file',
10911134
new File([getBlob(8 * MB)], 'someName'),
10921135
['JCnBsQ==', 'HELzGQ=='],
1136+
'/YBlgg==',
1137+
],
1138+
['blob', getBlob(8 * MB), ['JCnBsQ==', 'HELzGQ=='], '/YBlgg=='],
1139+
['string', 'Ü'.repeat(4 * MB), ['DL735w==', 'Akga7g=='], 'dPB9Lw=='],
1140+
[
1141+
'arrayBuffer',
1142+
new ArrayBuffer(8 * MB),
1143+
['yTuzdQ==', 'eXJPxg=='],
1144+
'GtK8RQ==',
1145+
],
1146+
[
1147+
'arrayBufferView',
1148+
new Uint8Array(8 * MB),
1149+
['yTuzdQ==', 'eXJPxg=='],
1150+
'GtK8RQ==',
10931151
],
1094-
['blob', getBlob(8 * MB), ['JCnBsQ==', 'HELzGQ==']],
1095-
['string', 'Ü'.repeat(4 * MB), ['DL735w==', 'Akga7g==']],
1096-
['arrayBuffer', new ArrayBuffer(8 * MB), ['yTuzdQ==', 'eXJPxg==']],
1097-
['arrayBufferView', new Uint8Array(8 * MB), ['yTuzdQ==', 'eXJPxg==']],
10981152
])(
10991153
`should create crc32 for %s type body`,
1100-
async (_, twoPartsPayload, expectedCrc32) => {
1154+
async (_, twoPartsPayload, expectedCrc32, finalCrc32) => {
11011155
mockMultipartUploadSuccess();
11021156
const { multipartUploadJob } = getMultipartUploadHandlers(
11031157
{
@@ -1112,17 +1166,12 @@ describe('getMultipartUploadHandlers with path', () => {
11121166
await multipartUploadJob();
11131167

11141168
/**
1115-
* final crc32 calculation calls calculateContentCRC32 3 times
1116-
* 1 time for each of the 2 parts
1117-
* 1 time to combine the resulting hash for each of the two parts
1118-
*
1119-
* uploading each part calls calculateContentCRC32 1 time each
1120-
*
1121-
* 1 time for optionsHash
1122-
*
1123-
* these steps results in 6 calls in total
1169+
* `calculateContentCRC32` is called 4 times with Full-body checksum:
1170+
* * 1 time when calculating final crc32 for the whole object to be uploaded.
1171+
* * 1 time when calculating optionsHash.
1172+
* * 1 time each when uploading part calls, 2 calls in total.
11241173
*/
1125-
expect(calculateContentCRC32).toHaveBeenCalledTimes(6);
1174+
expect(calculateContentCRC32).toHaveBeenCalledTimes(4);
11261175
expect(calculateContentMd5).not.toHaveBeenCalled();
11271176
expect(mockUploadPart).toHaveBeenCalledTimes(2);
11281177
expect(mockUploadPart).toHaveBeenCalledWith(
@@ -1133,6 +1182,13 @@ describe('getMultipartUploadHandlers with path', () => {
11331182
expect.anything(),
11341183
expect.objectContaining({ ChecksumCRC32: expectedCrc32[1] }),
11351184
);
1185+
expect(mockCompleteMultipartUpload).toHaveBeenCalledWith(
1186+
expect.anything(),
1187+
expect.objectContaining({
1188+
ChecksumCRC32: finalCrc32,
1189+
ChecksumType: 'FULL_OBJECT',
1190+
}),
1191+
);
11361192
},
11371193
);
11381194

@@ -1203,7 +1259,7 @@ describe('getMultipartUploadHandlers with path', () => {
12031259
file.size,
12041260
);
12051261
await multipartUploadJob();
1206-
expect(file.slice).toHaveBeenCalledTimes(10_000 * 2); // S3 limit of parts count double for crc32 calculations
1262+
expect(file.slice).toHaveBeenCalledTimes(10_000);
12071263
expect(mockCreateMultipartUpload).toHaveBeenCalledTimes(1);
12081264
expect(mockUploadPart).toHaveBeenCalledTimes(10_000);
12091265
expect(mockCompleteMultipartUpload).toHaveBeenCalledTimes(1);
@@ -1413,6 +1469,7 @@ describe('getMultipartUploadHandlers with path', () => {
14131469
bucket,
14141470
key: defaultKey,
14151471
finalCrc32: 'mock-crc32',
1472+
lastTouched: Date.now(),
14161473
},
14171474
}),
14181475
);
@@ -1519,7 +1576,7 @@ describe('getMultipartUploadHandlers with path', () => {
15191576
await multipartUploadJob();
15201577
expect(mockCalculateContentCRC32).toHaveBeenNthCalledWith(
15211578
1,
1522-
JSON.stringify(serializableOptions),
1579+
JSON.stringify({ ...serializableOptions, checksumType: 'FULL_OBJECT' }),
15231580
);
15241581
});
15251582

@@ -1583,14 +1640,46 @@ describe('getMultipartUploadHandlers with path', () => {
15831640
]);
15841641
});
15851642

1643+
it('should clean any outdated in-progress uploads', async () => {
1644+
mockDefaultStorage.getItem.mockResolvedValue(
1645+
JSON.stringify({
1646+
'other-outdated-update': {
1647+
uploadId: '000',
1648+
bucket,
1649+
key: defaultKey,
1650+
lastTouched: Date.now() - 2 * 60 * 60 * 1000, // 2 hours ago
1651+
},
1652+
}),
1653+
);
1654+
mockMultipartUploadSuccess();
1655+
mockListParts.mockResolvedValueOnce({ Parts: [], $metadata: {} });
1656+
const size = 8 * MB;
1657+
const { multipartUploadJob } = getMultipartUploadHandlers(
1658+
{
1659+
path: testPath,
1660+
data: new File([new ArrayBuffer(size)], 'someName'),
1661+
options: {
1662+
resumableUploadsCache: mockDefaultStorage,
1663+
},
1664+
},
1665+
size,
1666+
);
1667+
await multipartUploadJob();
1668+
// 1 for evicting outdated cached uploads;
1669+
// 1 for caching upload task;
1670+
// 1 for remove cache after upload is completed
1671+
expect(mockDefaultStorage.setItem).toHaveBeenCalledTimes(3);
1672+
expect(mockDefaultStorage.setItem.mock.calls[0][1]).toEqual('{}');
1673+
});
1674+
15861675
it('should send listParts request if the upload task is cached', async () => {
15871676
mockDefaultStorage.getItem.mockResolvedValue(
15881677
JSON.stringify({
15891678
[testPathCacheKey]: {
15901679
uploadId: 'uploadId',
15911680
bucket,
15921681
key: testPath,
1593-
lastModified: Date.now(),
1682+
lastTouched: Date.now(),
15941683
},
15951684
}),
15961685
);
@@ -1808,6 +1897,7 @@ describe('getMultipartUploadHandlers with path', () => {
18081897
uploadId: 'uploadId',
18091898
bucket,
18101899
key: testPath,
1900+
lastTouched: Date.now(),
18111901
},
18121902
}),
18131903
);

packages/storage/__tests__/providers/s3/utils/client/S3/cases/completeMultipartUpload.ts

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -59,9 +59,18 @@ const completeMultipartUploadHappyCase: ApiFunctionalTestCase<
5959
},
6060
],
6161
},
62+
ChecksumCRC32: '123',
63+
ChecksumType: 'FULL_OBJECT',
6264
UploadId: 'uploadId',
6365
},
64-
expect.objectContaining(defaultExpectedRequest),
66+
expect.objectContaining({
67+
...defaultExpectedRequest,
68+
headers: {
69+
'content-type': 'application/xml',
70+
'x-amz-checksum-crc32': '123',
71+
'x-amz-checksum-type': 'FULL_OBJECT',
72+
},
73+
}),
6574
{
6675
status: 200,
6776
headers: { ...DEFAULT_RESPONSE_HEADERS },
@@ -94,10 +103,10 @@ const completeMultipartUploadHappyCaseIfNoneMatch: ApiFunctionalTestCase<
94103
},
95104
expect.objectContaining({
96105
...defaultExpectedRequest,
97-
headers: {
106+
headers: expect.objectContaining({
98107
'content-type': 'application/xml',
99108
'If-None-Match': 'mock-if-none-match',
100-
},
109+
}),
101110
}),
102111
completeMultipartUploadHappyCase[6],
103112
completeMultipartUploadHappyCase[7],

0 commit comments

Comments
 (0)