You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@jclouds.apache.org by Roman Coedo <no...@github.com> on 2014/06/17 12:40:07 UTC
[jclouds-labs-aws] JCLOUDS-457: Added deleteArchive and uploadArchive
operations. (#13)
Now the Glacier client supports upload and delete archive
operations.
An static TestUtils class has been made for the archive operations
tests. This class allows us to build payloads and build ByteSources.
TreeHash has been refactored too to use this class instead of private
methods.
The VaultNameValidator has been refactored to use buildData when
testing big names. This makes the code more readable.
You can merge this Pull Request by running:
git pull https://github.com/rcoedo/jclouds-labs-aws archiveops
Or you can view, comment on it, or merge it online at:
https://github.com/jclouds/jclouds-labs-aws/pull/13
-- Commit Summary --
* JCLOUDS-457: Added deleteArchive and uploadArchive operations.
-- File Changes --
M glacier/src/main/java/org/jclouds/glacier/GlacierAsyncClient.java (42)
M glacier/src/main/java/org/jclouds/glacier/GlacierClient.java (34)
A glacier/src/main/java/org/jclouds/glacier/binders/BindDescriptionToHeaders.java (43)
A glacier/src/main/java/org/jclouds/glacier/binders/BindHashesToHeaders.java (59)
A glacier/src/main/java/org/jclouds/glacier/functions/ParseArchiveIdHeader.java (40)
A glacier/src/main/java/org/jclouds/glacier/predicates/validators/DescriptionValidator.java (57)
A glacier/src/main/java/org/jclouds/glacier/predicates/validators/PayloadValidator.java (50)
M glacier/src/main/java/org/jclouds/glacier/reference/GlacierHeaders.java (4)
M glacier/src/test/java/org/jclouds/glacier/GlacierClientMockTest.java (56)
A glacier/src/test/java/org/jclouds/glacier/predicates/validators/DescriptionValidatorTest.java (51)
A glacier/src/test/java/org/jclouds/glacier/predicates/validators/PayloadValidatorTest.java (51)
M glacier/src/test/java/org/jclouds/glacier/predicates/validators/VaultNameValidatorTest.java (15)
A glacier/src/test/java/org/jclouds/glacier/util/TestUtils.java (45)
M glacier/src/test/java/org/jclouds/glacier/util/TreeHashTest.java (29)
-- Patch Links --
https://github.com/jclouds/jclouds-labs-aws/pull/13.patch
https://github.com/jclouds/jclouds-labs-aws/pull/13.diff
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-aws/pull/13
Re: [jclouds-labs-aws] JCLOUDS-457: Added deleteArchive and
uploadArchive operations. (#13)
Posted by Andrew Gaul <no...@github.com>.
Do you want to break out some of these cleanups into a separate commit?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-aws/pull/13#issuecomment-46369496
Re: [jclouds-labs-aws] JCLOUDS-457: Added deleteArchive and
uploadArchive operations. (#13)
Posted by Andrew Gaul <no...@github.com>.
> +
> + private static final DescriptionValidator VALIDATOR = new DescriptionValidator();
> +
> + public void testValidate() throws IOException {
> + VALIDATOR.validate("This is a valid description");
> + VALIDATOR.validate("This_is*A#valid@Description");
> + VALIDATOR.validate("This~Is~A~Valid~Description");
> + VALIDATOR.validate("&Valid$Description");
> + VALIDATOR.validate("");
> + VALIDATOR.validate(buildData(1024).asCharSource(UTF_8).read());
> + }
> +
> + @Test(expectedExceptions = IllegalArgumentException.class)
> + public void testIllegalCharacters() {
> + VALIDATOR.validate(Character.toString((char) 31));
> + VALIDATOR.validate(Character.toString((char) 127));
Does this test both calls to `validate` or only the first?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-aws/pull/13/files#r13891739
Re: [jclouds-labs-aws] JCLOUDS-457: Added deleteArchive and
uploadArchive operations. (#13)
Posted by Roman Coedo <no...@github.com>.
> @@ -23,12 +29,13 @@
>
> private static final VaultNameValidator VALIDATOR = new VaultNameValidator();
>
> - public void testValidate() {
> + public void testValidate() throws UnsupportedEncodingException, IOException {
It was related this: new String(buildData(255).read(), UTF_8) but it's solved with your proposed code.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-aws/pull/13/files#r13887740
Re: [jclouds-labs-aws] JCLOUDS-457: Added deleteArchive and
uploadArchive operations. (#13)
Posted by Andrew Gaul <no...@github.com>.
> + * Unless required by applicable law or agreed to in writing, software
> + * 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.glacier.predicates.validators;
> +
> +import org.jclouds.io.Payload;
> +import org.jclouds.predicates.Validator;
> +
> +import com.google.inject.Singleton;
> +
> +/**
> + * Validates the Glacier archive payload being uploaded.
> + *
Extra line needed?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-aws/pull/13/files#r13887657
Re: [jclouds-labs-aws] JCLOUDS-457: Added deleteArchive and
uploadArchive operations. (#13)
Posted by Andrew Gaul <no...@github.com>.
Make the `CharMatcher` a static instance, addressed some minor formatting issues, and pushed to master.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-aws/pull/13#issuecomment-46378640
Re: [jclouds-labs-aws] JCLOUDS-457: Added deleteArchive and
uploadArchive operations. (#13)
Posted by Andrew Gaul <no...@github.com>.
> VALIDATOR.validate("VALID_NAME");
> VALIDATOR.validate("VALID-NAME");
> VALIDATOR.validate("VALID255NAME");
> VALIDATOR.validate("255VALID-NAME");
> VALIDATOR.validate("VALID.NAME");
> + VALIDATOR.validate(new String(buildData(255).read(), UTF_8));
Consider `buildData(255).asCharSource(UTF_8).read()` instead.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-aws/pull/13/files#r13887499
Re: [jclouds-labs-aws] JCLOUDS-457: Added deleteArchive and
uploadArchive operations. (#13)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-labs-aws-pull-requests #30](https://jclouds.ci.cloudbees.com/job/jclouds-labs-aws-pull-requests/30/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-aws/pull/13#issuecomment-46375729
Re: [jclouds-labs-aws] JCLOUDS-457: Added deleteArchive and
uploadArchive operations. (#13)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-labs-aws-pull-requests #28](https://jclouds.ci.cloudbees.com/job/jclouds-labs-aws-pull-requests/28/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-aws/pull/13#issuecomment-46291987
Re: [jclouds-labs-aws] JCLOUDS-457: Added deleteArchive and
uploadArchive operations. (#13)
Posted by Andrew Gaul <no...@github.com>.
> + */
> +package org.jclouds.glacier.predicates.validators;
> +
> +import org.jclouds.io.Payload;
> +import org.jclouds.predicates.Validator;
> +
> +import com.google.inject.Singleton;
> +
> +/**
> + * Validates the Glacier archive payload being uploaded.
> + *
> + */
> +@Singleton
> +public class PayloadValidator extends Validator<Payload> {
> +
> + private static final long MAX_CONTENT_SIZE = 4294967296L; // 4GiB
Can you write this as 1L << 32 for clarity?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-aws/pull/13/files#r13886684
Re: [jclouds-labs-aws] JCLOUDS-457: Added deleteArchive and
uploadArchive operations. (#13)
Posted by Andrew Gaul <no...@github.com>.
> +
> +import org.jclouds.io.ByteSources;
> +import org.jclouds.io.Payload;
> +import org.jclouds.io.payloads.ByteSourcePayload;
> +
> +import com.google.common.io.ByteSource;
> +
> +public class TestUtils {
> + public static final long MiB = 1024 * 1024;
> + public static final long GiB = 1024 * 1024 * 1024;
> + public static final long TiB = 1024 * 1024 * 1024 * 1024;
> +
> + public static Payload buildPayload(long size) {
> + ByteSource data = buildData(size);
> + Payload payload = new ByteSourcePayload(data);
> + payload.getContentMetadata().setContentType("text/plain");
Can you use `MediaType.PLAIN_TEXT_UTF_8` instead?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-aws/pull/13/files#r13887425
Re: [jclouds-labs-aws] JCLOUDS-457: Added deleteArchive and
uploadArchive operations. (#13)
Posted by Roman Coedo <no...@github.com>.
> +
> + private static final DescriptionValidator VALIDATOR = new DescriptionValidator();
> +
> + public void testValidate() throws IOException {
> + VALIDATOR.validate("This is a valid description");
> + VALIDATOR.validate("This_is*A#valid@Description");
> + VALIDATOR.validate("This~Is~A~Valid~Description");
> + VALIDATOR.validate("&Valid$Description");
> + VALIDATOR.validate("");
> + VALIDATOR.validate(buildData(1024).asCharSource(UTF_8).read());
> + }
> +
> + @Test(expectedExceptions = IllegalArgumentException.class)
> + public void testIllegalCharacters() {
> + VALIDATOR.validate(Character.toString((char) 31));
> + VALIDATOR.validate(Character.toString((char) 127));
These should be in two separated methods.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-aws/pull/13/files#r13891824
Re: [jclouds-labs-aws] JCLOUDS-457: Added deleteArchive and
uploadArchive operations. (#13)
Posted by Andrew Gaul <no...@github.com>.
> + "/111122223333/vaults/examplevault/archives/NkbByEejwEggmBz2fTHgJrg0XBoDfjP4q6iu87-TjhqG6eGoOY9Z8i1_AUyUsuhPAdTqLHy8pTl5nfCFJmDl2yEZONi5L26Omw12vcs01MNGntHEQL8MBfGlqrEXAMPLEArchiveId");
> + mr.addHeader("x-amz-archive-id", responseId);
> + MockWebServer server = new MockWebServer();
> + server.enqueue(mr);
> + server.play();
> +
> + // Send the request and check the response
> + try {
> + GlacierClient client = getGlacierClient(server.getUrl("/"));
> + String id = client.uploadArchive("examplevault", buildPayload(10), "test description");
> + RecordedRequest request = server.takeRequest();
> + assertEquals(id, responseId);
> + assertEquals(request.getRequestLine(), "POST /-/vaults/examplevault/archives HTTP/1.1");
> + assertEquals(request.getHeader("x-amz-archive-description"), "test description");
> + assertNotNull(request.getHeaders("x-amz-sha256-tree-hash"));
> + assertNotNull(request.getHeaders("x-amz-content-sha256"));
Can you factor out some of these String literals into constants?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-aws/pull/13/files#r13887692
Re: [jclouds-labs-aws] JCLOUDS-457: Added deleteArchive and
uploadArchive operations. (#13)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds-labs-aws #972](https://buildhive.cloudbees.com/job/jclouds/job/jclouds-labs-aws/972/) 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-aws/pull/13#issuecomment-46378287
Re: [jclouds-labs-aws] JCLOUDS-457: Added deleteArchive and
uploadArchive operations. (#13)
Posted by Andrew Gaul <no...@github.com>.
> @@ -23,12 +29,13 @@
>
> private static final VaultNameValidator VALIDATOR = new VaultNameValidator();
>
> - public void testValidate() {
> + public void testValidate() throws UnsupportedEncodingException, IOException {
How do we throw `UnsupportedEncodingException`?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-aws/pull/13/files#r13887518
Re: [jclouds-labs-aws] JCLOUDS-457: Added deleteArchive and
uploadArchive operations. (#13)
Posted by Roman Coedo <no...@github.com>.
Since we need the Job operations to make the live tests, they will come in a later commit,
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-aws/pull/13#issuecomment-46377362
Re: [jclouds-labs-aws] JCLOUDS-457: Added deleteArchive and
uploadArchive operations. (#13)
Posted by Roman Coedo <no...@github.com>.
I separated the cleanups, these will come in a new PR. This PR adds only the archive operations now.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-aws/pull/13#issuecomment-46375553
Re: [jclouds-labs-aws] JCLOUDS-457: Added deleteArchive and
uploadArchive operations. (#13)
Posted by Andrew Gaul <no...@github.com>.
> +public class DescriptionValidator extends Validator<String> {
> +
> + private static final int MAX_DESC_LENGTH = 1024;
> +
> + @Override
> + public void validate(String description) {
> + if (isNullOrEmpty(description))
> + return;
> + if (description.length() > MAX_DESC_LENGTH)
> + throw exception("Description can't be longer than " + MAX_DESC_LENGTH + " characters" + " but was " + description.length());
> + CharMatcher range = getDescriptionAcceptableRange();
> + if (!range.matchesAllOf(description))
> + throw exception("Description should have ASCII values between 32 and 126.");
> + }
> +
> + private static CharMatcher getDescriptionAcceptableRange() {
Can you make the `CharMatcher` a static final variable?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-aws/pull/13/files#r13886605
Re: [jclouds-labs-aws] JCLOUDS-457: Added deleteArchive and
uploadArchive operations. (#13)
Posted by Andrew Gaul <no...@github.com>.
> + * limitations under the License.
> + */
> +package org.jclouds.glacier.util;
> +
> +import java.util.Arrays;
> +
> +import org.jclouds.io.ByteSources;
> +import org.jclouds.io.Payload;
> +import org.jclouds.io.payloads.ByteSourcePayload;
> +
> +import com.google.common.io.ByteSource;
> +
> +public class TestUtils {
> + public static final long MiB = 1024 * 1024;
> + public static final long GiB = 1024 * 1024 * 1024;
> + public static final long TiB = 1024 * 1024 * 1024 * 1024;
This overflows since literals default to ints. Consider `1L << 40` instead.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-aws/pull/13/files#r13886925
Re: [jclouds-labs-aws] JCLOUDS-457: Added deleteArchive and
uploadArchive operations. (#13)
Posted by Andrew Gaul <no...@github.com>.
Closed #13.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-aws/pull/13#event-132432756
Re: [jclouds-labs-aws] JCLOUDS-457: Added deleteArchive and
uploadArchive operations. (#13)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds-labs-aws #967](https://buildhive.cloudbees.com/job/jclouds/job/jclouds-labs-aws/967/) 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-aws/pull/13#issuecomment-46292019
Re: [jclouds-labs-aws] JCLOUDS-457: Added deleteArchive and
uploadArchive operations. (#13)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-labs-aws-pull-requests #31](https://jclouds.ci.cloudbees.com/job/jclouds-labs-aws-pull-requests/31/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-aws/pull/13#issuecomment-46378057
Re: [jclouds-labs-aws] JCLOUDS-457: Added deleteArchive and
uploadArchive operations. (#13)
Posted by Andrew Gaul <no...@github.com>.
> + *
> + * @see GlacierClient#uploadArchive
> + */
> + String uploadArchive(String vaultName, Payload payload);
> +
> + /**
> + * Deletes an archive from a vault.
> + *
> + * @param vaultName
> + * Name of the Vault where the archive is stored.
> + * @param archiveId
> + * Amazon Glacier archive identifier.
> + * @return False if the archive was not deleted, true otherwise.
> + * @see <a href="http://docs.aws.amazon.com/amazonglacier/latest/dev/api-archive-delete.html" />
> + */
> + Boolean deleteArchive(String vaultName, String archiveId);
Should this return a primitive boolean?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-aws/pull/13/files#r13887604