Fix for FileSystem blob store clearContainer with options
diff --git a/apis/filesystem/src/main/java/org/jclouds/filesystem/strategy/internal/FilesystemStorageStrategyImpl.java b/apis/filesystem/src/main/java/org/jclouds/filesystem/strategy/internal/FilesystemStorageStrategyImpl.java
index b557845..a9f0a9c 100644
--- a/apis/filesystem/src/main/java/org/jclouds/filesystem/strategy/internal/FilesystemStorageStrategyImpl.java
+++ b/apis/filesystem/src/main/java/org/jclouds/filesystem/strategy/internal/FilesystemStorageStrategyImpl.java
@@ -27,6 +27,7 @@
import static java.nio.file.Files.probeContentType;
import static java.nio.file.Files.readAttributes;
import static java.nio.file.Files.setPosixFilePermissions;
+import static java.nio.file.Files.newDirectoryStream;
import static org.jclouds.filesystem.util.Utils.delete;
import static org.jclouds.filesystem.util.Utils.isPrivate;
import static org.jclouds.filesystem.util.Utils.isWindows;
@@ -41,6 +42,7 @@
import java.nio.ByteBuffer;
import java.nio.charset.StandardCharsets;
import java.nio.file.AccessDeniedException;
+import java.nio.file.DirectoryStream;
import java.nio.file.NoSuchFileException;
import java.nio.file.Path;
import java.nio.file.attribute.BasicFileAttributes;
@@ -58,6 +60,7 @@
import javax.inject.Named;
import javax.inject.Provider;
+import com.google.common.base.Strings;
import org.jclouds.blobstore.ContainerNotFoundException;
import org.jclouds.blobstore.KeyNotFoundException;
import org.jclouds.blobstore.LocalStorageStrategy;
@@ -253,16 +256,45 @@
@Override
public void clearContainer(String container, ListContainerOptions options) {
filesystemContainerNameValidator.validate(container);
- // TODO: these require calling removeDirectoriesTreeOfBlobKey
- checkArgument(options.getDir() == null && options.getPrefix() == null, "cannot specify directory or prefix");
+ checkArgument(options.getDir() == null || options.getPrefix() == null, "cannot specify both directory and prefix");
+ String optsPrefix = Strings.nullToEmpty(options.getDir() == null ? options.getPrefix() : options.getDir());
+ String normalizedOptsPath = normalize(optsPrefix);
+ String basePath = buildPathStartingFromBaseDir(container, normalizedOptsPath);
+ filesystemBlobKeyValidator.validate(basePath);
try {
- File containerFile = openFolder(container);
- File[] children = containerFile.listFiles();
- if (null != children) {
- for (File child : children)
- if (options.isRecursive() || child.isFile()) {
- Utils.deleteRecursively(child);
+ File object = new File(basePath);
+ if (object.isFile()) {
+ // To mimic the S3 type blobstores, a prefix for an object blob
+ // should also get deleted
+ delete(object);
+ }
+ else if (object.isDirectory() && (optsPrefix.endsWith(File.separator) || isNullOrEmpty(optsPrefix))) {
+ // S3 blobstores will only match prefixes that end with a trailing slash/file separator
+ // For instance, if we have a blob at /path/1/2/a, a prefix of /path/1/2 will not list /path/1/2/a
+ // but a prefix of /path/1/2/ will
+ File containerFile = openFolder(container + File.separator + normalizedOptsPath);
+ File[] children = containerFile.listFiles();
+ if (null != children) {
+ for (File child : children) {
+ if (options.isRecursive()) {
+ Utils.deleteRecursively(child);
+ } else {
+ if (child.isFile()) {
+ Utils.delete(child);
+ }
+ }
}
+ }
+
+ // Empty dirs in path if they don't have any objects
+ if (!optsPrefix.isEmpty()) {
+ if (options.isRecursive()) {
+ //first, remove the empty dir. It should be totally empty if it was a
+ // recursive delete
+ deleteDirectory(container, optsPrefix);
+ }
+ removeDirectoriesTreeOfBlobKey(container, optsPrefix);
+ }
}
} catch (IOException e) {
logger.error(e, "An error occurred while clearing container %s", container);
@@ -859,6 +891,18 @@
}
/**
+ * Checks if a directory is empty using a DirectoryStream iterator
+ *
+ * @param directoryPath
+ */
+ private boolean isDirEmpty(String directoryPath) throws IOException {
+ Path path = new File(directoryPath).toPath();
+ try (DirectoryStream<Path> dirStream = newDirectoryStream(path)) {
+ return !dirStream.iterator().hasNext();
+ }
+ }
+
+ /**
* Removes recursively the directory structure of a complex blob key, only if the directory is
* empty
*
@@ -889,16 +933,21 @@
logger.debug("Could not look for attributes from %s: %s", directory, e);
}
- String[] children = directory.list();
- if (null == children || children.length == 0) {
- try {
- delete(directory);
- } catch (IOException e) {
- logger.debug("Could not delete %s: %s", directory, e);
- return;
+ // Don't need to do a listing on the dir, which could be costly. The iterator should be more performant.
+ try {
+ if (isDirEmpty(directory.getPath())) {
+ try {
+ delete(directory);
+ } catch (IOException e) {
+ logger.debug("Could not delete %s: %s", directory, e);
+ return;
+ }
+ // recursively call for removing other path
+ removeDirectoriesTreeOfBlobKey(container, parentPath);
}
- // recursively call for removing other path
- removeDirectoriesTreeOfBlobKey(container, parentPath);
+ } catch (IOException e) {
+ logger.debug("Could not locate directory %s", directory, e);
+ return;
}
}
}
diff --git a/apis/filesystem/src/test/java/org/jclouds/filesystem/integration/FilesystemContainerIntegrationTest.java b/apis/filesystem/src/test/java/org/jclouds/filesystem/integration/FilesystemContainerIntegrationTest.java
index c9fee73..45f88d0 100644
--- a/apis/filesystem/src/test/java/org/jclouds/filesystem/integration/FilesystemContainerIntegrationTest.java
+++ b/apis/filesystem/src/test/java/org/jclouds/filesystem/integration/FilesystemContainerIntegrationTest.java
@@ -194,8 +194,4 @@
throw new SkipException("filesystem does not support anonymous access");
}
- @Override
- public void testClearWithOptions() throws InterruptedException {
- throw new SkipException("filesystem does not support clear with options");
- }
}
diff --git a/blobstore/src/test/java/org/jclouds/blobstore/integration/internal/BaseContainerIntegrationTest.java b/blobstore/src/test/java/org/jclouds/blobstore/integration/internal/BaseContainerIntegrationTest.java
index 6364c98..0c7b503 100644
--- a/blobstore/src/test/java/org/jclouds/blobstore/integration/internal/BaseContainerIntegrationTest.java
+++ b/blobstore/src/test/java/org/jclouds/blobstore/integration/internal/BaseContainerIntegrationTest.java
@@ -182,6 +182,8 @@
try {
ListContainerOptions options;
+ // Should wipe out all objects, as there are empty folders
+ // above
add5NestedBlobsToContainer(containerName);
options = new ListContainerOptions();
options.prefix("path/1/");
@@ -195,7 +197,9 @@
options.prefix("path/1/2/3");
options.recursive();
view.getBlobStore().clearContainer(containerName, options);
- assertConsistencyAwareContainerSize(containerName, 2);
+ assertConsistencyAwareBlobExists(containerName, "path/1/a");
+ assertConsistencyAwareBlobExists(containerName, "path/1/2/b");
+ assertConsistencyAwareBlobDoesntExist(containerName, "path/1/2/3");
view.getBlobStore().clearContainer(containerName);
add5NestedBlobsToContainer(containerName);
@@ -203,7 +207,10 @@
options.prefix("path/1/2/3/4/");
options.recursive();
view.getBlobStore().clearContainer(containerName, options);
- assertConsistencyAwareContainerSize(containerName, 4);
+ assertConsistencyAwareBlobExists(containerName, "path/1/a");
+ assertConsistencyAwareBlobExists(containerName, "path/1/2/b");
+ assertConsistencyAwareBlobExists(containerName, "path/1/2/3/5/e");
+ assertConsistencyAwareBlobDoesntExist(containerName, "path/1/2/3/4");
// non-recursive, should not clear anything, as prefix does not match
view.getBlobStore().clearContainer(containerName);
@@ -211,7 +218,11 @@
options = new ListContainerOptions();
options.prefix("path/1/2/3");
view.getBlobStore().clearContainer(containerName, options);
- assertConsistencyAwareContainerSize(containerName, 5);
+ assertConsistencyAwareBlobExists(containerName, "path/1/a");
+ assertConsistencyAwareBlobExists(containerName, "path/1/2/b");
+ assertConsistencyAwareBlobExists(containerName, "path/1/2/3/c");
+ assertConsistencyAwareBlobExists(containerName, "path/1/2/3/5/e");
+
// non-recursive, should only clear path/1/2/3/c
view.getBlobStore().clearContainer(containerName);
@@ -219,7 +230,10 @@
options = new ListContainerOptions();
options.prefix("path/1/2/3/");
view.getBlobStore().clearContainer(containerName, options);
- assertConsistencyAwareContainerSize(containerName, 4);
+ assertConsistencyAwareBlobExists(containerName, "path/1/a");
+ assertConsistencyAwareBlobExists(containerName, "path/1/2/b");
+ assertConsistencyAwareBlobExists(containerName, "path/1/2/3/4/d");
+ assertConsistencyAwareBlobDoesntExist(containerName, "path/1/2/3/c");
// non-recursive, should only clear path/1/2/3/c
view.getBlobStore().clearContainer(containerName);
@@ -227,7 +241,10 @@
options = new ListContainerOptions();
options.prefix("path/1/2/3/c");
view.getBlobStore().clearContainer(containerName, options);
- assertConsistencyAwareContainerSize(containerName, 4);
+ assertConsistencyAwareBlobExists(containerName, "path/1/a");
+ assertConsistencyAwareBlobExists(containerName, "path/1/2/b");
+ assertConsistencyAwareBlobExists(containerName, "path/1/2/3/4/d");
+ assertConsistencyAwareBlobDoesntExist(containerName, "path/1/2/3/c");
} finally {
returnContainer(containerName);
}
