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/17 21:52:10 UTC

[jclouds-labs-google] Added Default object access controls with expected tests (#29)

Default object access controls with expected tests. This is also tested with live tests. But live test will be commited after adding Bucket operations  
You can merge this Pull Request by running:

  git pull https://github.com/hsbhathiya/jclouds-labs-google DOACLstream

Or you can view, comment on it, or merge it online at:

  https://github.com/jclouds/jclouds-labs-google/pull/29

-- Commit Summary --

  * GCS sketch with authentication
  * Added Default object access controls with expected tests

-- File Changes --

    A google-cloud-storage/README.md (46)
    A google-cloud-storage/pom.xml (126)
    A google-cloud-storage/src/main/java/org/jclouds/googlecloudstorage/GoogleCloudStorageApi.java (41)
    A google-cloud-storage/src/main/java/org/jclouds/googlecloudstorage/GoogleCloudStorageApiMetadata.java (93)
    A google-cloud-storage/src/main/java/org/jclouds/googlecloudstorage/config/GoogleCloudStorageHttpApiModule.java (31)
    A google-cloud-storage/src/main/java/org/jclouds/googlecloudstorage/config/GoogleCloudStorageParserModule.java (49)
    A google-cloud-storage/src/main/java/org/jclouds/googlecloudstorage/config/OAuthModuleWithoutTypeAdapters.java (49)
    A google-cloud-storage/src/main/java/org/jclouds/googlecloudstorage/domain/DefaultObjectAccessControls.java (248)
    A google-cloud-storage/src/main/java/org/jclouds/googlecloudstorage/domain/DefaultObjectAccessControlsTemplate.java (63)
    A google-cloud-storage/src/main/java/org/jclouds/googlecloudstorage/domain/ListDefaultObjectAccessControls.java (112)
    A google-cloud-storage/src/main/java/org/jclouds/googlecloudstorage/domain/ListPage.java (132)
    A google-cloud-storage/src/main/java/org/jclouds/googlecloudstorage/domain/ObjectRole.java (21)
    A google-cloud-storage/src/main/java/org/jclouds/googlecloudstorage/domain/Resource.java (160)
    A google-cloud-storage/src/main/java/org/jclouds/googlecloudstorage/features/ApiResourceRefferences.java (47)
    A google-cloud-storage/src/main/java/org/jclouds/googlecloudstorage/features/DefaultObjectAccessControlsApi.java (217)
    A google-cloud-storage/src/main/java/org/jclouds/googlecloudstorage/handlers/DefaultObjectAccessControlsBinder.java (43)
    A google-cloud-storage/src/main/java/org/jclouds/googlecloudstorage/reference/GoogleCloudStorageConstants.java (50)
    A google-cloud-storage/src/main/resources/META-INF/services/org.jclouds.apis.ApiMetadata (19)
    A google-cloud-storage/src/test/java/org/jclouds/googlecloudstorage/GoogleCloudStorageApiMetadataTest.java (31)
    A google-cloud-storage/src/test/java/org/jclouds/googlecloudstorage/GoogleCloudStorageAuthenticatedRestContextLiveTest.java (36)
    A google-cloud-storage/src/test/java/org/jclouds/googlecloudstorage/features/DefaultObjectAccessControlsApiExpectTest.java (225)
    A google-cloud-storage/src/test/java/org/jclouds/googlecloudstorage/internal/BaseGoogleCloudStorageApiExpectTest.java (31)
    A google-cloud-storage/src/test/java/org/jclouds/googlecloudstorage/internal/BaseGoogleCloudStorageApiLiveTest.java (39)
    A google-cloud-storage/src/test/java/org/jclouds/googlecloudstorage/internal/BaseGoogleCloudStorageExpectTest.java (175)
    A google-cloud-storage/src/test/java/org/jclouds/googlecloudstorage/internal/BaseGoogleCloudStorageParseTest.java (32)
    A google-cloud-storage/src/test/java/org/jclouds/googlecloudstorage/parse/DefaultObjectaclGetTest.java (41)
    A google-cloud-storage/src/test/java/org/jclouds/googlecloudstorage/parse/DefaultObjectaclInsertTest.java (39)
    A google-cloud-storage/src/test/java/org/jclouds/googlecloudstorage/parse/DefaultObjectaclListTest.java (49)
    A google-cloud-storage/src/test/resources/default_objectacl_get.json (11)
    A google-cloud-storage/src/test/resources/default_objectacl_insert_requestpayload.json (4)
    A google-cloud-storage/src/test/resources/default_objectacl_insert_response.json (6)
    A google-cloud-storage/src/test/resources/default_objectacl_list.json (16)
    A google-cloud-storage/src/test/resources/default_objectacl_update_initial.json (6)
    A google-cloud-storage/src/test/resources/default_objectacl_update_requestpayload.json (5)
    M pom.xml (6)

-- Patch Links --

https://github.com/jclouds/jclouds-labs-google/pull/29.patch
https://github.com/jclouds/jclouds-labs-google/pull/29.diff

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/29

Re: [jclouds-labs-google] Added Default object access controls with expected tests (#29)

Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-labs-google-pull-requests #83](https://jclouds.ci.cloudbees.com/job/jclouds-labs-google-pull-requests/83/) SUCCESS
This pull request looks good

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/29#issuecomment-46587450

Re: [jclouds-labs-google] Added Default object access controls with expected tests (#29)

Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-labs-google-pull-requests #76](https://jclouds.ci.cloudbees.com/job/jclouds-labs-google-pull-requests/76/) SUCCESS
This pull request looks good

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/29#issuecomment-46356716

Re: [jclouds-labs-google] Added Default object access controls with expected tests (#29)

Posted by BuildHive <no...@github.com>.
[jclouds » jclouds-labs-google #1015](https://buildhive.cloudbees.com/job/jclouds/job/jclouds-labs-google/1015/) 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/29#issuecomment-46390349

Re: [jclouds-labs-google] Added Default object access controls with expected tests (#29)

Posted by Andrew Gaul <no...@github.com>.
Closed #29.

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/29#event-133249914

Re: [jclouds-labs-google] Added Default object access controls with expected tests (#29)

Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-labs-google-pull-requests #82](https://jclouds.ci.cloudbees.com/job/jclouds-labs-google-pull-requests/82/) SUCCESS
This pull request looks good

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/29#issuecomment-46390404

Re: [jclouds-labs-google] Added Default object access controls with expected tests (#29)

Posted by Andrew Gaul <no...@github.com>.
> + * See the License for the specific language governing permissions and
> + * limitations under the License.
> + */
> +package org.jclouds.googlecloudstorage.domain;
> +
> +import static com.google.common.base.Objects.equal;
> +import static com.google.common.base.Objects.toStringHelper;
> +import static com.google.common.base.Preconditions.checkNotNull;
> +
> +import java.beans.ConstructorProperties;
> +import java.net.URI;
> +
> +import com.google.common.base.Objects;
> +
> +/**
> + * Represents a DefaultObjectAccessControls Resource * 

Should the trailing asterisk be on the following line?

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/29/files#r13941270

Re: [jclouds-labs-google] Added Default object access controls with expected tests (#29)

Posted by BuildHive <no...@github.com>.
[jclouds » jclouds-labs-google #1014](https://buildhive.cloudbees.com/job/jclouds/job/jclouds-labs-google/1014/) 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/29#issuecomment-46389422

Re: [jclouds-labs-google] Added Default object access controls with expected tests (#29)

Posted by BuildHive <no...@github.com>.
[jclouds » jclouds-labs-google #1021](https://buildhive.cloudbees.com/job/jclouds/job/jclouds-labs-google/1021/) 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/29#issuecomment-46587553

Re: [jclouds-labs-google] Added Default object access controls with expected tests (#29)

Posted by Andrew Gaul <no...@github.com>.
Mostly looks good.  I am still studying GCS ACLs and do not yet understand some of this code.  However, my ignorance need not block merging since this is a foundational commit needed for progress on bucket operations.  Please address the one logic error and some style nits so we can merge and move forward.

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/29#issuecomment-46498890

Re: [jclouds-labs-google] Added Default object access controls with expected tests (#29)

Posted by Andrew Gaul <no...@github.com>.
> +
> +      String payload = "grant_type=urn%3Aietf%3Aparams%3Aoauth%3Agrant-type%3Ajwt-bearer&" +
> +      // Base64 Encoded Header
> +               "assertion=" + base64Url().omitPadding().encode(header.getBytes(UTF_8)) + "." +
> +               // Base64 Encoded Claims
> +               base64Url().omitPadding().encode(claims.getBytes(UTF_8)) + ".";
> +
> +      return HttpRequest.builder().method("POST").endpoint(URI.create("https://accounts.google.com/o/oauth2/token"))
> +               .addHeader("Accept", MediaType.APPLICATION_JSON)
> +               .payload(payloadFromStringWithContentType(payload, "application/x-www-form-urlencoded")).build();
> +   }
> +
> +   protected static Payload staticPayloadFromResource(String resource) {
> +      try {
> +         return payloadFromString(Strings2.toStringAndClose(BaseGoogleCloudStorageExpectTest.class
> +                  .getResourceAsStream(resource)));

This is kind of awkward, can you call `getClass` instead to get `BaseGoogleCloudStorageExpectTest.class`?  Similarly, `BaseRestApiExpectTest.payloadFromResource` implements similar functionality.  The existing code is probably not a good guide; would the following work better:

```
return new ByteSourcePayload(Resources.asByteSource(Resources.getResource(getClass(), resource)));
```

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/29/files#r13942099

Re: [jclouds-labs-google] Added Default object access controls with expected tests (#29)

Posted by BuildHive <no...@github.com>.
[jclouds » jclouds-labs-google #1005](https://buildhive.cloudbees.com/job/jclouds/job/jclouds-labs-google/1005/) 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/29#issuecomment-46356916

Re: [jclouds-labs-google] Added Default object access controls with expected tests (#29)

Posted by Andrew Gaul <no...@github.com>.
> + * distributed under the License is distributed on an "AS IS" BASIS,
> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
> + * See the License for the specific language governing permissions and
> + * limitations under the License.
> + */
> +package org.jclouds.googlecloudstorage.features;
> +
> +import com.google.common.base.CaseFormat;
> +
> +public final class ApiResourceRefferences {
> +
> +   private ApiResourceRefferences() {
> +   }
> +
> +   public enum Projection {
> +      noAcl, full

Most Java enums have ALL_CAPS style.

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/29/files#r13941383

Re: [jclouds-labs-google] Added Default object access controls with expected tests (#29)

Posted by Andrew Gaul <no...@github.com>.
>  /**
> - * Provide access to GoogleCloudStorage
> - *
> + * Provide access to GoogleCloudStorage.

Any reason for this spurious change?  The existing code follow Javadoc norms.

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/29/files#r13941247

Re: [jclouds-labs-google] Added Default object access controls with expected tests (#29)

Posted by Andrew Gaul <no...@github.com>.
> +               .addHeader("Authorization", "Bearer " + TOKEN)
> +               .payload(payloadFromResourceWithContentType("/default_objectacl_insert_requestpayload.json",
> +                        MediaType.APPLICATION_JSON)).build();
> +
> +      DefaultObjectAccessControlsApi api = requestsSendResponses(requestForScopes(STORAGE_FULLCONTROL_SCOPE),
> +               TOKEN_RESPONSE, insertRequest, CREATE_DEFAULT_OBJECTACL_RESPONSE).getDefaultObjectAccessControlsApi();
> +
> +      DefaultObjectAccessControlsTemplate template = new DefaultObjectAccessControlsTemplate().entity("allUsers").role(
> +               ObjectRole.OWNER);
> +
> +      assertEquals(api.createDefaultObjectAccessControls(EXPECTED_TEST_BUCKET, template),
> +               new DefaultObjectaclInsertTest().expected());
> +   }
> +
> +   // Test deleteDefaultObjectAccessControls
> +   public void testDeleteObjectaclResponseIs2xx() throws Exception {

Should this be capitalized with Acl?

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/29/files#r13942276

Re: [jclouds-labs-google] Added Default object access controls with expected tests (#29)

Posted by Andrew Gaul <no...@github.com>.
Reworded commit message, removed some whitespace noise, tested locally, and pushed to master.

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/29#issuecomment-46607685

Re: [jclouds-labs-google] Added Default object access controls with expected tests (#29)

Posted by Andrew Gaul <no...@github.com>.
> +
> +   }
> +
> +   @Override
> +   protected Properties setupProperties() {
> +      Properties props = super.setupProperties();
> +      // use no sig algorithm for expect tests (means no credential is required either)
> +      props.put(OAuthProperties.SIGNATURE_OR_MAC_ALGORITHM, OAuthConstants.NO_ALGORITHM);
> +      return props;
> +   }
> +
> +   @Override
> +   protected HttpRequestComparisonType compareHttpRequestAsType(HttpRequest input) {
> +      HttpRequestComparisonType reqType = HttpRequestComparisonType.DEFAULT;
> +      if (input.getPayload() != null) {
> +         if (input.getPayload().getContentMetadata().getContentType().equals(MediaType.APPLICATION_JSON)) {

`ContentMetadata.getContentType` returns a nullable String, not a `MediaType`.  Compare instead:

```
MediaType.APPLICATION_JSON.toString().equals(input.getPayload().getContentMetadata().getContentType())
```

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/29/files#r13941532

Re: [jclouds-labs-google] Added Default object access controls with expected tests (#29)

Posted by Andrew Gaul <no...@github.com>.
> @@ -0,0 +1,5 @@
> +{
> + "kind": "storage#objectAccessControl",
> +  "entity": "allUsers",

these have a wonky formatting; can you stick with 1- or 2-space indents consistently?  Also some files lack a trailing newline.

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/29/files#r13941636

Re: [jclouds-labs-google] Added Default object access controls with expected tests (#29)

Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-labs-google-pull-requests #81](https://jclouds.ci.cloudbees.com/job/jclouds-labs-google-pull-requests/81/) 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/29#issuecomment-46389223