diff --git a/src/commands/folders/download.js b/src/commands/folders/download.js index 526c6aae..63991d2a 100644 --- a/src/commands/folders/download.js +++ b/src/commands/folders/download.js @@ -81,6 +81,10 @@ class FoldersDownloadCommand extends BoxCommand { 'YYYY-MM-DDTHH_mm_ss_SSS' )}.zip`; rootItemPath = fileName; + // Wait for the archiver module before starting traversal so this.zip is + // assigned before the first item is processed; otherwise items take the + // non-zip path and the archive is never finalized. + await this._loadZipArchive(); outputFinalized = this._setupZip( path.join(destinationPath, fileName) ); @@ -219,10 +223,20 @@ class FoldersDownloadCommand extends BoxCommand { * @throws BoxCLIError * @private */ - async _setupZip(destinationPath) { + async _loadZipArchive() { if (!ZipArchive) { ({ ZipArchive } = await import('archiver')); } + } + + /** + * Sets up a zip archive writing to the given destination. Requires + * _loadZipArchive() to have completed first. + * @param {string} destinationPath The path to write the zip file to + * @returns {Promise} A promise resolving when the archive is finalized and written to disk + * @private + */ + _setupZip(destinationPath) { this.zip = new ZipArchive({ zlib: { level: 9 }, }); diff --git a/test/commands/folders.test.js b/test/commands/folders.test.js index 1ca0af85..e6dce411 100644 --- a/test/commands/folders.test.js +++ b/test/commands/folders.test.js @@ -15,8 +15,48 @@ const { } = require('../helpers/test-helper'); const os = require('node:os'); const leche = require('leche'); +const sinon = require('sinon'); +const FoldersDownloadCommand = require('../../src/commands/folders/download'); const _ = require('lodash'); +/** + * List the entry names of a zip file by walking its central directory, so the + * tests do not need an extra unzip dependency. + * @param {string} zipPath Path to the zip file + * @returns {string[]} Sorted entry names with forward slashes + */ +function getZipEntryNames(zipPath) { + const buffer = fs.readFileSync(zipPath); + const EOCD_SIGNATURE = 0x06_05_4b_50; + const CENTRAL_HEADER_SIGNATURE = 0x02_01_4b_50; + let eocdOffset = buffer.length - 22; + while ( + eocdOffset >= 0 && + buffer.readUInt32LE(eocdOffset) !== EOCD_SIGNATURE + ) { + eocdOffset -= 1; + } + if (eocdOffset < 0) { + return []; + } + const entryCount = buffer.readUInt16LE(eocdOffset + 10); + let offset = buffer.readUInt32LE(eocdOffset + 16); + const names = []; + for (let i = 0; i < entryCount; i++) { + assert.equal(buffer.readUInt32LE(offset), CENTRAL_HEADER_SIGNATURE); + const nameLength = buffer.readUInt16LE(offset + 28); + const extraLength = buffer.readUInt16LE(offset + 30); + const commentLength = buffer.readUInt16LE(offset + 32); + names.push( + buffer + .toString('utf8', offset + 46, offset + 46 + nameLength) + .replaceAll('\\', '/') + ); + offset += 46 + nameLength + extraLength + commentLength; + } + return names.sort(); +} + describe('Folders', function () { describe('folders:get', function () { let folderId = '0', @@ -1871,6 +1911,67 @@ describe('Folders', function () { } ); + // The archiver module is imported lazily and cached at module level, so the + // race this guards against only happens on the first --zip download in a + // process. Mocha runs a suite's own tests in order, so keep this test ahead + // of the other --zip tests. + let zipReadyBeforeTraversal; + test.do(() => { + const originalGetItems = FoldersDownloadCommand.prototype._getItems; + zipReadyBeforeTraversal = undefined; + sinon + .stub(FoldersDownloadCommand.prototype, '_getItems') + .callsFake(function (...args) { + zipReadyBeforeTraversal = Boolean(this.zip); + return originalGetItems.apply(this, args); + }); + }) + .nock(TEST_API_ROOT, (api) => + api + .get(`/2.0/folders/${folderID}`) + .reply(200, getFolderFixture) + .get('/2.0/folders/22222') + .reply(200, getSubfolderFixture) + .get('/2.0/files/77777/content') + .reply(302, '', { Location: `${TEST_DOWNLOAD_ROOT}/77777` }) + .get('/2.0/files/44444/content') + .reply(302, '', { Location: `${TEST_DOWNLOAD_ROOT}/44444` }) + .get('/2.0/files/55555/content') + .reply(302, '', { Location: `${TEST_DOWNLOAD_ROOT}/55555` }) + ) + .nock(TEST_DOWNLOAD_ROOT, (api) => + api + .get('/44444') + .reply(200, expectedContents['file 1.txt']) + .get('/55555') + .reply(200, expectedContents['file 2.txt']) + .get('/77777') + .reply( + 200, + expectedContents.subfolder['subfolder file 1.txt'] + ) + ) + .stdout() + .stderr() + .command([ + 'folders:download', + folderID, + `--destination=${downloadPath}`, + '--zip', + '--token=test', + ]) + .it( + 'should initialize the zip archive before traversing the folder', + async () => { + sinon.restore(); + await fs.remove(downloadPath); + assert.isTrue( + zipReadyBeforeTraversal, + 'this.zip must be set before _getItems() runs' + ); + } + ); + test.nock(TEST_API_ROOT, (api) => api .get(`/2.0/folders/${folderID}`) @@ -1908,18 +2009,25 @@ describe('Folders', function () { .it( 'should download folder to zip file when --zip flag is passed', async (context) => { - // Find zip file in directory const files = await fs.readdir(downloadPath); let filename = files.find( (f) => f.startsWith(`folders-download-${folderID}`) && f.endsWith('.zip') ); + assert.ok(filename, 'Zip file should have been found'); + let zipEntries = getZipEntryNames( + path.join(downloadPath, filename) + ); await fs.remove(downloadPath); - // @TODO(2018-10-30): Verify contents of zip file - - assert.ok(filename, 'File shoudl have been found'); + // Everything must be inside the archive; nothing may be written next to it + assert.deepEqual(files, [filename]); + assert.deepEqual(zipEntries, [ + `${folderName}/file 1.txt`, + `${folderName}/file 2.txt`, + `${folderName}/subfolder/subfolder file 1.txt`, + ]); assert.equal(context.stdout, ''); } );