Fix for FileSystem blob store clearContainer with options

This commit is contained in:
Joe Meiring 2018-11-16 14:33:25 -06:00 committed by Andrew Gaul
parent f9cebd59f8
commit a36c9dcef0
3 changed files with 88 additions and 26 deletions

View File

@ -27,6 +27,7 @@ import static java.nio.file.Files.getPosixFilePermissions;
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.io.InputStream;
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.Inject;
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 @@ public class FilesystemStorageStrategyImpl implements LocalStorageStrategy {
@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);
@ -858,6 +890,18 @@ public class FilesystemStorageStrategyImpl implements LocalStorageStrategy {
return pathToBeCleaned.substring(beginIndex, endIndex);
}
/**
* 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 @@ public class FilesystemStorageStrategyImpl implements LocalStorageStrategy {
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;
}
}
}

View File

@ -194,8 +194,4 @@ public class FilesystemContainerIntegrationTest extends BaseContainerIntegration
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");
}
}

View File

@ -182,6 +182,8 @@ public class BaseContainerIntegrationTest extends BaseBlobStoreIntegrationTest {
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 @@ public class BaseContainerIntegrationTest extends BaseBlobStoreIntegrationTest {
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 @@ public class BaseContainerIntegrationTest extends BaseBlobStoreIntegrationTest {
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 @@ public class BaseContainerIntegrationTest extends BaseBlobStoreIntegrationTest {
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 @@ public class BaseContainerIntegrationTest extends BaseBlobStoreIntegrationTest {
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 @@ public class BaseContainerIntegrationTest extends BaseBlobStoreIntegrationTest {
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);
}