From 03248b4c9406f536bfdcbe3bce9d5b7163b93e49 Mon Sep 17 00:00:00 2001 From: Timur Alperovich Date: Tue, 30 Jun 2015 23:20:37 -0700 Subject: [PATCH] JCLOUDS-930: LocalStore -- fixup for prefixes. When filtering results, we have to consider that a result was added to the common prefixes list in its entirety (i.e. a directory). Such results should be filtered out from the delimiter test. An example that demonstrates this problem is if one creates a directory "foo" and an object within it called "file". When listing the results, they will include the directory object "foo" and the object under "foo/file". During a non-recursive listing, we create a list of common prefixes ("foo"). Subsequently, jclouds should remove all objects that include the delimiter ("/"), however, that would not apply to "foo". With the change to include the delimiter in the listing, we need to be careful not to return two values "foo" and "foo/". A unit test for the local blob store to highlight this problem is included. An integration test "testDirectory" also catches this issue. --- .../blobstore/config/LocalBlobStore.java | 23 ++++++++++--------- .../strategy/internal/ListContainerTest.java | 20 ++++++++++++++++ 2 files changed, 32 insertions(+), 11 deletions(-) diff --git a/blobstore/src/main/java/org/jclouds/blobstore/config/LocalBlobStore.java b/blobstore/src/main/java/org/jclouds/blobstore/config/LocalBlobStore.java index 47b16bc2c5..e8702a0d0b 100644 --- a/blobstore/src/main/java/org/jclouds/blobstore/config/LocalBlobStore.java +++ b/blobstore/src/main/java/org/jclouds/blobstore/config/LocalBlobStore.java @@ -353,7 +353,7 @@ public final class LocalBlobStore implements BlobStore { transform(contents, new CommonPrefixes(prefix, delimiter))); commonPrefixes.remove(CommonPrefixes.NO_PREFIX); - contents = newTreeSet(filter(contents, new DelimiterFilter(prefix, delimiter))); + contents = newTreeSet(filter(contents, new DelimiterFilter(prefix, delimiter, commonPrefixes))); for (String o : commonPrefixes) { MutableStorageMetadata md = new MutableStorageMetadataImpl(); @@ -362,7 +362,7 @@ public final class LocalBlobStore implements BlobStore { if (prefix != null) { o = prefix + o; } - md.setName(o); + md.setName(o + delimiter); contents.add(md); } return contents; @@ -457,21 +457,22 @@ public final class LocalBlobStore implements BlobStore { private static class DelimiterFilter implements Predicate { private final String prefix; private final String delimiter; + private final Set commonPrefixes; - public DelimiterFilter(String prefix, String delimiter) { + public DelimiterFilter(String prefix, String delimiter, final Set commonPrefixes) { this.prefix = prefix; this.delimiter = delimiter; + this.commonPrefixes = commonPrefixes; } public boolean apply(StorageMetadata metadata) { String name = metadata.getName(); - if (metadata.getType() == StorageType.RELATIVE_PATH) { - // For directories, we need to make sure to include the separator character, as that is what the common - // prefix will include. - name += '/'; - } - if (prefix == null || prefix.isEmpty()) + if (prefix == null || prefix.isEmpty()) { + if (commonPrefixes.contains(name)) { + return false; + } return name.indexOf(delimiter) == -1; + } String prefixMatch; if (prefix.endsWith(delimiter)) { prefixMatch = "^" + Pattern.quote(prefix) + ".*"; @@ -481,7 +482,7 @@ public final class LocalBlobStore implements BlobStore { } if (name.matches(prefixMatch)) { String unprefixedName = name.replaceFirst(prefix, ""); - if (unprefixedName.equals("")) { + if (unprefixedName.equals("") || commonPrefixes.contains(unprefixedName)) { // we are the prefix in this case, return false return false; } @@ -512,7 +513,7 @@ public final class LocalBlobStore implements BlobStore { } if (working.indexOf(delimiter) >= 0) { // include the delimiter in the result - return working.substring(0, working.indexOf(delimiter) + 1); + return working.substring(0, working.indexOf(delimiter)); } else { return NO_PREFIX; } diff --git a/blobstore/src/test/java/org/jclouds/blobstore/strategy/internal/ListContainerTest.java b/blobstore/src/test/java/org/jclouds/blobstore/strategy/internal/ListContainerTest.java index 27aa8e33aa..a71429adfe 100644 --- a/blobstore/src/test/java/org/jclouds/blobstore/strategy/internal/ListContainerTest.java +++ b/blobstore/src/test/java/org/jclouds/blobstore/strategy/internal/ListContainerTest.java @@ -145,4 +145,24 @@ public class ListContainerTest { ListContainerOptions.Builder.maxResults(1).afterMarker(results.getNextMarker())); assertThat(results.getNextMarker()).isEqualTo(null); } + + public void testDirectoryListing() { + String containerName = "testDirectoryListing"; + blobStore.createContainerInLocation(null, containerName); + blobStore.createDirectory(containerName, "dir"); + blobStore.createDirectory(containerName, "dir/dir"); + + PageSet results = blobStore.list(containerName); + assertThat(results.size()).isEqualTo(1); + assertThat(Iterables.get(results, 0).getName()).isEqualTo("dir/"); + + results = blobStore.list(containerName, ListContainerOptions.Builder.inDirectory("dir")); + assertThat(results.size()).isEqualTo(1); + assertThat(Iterables.get(results, 0).getName()).isEqualTo("dir/dir"); + + blobStore.putBlob(containerName, blobStore.blobBuilder("dir/dir/blob").payload("").build()); + results = blobStore.list(containerName, ListContainerOptions.Builder.inDirectory("dir")); + assertThat(results.size()).isEqualTo(1); + assertThat(Iterables.get(results, 0).getName()).isEqualTo("dir/dir/"); + } }