Skip to content
This repository was archived by the owner on Mar 3, 2026. It is now read-only.

feat: add crc32c validation option to chunked download, cleanup naming #2110

Merged
ddelgrosso1 merged 2 commits into from
Dec 6, 2022
Merged

feat: add crc32c validation option to chunked download, cleanup naming #2110

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
bb4d849
feat: add crc32c validation option to chunked download, cleanup namin…
ddelgrosso1 Dec 5, 2022
d220f33
close file in finally block
ddelgrosso1 Dec 5, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 8 additions & 7 deletions internal-tooling/performTransferManagerTest.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,9 +60,9 @@ const argv = yargs(process.argv.slice(2))
testtype: {
type: 'string',
choices: [
TRANSFER_MANAGER_TEST_TYPES.TRANSFER_MANAGER_UPLOAD_MULTIPLE_OBJECTS,
TRANSFER_MANAGER_TEST_TYPES.TRANSFER_MANAGER_DOWNLOAD_MULTIPLE_OBJECTS,
TRANSFER_MANAGER_TEST_TYPES.TRANSFER_MANAGER_LARGE_FILE_DOWNLOAD,
TRANSFER_MANAGER_TEST_TYPES.TRANSFER_MANAGER_UPLOAD_MANY_FILES,
TRANSFER_MANAGER_TEST_TYPES.TRANSFER_MANAGER_DOWNLOAD_MANY_FILES,
TRANSFER_MANAGER_TEST_TYPES.TRANSFER_MANAGER_CHUNKED_FILE_DOWNLOAD,
],
},
})
Expand Down Expand Up @@ -92,13 +92,13 @@ async function main() {
));

switch (argv.testtype) {
case TRANSFER_MANAGER_TEST_TYPES.TRANSFER_MANAGER_UPLOAD_MULTIPLE_OBJECTS:
case TRANSFER_MANAGER_TEST_TYPES.TRANSFER_MANAGER_UPLOAD_MANY_FILES:
result = await performUploadManyFilesTest();
break;
case TRANSFER_MANAGER_TEST_TYPES.TRANSFER_MANAGER_DOWNLOAD_MULTIPLE_OBJECTS:
case TRANSFER_MANAGER_TEST_TYPES.TRANSFER_MANAGER_DOWNLOAD_MANY_FILES:
result = await performDownloadManyFilesTest();
break;
case TRANSFER_MANAGER_TEST_TYPES.TRANSFER_MANAGER_LARGE_FILE_DOWNLOAD:
case TRANSFER_MANAGER_TEST_TYPES.TRANSFER_MANAGER_CHUNKED_FILE_DOWNLOAD:
result = await performDownloadFileInChunksTest();
break;
default:
Expand Down Expand Up @@ -225,6 +225,7 @@ async function performDownloadFileInChunksTest(): Promise<TestResult> {
concurrencyLimit: argv.numpromises,
chunkSizeBytes: argv.chunksize,
destination: path.join(__dirname, fileName),
validation: checkType === 'crc32c' ? checkType : false,
});
const end = performance.now();

Expand All @@ -235,7 +236,7 @@ async function performDownloadFileInChunksTest(): Promise<TestResult> {
objectSize: sizeInBytes,
appBufferSize: BLOCK_SIZE_IN_BYTES,
libBufferSize: NODE_DEFAULT_HIGHWATER_MARK_BYTES,
crc32Enabled: false,
crc32Enabled: checkType === 'crc32c',
md5Enabled: false,
apiName: 'JSON',
elapsedTimeUs: Math.round((end - start) * 1000),
Expand Down
18 changes: 9 additions & 9 deletions internal-tooling/performanceTest.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,9 @@ const CSV_HEADERS =
const START_TIME = Date.now();
export const enum TRANSFER_MANAGER_TEST_TYPES {
WRITE_ONE_READ_THREE = 'w1r3',
TRANSFER_MANAGER_UPLOAD_MULTIPLE_OBJECTS = 'tm-upload',
TRANSFER_MANAGER_DOWNLOAD_MULTIPLE_OBJECTS = 'tm-download',
TRANSFER_MANAGER_LARGE_FILE_DOWNLOAD = 'tm-large',
TRANSFER_MANAGER_UPLOAD_MANY_FILES = 'tm-upload',
TRANSFER_MANAGER_DOWNLOAD_MANY_FILES = 'tm-download',
TRANSFER_MANAGER_CHUNKED_FILE_DOWNLOAD = 'tm-chunked',
APPLICATION_LARGE_FILE_DOWNLOAD = 'application-large',
APPLICATION_UPLOAD_MULTIPLE_OBJECTS = 'application-upload',
APPLICATION_DOWNLOAD_MULTIPLE_OBJECTS = 'application-download',
Expand All @@ -45,9 +45,9 @@ const argv = yargs(process.argv.slice(2))
type: 'string',
choices: [
TRANSFER_MANAGER_TEST_TYPES.WRITE_ONE_READ_THREE,
TRANSFER_MANAGER_TEST_TYPES.TRANSFER_MANAGER_UPLOAD_MULTIPLE_OBJECTS,
TRANSFER_MANAGER_TEST_TYPES.TRANSFER_MANAGER_DOWNLOAD_MULTIPLE_OBJECTS,
TRANSFER_MANAGER_TEST_TYPES.TRANSFER_MANAGER_LARGE_FILE_DOWNLOAD,
TRANSFER_MANAGER_TEST_TYPES.TRANSFER_MANAGER_UPLOAD_MANY_FILES,
TRANSFER_MANAGER_TEST_TYPES.TRANSFER_MANAGER_DOWNLOAD_MANY_FILES,
TRANSFER_MANAGER_TEST_TYPES.TRANSFER_MANAGER_CHUNKED_FILE_DOWNLOAD,
TRANSFER_MANAGER_TEST_TYPES.APPLICATION_DOWNLOAD_MULTIPLE_OBJECTS,
TRANSFER_MANAGER_TEST_TYPES.APPLICATION_LARGE_FILE_DOWNLOAD,
TRANSFER_MANAGER_TEST_TYPES.APPLICATION_UPLOAD_MULTIPLE_OBJECTS,
Expand Down Expand Up @@ -95,11 +95,11 @@ function createWorker() {
testPath = `${__dirname}/performPerformanceTest.js`;
} else if (
argv.testtype ===
TRANSFER_MANAGER_TEST_TYPES.TRANSFER_MANAGER_UPLOAD_MULTIPLE_OBJECTS ||
TRANSFER_MANAGER_TEST_TYPES.TRANSFER_MANAGER_UPLOAD_MANY_FILES ||
argv.testtype ===
TRANSFER_MANAGER_TEST_TYPES.TRANSFER_MANAGER_LARGE_FILE_DOWNLOAD ||
TRANSFER_MANAGER_TEST_TYPES.TRANSFER_MANAGER_CHUNKED_FILE_DOWNLOAD ||
argv.testtype ===
TRANSFER_MANAGER_TEST_TYPES.TRANSFER_MANAGER_DOWNLOAD_MULTIPLE_OBJECTS
TRANSFER_MANAGER_TEST_TYPES.TRANSFER_MANAGER_DOWNLOAD_MANY_FILES
) {
testPath = `${__dirname}/performTransferManagerTest.js`;
} else if (
Expand Down
32 changes: 24 additions & 8 deletions src/transfer-manager.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import * as pLimit from 'p-limit';
import * as path from 'path';
import * as extend from 'extend';
import {promises as fsp} from 'fs';
import {CRC32C} from './crc32c';

/**
* Default number of concurrently executing promises to use when calling uploadManyFiles.
Expand Down Expand Up @@ -65,6 +66,7 @@ export interface DownloadFileInChunksOptions {
concurrencyLimit?: number;
chunkSizeBytes?: number;
destination?: string;
validation?: 'crc32c' | false;
}

/**
Expand Down Expand Up @@ -264,6 +266,7 @@ export class TransferManager {
* @property {number} [concurrencyLimit] The number of concurrently executing promises
* to use when downloading the file.
* @property {number} [chunkSizeBytes] The size in bytes of each chunk to be downloaded.
* @property {string | boolean} [validation] Whether or not to perform a CRC32C validation check when download is complete.
* @experimental
*/
/**
Expand Down Expand Up @@ -299,7 +302,7 @@ export class TransferManager {
let limit = pLimit(
options.concurrencyLimit || DEFAULT_PARALLEL_CHUNKED_DOWNLOAD_LIMIT
);
const promises = [];
const promises: Promise<{bytesWritten: number; buffer: Buffer}>[] = [];
const file: File =
typeof fileOrName === 'string'
? this.bucket.file(fileOrName)
Expand Down Expand Up @@ -331,12 +334,25 @@ export class TransferManager {
start += chunkSize;
}

return Promise.all(promises)
.then(results => {
return results.map(result => result.buffer) as DownloadResponse;
})
.finally(async () => {
await fileToWrite.close();
});
return new Promise((resolve, reject) => {
let results: DownloadResponse;
Promise.all(promises)
.then(data => {
results = data.map(result => result.buffer) as DownloadResponse;
if (options.validation === 'crc32c') {
return CRC32C.fromFile(filePath);
Copy link
Copy Markdown
Contributor

@frankyn frankyn Dec 6, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this a post processing hash validation step once for both single download and multi-chunk download?

Copy link
Copy Markdown
Contributor Author

@ddelgrosso1 ddelgrosso1 Dec 6, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a post-file download CRC32C validation (if enabled). It will occur if the user opts in and the file is downloaded as a single chunk or mutichunks

}
return;
})
.then(() => {
resolve(results);
})
.catch(e => {
reject(e);
})
.finally(() => {
fileToWrite.close();
});
});
}
}
14 changes: 14 additions & 0 deletions test/transfer-manager.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -290,5 +290,19 @@ describe('Transfer Manager', () => {
await transferManager.downloadFileInChunks(file);
assert.strictEqual(downloadCallCount, 1);
});

it('should call fromFile when validation is set to crc32c', async () => {
let callCount = 0;
file.download = () => {
return Promise.resolve([Buffer.alloc(0)]);
};
CRC32C.fromFile = () => {
callCount++;
return Promise.resolve(new CRC32C(0));
};

await transferManager.downloadFileInChunks(file, {validation: 'crc32c'});
assert.strictEqual(callCount, 1);
});
});
});