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.
This commit is contained in:
Timur Alperovich 2015-06-30 23:20:37 -07:00 committed by Ignasi Barrera
parent 3009da3761
commit 03248b4c94
2 changed files with 32 additions and 11 deletions

View File

@ -353,7 +353,7 @@ public final class LocalBlobStore implements BlobStore {
transform(contents, new CommonPrefixes(prefix, delimiter))); transform(contents, new CommonPrefixes(prefix, delimiter)));
commonPrefixes.remove(CommonPrefixes.NO_PREFIX); 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) { for (String o : commonPrefixes) {
MutableStorageMetadata md = new MutableStorageMetadataImpl(); MutableStorageMetadata md = new MutableStorageMetadataImpl();
@ -362,7 +362,7 @@ public final class LocalBlobStore implements BlobStore {
if (prefix != null) { if (prefix != null) {
o = prefix + o; o = prefix + o;
} }
md.setName(o); md.setName(o + delimiter);
contents.add(md); contents.add(md);
} }
return contents; return contents;
@ -457,21 +457,22 @@ public final class LocalBlobStore implements BlobStore {
private static class DelimiterFilter implements Predicate<StorageMetadata> { private static class DelimiterFilter implements Predicate<StorageMetadata> {
private final String prefix; private final String prefix;
private final String delimiter; private final String delimiter;
private final Set<String> commonPrefixes;
public DelimiterFilter(String prefix, String delimiter) { public DelimiterFilter(String prefix, String delimiter, final Set<String> commonPrefixes) {
this.prefix = prefix; this.prefix = prefix;
this.delimiter = delimiter; this.delimiter = delimiter;
this.commonPrefixes = commonPrefixes;
} }
public boolean apply(StorageMetadata metadata) { public boolean apply(StorageMetadata metadata) {
String name = metadata.getName(); String name = metadata.getName();
if (metadata.getType() == StorageType.RELATIVE_PATH) { if (prefix == null || prefix.isEmpty()) {
// For directories, we need to make sure to include the separator character, as that is what the common if (commonPrefixes.contains(name)) {
// prefix will include. return false;
name += '/'; }
}
if (prefix == null || prefix.isEmpty())
return name.indexOf(delimiter) == -1; return name.indexOf(delimiter) == -1;
}
String prefixMatch; String prefixMatch;
if (prefix.endsWith(delimiter)) { if (prefix.endsWith(delimiter)) {
prefixMatch = "^" + Pattern.quote(prefix) + ".*"; prefixMatch = "^" + Pattern.quote(prefix) + ".*";
@ -481,7 +482,7 @@ public final class LocalBlobStore implements BlobStore {
} }
if (name.matches(prefixMatch)) { if (name.matches(prefixMatch)) {
String unprefixedName = name.replaceFirst(prefix, ""); String unprefixedName = name.replaceFirst(prefix, "");
if (unprefixedName.equals("")) { if (unprefixedName.equals("") || commonPrefixes.contains(unprefixedName)) {
// we are the prefix in this case, return false // we are the prefix in this case, return false
return false; return false;
} }
@ -512,7 +513,7 @@ public final class LocalBlobStore implements BlobStore {
} }
if (working.indexOf(delimiter) >= 0) { if (working.indexOf(delimiter) >= 0) {
// include the delimiter in the result // include the delimiter in the result
return working.substring(0, working.indexOf(delimiter) + 1); return working.substring(0, working.indexOf(delimiter));
} else { } else {
return NO_PREFIX; return NO_PREFIX;
} }

View File

@ -145,4 +145,24 @@ public class ListContainerTest {
ListContainerOptions.Builder.maxResults(1).afterMarker(results.getNextMarker())); ListContainerOptions.Builder.maxResults(1).afterMarker(results.getNextMarker()));
assertThat(results.getNextMarker()).isEqualTo(null); 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<? extends StorageMetadata> 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/");
}
} }