You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@jclouds.apache.org by Timur Alperovich <no...@github.com> on 2015/06/09 02:54:08 UTC

[jclouds] Implement generic and S3+Swift delimiter support. (#764)

The patch adds delimiter option support in the generic blob store
interface. The parameter -- a char -- is correctly used in the
openstack-swift and S3 APIs. The openstack-swift API no longer uses
the "path" parameter, as it does not include subdirectories (subdir
entries) and instead relies on a combination of delimiter and prefix
settings.

GCS, Azure, Atmos, and HPCS do not support the delimiter option. The
support can be added in subsequent patches.
You can view, comment on, or merge this pull request online at:

  https://github.com/jclouds/jclouds/pull/764

-- Commit Summary --

  * Implement generic and S3+Swift delimiter support.

-- File Changes --

    M apis/atmos/src/test/java/org/jclouds/atmos/blobstore/integration/AtmosContainerLiveTest.java (7)
    M apis/openstack-swift/src/main/java/org/jclouds/openstack/swift/v1/blobstore/functions/ToBlobMetadata.java (3)
    M apis/openstack-swift/src/main/java/org/jclouds/openstack/swift/v1/blobstore/functions/ToListContainerOptions.java (38)
    M apis/openstack-swift/src/main/java/org/jclouds/openstack/swift/v1/functions/ParseObjectListFromResponse.java (12)
    M apis/s3/src/main/java/org/jclouds/s3/blobstore/functions/ContainerToBucketListOptions.java (16)
    M apis/s3/src/test/java/org/jclouds/s3/blobstore/functions/BucketToContainerListOptions.java (4)
    M apis/swift/src/test/java/org/jclouds/openstack/swift/blobstore/integration/SwiftContainerLiveTest.java (7)
    M blobstore/src/main/java/org/jclouds/blobstore/options/ListContainerOptions.java (37)
    M blobstore/src/test/java/org/jclouds/blobstore/integration/internal/BaseContainerLiveTest.java (51)
    M blobstore/src/test/java/org/jclouds/blobstore/options/ListOptionsTest.java (7)
    M providers/azureblob/src/test/java/org/jclouds/azureblob/blobstore/integration/AzureBlobContainerLiveTest.java (7)
    M providers/hpcloud-objectstorage/src/test/java/org/jclouds/hpcloud/objectstorage/blobstore/integration/HPCloudObjectStorageContainerLiveTest.java (5)

-- Patch Links --

https://github.com/jclouds/jclouds/pull/764.patch
https://github.com/jclouds/jclouds/pull/764.diff

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

Re: [jclouds] Implement generic and S3+Swift delimiter support. (#764)

Posted by Timur Alperovich <no...@github.com>.
> @@ -139,4 +177,17 @@ private void runCreateContainerInLocation(String payload, Location nonDefault) t
>        }
>     }
>  
> +   private void createContainer(String containerName, Location location) throws InterruptedException {

Trying to factor out the container creation code (as there were additional assertions that the tests were doing).

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/764/files#r32043361

Re: [jclouds] JCLOUDS-929: Implement delimiter support. (#764)

Posted by Andrew Gaul <no...@github.com>.
Pushed to master as a29d75a5d11f4d7ea330815035d0f307d8518551, 6ec11fd6ecaa88569464a278064ad4a14a0df916, fe13b07233c520b21fcf4e86c093696401cf8e69, 4dc33ab56489f1faf3ef0312238902b476e631be, 2c1ca89e1ad95da56042df27f8d4aeaebcde6c7e.

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/764#issuecomment-121070665

Re: [jclouds] Implement generic and S3+Swift delimiter support. (#764)

Posted by Timur Alperovich <no...@github.com>.
Will open the JIRA issue and update the commit message. The "do not support" wording is indeed "not yet implemented in the jclouds provider". All of the providers/APIs support the delimiter option as far as I can tell.

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/764#issuecomment-110447281

Re: [jclouds] Implement generic and S3+Swift delimiter support. (#764)

Posted by Timur Alperovich <no...@github.com>.
> +      String payload = "foo";
> +      try {
> +         createContainer(containerName, defaultLocation);
> +         blobStore.putBlob(containerName, blobStore.blobBuilder("test/foo/foo").payload(payload).build());
> +         blobStore.putBlob(containerName, blobStore.blobBuilder("test/bar/foo").payload(payload).build());
> +         blobStore.putBlob(containerName, blobStore.blobBuilder("foo").payload(payload).build());
> +         checkListedSet(ImmutableSet.of("foo", "test"), blobStore.list(
> +                 containerName, ListContainerOptions.Builder.delimiter('/')));
> +         checkListedSet(ImmutableSet.of("test/foo/foo", "test/bar/foo", "foo"), blobStore.list(
> +                 containerName, ListContainerOptions.Builder.delimiter('\\')));
> +         checkListedSet(ImmutableSet.of("test/foo", "test/bar"), blobStore.list(
> +                 containerName, ListContainerOptions.Builder.delimiter('/').inDirectory("test")));
> +         checkListedSet(ImmutableSet.of("test/bar/foo"), blobStore.list(
> +                 containerName, ListContainerOptions.Builder.delimiter('/').inDirectory("test/bar")));
> +      } finally {
> +         recycleContainerAndAddToPool(containerName);

Done

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/764/files#r32044288

Re: [jclouds] Implement generic and S3+Swift delimiter support. (#764)

Posted by Andrew Gaul <no...@github.com>.
> +      String payload = "foo";
> +      try {
> +         createContainer(containerName, defaultLocation);
> +         blobStore.putBlob(containerName, blobStore.blobBuilder("test/foo/foo").payload(payload).build());
> +         blobStore.putBlob(containerName, blobStore.blobBuilder("test/bar/foo").payload(payload).build());
> +         blobStore.putBlob(containerName, blobStore.blobBuilder("foo").payload(payload).build());
> +         checkListedSet(ImmutableSet.of("foo", "test"), blobStore.list(
> +                 containerName, ListContainerOptions.Builder.delimiter('/')));
> +         checkListedSet(ImmutableSet.of("test/foo/foo", "test/bar/foo", "foo"), blobStore.list(
> +                 containerName, ListContainerOptions.Builder.delimiter('\\')));
> +         checkListedSet(ImmutableSet.of("test/foo", "test/bar"), blobStore.list(
> +                 containerName, ListContainerOptions.Builder.delimiter('/').inDirectory("test")));
> +         checkListedSet(ImmutableSet.of("test/bar/foo"), blobStore.list(
> +                 containerName, ListContainerOptions.Builder.delimiter('/').inDirectory("test/bar")));
> +      } finally {
> +         recycleContainerAndAddToPool(containerName);

Similarly, do you want `returnContainer` here?

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/764/files#r32042656

Re: [jclouds] Implement generic and S3+Swift delimiter support. (#764)

Posted by Andrew Gaul <no...@github.com>.
Might be nice to break this into a few commits within the same pull request, at least for the not-yet-implemented providers.  See the multipart upload commit train for an example.

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/764#issuecomment-110449662

Re: [jclouds] Implement generic and S3+Swift delimiter support. (#764)

Posted by Timur Alperovich <no...@github.com>.
> @@ -112,12 +116,46 @@ public void testPublicAccessInNonDefaultLocationWithBigBlob() throws Interrupted
>        runCreateContainerInLocation(payload, nonDefault);
>     }
>  
> +   @Test(groups = "live", dependsOnMethods = "testPublicAccess")
> +   public void testDelimiterList() throws InterruptedException {
> +      final String containerName = getScratchContainerName();

The latter makes sense then. I was following the existing tests.

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/764/files#r32043270

Re: [jclouds] Implement generic and S3+Swift delimiter support. (#764)

Posted by Andrew Gaul <no...@github.com>.
> @@ -112,12 +116,46 @@ public void testPublicAccessInNonDefaultLocationWithBigBlob() throws Interrupted
>        runCreateContainerInLocation(payload, nonDefault);
>     }
>  
> +   @Test(groups = "live", dependsOnMethods = "testPublicAccess")
> +   public void testDelimiterList() throws InterruptedException {
> +      final String containerName = getScratchContainerName();

Do you actually want `getScratchContainerName` or simply `getContainerName`?  The latter uses the recycled containers instead of allocating a new one.

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/764/files#r32042636

Re: [jclouds] Implement generic and S3+Swift delimiter support. (#764)

Posted by Andrew Gaul <no...@github.com>.
> +         checkListedSet(ImmutableSet.of("test/bar/foo"), blobStore.list(
> +                 containerName, ListContainerOptions.Builder.delimiter('/').inDirectory("test/bar")));
> +      } finally {
> +         recycleContainerAndAddToPool(containerName);
> +      }
> +   }
> +
> +   private void checkListedSet(ImmutableSet<String> expectedSet, PageSet<? extends StorageMetadata> results) {
> +      Set<String> resultSet = new HashSet<String>();
> +      for (StorageMetadata sm : results) {
> +         if (!resultSet.contains(sm.getName())) {
> +            resultSet.add(sm.getName());
> +         }
> +      }
> +      assertEquals(resultSet, expectedSet);
> +   }

Would this be more clear as a `storageMetadataToNames` method combined with `assertThat(Iterable).containsExactly(Iterable)`?

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/764/files#r32043892

Re: [jclouds] Implement generic and S3+Swift delimiter support. (#764)

Posted by Timur Alperovich <no...@github.com>.
> +         checkListedSet(ImmutableSet.of("test/bar/foo"), blobStore.list(
> +                 containerName, ListContainerOptions.Builder.delimiter('/').inDirectory("test/bar")));
> +      } finally {
> +         recycleContainerAndAddToPool(containerName);
> +      }
> +   }
> +
> +   private void checkListedSet(ImmutableSet<String> expectedSet, PageSet<? extends StorageMetadata> results) {
> +      Set<String> resultSet = new HashSet<String>();
> +      for (StorageMetadata sm : results) {
> +         if (!resultSet.contains(sm.getName())) {
> +            resultSet.add(sm.getName());
> +         }
> +      }
> +      assertEquals(resultSet, expectedSet);
> +   }

Done

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/764/files#r32044315

Re: [jclouds] Implement generic and S3+Swift delimiter support. (#764)

Posted by Timur Alperovich <no...@github.com>.
Sure -- will break this up in a similar fashion: base blobstore changes, S3 changes, Swift changes, and so on.

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/764#issuecomment-110451374

Re: [jclouds] Implement generic and S3+Swift delimiter support. (#764)

Posted by Andrew Gaul <no...@github.com>.
> GCS, Azure, Atmos, and HPCS do not support the delimiter option. The
support can be added in subsequent patches.

Is it correct to say that the providers do not support this, or that the support is not yet implemented?

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/764#issuecomment-110444703

Re: [jclouds] Implement generic and S3+Swift delimiter support. (#764)

Posted by Andrew Gaul <no...@github.com>.
Can you open a JIRA issue explaining the feature and your motivation?  This will highlight this work in the release notes and track the not-yet-implemented vs. cannot-be-implemented features.

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/764#issuecomment-110444898

Re: [jclouds] JCLOUDS-929: Implement delimiter support. (#764)

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

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/764#event-354820654

Re: [jclouds] Implement generic and S3+Swift delimiter support. (#764)

Posted by Andrew Gaul <no...@github.com>.
> @@ -139,4 +177,17 @@ private void runCreateContainerInLocation(String payload, Location nonDefault) t
>        }
>     }
>  
> +   private void createContainer(String containerName, Location location) throws InterruptedException {

What is the purpose of this method?

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/764/files#r32042711