You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@jclouds.apache.org by Jeremy Daggett <no...@github.com> on 2014/04/02 20:05:56 UTC
[jclouds-labs-openstack] Added missing methods to Container and
Object APIs (#89)
This PR removes the dependency on the _varargs_ for the **ObjectApi.list()** method.
It also adds the **ContainerApi.head()** method to get just the Container metadata, rather than retrieving the entire object. This method is utilized to determine if a container exists or not.
This will need to be merged to master and the 1.7 branch. Thx!
You can merge this Pull Request by running:
git pull https://github.com/rackerlabs/jclouds-labs-openstack api-updates
Or you can view, comment on it, or merge it online at:
https://github.com/jclouds/jclouds-labs-openstack/pull/89
-- Commit Summary --
* Added explicit list() method to ObjectApi and head() to ContainerApi along with tests.
* Reverted copy/paste issue in ContainerApiMockTest
-- File Changes --
M openstack-swift/src/main/java/org/jclouds/openstack/swift/v1/features/ContainerApi.java (17)
M openstack-swift/src/main/java/org/jclouds/openstack/swift/v1/features/ObjectApi.java (17)
M openstack-swift/src/test/java/org/jclouds/openstack/swift/v1/features/ContainerApiMockTest.java (28)
M openstack-swift/src/test/java/org/jclouds/openstack/swift/v1/features/ObjectApiLiveTest.java (12)
M openstack-swift/src/test/java/org/jclouds/openstack/swift/v1/features/ObjectApiMockTest.java (24)
-- Patch Links --
https://github.com/jclouds/jclouds-labs-openstack/pull/89.patch
https://github.com/jclouds/jclouds-labs-openstack/pull/89.diff
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/89
Re: [jclouds-labs-openstack] Added missing methods to Container and
Object APIs (#89)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds-labs-openstack #1014](https://buildhive.cloudbees.com/job/jclouds/job/jclouds-labs-openstack/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-openstack/pull/89#issuecomment-39449487
Re: [jclouds-labs-openstack] Added missing methods to Container and
Object APIs (#89)
Posted by Everett Toews <no...@github.com>.
Needs a live test for the head() method.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/89#issuecomment-39378402
Re: [jclouds-labs-openstack] Added missing methods to Container and
Object APIs (#89)
Posted by Ignasi Barrera <no...@github.com>.
Thanks for the cleanup. I'm OK to change that post 1.7.2 as that behavior was already there and was not the purpose of this PR. It would be better addressed in a new one.
Regarding the test that throws the exception and reading it again (my apologies for not having suggested this before), I think we should keep the assertions that verify the recorded request, to assert that the sent request is the expected one, regardless of the response or if it translates to an exception. Those could be added in a catch block and explicitly fail the test if the exception is not thrown. WDYT?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/89#issuecomment-39514966
Re: [jclouds-labs-openstack] Added missing methods to Container and
Object APIs (#89)
Posted by Ignasi Barrera <no...@github.com>.
> + assertEquals(container.getName(), "myContainer");
> + assertEquals(container.getObjectCount(), 42l);
> + assertEquals(container.getBytesUsed(), 323479l);
> + for (Entry<String, String> entry : container.getMetadata().entrySet()) {
> + assertEquals(container.getMetadata().get(entry.getKey().toLowerCase()), entry.getValue());
> + }
> + assertEquals(server.getRequestCount(), 3);
> + assertAuthentication(server);
> + assertRequest(server.takeRequest(), "PUT", "/v1/MossoCloudFS_5bcf396e-39dd-45ff-93a1-712b9aba90a9/myContainer");
> + assertRequest(server.takeRequest(), "HEAD", "/v1/MossoCloudFS_5bcf396e-39dd-45ff-93a1-712b9aba90a9/myContainer");
> + } finally {
> + server.shutdown();
> + }
> + }
> +
> + @Test(expectedExceptions = ContainerNotFoundException.class)
Which method is throwing this exception? If it is in the `head` call, then all its following code could be removed, just to make it clear that the method stops there.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/89/files#r11250962
Re: [jclouds-labs-openstack] Added missing methods to Container and
Object APIs (#89)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-labs-openstack-pull-requests #199](https://jclouds.ci.cloudbees.com/job/jclouds-labs-openstack-pull-requests/199/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/89#issuecomment-39364532
Re: [jclouds-labs-openstack] Added missing methods to Container and
Object APIs (#89)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-labs-openstack-pull-requests #215](https://jclouds.ci.cloudbees.com/job/jclouds-labs-openstack-pull-requests/215/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/89#issuecomment-39507714
Re: [jclouds-labs-openstack] Added missing methods to Container and
Object APIs (#89)
Posted by Jeremy Daggett <no...@github.com>.
@nacx Do you agree with the state of this PR. Are we good to merge?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/89#issuecomment-39508020
Re: [jclouds-labs-openstack] Added missing methods to Container and
Object APIs (#89)
Posted by Andrew Phillips <no...@github.com>.
Thanks, @jdaggett and @nacx! Added one minor comment, which can perhaps be addressed in the post-1.7.2 PR that was discussed to change the 404 behaviour?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/89#issuecomment-39649138
Re: [jclouds-labs-openstack] Added missing methods to Container and
Object APIs (#89)
Posted by Jeremy Daggett <no...@github.com>.
> @@ -80,6 +80,23 @@
> @Fallback(NullOnNotFoundOr404.class)
> @Path("/")
> @Nullable
> + ObjectList list();
> +
> + /**
> + * Lists up to 10,000 objects.
> + *
> + * @param options
> + * the {@link ListContainerOptions} for controlling the returned list.
> + *
> + * @return an {@link ObjectList} of {@link SwiftObject} ordered by name or {@code null}.
> + */
> + @Named("object:list")
> + @GET
> + @QueryParams(keys = "format", values = "json")
> + @ResponseParser(ParseObjectListFromResponse.class)
> + @Fallback(NullOnNotFoundOr404.class)
As discussed in IRC, we can fix this post-1.7.2 to ensure that the default behavior is to return an empty ObjectList.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/89/files#r11273935
Re: [jclouds-labs-openstack] Added missing methods to Container and
Object APIs (#89)
Posted by Jeremy Daggett <no...@github.com>.
@nacx Great! If we are good to go, then I can add those changes to a new PR post-1.7.2 . I just made a note to remind me about updating the test. :)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/89#issuecomment-39515652
Re: [jclouds-labs-openstack] Added missing methods to Container and
Object APIs (#89)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-labs-openstack-pull-requests #209](https://jclouds.ci.cloudbees.com/job/jclouds-labs-openstack-pull-requests/209/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/89#issuecomment-39449580
Re: [jclouds-labs-openstack] Added missing methods to Container and
Object APIs (#89)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds-labs-openstack #1012](https://buildhive.cloudbees.com/job/jclouds/job/jclouds-labs-openstack/1012/) 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/89#issuecomment-39396579
Re: [jclouds-labs-openstack] Added missing methods to Container and
Object APIs (#89)
Posted by Jeremy Daggett <no...@github.com>.
> + assertEquals(container.getName(), "myContainer");
> + assertEquals(container.getObjectCount(), 42l);
> + assertEquals(container.getBytesUsed(), 323479l);
> + for (Entry<String, String> entry : container.getMetadata().entrySet()) {
> + assertEquals(container.getMetadata().get(entry.getKey().toLowerCase()), entry.getValue());
> + }
> + assertEquals(server.getRequestCount(), 3);
> + assertAuthentication(server);
> + assertRequest(server.takeRequest(), "PUT", "/v1/MossoCloudFS_5bcf396e-39dd-45ff-93a1-712b9aba90a9/myContainer");
> + assertRequest(server.takeRequest(), "HEAD", "/v1/MossoCloudFS_5bcf396e-39dd-45ff-93a1-712b9aba90a9/myContainer");
> + } finally {
> + server.shutdown();
> + }
> + }
> +
> + @Test(expectedExceptions = ContainerNotFoundException.class)
Removed that dead code and made a comment in the test.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/89/files#r11273949
Re: [jclouds-labs-openstack] Added missing methods to Container and
Object APIs (#89)
Posted by Ignasi Barrera <no...@github.com>.
> @@ -80,6 +80,23 @@
> @Fallback(NullOnNotFoundOr404.class)
> @Path("/")
> @Nullable
> + ObjectList list();
> +
> + /**
> + * Lists up to 10,000 objects.
> + *
> + * @param options
> + * the {@link ListContainerOptions} for controlling the returned list.
> + *
> + * @return an {@link ObjectList} of {@link SwiftObject} ordered by name or {@code null}.
> + */
> + @Named("object:list")
> + @GET
> + @QueryParams(keys = "format", values = "json")
> + @ResponseParser(ParseObjectListFromResponse.class)
> + @Fallback(NullOnNotFoundOr404.class)
I've just seen this. In list methods, we should return empty lists instead of null ones. This was already returning null, so I'm just asking. Can we change the behavior of the list methods to return empty lists?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/89/files#r11248044
Re: [jclouds-labs-openstack] Added missing methods to Container and
Object APIs (#89)
Posted by Everett Toews <no...@github.com>.
Closed #89.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/89
Re: [jclouds-labs-openstack] Added missing methods to Container and
Object APIs (#89)
Posted by Andrew Phillips <no...@github.com>.
> @@ -123,6 +123,18 @@ public void testCopyObject() throws Exception {
> public void testList() throws Exception {
> for (String regionId : regions) {
> ObjectApi objectApi = api.objectApiInRegionForContainer(regionId, containerName);
> + ObjectList response = objectApi.list();
> + assertEquals(response.getContainer(), api.containerApiInRegion(regionId).get(containerName));
> + assertNotNull(response);
Don't need this assertion?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/89/files#r11227400
Re: [jclouds-labs-openstack] Added missing methods to Container and
Object APIs (#89)
Posted by Andrew Phillips <no...@github.com>.
> @@ -80,6 +80,23 @@
> @Fallback(NullOnNotFoundOr404.class)
> @Path("/")
> @Nullable
> + ObjectList list();
> +
> + /**
> + * Lists up to 10,000 objects.
If we're going to list this limitation here, also suggest what to do if you need to retrieve more items? Is there a different call you can make, or should you simply make multiple calls?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/89/files#r11225649
Re: [jclouds-labs-openstack] Added missing methods to Container and
Object APIs (#89)
Posted by Jeremy Daggett <no...@github.com>.
> +
> + Container container = api.containerApiInRegion("DFW").head("myContainer");
> + assertEquals(container.getName(), "myContainer");
> + assertEquals(container.getObjectCount(), 42l);
> + assertEquals(container.getBytesUsed(), 323479l);
> + for (Entry<String, String> entry : container.getMetadata().entrySet()) {
> + assertEquals(container.getMetadata().get(entry.getKey().toLowerCase()), entry.getValue());
> + }
> + assertEquals(server.getRequestCount(), 3);
> + assertAuthentication(server);
> + assertRequest(server.takeRequest(), "PUT", "/v1/MossoCloudFS_5bcf396e-39dd-45ff-93a1-712b9aba90a9/myContainer");
> + assertRequest(server.takeRequest(), "HEAD", "/v1/MossoCloudFS_5bcf396e-39dd-45ff-93a1-712b9aba90a9/myContainer");
> + } finally {
> + server.shutdown();
> + }
> + }
Fixed.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/89/files#r11249684
Re: [jclouds-labs-openstack] Added missing methods to Container and
Object APIs (#89)
Posted by Everett Toews <no...@github.com>.
Merged to master and 1.7.x
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/89#issuecomment-39516488
Re: [jclouds-labs-openstack] Added missing methods to Container and
Object APIs (#89)
Posted by Jeremy Daggett <no...@github.com>.
> @@ -80,6 +80,23 @@
> @Fallback(NullOnNotFoundOr404.class)
> @Path("/")
> @Nullable
> + ObjectList list();
> +
> + /**
> + * Lists up to 10,000 objects.
> + *
> + * @param options
> + * the {@link ListContainerOptions} for controlling the returned list.
> + *
> + * @return an {@link ObjectList} of {@link SwiftObject} ordered by name or {@code null}.
> + */
> + @Named("object:list")
> + @GET
> + @QueryParams(keys = "format", values = "json")
> + @ResponseParser(ParseObjectListFromResponse.class)
> + @Fallback(NullOnNotFoundOr404.class)
ObjectList is an object, so returning null here was appropriate. We could revisit this later since the APIs are beta.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/89/files#r11250118
Re: [jclouds-labs-openstack] Added missing methods to Container and
Object APIs (#89)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-labs-openstack-pull-requests #207](https://jclouds.ci.cloudbees.com/job/jclouds-labs-openstack-pull-requests/207/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/89#issuecomment-39396478
Re: [jclouds-labs-openstack] Added missing methods to Container and
Object APIs (#89)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-labs-openstack-pull-requests #203](https://jclouds.ci.cloudbees.com/job/jclouds-labs-openstack-pull-requests/203/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/89#issuecomment-39383404
Re: [jclouds-labs-openstack] Added missing methods to Container and
Object APIs (#89)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-labs-openstack-pull-requests #214](https://jclouds.ci.cloudbees.com/job/jclouds-labs-openstack-pull-requests/214/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/89#issuecomment-39506929
Re: [jclouds-labs-openstack] Added missing methods to Container and
Object APIs (#89)
Posted by Ignasi Barrera <no...@github.com>.
> @@ -80,6 +80,23 @@
> @Fallback(NullOnNotFoundOr404.class)
> @Path("/")
> @Nullable
> + ObjectList list();
> +
> + /**
> + * Lists up to 10,000 objects.
> + *
> + * @param options
> + * the {@link ListContainerOptions} for controlling the returned list.
> + *
> + * @return an {@link ObjectList} of {@link SwiftObject} ordered by name or {@code null}.
> + */
> + @Named("object:list")
> + @GET
> + @QueryParams(keys = "format", values = "json")
> + @ResponseParser(ParseObjectListFromResponse.class)
> + @Fallback(NullOnNotFoundOr404.class)
It is actually a [List](https://github.com/rackerlabs/jclouds-labs-openstack/blob/api-updates/openstack-swift/src/main/java/org/jclouds/openstack/swift/v1/domain/ObjectList.java#L34), as it extends `ForwardingList`. As its javadoc says, it represents a list of objects in a container.
So what does a `404` mean in this case? One could assume that containers should already exist, as you get this api with something like: `api.objectApiInRegionForContainer("DFW", "myContainer")`.
* If the container exists when calling the `list` method, I think the method should return the `ObjectList` instance with the `container` variable set with the container details, and the `objects` one set to an empty list.
* If the container does not exist, I think we should propagate an exception, as returning `null` is very ambiguous (what does `null` mean in this case? This is one of the reasons jclouds avoids using `null` for lists).
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/89/files#r11250700
Re: [jclouds-labs-openstack] Added missing methods to Container and
Object APIs (#89)
Posted by Andrew Phillips <no...@github.com>.
> jclouds-labs-openstack-pull-requests #199 SUCCESS
[Happy Checkstyle](https://jclouds.ci.cloudbees.com/job/jclouds-labs-openstack-pull-requests/199/violations/)!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/89#issuecomment-39382895
Re: [jclouds-labs-openstack] Added missing methods to Container and
Object APIs (#89)
Posted by Jeremy Daggett <no...@github.com>.
> @@ -123,6 +123,18 @@ public void testCopyObject() throws Exception {
> public void testList() throws Exception {
> for (String regionId : regions) {
> ObjectApi objectApi = api.objectApiInRegionForContainer(regionId, containerName);
> + ObjectList response = objectApi.list();
> + assertEquals(response.getContainer(), api.containerApiInRegion(regionId).get(containerName));
> + assertNotNull(response);
Ha, at that point? No. I removed it from the **testListWithOptions()** as well
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/89/files#r11232588
Re: [jclouds-labs-openstack] Added missing methods to Container and
Object APIs (#89)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds-labs-openstack #1021](https://buildhive.cloudbees.com/job/jclouds/job/jclouds-labs-openstack/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-openstack/pull/89#issuecomment-39509047
Re: [jclouds-labs-openstack] Added missing methods to Container and
Object APIs (#89)
Posted by Ignasi Barrera <no...@github.com>.
> +
> + Container container = api.containerApiInRegion("DFW").head("myContainer");
> + assertEquals(container.getName(), "myContainer");
> + assertEquals(container.getObjectCount(), 42l);
> + assertEquals(container.getBytesUsed(), 323479l);
> + for (Entry<String, String> entry : container.getMetadata().entrySet()) {
> + assertEquals(container.getMetadata().get(entry.getKey().toLowerCase()), entry.getValue());
> + }
> + assertEquals(server.getRequestCount(), 3);
> + assertAuthentication(server);
> + assertRequest(server.takeRequest(), "PUT", "/v1/MossoCloudFS_5bcf396e-39dd-45ff-93a1-712b9aba90a9/myContainer");
> + assertRequest(server.takeRequest(), "HEAD", "/v1/MossoCloudFS_5bcf396e-39dd-45ff-93a1-712b9aba90a9/myContainer");
> + } finally {
> + server.shutdown();
> + }
> + }
Remember not to check only the happy path! :) A test should also be added to verify the behavior when the HEAD request returns a 404.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/89/files#r11247941
Re: [jclouds-labs-openstack] Added missing methods to Container and
Object APIs (#89)
Posted by Andrew Phillips <no...@github.com>.
> @@ -68,7 +68,16 @@ public void testListWithOptions() throws Exception {
> assertTrue(container.getBytesUsed() == 0);
> }
> }
> -
> +
> + public void testHead() throws Exception {
> + for (String regionId : regions) {
> + Container container = api.containerApiInRegion(regionId).head(name);
> + assertEquals(container.getName(), name);
> + assertTrue(container.getObjectCount() == 0);
> + assertTrue(container.getBytesUsed() == 0);
Use `assertEquals(..., 0)` here?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/89/files#r11322899
Re: [jclouds-labs-openstack] Added missing methods to Container and
Object APIs (#89)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds-labs-openstack #1022](https://buildhive.cloudbees.com/job/jclouds/job/jclouds-labs-openstack/1022/) 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/89#issuecomment-39509865
Re: [jclouds-labs-openstack] Added missing methods to Container and
Object APIs (#89)
Posted by Ignasi Barrera <no...@github.com>.
> + .addHeader(CONTAINER_READ, CONTAINER_ACL_ANYBODY_READ)
> + .setBody(stringFromResource("/object_list.json"))));
> +
> + try {
> + SwiftApi api = api(server.getUrl("/").toString(), "openstack-swift");
> + ObjectList objects = api.objectApiInRegionForContainer("DFW", "myContainer").list(new ListContainerOptions());
> + assertEquals(objects, parsedObjectsForUrl(server.getUrl("/").toString()));
> + assertEquals(objects.getContainer().getName(), "myContainer");
> + assertTrue(objects.getContainer().getAnybodyRead().get());
> +
> + assertEquals(server.getRequestCount(), 2);
> + assertAuthentication(server);
> + assertRequest(server.takeRequest(), "GET", "/v1/MossoCloudFS_5bcf396e-39dd-45ff-93a1-712b9aba90a9/myContainer/?format=json");
> + } finally {
> + server.shutdown();
> + }
Same as above, add also the tests for responses not returning a 200.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/89/files#r11248065
Re: [jclouds-labs-openstack] Added missing methods to Container and
Object APIs (#89)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds-labs-openstack #1007](https://buildhive.cloudbees.com/job/jclouds/job/jclouds-labs-openstack/1007/) 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/89#issuecomment-39383333