fix: allow files in directories to be downloaded onto local machine (googleapis#2199)

* fix: allow files in nested folders to be downloaded onto local machine

* fix: revert removal of options.prefix check

* test: file.download should create directory paths recursively

* fix: move the filtering of directory objects from TransferManager.downloadManyFiles to File.download

* fix: handle windows path

* chore: remove temporary comments

* test: add scenarios where destination is set in Transfer Manager

* chore: remove console.warn

* fix: merge with main

* fix: added transfer manager tests for prefix

* fix: address PR comments

* fix: paths for windows ci

* fix: migrate mkdirsync to await fs/promises/mkdir

* fix: var name

---------

Co-authored-by: Vishwaraj Anand <vishwaraj.anand00@gmail.com>

Author: hochoy

handwritten/storage/src/file.ts

@@ -2288,12 +2288,13 @@ class File extends ServiceObject<File, FileMetadata> {
 
     const fileStream = this.createReadStream(options);
     let receivedData = false;
+
     if (destination) {
       fileStream
         .on('error', callback)
         .once('data', data => {
-          // We know that the file exists the server - now we can truncate/write to a file
           receivedData = true;
+          // We know that the file exists the server - now we can truncate/write to a file
           const writable = fs.createWriteStream(destination);
           writable.write(data);
           fileStream

handwritten/storage/src/transfer-manager.ts

@@ -526,7 +526,9 @@ export class TransferManager {
    *
    * @param {array | string} [filesOrFolder] An array of file name strings or file objects to be downloaded. If
    * a string is provided this will be treated as a GCS prefix and all files with that prefix will be downloaded.
-   * @param {DownloadManyFilesOptions} [options] Configuration options.
+   * @param {DownloadManyFilesOptions} [options] Configuration options. Setting options.prefix or options.stripPrefix
+   * or options.passthroughOptions.destination will cause the downloaded files to be written to the file system
+   * instead of being returned as a buffer.
    * @returns {Promise<DownloadResponse[]>}
    *
    * @example
@@ -587,7 +589,7 @@ export class TransferManager {
         [GCCL_GCS_CMD_KEY]: GCCL_GCS_CMD_FEATURE.DOWNLOAD_MANY,
       };
 
-      if (options.prefix) {
+      if (options.prefix || passThroughOptionsCopy.destination) {
         passThroughOptionsCopy.destination = path.join(
           options.prefix || '',
           passThroughOptionsCopy.destination || '',
@@ -598,7 +600,19 @@ export class TransferManager {
         passThroughOptionsCopy.destination = file.name.replace(regex, '');
       }
 
-      promises.push(limit(() => file.download(passThroughOptionsCopy)));
+      promises.push(
+        limit(async () => {
+          const destination = passThroughOptionsCopy.destination;
+          if (destination && destination.endsWith(path.sep)) {
+            await fsp.mkdir(destination, {recursive: true});
+            return Promise.resolve([
+              Buffer.alloc(0),
+            ]) as Promise<DownloadResponse>;
+          }
+
+          return file.download(passThroughOptionsCopy);
+        })
+      );
     }
 
     return Promise.all(promises);

handwritten/storage/test/file.ts

@@ -28,6 +28,7 @@ import assert from 'assert';
 import * as crypto from 'crypto';
 import duplexify from 'duplexify';
 import * as fs from 'fs';
+import * as path from 'path';
 import proxyquire from 'proxyquire';
 import * as resumableUpload from '../src/resumable-upload.js';
 import * as sinon from 'sinon';
@@ -2571,6 +2572,12 @@ describe('File', () => {
     });
 
     describe('with destination', () => {
+      const sandbox = sinon.createSandbox();
+
+      afterEach(() => {
+        sandbox.restore();
+      });
+
       it('should write the file to a destination if provided', done => {
         tmp.setGracefulCleanup();
         tmp.file((err, tmpFilePath) => {
@@ -2694,6 +2701,29 @@ describe('File', () => {
           });
         });
       });
+
+      it('should fail if provided destination directory does not exist', done => {
+        tmp.setGracefulCleanup();
+        tmp.dir(async (err, tmpDirPath) => {
+          assert.ifError(err);
+
+          const fileContents = 'nested-abcdefghijklmnopqrstuvwxyz';
+
+          Object.assign(fileReadStream, {
+            _read(this: Readable) {
+              this.push(fileContents);
+              this.push(null);
+            },
+          });
+
+          const nestedPath = path.join(tmpDirPath, 'a', 'b', 'c', 'file.txt');
+
+          file.download({destination: nestedPath}, (err: Error) => {
+            assert.ok(err);
+            done();
+          });
+        });
+      });
     });
   });
 

handwritten/storage/test/transfer-manager.ts

@@ -19,6 +19,7 @@ import {
   Bucket,
   File,
   CRC32C,
+  DownloadCallback,
   DownloadOptions,
   IdempotencyStrategy,
   MultiPartHelperGenerator,
@@ -233,6 +234,78 @@ describe('Transfer Manager', () => {
 
       await transferManager.downloadManyFiles([file]);
     });
+
+    it('sets the destination correctly when provided a passthroughOptions.destination', async () => {
+      const passthroughOptions = {
+        destination: 'test-destination',
+      };
+      const filename = 'first.txt';
+      const expectedDestination = path.normalize(
+        `${passthroughOptions.destination}/${filename}`
+      );
+      const download = (optionsOrCb?: DownloadOptions | DownloadCallback) => {
+        if (typeof optionsOrCb === 'function') {
+          optionsOrCb(null, Buffer.alloc(0));
+        } else if (optionsOrCb) {
+          assert.strictEqual(optionsOrCb.destination, expectedDestination);
+        }
+        return Promise.resolve([Buffer.alloc(0)]) as Promise<DownloadResponse>;
+      };
+
+      const file = new File(bucket, filename);
+      file.download = download;
+      await transferManager.downloadManyFiles([file], {passthroughOptions});
+    });
+
+    it('does not set the destination when prefix, strip prefix and passthroughOptions.destination are not provided', async () => {
+      const options = {};
+      const filename = 'first.txt';
+      const download = (optionsOrCb?: DownloadOptions | DownloadCallback) => {
+        if (typeof optionsOrCb === 'function') {
+          optionsOrCb(null, Buffer.alloc(0));
+        } else if (optionsOrCb) {
+          assert.strictEqual(optionsOrCb.destination, undefined);
+        }
+        return Promise.resolve([Buffer.alloc(0)]) as Promise<DownloadResponse>;
+      };
+
+      const file = new File(bucket, filename);
+      file.download = download;
+      await transferManager.downloadManyFiles([file], options);
+    });
+
+    it('should recursively create directory and write file contents if destination path is nested', async () => {
+      const prefix = 'text-prefix';
+      const folder = 'nestedFolder/';
+      const file = 'first.txt';
+      const filesOrFolder = [folder, path.join(folder, file)];
+      const expectedFilePath = path.join(prefix, folder, file);
+      const expectedDir = path.join(prefix, folder);
+      const mkdirSpy = sandbox.spy(fsp, 'mkdir');
+      const download = (optionsOrCb?: DownloadOptions | DownloadCallback) => {
+        if (typeof optionsOrCb === 'function') {
+          optionsOrCb(null, Buffer.alloc(0));
+        } else if (optionsOrCb) {
+          assert.strictEqual(optionsOrCb.destination, expectedFilePath);
+        }
+        return Promise.resolve([Buffer.alloc(0)]) as Promise<DownloadResponse>;
+      };
+
+      sandbox.stub(bucket, 'file').callsFake(filename => {
+        const file = new File(bucket, filename);
+        file.download = download;
+        return file;
+      });
+      await transferManager.downloadManyFiles(filesOrFolder, {
+        prefix: prefix,
+      });
+      assert.strictEqual(
+        mkdirSpy.calledOnceWith(expectedDir, {
+          recursive: true,
+        }),
+        true
+      );
+    });
   });
 
   describe('downloadFileInChunks', () => {