From b77ddb3351a83a6aac9936eac6519115f5657b9f Mon Sep 17 00:00:00 2001 From: Adrian Cole Date: Mon, 3 Nov 2014 20:48:12 -0800 Subject: [PATCH] JCLOUDS-458 Fix some obvious bugs in google-cloud-storage. --- .../binders/MultipartUploadBinder.java | 19 ++++--- .../blobstore/GCSBlobStore.java | 50 +++++++++---------- .../ObjectListToStorageMetadata.java | 1 - .../SequentialMultipartUploadStrategy.java | 25 +++------- .../googlecloudstorage/domain/GCSObject.java | 22 ++++---- .../features/ObjectApi.java | 3 +- 6 files changed, 55 insertions(+), 65 deletions(-) diff --git a/providers/google-cloud-storage/src/main/java/org/jclouds/googlecloudstorage/binders/MultipartUploadBinder.java b/providers/google-cloud-storage/src/main/java/org/jclouds/googlecloudstorage/binders/MultipartUploadBinder.java index dfd4f7e840..ca6422ba60 100644 --- a/providers/google-cloud-storage/src/main/java/org/jclouds/googlecloudstorage/binders/MultipartUploadBinder.java +++ b/providers/google-cloud-storage/src/main/java/org/jclouds/googlecloudstorage/binders/MultipartUploadBinder.java @@ -29,30 +29,35 @@ import org.jclouds.io.Payloads; import org.jclouds.io.payloads.MultipartForm; import org.jclouds.io.payloads.Part; import org.jclouds.io.payloads.StringPayload; +import org.jclouds.json.Json; import org.jclouds.rest.MapBinder; -import com.google.gson.Gson; - -public class MultipartUploadBinder implements MapBinder { +import com.google.inject.Inject; +public final class MultipartUploadBinder implements MapBinder { private static final String BOUNDARY_HEADER = "multipart_boundary"; + private final Json json; + + @Inject MultipartUploadBinder(Json json){ + this.json = json; + } + @Override public R bindToRequest(R request, Map postParams) { ObjectTemplate template = (ObjectTemplate) postParams.get("template"); Payload payload = (Payload) postParams.get("payload"); - String contentType = checkNotNull(template.cacheControl(), "contentType"); + String contentType = checkNotNull(template.contentType(), "contentType"); Long length = checkNotNull(template.size(), "contentLength"); - StringPayload jsonPayload = Payloads.newStringPayload(new Gson().toJson(template)); + StringPayload jsonPayload = Payloads.newStringPayload(json.toJson(template)); payload.getContentMetadata().setContentLength(length); Part jsonPart = Part.create("Metadata", jsonPayload, new Part.PartOptions().contentType(APPLICATION_JSON)); Part mediaPart = Part.create(template.name(), payload, new Part.PartOptions().contentType(contentType)); - MultipartForm compPayload = new MultipartForm(BOUNDARY_HEADER, jsonPart, mediaPart); - request.setPayload(compPayload); + request.setPayload(new MultipartForm(BOUNDARY_HEADER, jsonPart, mediaPart)); // HeaderPart request.toBuilder().replaceHeader(CONTENT_TYPE, "Multipart/related; boundary= " + BOUNDARY_HEADER).build(); return request; diff --git a/providers/google-cloud-storage/src/main/java/org/jclouds/googlecloudstorage/blobstore/GCSBlobStore.java b/providers/google-cloud-storage/src/main/java/org/jclouds/googlecloudstorage/blobstore/GCSBlobStore.java index c38885782f..fd8cfdb775 100644 --- a/providers/google-cloud-storage/src/main/java/org/jclouds/googlecloudstorage/blobstore/GCSBlobStore.java +++ b/providers/google-cloud-storage/src/main/java/org/jclouds/googlecloudstorage/blobstore/GCSBlobStore.java @@ -18,12 +18,13 @@ package org.jclouds.googlecloudstorage.blobstore; import static com.google.common.base.Preconditions.checkNotNull; import static com.google.common.io.BaseEncoding.base64; +import static org.jclouds.googlecloudstorage.domain.DomainResourceReferences.ObjectRole.READER; import java.io.UnsupportedEncodingException; import java.net.URLEncoder; import java.util.Set; -import javax.inject.Singleton; +import javax.inject.Inject; import org.jclouds.blobstore.BlobStoreContext; import org.jclouds.blobstore.domain.Blob; @@ -35,6 +36,7 @@ import org.jclouds.blobstore.domain.internal.BlobImpl; import org.jclouds.blobstore.domain.internal.PageSetImpl; import org.jclouds.blobstore.internal.BaseBlobStore; import org.jclouds.blobstore.options.CreateContainerOptions; +import org.jclouds.blobstore.options.GetOptions; import org.jclouds.blobstore.options.ListContainerOptions; import org.jclouds.blobstore.options.PutOptions; import org.jclouds.blobstore.strategy.internal.FetchBlobMetadata; @@ -50,7 +52,7 @@ import org.jclouds.googlecloudstorage.blobstore.functions.ObjectToBlobMetadata; import org.jclouds.googlecloudstorage.blobstore.strategy.internal.MultipartUploadStrategy; import org.jclouds.googlecloudstorage.config.UserProject; import org.jclouds.googlecloudstorage.domain.Bucket; -import org.jclouds.googlecloudstorage.domain.DomainResourceReferences.ObjectRole; +import org.jclouds.googlecloudstorage.domain.DomainResourceReferences; import org.jclouds.googlecloudstorage.domain.GCSObject; import org.jclouds.googlecloudstorage.domain.ListPage; import org.jclouds.googlecloudstorage.domain.templates.BucketTemplate; @@ -66,24 +68,21 @@ import com.google.common.base.Supplier; import com.google.common.base.Throwables; import com.google.common.collect.Iterables; import com.google.common.hash.HashCode; -import com.google.inject.Inject; import com.google.inject.Provider; -@Singleton -public class GCSBlobStore extends BaseBlobStore { +public final class GCSBlobStore extends BaseBlobStore { - GoogleCloudStorageApi api; - BucketToStorageMetadata bucketToStorageMetadata; - ObjectToBlobMetadata objectToBlobMetadata; - ObjectListToStorageMetadata objectListToStorageMetadata; - Provider fetchBlobMetadataProvider; - BlobMetadataToObjectTemplate blobMetadataToObjectTemplate; - BlobStoreListContainerOptionsToListObjectOptions listContainerOptionsToListObjectOptions; - MultipartUploadStrategy multipartUploadStrategy; - Supplier projectId; + private final GoogleCloudStorageApi api; + private final BucketToStorageMetadata bucketToStorageMetadata; + private final ObjectToBlobMetadata objectToBlobMetadata; + private final ObjectListToStorageMetadata objectListToStorageMetadata; + private final Provider fetchBlobMetadataProvider; + private final BlobMetadataToObjectTemplate blobMetadataToObjectTemplate; + private final BlobStoreListContainerOptionsToListObjectOptions listContainerOptionsToListObjectOptions; + private final MultipartUploadStrategy multipartUploadStrategy; + private final Supplier projectId; - @Inject - protected GCSBlobStore(BlobStoreContext context, BlobUtils blobUtils, Supplier defaultLocation, + @Inject GCSBlobStore(BlobStoreContext context, BlobUtils blobUtils, Supplier defaultLocation, @Memoized Supplier> locations, GoogleCloudStorageApi api, BucketToStorageMetadata bucketToStorageMetadata, ObjectToBlobMetadata objectToBlobMetadata, ObjectListToStorageMetadata objectListToStorageMetadata, @@ -105,9 +104,10 @@ public class GCSBlobStore extends BaseBlobStore { @Override public PageSet list() { - return new Function, org.jclouds.blobstore.domain.PageSet>() { - public org.jclouds.blobstore.domain.PageSet apply(ListPage from) { - return new PageSetImpl(Iterables.transform(from, bucketToStorageMetadata), null); + return new Function, PageSet>() { + public PageSet apply(ListPage from) { + return new PageSetImpl(Iterables.transform(from, bucketToStorageMetadata), + from.nextPageToken()); } }.apply(api.getBucketApi().listBucket(projectId.get())); } @@ -121,8 +121,7 @@ public class GCSBlobStore extends BaseBlobStore { public boolean createContainerInLocation(Location location, String container) { BucketTemplate template = new BucketTemplate().name(container); if (location != null) { - org.jclouds.googlecloudstorage.domain.DomainResourceReferences.Location gcsLocation = org.jclouds.googlecloudstorage.domain.DomainResourceReferences.Location - .fromValue(location.getId()); + DomainResourceReferences.Location gcsLocation = DomainResourceReferences.Location.fromValue(location.getId()); template = template.location(gcsLocation); } return api.getBucketApi().createBucket(projectId.get(), template) != null; @@ -132,15 +131,13 @@ public class GCSBlobStore extends BaseBlobStore { public boolean createContainerInLocation(Location location, String container, CreateContainerOptions options) { BucketTemplate template = new BucketTemplate().name(container); if (location != null) { - org.jclouds.googlecloudstorage.domain.DomainResourceReferences.Location gcsLocation = org.jclouds.googlecloudstorage.domain.DomainResourceReferences.Location - .fromValue(location.getId()); + DomainResourceReferences.Location gcsLocation = DomainResourceReferences.Location.fromValue(location.getId()); template = template.location(gcsLocation); } Bucket bucket = api.getBucketApi().createBucket(projectId.get(), template); if (options.isPublicRead()) { try { - ObjectAccessControlsTemplate doAclTemplate = ObjectAccessControlsTemplate - .create("allUsers", ObjectRole.READER); + ObjectAccessControlsTemplate doAclTemplate = ObjectAccessControlsTemplate.create("allUsers", READER); api.getDefaultObjectAccessControlsApi().createDefaultObjectAccessControls(container, doAclTemplate); } catch (HttpResponseException e) { // If DefaultObjectAccessControls operation fail, Reverse create operation the operation. @@ -162,7 +159,6 @@ public class GCSBlobStore extends BaseBlobStore { @Override public PageSet list(String container, ListContainerOptions options) { - if (options != null && options != ListContainerOptions.NONE) { ListObjectOptions listOptions = listContainerOptionsToListObjectOptions.apply(options); ListPage gcsList = api.getObjectApi().listObjects(container, listOptions); @@ -218,7 +214,7 @@ public class GCSBlobStore extends BaseBlobStore { } @Override - public Blob getBlob(String container, String name, org.jclouds.blobstore.options.GetOptions options) { + public Blob getBlob(String container, String name, GetOptions options) { GCSObject gcsObject = api.getObjectApi().getObject(container, name); if (gcsObject == null) { return null; diff --git a/providers/google-cloud-storage/src/main/java/org/jclouds/googlecloudstorage/blobstore/functions/ObjectListToStorageMetadata.java b/providers/google-cloud-storage/src/main/java/org/jclouds/googlecloudstorage/blobstore/functions/ObjectListToStorageMetadata.java index a5c6f73b72..1fc612df69 100644 --- a/providers/google-cloud-storage/src/main/java/org/jclouds/googlecloudstorage/blobstore/functions/ObjectListToStorageMetadata.java +++ b/providers/google-cloud-storage/src/main/java/org/jclouds/googlecloudstorage/blobstore/functions/ObjectListToStorageMetadata.java @@ -58,6 +58,5 @@ public class ObjectListToStorageMetadata implements Function return input; } }), from.nextPageToken()); - } } diff --git a/providers/google-cloud-storage/src/main/java/org/jclouds/googlecloudstorage/blobstore/strategy/internal/SequentialMultipartUploadStrategy.java b/providers/google-cloud-storage/src/main/java/org/jclouds/googlecloudstorage/blobstore/strategy/internal/SequentialMultipartUploadStrategy.java index c244f5c265..3ee612d8ec 100644 --- a/providers/google-cloud-storage/src/main/java/org/jclouds/googlecloudstorage/blobstore/strategy/internal/SequentialMultipartUploadStrategy.java +++ b/providers/google-cloud-storage/src/main/java/org/jclouds/googlecloudstorage/blobstore/strategy/internal/SequentialMultipartUploadStrategy.java @@ -20,13 +20,10 @@ import static com.google.common.base.Preconditions.checkNotNull; import java.util.List; -import javax.annotation.Resource; -import javax.inject.Named; import javax.inject.Provider; import org.jclouds.blobstore.domain.Blob; import org.jclouds.blobstore.domain.BlobBuilder; -import org.jclouds.blobstore.reference.BlobStoreConstants; import org.jclouds.googlecloudstorage.GoogleCloudStorageApi; import org.jclouds.googlecloudstorage.blobstore.functions.BlobMetadataToObjectTemplate; import org.jclouds.googlecloudstorage.domain.GCSObject; @@ -34,16 +31,11 @@ import org.jclouds.googlecloudstorage.domain.templates.ComposeObjectTemplate; import org.jclouds.googlecloudstorage.domain.templates.ObjectTemplate; import org.jclouds.io.Payload; import org.jclouds.io.PayloadSlicer; -import org.jclouds.logging.Logger; import com.google.common.collect.Lists; import com.google.inject.Inject; -public class SequentialMultipartUploadStrategy extends MultipartUploadStrategy { - - @Resource - @Named(BlobStoreConstants.BLOBSTORE_LOGGER) - private Logger logger = Logger.NULL; +public final class SequentialMultipartUploadStrategy extends MultipartUploadStrategy { private final GoogleCloudStorageApi api; private final Provider blobBuilders; @@ -52,16 +44,15 @@ public class SequentialMultipartUploadStrategy extends MultipartUploadStrategy { private final PayloadSlicer slicer; private final MultipartNamingStrategy namingStrategy; - @Inject - public SequentialMultipartUploadStrategy(GoogleCloudStorageApi api, Provider blobBuilders, + @Inject SequentialMultipartUploadStrategy(GoogleCloudStorageApi api, Provider blobBuilders, BlobMetadataToObjectTemplate blob2ObjectTemplate, MultipartUploadSlicingAlgorithm algorithm, PayloadSlicer slicer, MultipartNamingStrategy namingStrategy) { - this.api = checkNotNull(api, "api"); - this.blobBuilders = checkNotNull(blobBuilders, "blobBuilders"); - this.blob2ObjectTemplate = checkNotNull(blob2ObjectTemplate, "blob2Object"); - this.algorithm = checkNotNull(algorithm, "algorithm"); - this.slicer = checkNotNull(slicer, "slicer"); - this.namingStrategy = checkNotNull(namingStrategy, "namingStrategy"); + this.api = api; + this.blobBuilders = blobBuilders; + this.blob2ObjectTemplate = blob2ObjectTemplate; + this.algorithm = algorithm; + this.slicer = slicer; + this.namingStrategy = namingStrategy; } @Override diff --git a/providers/google-cloud-storage/src/main/java/org/jclouds/googlecloudstorage/domain/GCSObject.java b/providers/google-cloud-storage/src/main/java/org/jclouds/googlecloudstorage/domain/GCSObject.java index 7b3fc7f44d..77c13503c8 100644 --- a/providers/google-cloud-storage/src/main/java/org/jclouds/googlecloudstorage/domain/GCSObject.java +++ b/providers/google-cloud-storage/src/main/java/org/jclouds/googlecloudstorage/domain/GCSObject.java @@ -44,31 +44,31 @@ public abstract class GCSObject { public abstract String etag(); public abstract String name(); public abstract String bucket(); - public abstract Long generation(); - public abstract Long metageneration(); + public abstract long generation(); + public abstract long metageneration(); public abstract String contentType(); public abstract Date updated(); - public abstract Date timeDeleted(); + @Nullable public abstract Date timeDeleted(); public abstract StorageClass storageClass(); - public abstract Long size(); + public abstract long size(); @Nullable public abstract String md5Hash(); public abstract URI mediaLink(); public abstract Map metadata(); - public abstract String contentEncoding(); - public abstract String contentDisposition(); - public abstract String contentLanguage(); - public abstract String cacheControl(); + @Nullable public abstract String contentEncoding(); + @Nullable public abstract String contentDisposition(); + @Nullable public abstract String contentLanguage(); + @Nullable public abstract String cacheControl(); public abstract List acl(); public abstract Owner owner(); @Nullable public abstract String crc32c(); - public abstract Integer componentCount(); + @Nullable public abstract Integer componentCount(); @SerializedNames( { "id", "selfLink", "etag", "name", "bucket", "generation", "metageneration", "contentType", "updated", "timeDeleted", "storageClass", "size", "md5Hash", "mediaLink", "metadata", "contentEncoding", "contentDisposition", "contentLanguage", "cacheControl", "acl", "owner", "crc32c", "componentCount" }) - public static GCSObject create(String id, URI selfLink, String etag, String name, String bucket, Long generation, - Long metageneration, String contentType, Date updated, Date timeDeleted, StorageClass storageClass, Long size, + public static GCSObject create(String id, URI selfLink, String etag, String name, String bucket, long generation, + long metageneration, String contentType, Date updated, Date timeDeleted, StorageClass storageClass, long size, String md5Hash, URI mediaLink, Map metadata, String contentEncoding, String contentDisposition, String contentLanguage, String cacheControl, List acl, Owner owner, String crc32c, Integer componentCount) { diff --git a/providers/google-cloud-storage/src/main/java/org/jclouds/googlecloudstorage/features/ObjectApi.java b/providers/google-cloud-storage/src/main/java/org/jclouds/googlecloudstorage/features/ObjectApi.java index 77d6cf090b..75cdf2d724 100644 --- a/providers/google-cloud-storage/src/main/java/org/jclouds/googlecloudstorage/features/ObjectApi.java +++ b/providers/google-cloud-storage/src/main/java/org/jclouds/googlecloudstorage/features/ObjectApi.java @@ -474,7 +474,6 @@ public interface ObjectApi { * * @return a {@link GCSObject} */ - @Named("Object:multipartUpload") @POST @QueryParams(keys = "uploadType", values = "multipart") @@ -483,6 +482,6 @@ public interface ObjectApi { @OAuthScopes(STORAGE_FULLCONTROL_SCOPE) @MapBinder(MultipartUploadBinder.class) GCSObject multipartUpload(@PathParam("bucket") String bucketName, - @BinderParam(BindToJsonPayload.class) ObjectTemplate objectTemplate, + @PayloadParam("template") ObjectTemplate objectTemplate, @PayloadParam("payload") Payload payload); }