Operations on Buckets should use Storage.*Option types

[dimo414]

Some of the static classes in Storage, such as BlobTargetOption which is used by Storage.create(), are also being erroneously used in Bucket. This is problematic because Bucket keys of of path Strings, not BlobInfo instances, and therefore isn't able to extract information like the blob's generation, causing a line like this:

bucket.create(file, data, null, BlobTargetOption.generationMatch());

to fail:

 java.lang.IllegalArgumentException: Option ifGenerationMatch is missing a value
     at com.google.common.base.Preconditions.checkArgument(Preconditions.java:122)
     at com.google.gcloud.storage.StorageImpl.addToOptionMap(StorageImpl.java:651)
     at com.google.gcloud.storage.StorageImpl.addToOptionMap(StorageImpl.java:643)
     at com.google.gcloud.storage.StorageImpl.optionMap(StorageImpl.java:681)
     at com.google.gcloud.storage.StorageImpl.optionMap(StorageImpl.java:660)
     at com.google.gcloud.storage.StorageImpl.optionMap(StorageImpl.java:695)
     at com.google.gcloud.storage.StorageImpl.optionMap(StorageImpl.java:703)
     at com.google.gcloud.storage.StorageImpl.create(StorageImpl.java:140)
     at com.google.gcloud.storage.StorageImpl.create(StorageImpl.java:129)
     at com.google.gcloud.storage.Bucket.create(Bucket.java:360)

And there is no way in Storage.BlobTargetOption to actually specify a generation.

I don't think Bucket should have any dependency on the Storage.*Option types. There's clearly a good amount of overlap, but I don't think that's a good enough reason for them to share these types. Even where they behave the same it's confusing for the caller.

They could extend from a shared parent if we wanted, but that should be an implementation detail.

I can put together a patch to remove the dependencies on Storage.*Option from Bucket if that's desired.

[mziccard]

Nice catch!
We should provide dedicated BlobTargetOption and BlobWriteOption in Bucket. I am assigning this to myself and I'll work on it ASAP.

[mziccard]

#705 adds BlobTargetOption and BlobWriteOption to Bucket and fixes the bug reported by this issue. Storage.BlobListOption is still used in Bucket as it makes little sense to me to copy-paste the class given that it applies as-is.

[dimo414]

You could have Storage.BlobListOption and Bucket.BlobListOption extend a shared parent. That saves too much typing but makes it much clearer to callers that they're doing the right thing. When I was getting set up the Storage/Bucket overlap was one of the more confusing parts of the API.