You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@jclouds.apache.org by Jeremy Daggett <no...@github.com> on 2014/01/07 22:37:43 UTC
[jclouds-labs-openstack] Adds support for Swift Object copy (#73)
https://issues.apache.org/jira/browse/JCLOUDS-299
To run live tests against Rackspace:
mvn -Plive -Dtest.openstack-swift.identity=[username] -Dtest.test.openstack-swift.credential=[password] clean install
You can merge this Pull Request by running:
git pull https://github.com/rackerlabs/jclouds-labs-openstack copy-object
Or you can view, comment on it, or merge it online at:
https://github.com/jclouds/jclouds-labs-openstack/pull/73
-- Commit Summary --
* Removed deprecated accessRackspace refs, added access.json resource, and updated mock tests
* JCLOUDS-299: Added copy method to Object API
-- File Changes --
A openstack-swift/src/main/java/org/jclouds/openstack/swift/v1/CopyObjectException.java (54)
M openstack-swift/src/main/java/org/jclouds/openstack/swift/v1/features/ObjectApi.java (53)
M openstack-swift/src/main/java/org/jclouds/openstack/swift/v1/handlers/SwiftErrorHandler.java (17)
M openstack-swift/src/main/java/org/jclouds/openstack/swift/v1/options/CreateContainerOptions.java (1)
A openstack-swift/src/main/java/org/jclouds/openstack/swift/v1/reference/SwiftHeaders.java (51)
M openstack-swift/src/test/java/org/jclouds/openstack/swift/v1/AuthenticationMockTest.java (4)
M openstack-swift/src/test/java/org/jclouds/openstack/swift/v1/TemporaryUrlSignerLiveTest.java (2)
M openstack-swift/src/test/java/org/jclouds/openstack/swift/v1/TemporaryUrlSignerMockTest.java (8)
M openstack-swift/src/test/java/org/jclouds/openstack/swift/v1/features/AccountApiLiveTest.java (6)
M openstack-swift/src/test/java/org/jclouds/openstack/swift/v1/features/AccountApiMockTest.java (22)
M openstack-swift/src/test/java/org/jclouds/openstack/swift/v1/features/BulkApiLiveTest.java (12)
M openstack-swift/src/test/java/org/jclouds/openstack/swift/v1/features/BulkApiMockTest.java (4)
M openstack-swift/src/test/java/org/jclouds/openstack/swift/v1/features/ContainerApiLiveTest.java (16)
M openstack-swift/src/test/java/org/jclouds/openstack/swift/v1/features/ContainerApiMockTest.java (52)
M openstack-swift/src/test/java/org/jclouds/openstack/swift/v1/features/ObjectApiLiveTest.java (90)
M openstack-swift/src/test/java/org/jclouds/openstack/swift/v1/features/ObjectApiMockTest.java (110)
M openstack-swift/src/test/java/org/jclouds/openstack/swift/v1/features/StaticLargeObjectApiLiveTest.java (12)
M openstack-swift/src/test/java/org/jclouds/openstack/swift/v1/features/StaticLargeObjectApiMockTest.java (12)
M openstack-swift/src/test/java/org/jclouds/openstack/swift/v1/internal/BaseSwiftApiLiveTest.java (17)
A openstack-swift/src/test/resources/access.json (249)
-- Patch Links --
https://github.com/jclouds/jclouds-labs-openstack/pull/73.patch
https://github.com/jclouds/jclouds-labs-openstack/pull/73.diff
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/73
Re: [jclouds-labs-openstack] Adds support for Swift Object copy (#73)
Posted by Jeremy Daggett <no...@github.com>.
> *
> * @see <a
> * href="http://docs.openstack.org/api/openstack-object-storage/1.0/content/Update_Container_Metadata-d1e1900.html">
> * Create or Update Object Metadata API</a>
> *
> - * @return <code>true</code> if the Object Metadata was successfully created
> + * @return {@link true} if the metadata was successfully created
Missed that one, thx!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/73/files#r8740375
Re: [jclouds-labs-openstack] Adds support for Swift Object copy (#73)
Posted by Andrew Phillips <no...@github.com>.
> + private String destinationPath;
> +
> + public CopyObjectException() {
> + super();
> + }
> +
> + public CopyObjectException(String sourcePath, String destinationPath, String message) {
> + super(String.format("Either the source path %s or the destination path %s was not found. " +
> + "(message: %s)", sourcePath, destinationPath, message));
> + this.sourcePath = sourcePath;
> + this.destinationPath = destinationPath;
> + }
> +
> + public CopyObjectException(Exception from) {
> + super(from);
> + }
What is this needed for?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/73/files#r8709648
Re: [jclouds-labs-openstack] Adds support for Swift Object copy (#73)
Posted by Andrew Phillips <no...@github.com>.
> @@ -49,10 +51,21 @@ public void handleError(HttpCommand command, HttpResponse response) {
> exception = new AuthorizationException(exception.getMessage(), exception);
> break;
> case 404:
> - if (!command.getCurrentRequest().getMethod().equals("DELETE")) {
> + String sourcePath = command.getCurrentRequest().getFirstHeaderOrNull(SwiftHeaders.OBJECT_COPY_FROM);
> + Exception oldException = exception;
> +
> + if (sourcePath != null) {
> + String path = command.getCurrentRequest().getEndpoint().getPath();
> + int startOfDestinationPath = path.lastIndexOf("/", path.lastIndexOf("/") - 1);
> + String destinationPath = path.substring(startOfDestinationPath);
> +
> + exception = new CopyObjectException(sourcePath, destinationPath, message);
> + exception.initCause(oldException);
> + }
> + else if (!command.getCurrentRequest().getMethod().equals("DELETE")) {
[minor] `}` and `else if` on the same line?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/73/files#r8709877
Re: [jclouds-labs-openstack] Adds support for Swift Object copy (#73)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-labs-openstack-pull-requests #141](https://jclouds.ci.cloudbees.com/job/jclouds-labs-openstack-pull-requests/141/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/73#issuecomment-31783282
Re: [jclouds-labs-openstack] Adds support for Swift Object copy (#73)
Posted by Andrew Phillips <no...@github.com>.
> @@ -140,18 +146,18 @@ String replace(@PathParam("objectName") String objectName, @BinderParam(SetPaylo
> SwiftObject get(@PathParam("objectName") String objectName, GetOptions options);
>
> /**
> - * Creates or updates the Object metadata.
> + * Create or update the metadata for a {@link SwiftObject}.
"Creates or updates"
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/73/files#r8709779
Re: [jclouds-labs-openstack] Adds support for Swift Object copy (#73)
Posted by Andrew Phillips <no...@github.com>.
Zack's comments aside, looks good to me too. Thanks, @jdaggett!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/73#issuecomment-32530108
Re: [jclouds-labs-openstack] Adds support for Swift Object copy (#73)
Posted by Zack Shoylev <no...@github.com>.
> @@ -163,11 +163,11 @@ public void alreadyCreated() throws Exception {
> /** upper-cases first char, and lower-cases rest!! **/
> public void getKnowingServerMessesWithMetadataKeyCaseFormat() throws Exception {
> MockWebServer server = mockOpenStackServer();
> - server.enqueue(new MockResponse().setBody(accessRackspace));
> - server.enqueue(containerResponse() //
> + server.enqueue(addCommonHeaders(new MockResponse().setBody(stringFromResource("/access.json"))));
> + server.enqueue(addCommonHeaders(containerResponse() //
I have to mostly agree with what @everett-toews said above.
I would also suggest the following: 1) Remove IDE control metacode and 2) the Eclipse source-control formatter for jclouds should live in github (or, second option, the wiki) so it can be edited collaboratively as needed. I am specifically thinking Github, as changes to something like that usually come with ... discussions.
Now the reason I think this is important is that (and correct me if I am wrong) having it easily available and accessible will reduce barriers for new jclouds developers.
As for the current location of the formatter... I will have to check my old email.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/73/files#r8951854
Re: [jclouds-labs-openstack] Adds support for Swift Object copy (#73)
Posted by Andrew Phillips <no...@github.com>.
> @@ -163,11 +163,11 @@ public void alreadyCreated() throws Exception {
> /** upper-cases first char, and lower-cases rest!! **/
> public void getKnowingServerMessesWithMetadataKeyCaseFormat() throws Exception {
> MockWebServer server = mockOpenStackServer();
> - server.enqueue(new MockResponse().setBody(accessRackspace));
> - server.enqueue(containerResponse() //
> + server.enqueue(addCommonHeaders(new MockResponse().setBody(stringFromResource("/access.json"))));
> + server.enqueue(addCommonHeaders(containerResponse() //
> Wiki?
That sounds like the right place, off the top of my head...
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/73/files#r8948417
Re: [jclouds-labs-openstack] Adds support for Swift Object copy (#73)
Posted by Zack Shoylev <no...@github.com>.
> @@ -35,8 +35,8 @@ public void whenAccountApiIsNull() {
>
> public void whenAccountApiHasKey() throws Exception {
> MockWebServer server = mockOpenStackServer();
> - server.enqueue(new MockResponse().setBody(accessRackspace));
> - server.enqueue(accountResponse().addHeader("X-Account-Meta-Temp-URL-Key", "mykey"));
> + server.enqueue(addCommonHeaders(new MockResponse().setBody(stringFromResource("/access.json"))));
> + server.enqueue(addCommonHeaders(accountResponse().addHeader("X-Account-Meta-Temp-URL-Key", "mykey")));
Shouldn't this header be defined?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/73/files#r8942068
Re: [jclouds-labs-openstack] Adds support for Swift Object copy (#73)
Posted by Andrew Phillips <no...@github.com>.
> +
> +import org.jclouds.rest.ResourceNotFoundException;
> +
> +/**
> + * Thrown when an object cannot be copied.
> + *
> + * @author Everett Toews
> + */
> +public class CopyObjectException extends ResourceNotFoundException {
> +
> + private String sourcePath;
> + private String destinationPath;
> +
> + public CopyObjectException() {
> + super();
> + }
I guess this one is needed..?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/73/files#r8709553
Re: [jclouds-labs-openstack] Adds support for Swift Object copy (#73)
Posted by Andrew Phillips <no...@github.com>.
> + assertTrue(api.objectApiInRegionForContainer("DFW", destinationContainer)
> + .copy(destinationObject, sourceContainer, sourceObject));
> +
> + assertEquals(server.getRequestCount(), 2);
> + assertEquals(server.takeRequest().getRequestLine(), "POST /tokens HTTP/1.1");
> +
> + RecordedRequest copyRequest = server.takeRequest();
> + assertEquals(copyRequest.getRequestLine(),
> + "PUT /v1/MossoCloudFS_5bcf396e-39dd-45ff-93a1-712b9aba90a9/foo/bar.txt HTTP/1.1");
> + } finally {
> + server.shutdown();
> + }
> + }
> +
> + @Test(expectedExceptions = CopyObjectException.class)
> + public void copyFail() throws InterruptedException, IOException {
`testCopyFail()` or `testFailedCopy()` or so?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/73/files#r8710245
Re: [jclouds-labs-openstack] Adds support for Swift Object copy (#73)
Posted by Andrew Phillips <no...@github.com>.
> + // Create the destination object
> + assertNotNull(destApi.replace(destinationObject, data, ImmutableMap.<String, String> of()));
> + object = destApi.get(destinationObject, GetOptions.NONE);
> + checkObject(destApi.get(destinationObject, GetOptions.NONE));
> +
> + // check the copy operation
> + assertTrue(destApi.copy(destinationObject, sourceContainer, sourceObject));
> + assertNotNull(destApi.head(destinationObject));
> +
> + // now get a real SwiftObject
> + SwiftObject destSwiftObject = destApi.get(destinationObject, GetOptions.NONE);
> + assertEquals(Strings2.toString(destSwiftObject.payload()), stringPayload);
> +
> + // test exception thrown on bad source name
> + try {
> + assertFalse(destApi.copy(destinationObject, badSource, sourceObject));
Why an assert if the call is expected to throw an exception anyway?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/73/files#r8710166
Re: [jclouds-labs-openstack] Adds support for Swift Object copy (#73)
Posted by Andrew Phillips <no...@github.com>.
> @@ -49,10 +51,21 @@ public void handleError(HttpCommand command, HttpResponse response) {
> exception = new AuthorizationException(exception.getMessage(), exception);
> break;
> case 404:
> - if (!command.getCurrentRequest().getMethod().equals("DELETE")) {
> + String sourcePath = command.getCurrentRequest().getFirstHeaderOrNull(SwiftHeaders.OBJECT_COPY_FROM);
> + Exception oldException = exception;
> +
> + if (sourcePath != null) {
> + String path = command.getCurrentRequest().getEndpoint().getPath();
> + int startOfDestinationPath = path.lastIndexOf("/", path.lastIndexOf("/") - 1);
> + String destinationPath = path.substring(startOfDestinationPath);
This is getting the substring after the _second_-last `/` in the path, if I'm correct? Perhaps add a comment to that effect? Or a short comment to describe the expected syntax of the path here, i.e. why it makes sense to get everything from the second-last '/' onward?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/73/files#r8709859
Re: [jclouds-labs-openstack] Adds support for Swift Object copy (#73)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-labs-openstack-pull-requests #147](https://jclouds.ci.cloudbees.com/job/jclouds-labs-openstack-pull-requests/147/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/73#issuecomment-32103580
Re: [jclouds-labs-openstack] Adds support for Swift Object copy (#73)
Posted by Andrew Phillips <no...@github.com>.
> @@ -297,7 +301,63 @@ public void alreadyDeleted() throws Exception {
> server.shutdown();
> }
> }
> +
> + public void copy() throws Exception {
`testCopy()`? See above comment
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/73/files#r8710216
Re: [jclouds-labs-openstack] Adds support for Swift Object copy (#73)
Posted by Zack Shoylev <no...@github.com>.
> @@ -163,11 +163,11 @@ public void alreadyCreated() throws Exception {
> /** upper-cases first char, and lower-cases rest!! **/
> public void getKnowingServerMessesWithMetadataKeyCaseFormat() throws Exception {
> MockWebServer server = mockOpenStackServer();
> - server.enqueue(new MockResponse().setBody(accessRackspace));
> - server.enqueue(containerResponse() //
> + server.enqueue(addCommonHeaders(new MockResponse().setBody(stringFromResource("/access.json"))));
> + server.enqueue(addCommonHeaders(containerResponse() //
// ?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/73/files#r8942205
Re: [jclouds-labs-openstack] Adds support for Swift Object copy (#73)
Posted by Andrew Phillips <no...@github.com>.
Thanks for the update, @jdaggett! Skimmed through it a bit more quickly this time. One minor comment, otherwise looks good to me!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/73#issuecomment-32001727
Re: [jclouds-labs-openstack] Adds support for Swift Object copy (#73)
Posted by Andrew Phillips <no...@github.com>.
> private String name = getClass().getSimpleName();
> private String containerName = getClass().getSimpleName() + "Container";
> +
> + public void copy() throws Exception {
I personally am not a fan of `testX` names for tests, but that seems to be what we mainly use in jclouds. So `testCopy()`?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/73/files#r8710139
Re: [jclouds-labs-openstack] Adds support for Swift Object copy (#73)
Posted by Jeremy Daggett <no...@github.com>.
> @@ -49,10 +51,21 @@ public void handleError(HttpCommand command, HttpResponse response) {
> exception = new AuthorizationException(exception.getMessage(), exception);
> break;
> case 404:
> - if (!command.getCurrentRequest().getMethod().equals("DELETE")) {
> + String sourcePath = command.getCurrentRequest().getFirstHeaderOrNull(SwiftHeaders.OBJECT_COPY_FROM);
> + Exception oldException = exception;
> +
> + if (sourcePath != null) {
> + String path = command.getCurrentRequest().getEndpoint().getPath();
> + int startOfDestinationPath = path.lastIndexOf("/", path.lastIndexOf("/") - 1);
> + String destinationPath = path.substring(startOfDestinationPath);
I have added a quick comment about the syntax
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/73/files#r8771881
Re: [jclouds-labs-openstack] Adds support for Swift Object copy (#73)
Posted by Andrew Phillips <no...@github.com>.
> @@ -49,10 +51,21 @@ public void handleError(HttpCommand command, HttpResponse response) {
> exception = new AuthorizationException(exception.getMessage(), exception);
> break;
> case 404:
> - if (!command.getCurrentRequest().getMethod().equals("DELETE")) {
> + String sourcePath = command.getCurrentRequest().getFirstHeaderOrNull(SwiftHeaders.OBJECT_COPY_FROM);
> + Exception oldException = exception;
[minor] Reverse the order of these two lines? The "exception backup" is requires for multiple of the 'if...else' branches, but the "get source path" statement applies only to the first 'if..." branch?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/73/files#r8710001
Re: [jclouds-labs-openstack] Adds support for Swift Object copy (#73)
Posted by Andrew Phillips <no...@github.com>.
> private String name = getClass().getSimpleName();
> private String containerName = getClass().getSimpleName() + "Container";
> +
> + public void copy() throws Exception {
> + for (String regionId : regions) {
> + // source
> + String sourceContainer = "src" + containerName;
> + String sourceObject = "original.txt";
> + String badSource = "badSource";
> +
> + // destination
> + String destinationContainer = "dest"+ containerName;
[minor] Space after `"dest"`?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/73/files#r8710116
Re: [jclouds-labs-openstack] Adds support for Swift Object copy (#73)
Posted by Andrew Phillips <no...@github.com>.
> @@ -163,11 +163,11 @@ public void alreadyCreated() throws Exception {
> /** upper-cases first char, and lower-cases rest!! **/
> public void getKnowingServerMessesWithMetadataKeyCaseFormat() throws Exception {
> MockWebServer server = mockOpenStackServer();
> - server.enqueue(new MockResponse().setBody(accessRackspace));
> - server.enqueue(containerResponse() //
> + server.enqueue(addCommonHeaders(new MockResponse().setBody(stringFromResource("/access.json"))));
> + server.enqueue(addCommonHeaders(containerResponse() //
> I am specifically thinking Github, as changes to something like that usually come with ... discussions.
A document in the wiki should be able to provide the same editing and discussion options? I'm not necessarily saying the wiki is better, but since that's where we plan to keep other development guidelines..?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/73/files#r8957591
Re: [jclouds-labs-openstack] Adds support for Swift Object copy (#73)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-labs-openstack-pull-requests #146](https://jclouds.ci.cloudbees.com/job/jclouds-labs-openstack-pull-requests/146/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/73#issuecomment-31987673
Re: [jclouds-labs-openstack] Adds support for Swift Object copy (#73)
Posted by Andrew Phillips <no...@github.com>.
Thanks, @jdaggett! Main comment is 3rd vs. 2nd person usage in Javadocs, I guess - otherwise just a couple of small things.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/73#issuecomment-31789253
Re: [jclouds-labs-openstack] Adds support for Swift Object copy (#73)
Posted by Jeremy Daggett <no...@github.com>.
> +
> +import org.jclouds.rest.ResourceNotFoundException;
> +
> +/**
> + * Thrown when an object cannot be copied.
> + *
> + * @author Everett Toews
> + */
> +public class CopyObjectException extends ResourceNotFoundException {
> +
> + private String sourcePath;
> + private String destinationPath;
> +
> + public CopyObjectException() {
> + super();
> + }
This file was copied directly from the current [Swift API][1]:
[1]: https://github.com/jclouds/jclouds/blob/master/blobstore/src/main/java/org/jclouds/blobstore/ContainerNotFoundException.java
We can definitely nuke it!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/73/files#r8740254
Re: [jclouds-labs-openstack] Adds support for Swift Object copy (#73)
Posted by Jeremy Daggett <no...@github.com>.
> + assertTrue(api.objectApiInRegionForContainer("DFW", destinationContainer)
> + .copy(destinationObject, sourceContainer, sourceObject));
> +
> + assertEquals(server.getRequestCount(), 2);
> + assertEquals(server.takeRequest().getRequestLine(), "POST /tokens HTTP/1.1");
> +
> + RecordedRequest copyRequest = server.takeRequest();
> + assertEquals(copyRequest.getRequestLine(),
> + "PUT /v1/MossoCloudFS_5bcf396e-39dd-45ff-93a1-712b9aba90a9/foo/bar.txt HTTP/1.1");
> + } finally {
> + server.shutdown();
> + }
> + }
> +
> + @Test(expectedExceptions = CopyObjectException.class)
> + public void copyFail() throws InterruptedException, IOException {
copyObjectFail()
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/73/files#r8771900
Re: [jclouds-labs-openstack] Adds support for Swift Object copy (#73)
Posted by BuildHive <no...@github.com>.
[jclouds ยป jclouds-labs-openstack #765](https://buildhive.cloudbees.com/job/jclouds/job/jclouds-labs-openstack/765/) 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-openstack/pull/73#issuecomment-32103551
Re: [jclouds-labs-openstack] Adds support for Swift Object copy (#73)
Posted by Zack Shoylev <no...@github.com>.
I had to merge this properly just now. I think I messed up when pushing.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/73#issuecomment-32642040
Re: [jclouds-labs-openstack] Adds support for Swift Object copy (#73)
Posted by Andrew Phillips <no...@github.com>.
> @@ -162,15 +168,14 @@ boolean updateMetadata(@PathParam("objectName") String objectName,
> @BinderParam(BindObjectMetadataToHeaders.class) Map<String, String> metadata);
>
> /**
> - * Deletes Object metadata.
> + * Delete the metadata from a {@link SwiftObject}.
"Deletes" (see Javadoc style comment below)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/73/files#r8709772
Re: [jclouds-labs-openstack] Adds support for Swift Object copy (#73)
Posted by Jeremy Daggett <no...@github.com>.
> + server.enqueue(addCommonHeaders(new MockResponse().setBody(stringFromResource("/access.json"))));
> + server.enqueue(addCommonHeaders(new MockResponse().setResponseCode(404)
> + .addHeader(SwiftHeaders.OBJECT_COPY_FROM, "bar")));
> +
> + try {
> + SwiftApi api = api(server.getUrl("/").toString(), "openstack-swift");
> + api.objectApiInRegionForContainer("DFW", "foo")
> + .copy("bar.txt", "bar", "foo.txt");
> +
> + assertEquals(server.getRequestCount(), 2);
> + assertEquals(server.takeRequest().getRequestLine(), "POST /tokens HTTP/1.1");
> +
> + RecordedRequest copyRequest = server.takeRequest();
> + assertEquals(copyRequest.getRequestLine(),
> + "PUT /v1/MossoCloudFS_5bcf396e-39dd-45ff-93a1-712b9aba90a9/foo/bar.txt HTTP/1.1 ");
> + fail();
@demobox Learning more about how these MockWebServer test handle exceptions. What I found is that when you set the response code to 404 (line 330), that indeed invokes the SwiftErrorHandler. We really only need lines 334-336 here. WDYT?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/73/files#r8797448
Re: [jclouds-labs-openstack] Adds support for Swift Object copy (#73)
Posted by Jeremy Daggett <no...@github.com>.
> @@ -163,11 +163,11 @@ public void alreadyCreated() throws Exception {
> /** upper-cases first char, and lower-cases rest!! **/
> public void getKnowingServerMessesWithMetadataKeyCaseFormat() throws Exception {
> MockWebServer server = mockOpenStackServer();
> - server.enqueue(new MockResponse().setBody(accessRackspace));
> - server.enqueue(containerResponse() //
> + server.enqueue(addCommonHeaders(new MockResponse().setBody(stringFromResource("/access.json"))));
> + server.enqueue(addCommonHeaders(containerResponse() //
@demobox @everett-toews I know that someone had put together an Eclipse formatter config a while back to. Do you know where that is by chance? I have no problem removing these trailing double-slashes when I find them in the code going forward. Where do you suggest starting a best practices/coding conventions document? Wiki?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/73/files#r8948355
Re: [jclouds-labs-openstack] Adds support for Swift Object copy (#73)
Posted by Jeremy Daggett <no...@github.com>.
> private String name = getClass().getSimpleName();
> private String containerName = getClass().getSimpleName() + "Container";
> +
> + public void copy() throws Exception {
Changed it to "copyObject()" since that is more descriptive.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/73/files#r8771892
Re: [jclouds-labs-openstack] Adds support for Swift Object copy (#73)
Posted by Andrew Phillips <no...@github.com>.
> public BaseSwiftApiLiveTest() {
> provider = "openstack-swift";
> }
>
> + @BeforeClass
> + public void setupRegions() {
> + String key = "test." + provider + ".region";
> +
> + if (System.getProperties().containsKey(key)) {
> + regions.add(System.getProperty(key));
> + }
> + else {
> + regions = api.configuredRegions();
> + }
```
String providedRegion = System.getProperty("test." + provider + ".region");
if (providedRegion != null) {
regions = ImmutableSet.of(providedRegion);
} else {
regions = api.configuredRegions();
}
```
?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/73/files#r8710403
Re: [jclouds-labs-openstack] Adds support for Swift Object copy (#73)
Posted by Andrew Phillips <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.openstack.swift.v1.reference;
> +
> +/**
> + * Common headers in Swift.
> + *
> + * @author Jeremy Daggett
> + */
> +public interface SwiftHeaders {
> +
> + public static final String USER_METADATA_PREFIX = "X-Object-Meta-";
`public static final` not needed for all of this since fields in an interface are implicitly declared so? Cf. http://stackoverflow.com/questions/17592872/is-public-static-final-redunant-for-constant-from-java-interface
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/73/files#r8710068
Re: [jclouds-labs-openstack] Adds support for Swift Object copy (#73)
Posted by Andrew Phillips <no...@github.com>.
> + assertEquals(server.takeRequest().getRequestLine(), "POST /tokens HTTP/1.1");
> +
> + RecordedRequest copyRequest = server.takeRequest();
> + assertEquals(copyRequest.getRequestLine(),
> + "PUT /v1/MossoCloudFS_5bcf396e-39dd-45ff-93a1-712b9aba90a9/foo/bar.txt HTTP/1.1");
> + } finally {
> + server.shutdown();
> + }
> + }
> +
> + @Test(expectedExceptions = CopyObjectException.class)
> + public void copyFail() throws InterruptedException, IOException {
> + String sourceContainer = "bar";
> + String sourceObject = "foo.txt";
> + String destinationContainer = "foo";
> + String destinationObject = "bar.txt";
Inline these, since they're only used once?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/73/files#r8710261
Re: [jclouds-labs-openstack] Adds support for Swift Object copy (#73)
Posted by Andrew Phillips <no...@github.com>.
> @@ -184,7 +189,7 @@ boolean deleteMetadata(@PathParam("objectName") String objectName,
> @BinderParam(BindRemoveObjectMetadataToHeaders.class) Map<String, String> metadata);
>
> /**
> - * Deletes a object, if present.
> + * Delete an object, if present.
Use 3rd person rather than 2nd person style, as per the [Javadoc style guide](http://www.oracle.com/technetwork/java/javase/documentation/index-137868.html)?
"Use 3rd person (descriptive) not 2nd person (prescriptive).
The description is in 3rd person declarative rather than 2nd person imperative.
* Gets the label. (preferred)
* Get the label. (avoid)"
I.e. revert this change?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/73/files#r8709729
Re: [jclouds-labs-openstack] Adds support for Swift Object copy (#73)
Posted by Jeremy Daggett <no...@github.com>.
Thank you @demobox ! I addressed all of the small things, and will push shortly
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/73#issuecomment-31978997
Re: [jclouds-labs-openstack] Adds support for Swift Object copy (#73)
Posted by Zack Shoylev <no...@github.com>.
Looks good except for (minor) some header usage. +1
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/73#issuecomment-32504706
Re: [jclouds-labs-openstack] Adds support for Swift Object copy (#73)
Posted by Jeremy Daggett <no...@github.com>.
> public BaseSwiftApiLiveTest() {
> provider = "openstack-swift";
> }
>
> + @BeforeClass
> + public void setupRegions() {
> + String key = "test." + provider + ".region";
> +
> + if (System.getProperties().containsKey(key)) {
> + regions.add(System.getProperty(key));
> + }
> + else {
> + regions = api.configuredRegions();
> + }
I nuked the setupRegions() method and moved the logic to the setup() method. @everett-toews might benefit from this in the MarconiApi.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/73/files#r8771925
Re: [jclouds-labs-openstack] Adds support for Swift Object copy (#73)
Posted by Jeremy Daggett <no...@github.com>.
> @@ -35,8 +35,8 @@ public void whenAccountApiIsNull() {
>
> public void whenAccountApiHasKey() throws Exception {
> MockWebServer server = mockOpenStackServer();
> - server.enqueue(new MockResponse().setBody(accessRackspace));
> - server.enqueue(accountResponse().addHeader("X-Account-Meta-Temp-URL-Key", "mykey"));
> + server.enqueue(addCommonHeaders(new MockResponse().setBody(stringFromResource("/access.json"))));
> + server.enqueue(addCommonHeaders(accountResponse().addHeader("X-Account-Meta-Temp-URL-Key", "mykey")));
I can address this in a separate PR since that header needs to be updated else where. Cool?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/73/files#r8943214
Re: [jclouds-labs-openstack] Adds support for Swift Object copy (#73)
Posted by Jeremy Daggett <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.openstack.swift.v1.reference;
> +
> +/**
> + * Common headers in Swift.
> + *
> + * @author Jeremy Daggett
> + */
> +public interface SwiftHeaders {
> +
> + public static final String USER_METADATA_PREFIX = "X-Object-Meta-";
Thanks, old habits die hard. ;)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/73/files#r8771886
Re: [jclouds-labs-openstack] Adds support for Swift Object copy (#73)
Posted by Andrew Phillips <no...@github.com>.
> I had to merge this properly just now. I think I messed up when pushing.
All done, though? Or still work to come?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/73#issuecomment-32669617
Re: [jclouds-labs-openstack] Adds support for Swift Object copy (#73)
Posted by Andrew Phillips <no...@github.com>.
> + checkObject(destApi.get(destinationObject, GetOptions.NONE));
> +
> + // check the copy operation
> + assertTrue(destApi.copy(destinationObject, sourceContainer, sourceObject));
> + assertNotNull(destApi.head(destinationObject));
> +
> + // now get a real SwiftObject
> + SwiftObject destSwiftObject = destApi.get(destinationObject, GetOptions.NONE);
> + assertEquals(Strings2.toString(destSwiftObject.payload()), stringPayload);
> +
> + // test exception thrown on bad source name
> + try {
> + assertFalse(destApi.copy(destinationObject, badSource, sourceObject));
> + fail("Expected CopyObjectException");
> + }
> + catch (CopyObjectException e) {
[minor] `} catch` on one line?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/73/files#r8710189
Re: [jclouds-labs-openstack] Adds support for Swift Object copy (#73)
Posted by Andrew Phillips <no...@github.com>.
> +/**
> + * Thrown when an object cannot be copied.
> + *
> + * @author Everett Toews
> + */
> +public class CopyObjectException extends ResourceNotFoundException {
> +
> + private String sourcePath;
> + private String destinationPath;
> +
> + public CopyObjectException() {
> + super();
> + }
> +
> + public CopyObjectException(String sourcePath, String destinationPath, String message) {
> + super(String.format("Either the source path %s or the destination path %s was not found. " +
Perhaps change `%s` to `'%s'` or so to make nasty leading or trailing spaces more obvious?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/73/files#r8709634
Re: [jclouds-labs-openstack] Adds support for Swift Object copy (#73)
Posted by Andrew Phillips <no...@github.com>.
>
> public class BaseSwiftApiLiveTest extends BaseApiLiveTest<SwiftApi> {
>
> + protected Set<String> regions = Sets.newHashSet();
Leave uninitialized here and assign below?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/73/files#r8710370
Re: [jclouds-labs-openstack] Adds support for Swift Object copy (#73)
Posted by Andrew Phillips <no...@github.com>.
> *
> * @see <a
> * href="http://docs.openstack.org/api/openstack-object-storage/1.0/content/Update_Container_Metadata-d1e1900.html">
> * Create or Update Object Metadata API</a>
> *
> - * @return <code>true</code> if the Object Metadata was successfully created
> + * @return {@link true} if the metadata was successfully created
`{@code true}`?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/73/files#r8709664
Re: [jclouds-labs-openstack] Adds support for Swift Object copy (#73)
Posted by Andrew Phillips <no...@github.com>.
> + server.enqueue(addCommonHeaders(new MockResponse().setBody(stringFromResource("/access.json"))));
> + server.enqueue(addCommonHeaders(new MockResponse().setResponseCode(404)
> + .addHeader(SwiftHeaders.OBJECT_COPY_FROM, sourceContainer)));
> +
> + try {
> + SwiftApi api = api(server.getUrl("/").toString(), "openstack-swift");
> + api.objectApiInRegionForContainer("DFW", destinationContainer)
> + .copy(destinationObject, sourceContainer, sourceObject);
> +
> + assertEquals(server.getRequestCount(), 2);
> + assertEquals(server.takeRequest().getRequestLine(), "POST /tokens HTTP/1.1");
> +
> + RecordedRequest copyRequest = server.takeRequest();
> + assertEquals(copyRequest.getRequestLine(),
> + "PUT /v1/MossoCloudFS_5bcf396e-39dd-45ff-93a1-712b9aba90a9/foo/bar.txt HTTP/1.1");
> + fail();
Add message to `fail(...)`
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/73/files#r8710267
Re: [jclouds-labs-openstack] Adds support for Swift Object copy (#73)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-labs-openstack-pull-requests #150](https://jclouds.ci.cloudbees.com/job/jclouds-labs-openstack-pull-requests/150/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/73#issuecomment-32426820
Re: [jclouds-labs-openstack] Adds support for Swift Object copy (#73)
Posted by Andrew Phillips <no...@github.com>.
> + server.enqueue(addCommonHeaders(new MockResponse().setBody(stringFromResource("/access.json"))));
> + server.enqueue(addCommonHeaders(new MockResponse().setResponseCode(404)
> + .addHeader(SwiftHeaders.OBJECT_COPY_FROM, "bar")));
> +
> + try {
> + SwiftApi api = api(server.getUrl("/").toString(), "openstack-swift");
> + api.objectApiInRegionForContainer("DFW", "foo")
> + .copy("bar.txt", "bar", "foo.txt");
> +
> + assertEquals(server.getRequestCount(), 2);
> + assertEquals(server.takeRequest().getRequestLine(), "POST /tokens HTTP/1.1");
> +
> + RecordedRequest copyRequest = server.takeRequest();
> + assertEquals(copyRequest.getRequestLine(),
> + "PUT /v1/MossoCloudFS_5bcf396e-39dd-45ff-93a1-712b9aba90a9/foo/bar.txt HTTP/1.1 ");
> + fail();
I we can cut 337-344 I'd say let's do that!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/73/files#r8803687
Re: [jclouds-labs-openstack] Adds support for Swift Object copy (#73)
Posted by Jeremy Daggett <no...@github.com>.
> + private String destinationPath;
> +
> + public CopyObjectException() {
> + super();
> + }
> +
> + public CopyObjectException(String sourcePath, String destinationPath, String message) {
> + super(String.format("Either the source path %s or the destination path %s was not found. " +
> + "(message: %s)", sourcePath, destinationPath, message));
> + this.sourcePath = sourcePath;
> + this.destinationPath = destinationPath;
> + }
> +
> + public CopyObjectException(Exception from) {
> + super(from);
> + }
Leftover from the copied file. We don't need it since this is a very specific exception.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/73/files#r8740309
Re: [jclouds-labs-openstack] Adds support for Swift Object copy (#73)
Posted by Andrew Phillips <no...@github.com>.
> @@ -163,11 +163,11 @@ public void alreadyCreated() throws Exception {
> /** upper-cases first char, and lower-cases rest!! **/
> public void getKnowingServerMessesWithMetadataKeyCaseFormat() throws Exception {
> MockWebServer server = mockOpenStackServer();
> - server.enqueue(new MockResponse().setBody(accessRackspace));
> - server.enqueue(containerResponse() //
> + server.enqueue(addCommonHeaders(new MockResponse().setBody(stringFromResource("/access.json"))));
> + server.enqueue(addCommonHeaders(containerResponse() //
@zack-shoylev: I think these are to prevent reformatting of the lines...at least, that's what I remember the answer being when I asked in a different PR ;-)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/73/files#r8945543
Re: [jclouds-labs-openstack] Adds support for Swift Object copy (#73)
Posted by Everett Toews <no...@github.com>.
> @@ -163,11 +163,11 @@ public void alreadyCreated() throws Exception {
> /** upper-cases first char, and lower-cases rest!! **/
> public void getKnowingServerMessesWithMetadataKeyCaseFormat() throws Exception {
> MockWebServer server = mockOpenStackServer();
> - server.enqueue(new MockResponse().setBody(accessRackspace));
> - server.enqueue(containerResponse() //
> + server.enqueue(addCommonHeaders(new MockResponse().setBody(stringFromResource("/access.json"))));
> + server.enqueue(addCommonHeaders(containerResponse() //
Yuck. It's always nasty when this kind of IDE control code gets into the code base. It's terrible for readability.
Is there a code convention policy in jclouds about it? Do we even have such conventions at all?
If not, I'd say it's up to the author.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/73/files#r8947376
Re: [jclouds-labs-openstack] Adds support for Swift Object copy (#73)
Posted by Andrew Phillips <no...@github.com>.
> @@ -197,4 +202,32 @@ boolean deleteMetadata(@PathParam("objectName") String objectName,
> @Fallback(VoidOnNotFoundOr404.class)
> @Path("/{objectName}")
> void delete(@PathParam("objectName") String objectName);
> +
> + /**
> + * Copy an object from one container to another. Please note that this
"Copies" (see Javadoc style comment above)?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/73/files#r8709748
Re: [jclouds-labs-openstack] Adds support for Swift Object copy (#73)
Posted by Jeremy Daggett <no...@github.com>.
> +/**
> + * Thrown when an object cannot be copied.
> + *
> + * @author Everett Toews
> + */
> +public class CopyObjectException extends ResourceNotFoundException {
> +
> + private String sourcePath;
> + private String destinationPath;
> +
> + public CopyObjectException() {
> + super();
> + }
> +
> + public CopyObjectException(String sourcePath, String destinationPath, String message) {
> + super(String.format("Either the source path %s or the destination path %s was not found. " +
Agreed. That would help for readability.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/73/files#r8740300
Re: [jclouds-labs-openstack] Adds support for Swift Object copy (#73)
Posted by Jeremy Daggett <no...@github.com>.
> @@ -163,11 +163,11 @@ public void alreadyCreated() throws Exception {
> /** upper-cases first char, and lower-cases rest!! **/
> public void getKnowingServerMessesWithMetadataKeyCaseFormat() throws Exception {
> MockWebServer server = mockOpenStackServer();
> - server.enqueue(new MockResponse().setBody(accessRackspace));
> - server.enqueue(containerResponse() //
> + server.enqueue(addCommonHeaders(new MockResponse().setBody(stringFromResource("/access.json"))));
> + server.enqueue(addCommonHeaders(containerResponse() //
Ditto on the last comment. Will fix in another PR.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/73/files#r8943229
Re: [jclouds-labs-openstack] Adds support for Swift Object copy (#73)
Posted by Zack Shoylev <no...@github.com>.
> @@ -163,11 +163,11 @@ public void alreadyCreated() throws Exception {
> /** upper-cases first char, and lower-cases rest!! **/
> public void getKnowingServerMessesWithMetadataKeyCaseFormat() throws Exception {
> MockWebServer server = mockOpenStackServer();
> - server.enqueue(new MockResponse().setBody(accessRackspace));
> - server.enqueue(containerResponse() //
> + server.enqueue(addCommonHeaders(new MockResponse().setBody(stringFromResource("/access.json"))));
> + server.enqueue(addCommonHeaders(containerResponse() //
Good point. Wiki for consistency then :)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/73/files#r8969911
Re: [jclouds-labs-openstack] Adds support for Swift Object copy (#73)
Posted by Andrew Phillips <no...@github.com>.
> + server.enqueue(addCommonHeaders(new MockResponse().setBody(stringFromResource("/access.json"))));
> + server.enqueue(addCommonHeaders(new MockResponse().setResponseCode(404)
> + .addHeader(SwiftHeaders.OBJECT_COPY_FROM, "bar")));
> +
> + try {
> + SwiftApi api = api(server.getUrl("/").toString(), "openstack-swift");
> + api.objectApiInRegionForContainer("DFW", "foo")
> + .copy("bar.txt", "bar", "foo.txt");
> +
> + assertEquals(server.getRequestCount(), 2);
> + assertEquals(server.takeRequest().getRequestLine(), "POST /tokens HTTP/1.1");
> +
> + RecordedRequest copyRequest = server.takeRequest();
> + assertEquals(copyRequest.getRequestLine(),
> + "PUT /v1/MossoCloudFS_5bcf396e-39dd-45ff-93a1-712b9aba90a9/foo/bar.txt HTTP/1.1 ");
> + fail();
Add a message to this `fail()`? Actually, which line do we actually expect to throw the CopyObjectException here?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/73/files#r8779973