-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Description
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.