You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@jclouds.apache.org by Bhathiya <no...@github.com> on 2014/06/26 02:46:13 UTC
[jclouds-labs-google] JCLOUD-458:Added Bucket Operation with live
tests (#31)
JCLOUDS-458: Refactoring the domain resources (reducing repitition)
Address Checkstyle Violations
Minor changes
You can merge this Pull Request by running:
git pull https://github.com/hsbhathiya/jclouds-labs-google Refactorstream
Or you can view, comment on it, or merge it online at:
https://github.com/jclouds/jclouds-labs-google/pull/31
-- Commit Summary --
* JCLOUD-458:Added Bucket Operation with live tests
-- File Changes --
M google-cloud-storage/README.md (1)
M google-cloud-storage/pom.xml (1)
M google-cloud-storage/src/main/java/org/jclouds/googlecloudstorage/GoogleCloudStorageApi.java (8)
M google-cloud-storage/src/main/java/org/jclouds/googlecloudstorage/domain/BucketAccessControls.java (81)
A google-cloud-storage/src/main/java/org/jclouds/googlecloudstorage/domain/Buckets.java (296)
A google-cloud-storage/src/main/java/org/jclouds/googlecloudstorage/domain/BucketsTemplate.java (213)
M google-cloud-storage/src/main/java/org/jclouds/googlecloudstorage/domain/DefaultObjectAccessControls.java (77)
M google-cloud-storage/src/main/java/org/jclouds/googlecloudstorage/domain/DefaultObjectAccessControlsTemplate.java (2)
R google-cloud-storage/src/main/java/org/jclouds/googlecloudstorage/domain/DomainResourceRefferences.java (32)
M google-cloud-storage/src/main/java/org/jclouds/googlecloudstorage/domain/ListDefaultObjectAccessControls.java (1)
A google-cloud-storage/src/main/java/org/jclouds/googlecloudstorage/domain/internal/CommonInternal.java (179)
A google-cloud-storage/src/main/java/org/jclouds/googlecloudstorage/domain/internal/Cors.java (148)
A google-cloud-storage/src/main/java/org/jclouds/googlecloudstorage/domain/internal/LifeCycle.java (370)
A google-cloud-storage/src/main/java/org/jclouds/googlecloudstorage/domain/internal/Logging.java (99)
A google-cloud-storage/src/main/java/org/jclouds/googlecloudstorage/domain/internal/Versioning.java (92)
A google-cloud-storage/src/main/java/org/jclouds/googlecloudstorage/domain/internal/Website.java (99)
A google-cloud-storage/src/main/java/org/jclouds/googlecloudstorage/features/BucketsApi.java (313)
M google-cloud-storage/src/main/java/org/jclouds/googlecloudstorage/features/DefaultObjectAccessControlsApi.java (2)
A google-cloud-storage/src/main/java/org/jclouds/googlecloudstorage/handlers/BucketsBinder.java (43)
A google-cloud-storage/src/main/java/org/jclouds/googlecloudstorage/options/DeleteBucketsOptions.java (51)
A google-cloud-storage/src/main/java/org/jclouds/googlecloudstorage/options/GetBucketsOptions.java (61)
A google-cloud-storage/src/main/java/org/jclouds/googlecloudstorage/options/InsertBucketsOptions.java (53)
A google-cloud-storage/src/main/java/org/jclouds/googlecloudstorage/options/ListOptions.java (56)
A google-cloud-storage/src/main/java/org/jclouds/googlecloudstorage/options/UpdateBucketsOptions.java (73)
M google-cloud-storage/src/test/java/org/jclouds/googlecloudstorage/features/BucketAccessControlsApiExpectTest.java (2)
A google-cloud-storage/src/test/java/org/jclouds/googlecloudstorage/features/BucketsApiExpectTest.java (260)
A google-cloud-storage/src/test/java/org/jclouds/googlecloudstorage/features/BucketsApiLiveTest.java (220)
M google-cloud-storage/src/test/java/org/jclouds/googlecloudstorage/features/DefaultObjectAccessControlsApiExpectTest.java (2)
M google-cloud-storage/src/test/java/org/jclouds/googlecloudstorage/internal/BaseGoogleCloudStorageApiLiveTest.java (5)
M google-cloud-storage/src/test/java/org/jclouds/googlecloudstorage/parse/BucketAclGetTest.java (2)
M google-cloud-storage/src/test/java/org/jclouds/googlecloudstorage/parse/BucketAclInsertTest.java (2)
M google-cloud-storage/src/test/java/org/jclouds/googlecloudstorage/parse/BucketAclListTest.java (7)
M google-cloud-storage/src/test/java/org/jclouds/googlecloudstorage/parse/BucketAclUpdateTest.java (2)
A google-cloud-storage/src/test/java/org/jclouds/googlecloudstorage/parse/BucketUpdateTest.java (49)
M google-cloud-storage/src/test/java/org/jclouds/googlecloudstorage/parse/DefaultObjectAclGetTest.java (6)
M google-cloud-storage/src/test/java/org/jclouds/googlecloudstorage/parse/DefaultObjectAclInsertTest.java (2)
M google-cloud-storage/src/test/java/org/jclouds/googlecloudstorage/parse/DefaultObjectAclListTest.java (6)
A google-cloud-storage/src/test/java/org/jclouds/googlecloudstorage/parse/FullBucketGetTest.java (66)
A google-cloud-storage/src/test/java/org/jclouds/googlecloudstorage/parse/NoAclBucketListTest.java (53)
A google-cloud-storage/src/test/java/org/jclouds/googlecloudstorage/parse/NoAclBucketTest.java (49)
A google-cloud-storage/src/test/resources/bucket_insert_requestpayload.json (3)
A google-cloud-storage/src/test/resources/bucket_update_requestpayload.json (12)
A google-cloud-storage/src/test/resources/bucket_update_response.json (15)
M google-cloud-storage/src/test/resources/bucketacl_get.json (2)
M google-cloud-storage/src/test/resources/bucketacl_insert_response.json (2)
M google-cloud-storage/src/test/resources/bucketacl_update_initial.json (2)
A google-cloud-storage/src/test/resources/full_bucket_get.json (38)
A google-cloud-storage/src/test/resources/list_bucket_withoptions.json (21)
A google-cloud-storage/src/test/resources/noAcl_bucket.json (15)
A google-cloud-storage/src/test/resources/noAcl_bucket_list.json (22)
-- Patch Links --
https://github.com/jclouds/jclouds-labs-google/pull/31.patch
https://github.com/jclouds/jclouds-labs-google/pull/31.diff
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/31
Re: [jclouds-labs-google] JCLOUD-458:Added Bucket Operation with live
tests (#31)
Posted by Andrew Phillips <no...@github.com>.
> +
> + public static class Builder {
> + private String type;
> +
> + public Builder type(String type) {
> + this.type = type;
> + return this;
> + }
> +
> + public Action build() {
> + return new Action(this.type);
> + }
> +
> + public Builder fromAction(Action in) {
> + return this.type(in.getType());
> +
[minor] Remove blank line
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/31/files#r14528567
Re: [jclouds-labs-google] JCLOUD-458:Added Bucket Operation with live
tests (#31)
Posted by Andrew Phillips <no...@github.com>.
> }
>
> public enum ObjectRole {
> READER, OWNER
> }
>
> + public enum Location {
> + ASIA, EU, US, ASIA_EAST1, US_CENTRAL1, US_CENTRAL2, US_EAST1, US_EAST2, US_EAST3, US_WEST1;
> +
> + public String value() {
> + return CaseFormat.UPPER_UNDERSCORE.to(CaseFormat.LOWER_HYPHEN, name()).toUpperCase();
Just to ensure I'm understanding this...we go from A_B to a-b and then A-B again? In that case, how about just replacing '_' with '-'?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/31/files#r14528452
Re: [jclouds-labs-google] JCLOUD-458:Added Bucket Operation with live
tests (#31)
Posted by Andrew Phillips <no...@github.com>.
> + }
> +
> + public BucketTemplate acl(Set<BucketAccessControls> acl) {
> +
> + if (this.acl == null) {
> + this.acl = Sets.newLinkedHashSet();
> + }
> +
> + this.acl.addAll(acl);
> + return this;
> + }
> +
> + public BucketTemplate addDefaultObjectAccessControls(DefaultObjectAccessControls oac) {
> + if (this.defaultObjectAccessControls == null) {
> + this.defaultObjectAccessControls = Sets.newLinkedHashSet();
> + }
See above comment
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/31/files#r14528336
Re: [jclouds-labs-google] JCLOUD-458:Added Bucket Operation with live
tests (#31)
Posted by Andrew Phillips <no...@github.com>.
> @@ -111,6 +111,7 @@
> <jclouds.blobstore.httpstream.md5>${jclouds.blobstore.httpstream.md5}</jclouds.blobstore.httpstream.md5>
> <test.google-cloud-storage.identity>${test.google-cloud-storage.identity}</test.google-cloud-storage.identity>
> <test.google-cloud-storage.credential>${test.google-cloud-storage.credential}</test.google-cloud-storage.credential>
> + <test.google-cloud-storage.project-number>${test.google-cloud-storage.project-number}</test.google-cloud-storage.project-number>
[minor] Fix indent?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/31/files#r14484467
Re: [jclouds-labs-google] JCLOUD-458:Added Bucket Operation with live
tests (#31)
Posted by Andrew Phillips <no...@github.com>.
> +import static com.google.common.base.Objects.equal;
> +import static com.google.common.base.Objects.toStringHelper;
> +
> +import java.beans.ConstructorProperties;
> +
> +import com.google.common.base.Objects;
> +
> +/**
> + * The bucket's logging configuration, which defines the destination bucket and optional name prefix for the current
> + * bucket's logs.
> + */
> +
> +public final class ProjectTeam {
> +
> + public enum Team {
> + owners, editors, viewers;
Do these need to be lowercase for (de)serialization? If not, make uppercase?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/31/files#r14532864
Re: [jclouds-labs-google] JCLOUD-458:Added Bucket Operation with live
tests (#31)
Posted by Andrew Phillips <no...@github.com>.
> +
> + Iterator<Bucket> pageIterator = bucket.iterator();
> + assertTrue(pageIterator.hasNext());
> +
> + Bucket singlePageIterator = pageIterator.next();
> + List<Bucket> bucketAsList = Lists.newArrayList(singlePageIterator);
> +
> + assertNotNull(singlePageIterator);
> + assertSame(bucketAsList.size(), 1);
> +
> + }
> +
> + @Test(groups = "live", dependsOnMethods = "testCreateBucket")
> + public void testPatchBucket() {
> + Logging logging = new Logging(LOG_BUCKET_NAME, BUCKET_NAME);
> + BucketTemplate template = new BucketTemplate().name(BUCKET_NAME).logging(logging);
Use the builder?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/31/files#r14533712
Re: [jclouds-labs-google] JCLOUD-458:Added Bucket Operation with live
tests (#31)
Posted by Andrew Phillips <no...@github.com>.
> + @Consumes(MediaType.APPLICATION_JSON)
> + @Path("/b/{bucket}")
> + @OAuthScopes(STORAGE_FULLCONTROL_SCOPE)
> + @Fallback(NullOnNotFoundOr404.class)
> + @Nullable
> + HttpResponse deleteBuckets(@PathParam("bucket") String bucketName, DeleteBucketOptions options);
> +
> + /**
> + * Retrieves a list of buckets for a given project
> + *
> + * @param project
> + * Name of the project to retrieve the buckets
> + *
> + * @return a {@link ListPage<Bucket>}
> + */
> +
[minor] Remove blank line?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/31/files#r14533179
Re: [jclouds-labs-google] JCLOUD-458:Added Bucket Operation with live
tests (#31)
Posted by Andrew Phillips <no...@github.com>.
> + BucketLifeCycle lifeCycle, StorageClass storageClass) {
> +
> + super(Kind.BUCKET, id, selfLink, etag);
> + this.projectNumber = projectNumber;
> + this.timeCreated = checkNotNull(timeCreated, "timeCreated");
> + this.metageneration = checkNotNull(metageneration, "metageneration");
> + this.acl = acl.isEmpty() ? null : acl;
> + this.defaultObjectAcl = defaultObjectAcl.isEmpty() ? null : defaultObjectAcl;
> + this.owner = checkNotNull(owner, "Owner");
> + this.location = checkNotNull(location, "location");
> + this.website = website;
> + this.logging = logging;
> + this.versioning = versioning;
> + this.cors = cors.isEmpty() ? null : cors;
> + this.lifeCycle = lifeCycle;
> + this.storageClass = storageClass;
These last ones are all nullable? If so, add `@Nullable` to the constructor parameters?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/31/files#r14489236
Re: [jclouds-labs-google] JCLOUD-458:Added Bucket Operation with live
tests (#31)
Posted by Andrew Phillips <no...@github.com>.
> + * @param bucketName
> + * In the request body, supply a bucket resource with list of {@link BucketAccessControls} (acl[])
> + * @param options
> + * Supply {@link UpdateBucketOptions} with optional query parameters
> + *
> + * @return If successful,this method returns a {@link Bucket} resource in the response body
> + */
> +
> + @Named("Bucket:update")
> + @PUT
> + @Consumes(MediaType.APPLICATION_JSON)
> + @Produces(MediaType.APPLICATION_JSON)
> + @Path("/b/{bucket}")
> + @OAuthScopes(STORAGE_FULLCONTROL_SCOPE)
> + @Fallback(NullOnNotFoundOr404.class)
> + Bucket updateBuckets(@PathParam("bucket") String bucketName,
See all comments above
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/31/files#r14533243
Re: [jclouds-labs-google] JCLOUD-458:Added Bucket Operation with live
tests (#31)
Posted by Andrew Phillips <no...@github.com>.
> + private Website website;
> + private Logging logging;
> + private Versioning versioning;
> + private ImmutableSet.Builder<BucketCors> cors = ImmutableSet.builder();
> + private BucketLifeCycle lifeCycle;
> + private StorageClass storageClass;
> +
> + public Builder name(String name) {
> + this.name = name;
> + return this;
> + }
> +
> + public Builder projectNumber(Long projectNumber) {
> + this.projectNumber = projectNumber;
> + return this;
> +
[minor] Remove blank line?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/31/files#r14528197
Re: [jclouds-labs-google] JCLOUD-458:Added Bucket Operation with live
tests (#31)
Posted by Bhathiya <no...@github.com>.
@demobox @mattstep @andrewgaul
Addressed these issues in https://github.com/jclouds/jclouds-labs-google/pull/33
previous commits (PR 29&30) also contain some of the issues pointed here. I will address those in seperated PRs
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/31#issuecomment-48124465
Re: [jclouds-labs-google] JCLOUD-458:Added Bucket Operation with live
tests (#31)
Posted by Andrew Phillips <no...@github.com>.
> +
> +import javax.ws.rs.Consumes;
> +import javax.ws.rs.core.MediaType;
> +
> +import org.jclouds.date.internal.SimpleDateFormatDateService;
> +import org.jclouds.googlecloudstorage.domain.Bucket;
> +import org.jclouds.googlecloudstorage.domain.DomainResourceRefferences.Location;
> +import org.jclouds.googlecloudstorage.domain.DomainResourceRefferences.StorageClass;
> +import org.jclouds.googlecloudstorage.domain.ListPage;
> +import org.jclouds.googlecloudstorage.domain.Resource.Kind;
> +import org.jclouds.googlecloudstorage.domain.internal.Owner;
> +import org.jclouds.googlecloudstorage.internal.BaseGoogleCloudStorageParseTest;
> +
> +public class NoAclBucketListTest extends BaseGoogleCloudStorageParseTest<ListPage<Bucket>> {
> +
> + private Bucket item_1 = Bucket.builder().id("bhashbucket")
[minor] `item1` (no underscore)?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/31/files#r14533795
Re: [jclouds-labs-google] JCLOUD-458:Added Bucket Operation with live
tests (#31)
Posted by Bhathiya <no...@github.com>.
> + }
> +
> + public BucketTemplate lifeCycle(BucketLifeCycle lifeCycle) {
> + this.lifeCycle = lifeCycle;
> + return this;
> + }
> +
> + public BucketTemplate storageClass(StorageClass storageClass) {
> + this.storageClass = storageClass;
> + return this;
> + }
> +
> + public BucketTemplate addAcl(BucketAccessControls bucketAccessControls) {
> +
> + if (this.acl == null) {
> + this.acl = Sets.newLinkedHashSet();
Initializing to empty didn't work.Registered the BucketTemplateTypeAdapter overriding default gson serializing
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/31/files#r14576930
Re: [jclouds-labs-google] JCLOUD-458:Added Bucket Operation with live
tests (#31)
Posted by Andrew Phillips <no...@github.com>.
> + *
> + * @param projectId
> + * A valid API project identifier
> + * @param bucketTemplate
> + * In the request body, supply a bucket resource
> + *
> + * @return If successful, this method returns a {@link Bucket} resource in the response body
> + */
> +
> + @Named("Bucket:insert")
> + @POST
> + @Consumes(MediaType.APPLICATION_JSON)
> + @Path("/b")
> + @OAuthScopes(STORAGE_FULLCONTROL_SCOPE)
> + @MapBinder(BucketBinder.class)
> + Bucket createBuckets(@QueryParam("project") String projectId,
`createBucket`? And if the API call is actually `insert`, why not `insertBucket`?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/31/files#r14533061
Re: [jclouds-labs-google] JCLOUD-458:Added Bucket Operation with live
tests (#31)
Posted by Andrew Phillips <no...@github.com>.
> + @OAuthScopes(STORAGE_FULLCONTROL_SCOPE)
> + @Fallback(NullOnNotFoundOr404.class)
> + Bucket updateBuckets(@PathParam("bucket") String bucketName,
> + @BinderParam(BindToJsonPayload.class) BucketTemplate bucketTemplate, UpdateBucketOptions options);
> +
> + /**
> + * Updates a bucket supporting patch semantics.
> + *
> + * @param bucketName
> + * In the request body, supply a bucket resource with list of {@link BucketAccessControls} (acl[])
> + * @param bucketTemplate
> + * In the request body, supply the relevant portions of a bucket resource
> + *
> + * @return If successful, this method returns a {@link Bucket} resource in the response body
> + */
> +
[minor] Remove blank line?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/31/files#r14533266
Re: [jclouds-labs-google] JCLOUD-458:Added Bucket Operation with live
tests (#31)
Posted by Matt Stephenson <no...@github.com>.
Please flatten out your class definitions, use Bucket instead of Buckets (only use pluralization when naming a collection, as in a java collection), and look again through your commit wrt my comments. I'll comment on the entire PR when you've gotten through this.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/31#issuecomment-47246663
Re: [jclouds-labs-google] JCLOUD-458:Added Bucket Operation with live
tests (#31)
Posted by Andrew Phillips <no...@github.com>.
> +import org.jclouds.googlecloudstorage.domain.DomainResourceRefferences.Location;
> +import org.jclouds.googlecloudstorage.domain.DomainResourceRefferences.StorageClass;
> +import org.jclouds.googlecloudstorage.domain.internal.BucketCors;
> +import org.jclouds.googlecloudstorage.domain.internal.BucketLifeCycle;
> +import org.jclouds.googlecloudstorage.domain.internal.Logging;
> +import org.jclouds.googlecloudstorage.domain.internal.Owner;
> +import org.jclouds.googlecloudstorage.domain.internal.Versioning;
> +import org.jclouds.googlecloudstorage.domain.internal.Website;
> +
> +
> +
> +import com.google.common.base.Objects;
> +import com.google.common.collect.ImmutableSet;
> +
> +/**
> + * The Bucket represents a bucket in Google Cloud Storage There is a single global namespace shared by all buckets
[minor] Add some punctuation here?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/31/files#r14485325
Re: [jclouds-labs-google] JCLOUD-458:Added Bucket Operation with live
tests (#31)
Posted by Andrew Phillips <no...@github.com>.
> + @Consumes(MediaType.APPLICATION_JSON)
> + @Path("/b")
> + @OAuthScopes(STORAGE_FULLCONTROL_SCOPE)
> + @MapBinder(BucketBinder.class)
> + Bucket createBuckets(@QueryParam("project") String projectNumber,
> + @PayloadParam("template") BucketTemplate bucketTemplate, InsertBucketOptions options);
> +
> + /**
> + * Permanently deletes an empty Bucket
> + *
> + * @param bucketName
> + * Name of the bucket
> + *
> + * @return If successful, this method returns an empty response body
> + */
> +
[minor] Remove blank line?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/31/files#r14533109
Re: [jclouds-labs-google] JCLOUD-458:Added Bucket Operation with live
tests (#31)
Posted by Andrew Phillips <no...@github.com>.
> + public Builder addMethod(String method) {
> + this.methods.add(method);
> + return this;
> + }
> +
> + public Builder method(Set<String> method) {
> + this.methods.addAll(method);
> + return this;
> + }
> +
> + public Builder addResponseHeader(String responseHeader) {
> + this.reponseHeaders.add(responseHeader);
> + return this;
> + }
> +
> + public Builder responseHeader(Set<String> responseHeader) {
Should this be `responseHeaders` (both the method and the variable name)?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/31/files#r14532681
Re: [jclouds-labs-google] JCLOUD-458:Added Bucket Operation with live
tests (#31)
Posted by Matt Stephenson <no...@github.com>.
@andrewgaul I recalled that about a day ago when @hsbhathiya informed me his commit hadn't made it. I got the internal bidirectional github branch tracking scripts I setup at Google confused with those at Apache. I thought we had bidirectional sync, I'll work with infra to see if I can improve that. I haven't had access to a machine with my ssh key for a few days.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/31#issuecomment-47837323
Re: [jclouds-labs-google] JCLOUD-458:Added Bucket Operation with live
tests (#31)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds-labs-google #1052](https://buildhive.cloudbees.com/job/jclouds/job/jclouds-labs-google/1052/) SUCCESS
This pull request looks good
[(what's this?)](https://www.cloudbees.com/what-is-buildhive)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/31#issuecomment-47273894
Re: [jclouds-labs-google] JCLOUD-458:Added Bucket Operation with live
tests (#31)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-labs-google-pull-requests #86](https://jclouds.ci.cloudbees.com/job/jclouds-labs-google-pull-requests/86/) UNSTABLE
Looks like there's a problem with this pull request
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/31#issuecomment-47274114
Re: [jclouds-labs-google] JCLOUD-458:Added Bucket Operation with live
tests (#31)
Posted by Bhathiya <no...@github.com>.
> +
> +import com.google.common.base.Objects;
> +import com.google.common.collect.ImmutableSet;
> +
> +/**
> + * The bucket's lifecycle configuration.
> + *
> + * @see <a href= "https://developers.google.com/storage/docs/lifecycle" />
> + */
> +
> +public class BucketLifeCycle {
> +
> +
> + private final Set<Rule> rules;
> +
> + public BucketLifeCycle(Set<Rule> rule) {
Made the change. However this class need bit more testing. will make the changes and add the tests in a seperate PR
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/31/files#r14576939
Re: [jclouds-labs-google] JCLOUD-458:Added Bucket Operation with live
tests (#31)
Posted by Andrew Phillips <no...@github.com>.
> + }
> +
> + @Override
> + public String toString() {
> + return string().toString();
> + }
> +
> + public static Builder builder() {
> + return new Builder();
> + }
> +
> + public static final class Builder {
> +
> + private ImmutableSet.Builder<String> origins = ImmutableSet.builder();
> + private ImmutableSet.Builder<String> methods = ImmutableSet.builder();
> + private ImmutableSet.Builder<String> reponseHeaders = ImmutableSet.builder();
Typo: `responseHeaders`
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/31/files#r14532547
Re: [jclouds-labs-google] JCLOUD-458:Added Bucket Operation with live
tests (#31)
Posted by Andrew Phillips <no...@github.com>.
> +
> +import com.google.common.base.Objects;
> +
> +/**
> + * The bucket's logging configuration, which defines the destination bucket and optional name prefix for the current
> + * bucket's logs.
> + *
> + * @see <a href= "https://developers.google.com/storage/docs/accesslogs" />
> + */
> +
> +public final class Logging {
> + private final String logBucket;
> + private final String logObjectPrefix;
> +
> + public Logging(String logBucket, String logObjectPrefix) {
> +
[minor] Remove blank line?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/31/files#r14532784
Re: [jclouds-labs-google] JCLOUD-458:Added Bucket Operation with live
tests (#31)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds-labs-google #1050](https://buildhive.cloudbees.com/job/jclouds/job/jclouds-labs-google/1050/) SUCCESS
This pull request looks good
[(what's this?)](https://www.cloudbees.com/what-is-buildhive)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/31#issuecomment-47176839
Re: [jclouds-labs-google] JCLOUD-458:Added Bucket Operation with live
tests (#31)
Posted by Andrew Phillips <no...@github.com>.
> +
> +import java.util.Date;
> +
> +import com.google.common.base.Objects;
> +
> +/**
> + * This is an Internal Object used in BucketLifeCycles/Rules.
> + */
> +
> +public class Condition {
> + private final Integer age;
> + private final Date createdBefore;
> + private final Boolean isLive;
> + private final Integer numNewerVersions;
> +
> + public Condition(Integer age, Date createdBefore, Boolean isLive, Integer numNewerVersions) {
All `@Nullable`, I guess?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/31/files#r14532749
Re: [jclouds-labs-google] JCLOUD-458:Added Bucket Operation with live
tests (#31)
Posted by Bhathiya <no...@github.com>.
> }
>
> public enum ObjectRole {
> READER, OWNER
> }
>
> + public enum Location {
> + ASIA, EU, US, ASIA_EAST1, US_CENTRAL1, US_CENTRAL2, US_EAST1, US_EAST2, US_EAST3, US_WEST1;
> +
> + public String value() {
> + return CaseFormat.UPPER_UNDERSCORE.to(CaseFormat.LOWER_HYPHEN, name()).toUpperCase();
was thinking CaseFormat.UPPER_UNDERSCORE.to(CaseFormat.UPPER_HYPHEN, name())
But UPPER_HYPHEN is not available as a caseformat.
Now changed to name().replace('_', '-');
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/31/files#r14576932
Re: [jclouds-labs-google] JCLOUD-458:Added Bucket Operation with live
tests (#31)
Posted by Andrew Phillips <no...@github.com>.
> + * @param bucketName
> + * Name of the bucket
> + * @param bucket
> + * In the request body, supply a bucket resource with list of {@link BucketAccessControls}
> + *
> + * @return If successful, this method returns a {@link Bucket} resource in the response body
> + */
> +
> + @Named("Bucket:update")
> + @PUT
> + @Consumes(MediaType.APPLICATION_JSON)
> + @Produces(MediaType.APPLICATION_JSON)
> + @Path("/b/{bucket}")
> + @OAuthScopes(STORAGE_FULLCONTROL_SCOPE)
> + @Fallback(NullOnNotFoundOr404.class)
> + Bucket updateBuckets(@PathParam("bucket") String bucketName,
`updateBucket`?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/31/files#r14533218
Re: [jclouds-labs-google] JCLOUD-458:Added Bucket Operation with live
tests (#31)
Posted by Andrew Phillips <no...@github.com>.
> + * Name of the bucket
> + * @param options
> + * Supply {@link GetBucketOptions} with optional query parameters
> + *
> + * @return a {@link Bucket} resource
> + */
> +
> + @Named("Bucket:get")
> + @GET
> + @Consumes(MediaType.APPLICATION_JSON)
> + @Produces(MediaType.APPLICATION_JSON)
> + @Path("/b/{bucket}")
> + @OAuthScopes(STORAGE_READONLY_SCOPE)
> + @Fallback(NullOnNotFoundOr404.class)
> + @Nullable
> + Bucket getBuckets(@PathParam("bucket") String bucketName, GetBucketOptions options);
See all the comments above.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/31/files#r14533030
Re: [jclouds-labs-google] JCLOUD-458:Added Bucket Operation with live
tests (#31)
Posted by Matt Stephenson <no...@github.com>.
Reopened #31.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/31#event-136042307
Re: [jclouds-labs-google] JCLOUD-458:Added Bucket Operation with live
tests (#31)
Posted by Andrew Phillips <no...@github.com>.
> + @Produces(MediaType.APPLICATION_JSON)
> + @Path("/b")
> + @OAuthScopes(STORAGE_FULLCONTROL_SCOPE)
> + @Fallback(NullOnNotFoundOr404.class)
> + @Nullable
> + ListPage<Bucket> listBuckets(@QueryParam("project") String project, ListOptions options);
> +
> + /**
> + * Updates a bucket
> + *
> + * @param bucketName
> + * Name of the bucket
> + * @param bucket
> + * In the request body, supply a bucket resource with list of {@link BucketAccessControls}
> + *
> + * @return If successful, this method returns a {@link Bucket} resource in the response body
"If successful, returns the updated Bucket"?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/31/files#r14533229
Re: [jclouds-labs-google] JCLOUD-458:Added Bucket Operation with live
tests (#31)
Posted by Andrew Phillips <no...@github.com>.
> + this.acl.addAll(acl);
> + return this;
> + }
> +
> + public BucketTemplate addDefaultObjectAccessControls(DefaultObjectAccessControls oac) {
> + if (this.defaultObjectAccessControls == null) {
> + this.defaultObjectAccessControls = Sets.newLinkedHashSet();
> + }
> + this.defaultObjectAccessControls.add(oac);
> + return this;
> + }
> +
> + public BucketTemplate defaultObjectAccessControls(Set<DefaultObjectAccessControls> defaultObjectAcl) {
> + if (this.defaultObjectAccessControls == null) {
> + this.defaultObjectAccessControls = Sets.newLinkedHashSet();
> + }
See above comment
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/31/files#r14528344
Re: [jclouds-labs-google] JCLOUD-458:Added Bucket Operation with live
tests (#31)
Posted by Andrew Phillips <no...@github.com>.
> + * Supply {@link InsertBucketOptions} with optional query parameters
> + *
> + * @return If successful, this method returns a {@link Bucket} resource in the response body
> + */
> +
> + @Named("Bucket:insert")
> + @POST
> + @Consumes(MediaType.APPLICATION_JSON)
> + @Path("/b")
> + @OAuthScopes(STORAGE_FULLCONTROL_SCOPE)
> + @MapBinder(BucketBinder.class)
> + Bucket createBuckets(@QueryParam("project") String projectNumber,
> + @PayloadParam("template") BucketTemplate bucketTemplate, InsertBucketOptions options);
> +
> + /**
> + * Permanently deletes an empty Bucket
What if the bucket is not empty? Describe?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/31/files#r14533104
Re: [jclouds-labs-google] JCLOUD-458:Added Bucket Operation with live
tests (#31)
Posted by Andrew Phillips <no...@github.com>.
> + *
> + * @param bucketName
> + * Name of the bucket
> + *
> + * @return a {@link Bucket} resource
> + */
> +
> + @Named("Bucket:get")
> + @GET
> + @Consumes(MediaType.APPLICATION_JSON)
> + @Produces(MediaType.APPLICATION_JSON)
> + @Path("/b/{bucket}")
> + @OAuthScopes(STORAGE_READONLY_SCOPE)
> + @Fallback(NullOnNotFoundOr404.class)
> + @Nullable
> + Bucket getBuckets(@PathParam("bucket") String bucketName);
Should this be `getBucket`?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/31/files#r14533008
Re: [jclouds-labs-google] JCLOUD-458:Added Bucket Operation with live
tests (#31)
Posted by Andrew Phillips <no...@github.com>.
> + * In the request body, supply the relevant portions of a bucket resource
> + * @param options
> + * Supply {@link UpdateBucketOptions} with optional query parameters
> + *
> + * @return If successful, this method returns a {@link Bucket} resource in the response body
> + */
> +
> + @Named("Bucket:patch")
> + @PATCH
> + @Consumes(MediaType.APPLICATION_JSON)
> + @Produces(MediaType.APPLICATION_JSON)
> + @Path("/b/{bucket}")
> + @OAuthScopes(STORAGE_FULLCONTROL_SCOPE)
> + @Fallback(NullOnNotFoundOr404.class)
> + Bucket patchBuckets(@PathParam("bucket") String bucketName,
> + @BinderParam(BindToJsonPayload.class) BucketTemplate bucketTemplate, UpdateBucketOptions options);
See all comments above
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/31/files#r14533292
Re: [jclouds-labs-google] JCLOUD-458:Added Bucket Operation with live
tests (#31)
Posted by Andrew Phillips <no...@github.com>.
> +import org.jclouds.rest.annotations.RequestFilters;
> +import org.jclouds.rest.annotations.SkipEncoding;
> +import org.jclouds.rest.binders.BindToJsonPayload;
> +
> +/**
> + * Provides access to Bucket entities via their REST API.
> + *
> + * @see <a href = " https://developers.google.com/storage/docs/json_api/v1/buckets"/>
> + */
> +
> +@SkipEncoding({ '/', '=' })
> +@RequestFilters(OAuthAuthenticator.class)
> +public interface BucketApi {
> +
> + /**
> + * Returns metadata for the specified bucket.
Hm...so "Bucket" is actually metadata? Shouldn't this be "Get the bucket of the specified name" or "Get the metadata for the bucket of the specified name" or so?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/31/files#r14532980
Re: [jclouds-labs-google] JCLOUD-458:Added Bucket Operation with live
tests (#31)
Posted by Bhathiya <no...@github.com>.
@demobox Thanks for reviewing, I would make the necessary changes in a separate PR
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/31#issuecomment-47977274
Re: [jclouds-labs-google] JCLOUD-458:Added Bucket Operation with live
tests (#31)
Posted by Andrew Phillips <no...@github.com>.
> + @OAuthScopes(STORAGE_FULLCONTROL_SCOPE)
> + @Fallback(NullOnNotFoundOr404.class)
> + @Nullable
> + ListPage<Bucket> listBuckets(@QueryParam("project") String project, ListOptions options);
> +
> + /**
> + * Updates a bucket
> + *
> + * @param bucketName
> + * Name of the bucket
> + * @param bucket
> + * In the request body, supply a bucket resource with list of {@link BucketAccessControls}
> + *
> + * @return If successful, this method returns a {@link Bucket} resource in the response body
> + */
> +
[minor] Remove blank line?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/31/files#r14533235
Re: [jclouds-labs-google] JCLOUD-458:Added Bucket Operation with live
tests (#31)
Posted by Andrew Phillips <no...@github.com>.
> +import org.jclouds.date.internal.SimpleDateFormatDateService;
> +import org.jclouds.googlecloudstorage.domain.BucketAccessControls;
> +import org.jclouds.googlecloudstorage.domain.DomainResourceRefferences.Location;
> +import org.jclouds.googlecloudstorage.domain.DomainResourceRefferences.ObjectRole;
> +import org.jclouds.googlecloudstorage.domain.Bucket;
> +import org.jclouds.googlecloudstorage.domain.DefaultObjectAccessControls;
> +import org.jclouds.googlecloudstorage.domain.DomainResourceRefferences.Role;
> +import org.jclouds.googlecloudstorage.domain.DomainResourceRefferences.StorageClass;
> +import org.jclouds.googlecloudstorage.domain.internal.Owner;
> +import org.jclouds.googlecloudstorage.domain.internal.ProjectTeam;
> +import org.jclouds.googlecloudstorage.domain.internal.ProjectTeam.Team;
> +import org.jclouds.googlecloudstorage.internal.BaseGoogleCloudStorageParseTest;
> +
> +public class FullBucketGetTest extends BaseGoogleCloudStorageParseTest<Bucket> {
> +
> + private final BucketAccessControls acl_1 = BucketAccessControls
[minor] Call it `acl1` (i.e. no underscore)?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/31/files#r14533776
Re: [jclouds-labs-google] JCLOUD-458:Added Bucket Operation with live
tests (#31)
Posted by Andrew Phillips <no...@github.com>.
> + }
> +
> + public BucketTemplate lifeCycle(BucketLifeCycle lifeCycle) {
> + this.lifeCycle = lifeCycle;
> + return this;
> + }
> +
> + public BucketTemplate storageClass(StorageClass storageClass) {
> + this.storageClass = storageClass;
> + return this;
> + }
> +
> + public BucketTemplate addAcl(BucketAccessControls bucketAccessControls) {
> +
> + if (this.acl == null) {
> + this.acl = Sets.newLinkedHashSet();
Linked hash set because we need a consistent ordering for _testing_ or actually in the code? Also: if we're going to do this anyway, how about initializing the variable to an empty set straight away?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/31/files#r14528307
Re: [jclouds-labs-google] JCLOUD-458:Added Bucket Operation with live
tests (#31)
Posted by Andrew Phillips <no...@github.com>.
> +
> + BucketTemplate logTemplate = new BucketTemplate().name(LOG_BUCKET_NAME);
> + Bucket logResponse = api().createBuckets(PROJECT_NUMBER, logTemplate);
> + assertNotNull(logResponse);
> +
> + BucketAccessControls acl = BucketAccessControls.builder().bucket(BUCKET_NAME).entity("allUsers").role(Role.OWNER)
> + .build();
> + DefaultObjectAccessControls oac = DefaultObjectAccessControls.builder().bucket(BUCKET_NAME).entity("allUsers")
> + .role(ObjectRole.OWNER).build();
> + BucketCors bucketCors = BucketCors.builder().addOrigin("http://example.appspot.com").addMethod("GET").addMethod("HEAD")
> + .addResponseHeader("x-meta-goog-custom").maxAgeSeconds(10).build();
> + Versioning version = Versioning.builder().enalbled(true).build();
> +
> + Logging log = new Logging(LOG_BUCKET_NAME, BUCKET_NAME);
> +
> + BucketTemplate template = new BucketTemplate().name(BUCKET_NAME).addAcl(acl)
Does BucketTemplate have a builder? If so, use that?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/31/files#r14533499
Re: [jclouds-labs-google] JCLOUD-458:Added Bucket Operation with live
tests (#31)
Posted by Andrew Phillips <no...@github.com>.
> + assertEquals(response.getName(), BUCKET_NAME_WITHOPTIONS);
> + assertNotNull(response.getAcl());
> + }
> +
> + @Test(groups = "live", dependsOnMethods = "testCreateBucket")
> + public void testGetBucket() {
> + Bucket response = api().getBuckets(BUCKET_NAME);
> +
> + assertNotNull(response);
> + assertEquals(response.getName(), BUCKET_NAME);
> + assertEquals(response.getKind(), Kind.BUCKET);
> + }
> +
> + @Test(groups = "live", dependsOnMethods = "testUpdateBucketWithOptions")
> + public void testGetBucketWithOptions() {
> + GetBucketOptions options = new GetBucketOptions().ifMetagenerationMatch(metageneration);
Use the builder?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/31/files#r14533517
Re: [jclouds-labs-google] JCLOUD-458:Added Bucket Operation with live
tests (#31)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-labs-google-pull-requests #85](https://jclouds.ci.cloudbees.com/job/jclouds-labs-google-pull-requests/85/) FAILURE
Looks like there's a problem with this pull request
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/31#issuecomment-47176641
Re: [jclouds-labs-google] JCLOUD-458:Added Bucket Operation with live
tests (#31)
Posted by Matt Stephenson <no...@github.com>.
Merged #31.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/31#event-136105251
Re: [jclouds-labs-google] JCLOUD-458:Added Bucket Operation with live
tests (#31)
Posted by Andrew Phillips <no...@github.com>.
> + assertEquals(response.getKind(), Kind.BUCKET);
> + assertEquals(response.getName(), BUCKET_NAME);
> + assertEquals(response.getLocation(), Location.US_CENTRAL2);
> + assertTrue(response.getVersioning().isEnabled());
> + }
> +
> + @Test(groups = "live")
> + public void testCreateBucketWithOptions() {
> +
> + DefaultObjectAccessControls oac = DefaultObjectAccessControls.builder().bucket(BUCKET_NAME_WITHOPTIONS)
> + .entity("allUsers").role(ObjectRole.OWNER).build();
> + BucketCors bucketCors = BucketCors.builder().addOrigin("http://example.appspot.com").addMethod("GET").addMethod("HEAD")
> + .addResponseHeader("x-meta-goog-custom").maxAgeSeconds(10).build();
> + Versioning version = Versioning.builder().enalbled(true).build();
> +
> + BucketTemplate template = new BucketTemplate().name(BUCKET_NAME_WITHOPTIONS)
Use the builder?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/31/files#r14533640
Re: [jclouds-labs-google] JCLOUD-458:Added Bucket Operation with live
tests (#31)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-labs-google-pull-requests #87](https://jclouds.ci.cloudbees.com/job/jclouds-labs-google-pull-requests/87/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/31#issuecomment-47390252
Re: [jclouds-labs-google] JCLOUD-458:Added Bucket Operation with live
tests (#31)
Posted by Bhathiya <no...@github.com>.
> +/**
> + * The bucket's Cross-Origin Resource Sharing (CORS) configuration.
> + *
> + * @see <a href= "https://developers.google.com/storage/docs/cross-origin" />
> + */
> +
> +public final class BucketCors {
> + private final Set<String> origins;
> + private final Set<String> methods;
> + private final Set<String> responseHeaders;
> + private final Integer maxAgeSeconds;
> +
> + public BucketCors(@Nullable Set<String> origin, @Nullable Set<String> method, @Nullable Set<String> responseHeader,
> + Integer maxAgeSeconds) {
> +
> + this.origins = origin == null ? ImmutableSet.<String> of() : origin;
Now it is consistent with Bucket. Actually BucketCor class had some issues with empty sets.
Also changed methods,origins ,responseHeaders back to method ,origin & responseHeader
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/31/files#r14576935
Re: [jclouds-labs-google] JCLOUD-458:Added Bucket Operation with live
tests (#31)
Posted by Andrew Phillips <no...@github.com>.
> + BucketAccessControls bucketacl = BucketAccessControls.builder().bucket(BUCKET_NAME)
> + .entity("allAuthenticatedUsers").role(Role.OWNER).build();
> + BucketTemplate template = new BucketTemplate().name(BUCKET_NAME).addAcl(bucketacl);
> + Bucket response = api().updateBuckets(BUCKET_NAME, template);
> +
> + assertNotNull(response);
> + assertEquals(response.getName(), BUCKET_NAME);
> + assertNotNull(response.getAcl());
> + }
> +
> + @Test(groups = "live", dependsOnMethods = "testCreateBucketWithOptions")
> + public void testUpdateBucketWithOptions() {
> + BucketAccessControls bucketacl = BucketAccessControls.builder().bucket(BUCKET_NAME_WITHOPTIONS)
> + .entity("allAuthenticatedUsers").role(Role.OWNER).build();
> + UpdateBucketOptions options = new UpdateBucketOptions().projection(Projection.FULL);
> + BucketTemplate template = new BucketTemplate().name(BUCKET_NAME_WITHOPTIONS).addAcl(bucketacl);
Use the builder?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/31/files#r14533656
Re: [jclouds-labs-google] JCLOUD-458:Added Bucket Operation with live
tests (#31)
Posted by Andrew Phillips <no...@github.com>.
> +
> +import com.google.common.base.Objects;
> +
> +/**
> + * This is a internal object in bucket resource
> + *
> + * @see <a href= "https://developers.google.com/storage/docs/website-configuration" />
> + */
> +
> +public class Website {
> +
> + private final String mainPageSuffix;
> + private final String notFoundPage;
> +
> + public Website(String mainPageSuffix, String notFoundPage) {
> +
[minor] Remove blank link?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/31/files#r14532910
Re: [jclouds-labs-google] JCLOUD-458:Added Bucket Operation with live
tests (#31)
Posted by Andrew Phillips <no...@github.com>.
> + DefaultObjectAccessControls oac = DefaultObjectAccessControls.builder().bucket(BUCKET_NAME).entity("allUsers")
> + .role(ObjectRole.OWNER).build();
> + BucketCors bucketCors = BucketCors.builder().addOrigin("http://example.appspot.com").addMethod("GET").addMethod("HEAD")
> + .addResponseHeader("x-meta-goog-custom").maxAgeSeconds(10).build();
> + Versioning version = Versioning.builder().enalbled(true).build();
> +
> + Logging log = new Logging(LOG_BUCKET_NAME, BUCKET_NAME);
> +
> + BucketTemplate template = new BucketTemplate().name(BUCKET_NAME).addAcl(acl)
> + .addDefaultObjectAccessControls(oac).versioning(version).location(Location.US_CENTRAL2).logging(log)
> + .storageClass(StorageClass.DURABLE_REDUCED_AVAILABILITY).addCORS(bucketCors);
> +
> + Bucket response = api().createBuckets(PROJECT_NUMBER, template);
> +
> + assertNotNull(response);
> + assertNotNull(response.getCors());
Can we say something more details about cors? Non empty, size etc.?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/31/files#r14533432
Re: [jclouds-labs-google] JCLOUD-458:Added Bucket Operation with live
tests (#31)
Posted by Andrew Phillips <no...@github.com>.
> +
> +import javax.inject.Inject;
> +
> +import org.jclouds.googlecloudstorage.domain.BucketTemplate;
> +import org.jclouds.http.HttpRequest;
> +import org.jclouds.rest.MapBinder;
> +import org.jclouds.rest.binders.BindToJsonPayload;
> +
> +public class BucketBinder implements MapBinder {
> +
> + @Inject
> + private BindToJsonPayload jsonBinder;
> +
> + @Override
> + public <R extends HttpRequest> R bindToRequest(R request, Map<String, Object> postParams) {
> + BucketTemplate postBucket = (BucketTemplate) postParams.get("template");
Do we need any checks here for null etc.? Or will the input always be valid?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/31/files#r14533317
Re: [jclouds-labs-google] JCLOUD-458:Added Bucket Operation with live
tests (#31)
Posted by Andrew Phillips <no...@github.com>.
> +
> + public BucketTemplate addAcl(BucketAccessControls bucketAccessControls) {
> +
> + if (this.acl == null) {
> + this.acl = Sets.newLinkedHashSet();
> + }
> +
> + this.acl.add(bucketAccessControls);
> + return this;
> + }
> +
> + public BucketTemplate acl(Set<BucketAccessControls> acl) {
> +
> + if (this.acl == null) {
> + this.acl = Sets.newLinkedHashSet();
> + }
See above comment.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/31/files#r14528324
Re: [jclouds-labs-google] JCLOUD-458:Added Bucket Operation with live
tests (#31)
Posted by Andrew Phillips <no...@github.com>.
> + public Builder responseHeader(Set<String> responseHeader) {
> + this.reponseHeaders.addAll(responseHeader);
> + return this;
> + }
> +
> + public Builder maxAgeSeconds(Integer maxAgeSeconds) {
> + this.maxAgeSeconds = maxAgeSeconds;
> + return this;
> + }
> +
> + public BucketCors build() {
> + return new BucketCors(this.origins.build(), this.methods.build(), this.reponseHeaders.build(),
> + this.maxAgeSeconds);
> + }
> +
> + public Builder fromCors(BucketCors c) {
[minor] Rename the variable to `bc`, `cors`, `from` or `bucketCors`?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/31/files#r14532629
Re: [jclouds-labs-google] JCLOUD-458:Added Bucket Operation with live
tests (#31)
Posted by Andrew Phillips <no...@github.com>.
> + private final String name;
> + private final Long projectNumber;
> + private final Date timeCreated;
> + private final Long metageneration;
> + private final Set<BucketAccessControls> acl;
> + private final Set<DefaultObjectAccessControls> defaultObjectAcl;
> + private final Owner owner;
> + private final Location location;
> + private final Website website;
> + private final Logging logging;
> + private final Versioning versioning;
> + private final Set<BucketCors> cors;
> + private final BucketLifeCycle lifeCycle;
> + private final StorageClass storageClass;
> +
> + public Bucket(String id, URI selfLink, String name, String etag, Long projectNumber, Date timeCreated,
If this has a builder, should it also have a public constructor?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/31/files#r14489265
Re: [jclouds-labs-google] JCLOUD-458:Added Bucket Operation with live
tests (#31)
Posted by Andrew Phillips <no...@github.com>.
> +
> +import com.google.common.base.Objects;
> +import com.google.common.collect.ImmutableSet;
> +
> +/**
> + * The bucket's lifecycle configuration.
> + *
> + * @see <a href= "https://developers.google.com/storage/docs/lifecycle" />
> + */
> +
> +public class BucketLifeCycle {
> +
> +
> + private final Set<Rule> rules;
> +
> + public BucketLifeCycle(Set<Rule> rule) {
Should the variable be `rules` here?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/31/files#r14532720
Re: [jclouds-labs-google] JCLOUD-458:Added Bucket Operation with live
tests (#31)
Posted by Andrew Phillips <no...@github.com>.
Overall, nice work! A couple of comments:
* In the domain objects, we have many nullable fields. Sometimes they're `@Nullable` in the constructor, usually not. Either way works for me, but we should try to be consistent both in this PR and with jclouds usage elsewhere?
* In some domain objects, we do empty-collection-to-null conversion, in others it's the other way around. Again, be consistent in this PR and with the rest of jclouds, if possible?
* Expect tests rather than MockWebServer tests?
* Use builders in the tests?
Thanks, @hsbhathiya and @mattstep!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/31#issuecomment-47975001
Re: [jclouds-labs-google] JCLOUD-458:Added Bucket Operation with live
tests (#31)
Posted by Matt Stephenson <no...@github.com>.
Closed #31.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/31#event-136042292
Re: [jclouds-labs-google] JCLOUD-458:Added Bucket Operation with live
tests (#31)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds-labs-google #1057](https://buildhive.cloudbees.com/job/jclouds/job/jclouds-labs-google/1057/) SUCCESS
This pull request looks good
[(what's this?)](https://www.cloudbees.com/what-is-buildhive)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/31#issuecomment-47390196
Re: [jclouds-labs-google] JCLOUD-458:Added Bucket Operation with live
tests (#31)
Posted by Andrew Phillips <no...@github.com>.
> + * Name of the project to retrieve the buckets
> + * @param options
> + * Supply {@link ListOptions} with optional query parameters
> + *
> + *
> + */
> +
> + @Named("Bucket:list")
> + @GET
> + @Consumes(MediaType.APPLICATION_JSON)
> + @Produces(MediaType.APPLICATION_JSON)
> + @Path("/b")
> + @OAuthScopes(STORAGE_FULLCONTROL_SCOPE)
> + @Fallback(NullOnNotFoundOr404.class)
> + @Nullable
> + ListPage<Bucket> listBuckets(@QueryParam("project") String project, ListOptions options);
See all comments above
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/31/files#r14533208
Re: [jclouds-labs-google] JCLOUD-458:Added Bucket Operation with live
tests (#31)
Posted by Andrew Phillips <no...@github.com>.
> @@ -44,4 +45,11 @@
> @Delegate
> @Path("")
> BucketAccessControlsApi getBucketAccessControlsApi();
> +
> + /**
> + * Provides access to Bucket features
> + */
> + @Delegate
> + @Path("")
> + BucketApi getBucketsApi();
Should this be `getBucketApi()`?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/31/files#r14484479
Re: [jclouds-labs-google] JCLOUD-458:Added Bucket Operation with live
tests (#31)
Posted by Andrew Phillips <no...@github.com>.
> + return string().toString();
> + }
> +
> + public static Builder builder() {
> + return new Builder();
> + }
> +
> + public static class Builder {
> + ImmutableSet.Builder<Rule> rules = ImmutableSet.builder();
> +
> + public Builder addRule(Rule rule) {
> + this.rules.add(rule);
> + return this;
> + }
> +
> + public Builder rule(Set<Rule> rule) {
`rules` (variable and method name)?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/31/files#r14532730
Re: [jclouds-labs-google] JCLOUD-458:Added Bucket Operation with live
tests (#31)
Posted by Andrew Phillips <no...@github.com>.
>
> import com.google.common.base.Objects;
>
> /**
> * Represents a DefaultObjectAccessControls Resource
> - *
> + *
This might give a "trailing whitespace" warning...remove the trailing space?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/31/files#r14528388
Re: [jclouds-labs-google] JCLOUD-458:Added Bucket Operation with live
tests (#31)
Posted by Andrew Phillips <no...@github.com>.
> +/**
> + * The bucket's lifecycle configuration.
> + *
> + * @see <a href= "https://developers.google.com/storage/docs/lifecycle" />
> + */
> +
> +public class BucketLifeCycle {
> +
> +
> + private final Set<Rule> rules;
> +
> + public BucketLifeCycle(Set<Rule> rule) {
> + this.rules = rule == null ? ImmutableSet.<Rule> of() : rule;
> + }
> +
> + public Set<Rule> getRule() {
Should this be `getRules`?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/31/files#r14532705
Re: [jclouds-labs-google] JCLOUD-458:Added Bucket Operation with live
tests (#31)
Posted by Andrew Phillips <no...@github.com>.
> +
> + Bucket response = api().createBuckets(PROJECT_NUMBER, template, options);
> +
> + assertNotNull(response);
> + assertNotNull(response.getCors());
> + assertEquals(response.getKind(), Kind.BUCKET);
> + assertEquals(response.getName(), BUCKET_NAME_WITHOPTIONS);
> + assertEquals(response.getLocation(), Location.US_CENTRAL2);
> + assertTrue(response.getVersioning().isEnabled());
> + }
> +
> + @Test(groups = "live", dependsOnMethods = "testCreateBucket")
> + public void testUpdateBucket() {
> + BucketAccessControls bucketacl = BucketAccessControls.builder().bucket(BUCKET_NAME)
> + .entity("allAuthenticatedUsers").role(Role.OWNER).build();
> + BucketTemplate template = new BucketTemplate().name(BUCKET_NAME).addAcl(bucketacl);
Use the builder?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/31/files#r14533651
Re: [jclouds-labs-google] JCLOUD-458:Added Bucket Operation with live
tests (#31)
Posted by Andrew Phillips <no...@github.com>.
> + * @param bucketName
> + * Name of the bucket
> + * @param options
> + * Supply {@link DeleteBucketOptions} with optional query parameters
> + *
> + * @return If successful, this method returns an empty response body.
> + */
> +
> + @Named("Bucket:delete")
> + @DELETE
> + @Consumes(MediaType.APPLICATION_JSON)
> + @Path("/b/{bucket}")
> + @OAuthScopes(STORAGE_FULLCONTROL_SCOPE)
> + @Fallback(NullOnNotFoundOr404.class)
> + @Nullable
> + HttpResponse deleteBuckets(@PathParam("bucket") String bucketName, DeleteBucketOptions options);
See all comments above
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/31/files#r14533163
Re: [jclouds-labs-google] JCLOUD-458:Added Bucket Operation with live
tests (#31)
Posted by Andrew Phillips <no...@github.com>.
> + *
> + * @return If successful,this method returns a {@link Bucket} resource in the response body
> + */
> +
> + @Named("Bucket:update")
> + @PUT
> + @Consumes(MediaType.APPLICATION_JSON)
> + @Produces(MediaType.APPLICATION_JSON)
> + @Path("/b/{bucket}")
> + @OAuthScopes(STORAGE_FULLCONTROL_SCOPE)
> + @Fallback(NullOnNotFoundOr404.class)
> + Bucket updateBuckets(@PathParam("bucket") String bucketName,
> + @BinderParam(BindToJsonPayload.class) BucketTemplate bucketTemplate, UpdateBucketOptions options);
> +
> + /**
> + * Updates a bucket supporting patch semantics.
Explain how this is different from `updateBucket`?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/31/files#r14533255
Re: [jclouds-labs-google] JCLOUD-458:Added Bucket Operation with live
tests (#31)
Posted by Andrew Phillips <no...@github.com>.
> + * @param bucketName
> + * In the request body, supply a bucket resource with list of {@link BucketAccessControls} (acl[])
> + * @param bucketTemplate
> + * In the request body, supply the relevant portions of a bucket resource
> + *
> + * @return If successful, this method returns a {@link Bucket} resource in the response body
> + */
> +
> + @Named("Bucket:patch")
> + @PATCH
> + @Consumes(MediaType.APPLICATION_JSON)
> + @Produces(MediaType.APPLICATION_JSON)
> + @Path("/b/{bucket}")
> + @OAuthScopes(STORAGE_FULLCONTROL_SCOPE)
> + @Fallback(NullOnNotFoundOr404.class)
> + Bucket patchBuckets(@PathParam("bucket") String bucketName,
`patchBucket`?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/31/files#r14533278
Re: [jclouds-labs-google] JCLOUD-458:Added Bucket Operation with live
tests (#31)
Posted by Bhathiya <no...@github.com>.
> +import static com.google.common.base.Objects.equal;
> +import static com.google.common.base.Objects.toStringHelper;
> +
> +import java.beans.ConstructorProperties;
> +
> +import com.google.common.base.Objects;
> +
> +/**
> + * The bucket's logging configuration, which defines the destination bucket and optional name prefix for the current
> + * bucket's logs.
> + */
> +
> +public final class ProjectTeam {
> +
> + public enum Team {
> + owners, editors, viewers;
Changed
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/31/files#r14577033
Re: [jclouds-labs-google] JCLOUD-458:Added Bucket Operation with live
tests (#31)
Posted by Andrew Phillips <no...@github.com>.
> + /**
> + * Retrieves a list of buckets for a given project
> + *
> + * @param project
> + * Name of the project to retrieve the buckets
> + *
> + * @return a {@link ListPage<Bucket>}
> + */
> +
> + @Named("Bucket:list")
> + @GET
> + @Consumes(MediaType.APPLICATION_JSON)
> + @Produces(MediaType.APPLICATION_JSON)
> + @Path("/b")
> + @OAuthScopes(STORAGE_FULLCONTROL_SCOPE)
> + @Fallback(NullOnNotFoundOr404.class)
Would returning an empty ListPage make more sense here?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/31/files#r14533203
Re: [jclouds-labs-google] JCLOUD-458:Added Bucket Operation with live
tests (#31)
Posted by Ignasi Barrera <no...@github.com>.
@mattstep @andrewgaul also take into account that merging using the GitHub "merge button" will generate a merge commit even if the branch is rebased to the latest version (at least it did the last time I used it), anď we don't want that. Just FYI!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/31#issuecomment-48016776
Re: [jclouds-labs-google] JCLOUD-458:Added Bucket Operation with live
tests (#31)
Posted by Andrew Gaul <no...@github.com>.
@mattstep Note that merging via GitHub does *not* push to the official Apache git repository; jclouds-mirror pushes from Apache to GitHub. Committers must git push to the Apache repository. I reworded the commit message, made some whitespace changes, and pushed to master as b2d343d06a8275d0b9b8b6c9e30c49030f697fd9.
@hsbhathiya Please try to avoid trailing whitespace and removing newlines at the end of files, perhaps reconfigure your editor? You should be able to see these with `git show` if you add the following to your `.gitconfig`:
```
[color]
status=auto
branch=auto
interactive=auto
diff=auto
```
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/31#issuecomment-47836401
Re: [jclouds-labs-google] JCLOUD-458:Added Bucket Operation with live
tests (#31)
Posted by Matt Stephenson <no...@github.com>.
Closing and reopening to trigger cloudbees again
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/31#issuecomment-47389586
Re: [jclouds-labs-google] JCLOUD-458:Added Bucket Operation with live
tests (#31)
Posted by Andrew Phillips <no...@github.com>.
> + public int hashCode() {
> + return Objects.hashCode(entity, entityId);
> + }
> +
> + @Override
> + public boolean equals(Object obj) {
> + if (this == obj)
> + return true;
> + if (obj == null || getClass() != obj.getClass())
> + return false;
> + Owner that = Owner.class.cast(obj);
> + return equal(this.entity, that.entity);
> + }
> +
> + protected Objects.ToStringHelper string() {
> + return toStringHelper(this).omitNullValues().add("entiy", entity).add("entityId", entityId);
Typo: `entity`?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/31/files#r14532799
Re: [jclouds-labs-google] JCLOUD-458:Added Bucket Operation with live
tests (#31)
Posted by Andrew Phillips <no...@github.com>.
> + * Permanently deletes an empty Bucket
> + *
> + * @param bucketName
> + * Name of the bucket
> + *
> + * @return If successful, this method returns an empty response body
> + */
> +
> + @Named("Bucket:delete")
> + @DELETE
> + @Consumes(MediaType.APPLICATION_JSON)
> + @Path("/b/{bucket}")
> + @OAuthScopes(STORAGE_FULLCONTROL_SCOPE)
> + @Fallback(NullOnNotFoundOr404.class)
> + @Nullable
> + HttpResponse deleteBuckets(@PathParam("bucket") String bucketName);
`deleteBucket`?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/31/files#r14533092
Re: [jclouds-labs-google] JCLOUD-458:Added Bucket Operation with live
tests (#31)
Posted by Bhathiya <no...@github.com>.
> +import static com.google.common.base.Objects.equal;
> +import static com.google.common.base.Objects.toStringHelper;
> +
> +import com.google.common.base.Objects;
> +
> +/**
> + * This is an internal object used in both Bucket and Object representation
> + */
> +
> +public final class Owner {
> +
> + private final String entity;
> + private final String entityId;
> +
> + public Owner(String entity, String entityId) {
> + this.entity = entity;
>From api refference in BucketAccessCotrol
entity
"The entity holding the permission, in one of the following forms:
user-userId
user-email
group-groupId
group-email
domain-domain
project-team-projectId
allUsers
allAuthenticatedUsers"
entityId - "The ID for the entity, if any" .
Anyway entityId return null in all the tests i tried
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/31/files#r14577031
Re: [jclouds-labs-google] JCLOUD-458:Added Bucket Operation with live
tests (#31)
Posted by Andrew Phillips <no...@github.com>.
> + * @see <a href = " https://developers.google.com/storage/docs/json_api/v1/buckets"/>
> + */
> +
> +@SkipEncoding({ '/', '=' })
> +@RequestFilters(OAuthAuthenticator.class)
> +public interface BucketApi {
> +
> + /**
> + * Returns metadata for the specified bucket.
> + *
> + * @param bucketName
> + * Name of the bucket
> + *
> + * @return a {@link Bucket} resource
> + */
> +
[minor] Remove blank line?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/31/files#r14532996
Re: [jclouds-labs-google] JCLOUD-458:Added Bucket Operation with live
tests (#31)
Posted by Andrew Phillips <no...@github.com>.
> + Long metageneration, Set<BucketAccessControls> acl, Set<DefaultObjectAccessControls> defaultObjectAcl,
> + Owner owner, Location location, Website website, Logging logging, Versioning versioning, Set<BucketCors> cors,
> + BucketLifeCycle lifeCycle, StorageClass storageClass) {
> +
> + super(Kind.BUCKET, id, selfLink, etag);
> + this.projectNumber = projectNumber;
> + this.timeCreated = checkNotNull(timeCreated, "timeCreated");
> + this.metageneration = checkNotNull(metageneration, "metageneration");
> + this.acl = acl.isEmpty() ? null : acl;
> + this.defaultObjectAcl = defaultObjectAcl.isEmpty() ? null : defaultObjectAcl;
> + this.owner = checkNotNull(owner, "Owner");
> + this.location = checkNotNull(location, "location");
> + this.website = website;
> + this.logging = logging;
> + this.versioning = versioning;
> + this.cors = cors.isEmpty() ? null : cors;
We're converting an empty input set to `null` here...that's intentional, I take it?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/31/files#r14489206
Re: [jclouds-labs-google] JCLOUD-458:Added Bucket Operation with live
tests (#31)
Posted by Andrew Phillips <no...@github.com>.
> + *
> + * @param bucketTemplate
> + * In the request body, supply a {@link Bucket} resource
> + * @param options
> + * Supply {@link InsertBucketOptions} with optional query parameters
> + *
> + * @return If successful, this method returns a {@link Bucket} resource in the response body
> + */
> +
> + @Named("Bucket:insert")
> + @POST
> + @Consumes(MediaType.APPLICATION_JSON)
> + @Path("/b")
> + @OAuthScopes(STORAGE_FULLCONTROL_SCOPE)
> + @MapBinder(BucketBinder.class)
> + Bucket createBuckets(@QueryParam("project") String projectNumber,
See all comments above
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/31/files#r14533070
Re: [jclouds-labs-google] JCLOUD-458:Added Bucket Operation with live
tests (#31)
Posted by Andrew Phillips <no...@github.com>.
> + @Test(groups = "live", dependsOnMethods = { "testListBucket", "testGetBucket", "testUpdateBucket" })
> + public void testDeleteBucket() {
> + HttpResponse response = api().deleteBuckets(BUCKET_NAME);
> + HttpResponse response2 = api().deleteBuckets(LOG_BUCKET_NAME);
> +
> + assertNotNull(response);
> + assertEquals(response.getStatusCode(), 204);
> + assertNotNull(response2);
> + assertEquals(response2.getStatusCode(), 204);
> +
> + }
> +
> + @Test(groups = "live", dependsOnMethods = { "testGetBucketWithOptions" })
> + public void testDeleteBucketWithOptions() {
> +
> + DeleteBucketOptions options = new DeleteBucketOptions().ifMetagenerationMatch(metageneration)
Use the builder?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/31/files#r14533587
Re: [jclouds-labs-google] JCLOUD-458:Added Bucket Operation with live
tests (#31)
Posted by Andrew Phillips <no...@github.com>.
> +import static com.google.common.base.Objects.equal;
> +import static com.google.common.base.Objects.toStringHelper;
> +
> +import com.google.common.base.Objects;
> +
> +/**
> + * This is an internal object used in both Bucket and Object representation
> + */
> +
> +public final class Owner {
> +
> + private final String entity;
> + private final String entityId;
> +
> + public Owner(String entity, String entityId) {
> + this.entity = entity;
What is this if it's not the identifier? Is it the name? Or is "entity" a domain object that is always a string?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/31/files#r14532835
Re: [jclouds-labs-google] JCLOUD-458:Added Bucket Operation with live
tests (#31)
Posted by Andrew Phillips <no...@github.com>.
> +/**
> + * The bucket's Cross-Origin Resource Sharing (CORS) configuration.
> + *
> + * @see <a href= "https://developers.google.com/storage/docs/cross-origin" />
> + */
> +
> +public final class BucketCors {
> + private final Set<String> origins;
> + private final Set<String> methods;
> + private final Set<String> responseHeaders;
> + private final Integer maxAgeSeconds;
> +
> + public BucketCors(@Nullable Set<String> origin, @Nullable Set<String> method, @Nullable Set<String> responseHeader,
> + Integer maxAgeSeconds) {
> +
> + this.origins = origin == null ? ImmutableSet.<String> of() : origin;
Just curious: up in `Bucket` we had empty-collection-to-null conversion in the constructor, here it's exactly the opposite (null-to-empty-collection). Any reason for having these two different approaches?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/31/files#r14532492
Re: [jclouds-labs-google] JCLOUD-458:Added Bucket Operation with live
tests (#31)
Posted by Andrew Phillips <no...@github.com>.
> + GetBucketOptions options = new GetBucketOptions().ifMetagenerationMatch(metageneration);
> + Bucket response = api().getBuckets(BUCKET_NAME_WITHOPTIONS, options);
> +
> + assertNotNull(response);
> + assertEquals(response.getName(), BUCKET_NAME_WITHOPTIONS);
> + assertEquals(response.getKind(), Kind.BUCKET);
> + }
> +
> + @Test(groups = "live", dependsOnMethods = "testCreateBucket")
> + public void testListBucket() {
> + ListPage<Bucket> bucket = api().listBuckets(PROJECT_NUMBER);
> +
> + Iterator<Bucket> pageIterator = bucket.iterator();
> + assertTrue(pageIterator.hasNext());
> +
> + Bucket singlePageIterator = pageIterator.next();
Why is this called `singlePageIterator` when it's a Bucket?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/31/files#r14533697
Re: [jclouds-labs-google] JCLOUD-458:Added Bucket Operation with live
tests (#31)
Posted by Matt Stephenson <no...@github.com>.
@hsbhathiya @demobox brings up some good minor points, please open a separate PR when you have time with these changes.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/31#issuecomment-47840596
Re: [jclouds-labs-google] JCLOUD-458:Added Bucket Operation with live
tests (#31)
Posted by Andrew Phillips <no...@github.com>.
> + @OAuthScopes(STORAGE_READONLY_SCOPE)
> + @Fallback(NullOnNotFoundOr404.class)
> + @Nullable
> + Bucket getBuckets(@PathParam("bucket") String bucketName, GetBucketOptions options);
> +
> + /**
> + * Creates a new bucket
> + *
> + * @param projectId
> + * A valid API project identifier
> + * @param bucketTemplate
> + * In the request body, supply a bucket resource
> + *
> + * @return If successful, this method returns a {@link Bucket} resource in the response body
> + */
> +
[minor] Remove blank line
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/31/files#r14533040
Re: [jclouds-labs-google] JCLOUD-458:Added Bucket Operation with live
tests (#31)
Posted by Andrew Phillips <no...@github.com>.
> + private static final String BUCKET_NAME = "jcloudtestbucket" + (int) (Math.random() * 10000);
> +
> + private static final String BUCKET_NAME_WITHOPTIONS = "jcloudtestbucketoptions" + (int) (Math.random() * 10000);
> +
> + private static final String LOG_BUCKET_NAME = "jcloudtestbucket" + (int) (Math.random() * 10000);
> +
> + private Long metageneration;
> +
> + private BucketApi api() {
> + return api.getBucketsApi();
> + }
> +
> + @Test(groups = "live")
> + public void testCreateBucket() {
> +
> + BucketTemplate logTemplate = new BucketTemplate().name(LOG_BUCKET_NAME);
Use the builder?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/31/files#r14533631
Re: [jclouds-labs-google] JCLOUD-458:Added Bucket Operation with live
tests (#31)
Posted by Andrew Phillips <no...@github.com>.
> + }
> +
> + @Test(groups = "live")
> + public void testCreateBucketWithOptions() {
> +
> + DefaultObjectAccessControls oac = DefaultObjectAccessControls.builder().bucket(BUCKET_NAME_WITHOPTIONS)
> + .entity("allUsers").role(ObjectRole.OWNER).build();
> + BucketCors bucketCors = BucketCors.builder().addOrigin("http://example.appspot.com").addMethod("GET").addMethod("HEAD")
> + .addResponseHeader("x-meta-goog-custom").maxAgeSeconds(10).build();
> + Versioning version = Versioning.builder().enalbled(true).build();
> +
> + BucketTemplate template = new BucketTemplate().name(BUCKET_NAME_WITHOPTIONS)
> + .addDefaultObjectAccessControls(oac).versioning(version).location(Location.US_CENTRAL2)
> + .storageClass(StorageClass.DURABLE_REDUCED_AVAILABILITY).addCORS(bucketCors);
> +
> + InsertBucketOptions options = new InsertBucketOptions().projection(Projection.FULL);
Use the builder?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/31/files#r14533476