diff --git a/blobstore/src/main/java/org/jclouds/blobstore/strategy/internal/DeleteAllKeysInList.java b/blobstore/src/main/java/org/jclouds/blobstore/strategy/internal/DeleteAllKeysInList.java index dc8b816df2..e028975226 100644 --- a/blobstore/src/main/java/org/jclouds/blobstore/strategy/internal/DeleteAllKeysInList.java +++ b/blobstore/src/main/java/org/jclouds/blobstore/strategy/internal/DeleteAllKeysInList.java @@ -93,9 +93,9 @@ public class DeleteAllKeysInList implements ClearListStrategy, ClearContainerStr message = message + " recursively"; Map exceptions = Maps.newHashMap(); PageSet listing; - Iterable toDelete; int maxErrors = 3; // TODO parameterize for (int i = 0; i < maxErrors; ) { + // fetch partial directory listing try { listing = connection.list(containerName, options).get(); } catch (ExecutionException ee) { @@ -108,36 +108,52 @@ public class DeleteAllKeysInList implements ClearListStrategy, ClearContainerStr } catch (InterruptedException ie) { throw Throwables.propagate(ie); } - toDelete = filterListing(listing, options); - Map> responses = Maps.newHashMap(); - try { - for (final StorageMetadata md : toDelete) { + // recurse on subdirectories + if (options.isRecursive()) { + for (StorageMetadata md : listing) { String fullPath = parentIsFolder(options, md) ? options.getDir() + "/" + md.getName() : md.getName(); switch (md.getType()) { case BLOB: - responses.put(md, connection.removeBlob(containerName, fullPath)); break; case FOLDER: - if (options.isRecursive() && !fullPath.equals(options.getDir())) { - execute(containerName, options.clone().inDirectory(fullPath)); - } - responses.put(md, connection.deleteDirectory(containerName, fullPath)); - break; case RELATIVE_PATH: if (options.isRecursive() && !fullPath.equals(options.getDir())) { execute(containerName, options.clone().inDirectory(fullPath)); } - responses.put(md, connection.deleteDirectory(containerName, md.getName())); break; case CONTAINER: throw new IllegalArgumentException("Container type not supported"); } } - } finally { - exceptions = awaitCompletion(responses, userExecutor, maxTime, logger, message); } + + // remove blobs and now-empty subdirectories + Map> responses = Maps.newHashMap(); + for (StorageMetadata md : listing) { + String fullPath = parentIsFolder(options, md) ? options.getDir() + "/" + + md.getName() : md.getName(); + switch (md.getType()) { + case BLOB: + responses.put(md, connection.removeBlob(containerName, fullPath)); + break; + case FOLDER: + if (options.isRecursive()) { + responses.put(md, connection.deleteDirectory(containerName, fullPath)); + } + break; + case RELATIVE_PATH: + if (options.isRecursive()) { + responses.put(md, connection.deleteDirectory(containerName, md.getName())); + } + break; + case CONTAINER: + throw new IllegalArgumentException("Container type not supported"); + } + } + + exceptions = awaitCompletion(responses, userExecutor, maxTime, logger, message); if (!exceptions.isEmpty()) { ++i; retryHandler.imposeBackoffExponentialDelay(i, message); @@ -157,29 +173,4 @@ public class DeleteAllKeysInList implements ClearListStrategy, ClearContainerStr private boolean parentIsFolder(final ListContainerOptions options, final StorageMetadata md) { return (options.getDir() != null && md.getName().indexOf('/') == -1); } - - private Iterable filterListing( - final PageSet listing, - final ListContainerOptions options) { - Iterable toDelete = Iterables.filter(listing, - new Predicate() { - - @Override - public boolean apply(StorageMetadata input) { - switch (input.getType()) { - case BLOB: - return true; - case FOLDER: - case RELATIVE_PATH: - if (options.isRecursive()) - return true; - break; - } - return false; - } - - }); - return toDelete; - } - } diff --git a/blobstore/src/test/java/org/jclouds/blobstore/strategy/internal/DeleteAllKeysInListTest.java b/blobstore/src/test/java/org/jclouds/blobstore/strategy/internal/DeleteAllKeysInListTest.java index 47de7565e5..9565d20119 100644 --- a/blobstore/src/test/java/org/jclouds/blobstore/strategy/internal/DeleteAllKeysInListTest.java +++ b/blobstore/src/test/java/org/jclouds/blobstore/strategy/internal/DeleteAllKeysInListTest.java @@ -23,8 +23,8 @@ import static org.testng.Assert.assertEquals; import org.jclouds.ContextBuilder; import org.jclouds.blobstore.BlobStore; import org.jclouds.blobstore.options.ListContainerOptions; -import org.testng.annotations.AfterClass; -import org.testng.annotations.BeforeClass; +import org.testng.annotations.AfterMethod; +import org.testng.annotations.BeforeMethod; import org.testng.annotations.Test; import com.google.common.io.Closeables; @@ -38,40 +38,60 @@ import com.google.inject.Injector; public class DeleteAllKeysInListTest { private BlobStore blobstore; private DeleteAllKeysInList deleter; + private static final String containerName = "container"; + private static final String directoryName = "directory"; - @BeforeClass + @BeforeMethod void setupBlobStore() { Injector injector = ContextBuilder.newBuilder("transient").buildInjector(); blobstore = injector.getInstance(BlobStore.class); deleter = injector.getInstance(DeleteAllKeysInList.class); + createDataSet(); + } + + @AfterMethod + void close() { + Closeables.closeQuietly(blobstore.getContext()); } public void testExecuteWithoutOptionsClearsRecursively() { - blobstore.createContainerInLocation(null, "goodies"); - for (int i = 0; i < 1001; i++) { - blobstore.putBlob("goodies", blobstore.blobBuilder(i + "").payload(i + "").build()); - } - assertEquals(blobstore.countBlobs("goodies"), 1001); - deleter.execute("goodies"); - assertEquals(blobstore.countBlobs("goodies"), 0); + deleter.execute(containerName); + assertEquals(blobstore.countBlobs(containerName), 0); + } + + public void testExecuteRecursive() { + deleter.execute(containerName, ListContainerOptions.Builder.recursive()); + assertEquals(blobstore.countBlobs(containerName), 0); } public void testExecuteNonRecursive() { - blobstore.createContainerInLocation(null, "foo"); - for (int i = 0; i < 1001; i++) { - blobstore.putBlob("foo", blobstore.blobBuilder(i + "").payload(i + "").build()); - } - for (int i = 0; i < 1001; i++) { - blobstore.putBlob("foo", blobstore.blobBuilder("dir/" + i + "").payload(i + "").build()); - } - assertEquals(blobstore.countBlobs("foo"), 2002); - deleter.execute("foo", ListContainerOptions.Builder.inDirectory("dir")); - assertEquals(blobstore.countBlobs("foo"), 1001); + deleter.execute(containerName, ListContainerOptions.NONE); + assertEquals(blobstore.countBlobs(containerName), 2222); } - @AfterClass - void close() { - if (blobstore != null) - Closeables.closeQuietly(blobstore.getContext()); + public void testExecuteInDirectory() { + deleter.execute(containerName, ListContainerOptions.Builder.inDirectory(directoryName)); + assertEquals(blobstore.countBlobs(containerName), 1111); + } + + /** + * Create a container "container" with 1111 blobs named "blob-%d". Create a + * subdirectory "directory" which contains 2222 more blobs named + * "directory/blob-%d". + */ + private void createDataSet() { + String blobNameFmt = "blob-%d"; + String directoryBlobNameFmt = "%s/blob-%d"; + + blobstore.createContainerInLocation(null, containerName); + for (int i = 0; i < 1111; i++) { + String blobName = String.format(blobNameFmt, i); + blobstore.putBlob(containerName, blobstore.blobBuilder(blobName).payload(blobName).build()); + } + for (int i = 0; i < 2222; i++) { + String directoryBlobName = String.format(directoryBlobNameFmt, directoryName, i); + blobstore.putBlob(containerName, blobstore.blobBuilder(directoryBlobName).payload(directoryBlobName).build()); + } + assertEquals(blobstore.countBlobs(containerName), 3333); } }