support uploading directories

[stephenplusplus]

Fixes #412

To Dos

• Unit Tests
• System Tests
• Docs

This is a work in progress, but is an attempt to make a sane API for uploading directories.

zoo/
  chipmunks/
    chipmunk-1.jpg
  pandas/
    panda-1.jpg
  zoo-photos.zip

bucket.uploadDirectory("zoo", function (err, files) {
  files = [
    File("chipmunks/chipmunk-1.jpg"),
    File("pandas/panda-1.jpg"),
    File("zoo-photos.jpg")
  ]
})

Globs

bucket.uploadDirectory is a wrapper around bucket.upload, which now supports globs:

bucket.upload("zoo/**/*.jpg", function (err, files) {
  files = [
    File("chipmunk-1.jpg"),
    File("panda-1.jpg")
  ]
})

Note that the resulting file names aren't prefixed with a directory path (e.g. "chipmunks/chipmunk-1.jpg" vs "chipmunk-1.jpg"). Since a glob can include several files and directories in various locations on a hard drive, by default, just the filename is used as the uploaded file's name.

By specifying options.basePath, a user can tell us the common parent of all of the locations their glob matches, which is then used as a prefix for the file. This is how bucket.uploadDirectory works (simplified):

Bucket.prototype.uploadDirectory = function (directoryPath) {
  bucket.upload(path.join(directoryPath, "**/*"), { basePath: directoryPath })
}

Extending globs

bucket.upload accepts either a single glob or array of globs. It's passed directly to sindresorhus/globby, which uses isaacs/minimatch and isaacs/node-glob to return the matching paths. All minimatch glob patterns are accepted, as well as node-glob options via options.globOptions.

[stephenplusplus]

@ryanseys - If you can take a quick look at the first post to see if this is a useful addition, I'll start working on the tests for these to get it ready for code review & merge.

[ryanseys]

Yes this looks ok. What approach is taken when filenames conflict. E.g. /img/chipmunks/chippy-1.jpg and /img/woodchippers/chippy-1.jpg will both be uploaded as chippy-1.jpg when manually using bucket.upload('/img/**/*.jpg', function(err, files) {}); Does this just mean that one will be overwritten or can we somehow tell bucket.upload to be smart about it so that nothing is overwritten?

I'm thinking maybe we don't even need bucket.uploadDirectory (???), why not just fs.stat to figure out if the name given is a directory, and then take the approach of using basePath in that case. If they give a glob, maybe they can specify a { flatten: true } to explictly say they want to overwrite files, otherwise upload and maintain the structure from their base path?

Sorry if this doesn't make sense.

[ryanseys]

Or just always maintain structure when given a directory. If given a glob, always flatten. Seems reasonable.

[ryanseys]

I think that's probably what you're doing already except it's strewn across upload and uploadDirectory where I'm not convinced we need both.

[stephenplusplus]

What approach is taken when filenames conflict

The last one will win.

I'm thinking maybe we don't even need bucket.uploadDirectory (???), why not just fs.stat to figure out if the name given is a directory, and then take the approach of using basePath in that case.

I like this!

[ryanseys]

If it makes the logic of upload infinitely more difficult, I'd say it's not worth it, so if this is already effectively doing the same thing as:

just always maintain structure when given a directory. If given a glob, always flatten.

then I don't see any reason to change it now. But if it's easy to change, that'd be cool :)

[stephenplusplus]

I played around with it for a bit, but I couldn't find a way to make the change elegantly. For now, I'll continue with the two methods (upload for globs, uploadDirectory for directories), and maybe revisit later if a solution strikes me.

[ryanseys]

No worries. Might be beneficial and more intuitive for the users anyway. At
least that can be our excuse :)

On Thursday, April 16, 2015, Stephen Sawchuk notifications@github.com
wrote:

I played around with it for a bit, but I couldn't find a way to make the
change elegantly. For now, I'll continue with the two methods (upload for
globs, uploadDirectory for directories), and maybe revisit later if a
solution strikes me.

—
Reply to this email directly or view it on GitHub
#492 (comment)
.

[stephenplusplus]

Ready for a closer look 👓

[stephenplusplus]

(cont'd from #492 (comment))

After much thought and attempts to implement this, I don't think we need to support uploading directories or globs.

It's quite straightforward for a user to implement uploading a directory in their app:

var async = require("async")
var fs = require("fs")
var storage = require("gcloud")

var myBucket = gcloud.storage().bucket("pics")
uploadDirectory(myBucket, "/Users/stephen/Photos", function (err, files) {})

function uploadDirectory(bucket, directory, callback) {
  fs.readdir(directory, function (err, files) {
    async.map(files, bucket.upload, callback)
  })
}

If they want other things to happen, like uploading the files into a new directory in their bucket, recursing through child directories and maintaining that hierarchy, etc., their code is the best place to handle that. Our supporting it It leads to either too much guess work or confusing options.

[ryanseys]

Globs I agree are a little fancy and require weird options but just
uploading a path where the path is a directory should be easy to support.
May I take a stab at it? :) I'll work off of what you've already created.

On Monday, April 20, 2015, Stephen Sawchuk notifications@github.com wrote:

(cont'd from #492 (comment)
#492 (comment)
)

After much thought and attempts to implement this, I don't think we need
to support uploading directories or globs.

It's quite straightforward for a user to implement uploading a directory
in their app:

var async = require("async")var fs = require("fs")var storage = require("gcloud")
var myBucket = gcloud.storage().bucket("pics")
uploadDirectory(myBucket, "/Users/stephen/Photos", function (err, files) {})
function uploadDirectory(bucket, directory, callback) {
fs.readdir(directory, function (err, files) {
async.map(files, bucket.upload, callback)
})
}

If they want other things to happen, like uploading the files into a new
directory in their bucket, recursing through child directories and
maintaining that hierarchy, etc., their code is the best place to handle
that. Our supporting it It leads to either too much guess work or confusing
options.

—
Reply to this email directly or view it on GitHub
#492 (comment)
.

[stephenplusplus]

Sure, your code has yet to leave me unimpressed!

[stephenplusplus]

I still think this is outside the scope of what our lib should support. We allow uploading files via upload() and a writable stream. With those two things, a user who wants to mass upload multiple files can make it happen using any utility library / native looping mechanism they choose. I'm going to close as I don't think this is a large priority.

@ryanseys with all of your free time, if you're still interesting in taking a stab at a smaller implementation of this, please feel free :)

[ryanseys]

@ryanseys with all of your free time

Hahaha!

Like you said in #412, I believe this is not too difficult for a user to implement given our library and would allow them more flexibility than we could provide through our library in terms of handling errors and other responses on the fly.