jclouds-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From g...@apache.org
Subject jclouds git commit: Fix for FileSystem blob store clearContainer with options
Date Fri, 04 Jan 2019 22:17:19 GMT
Repository: jclouds
Updated Branches:
  refs/heads/master f9cebd59f -> a36c9dcef


Fix for FileSystem blob store clearContainer with options


Project: http://git-wip-us.apache.org/repos/asf/jclouds/repo
Commit: http://git-wip-us.apache.org/repos/asf/jclouds/commit/a36c9dce
Tree: http://git-wip-us.apache.org/repos/asf/jclouds/tree/a36c9dce
Diff: http://git-wip-us.apache.org/repos/asf/jclouds/diff/a36c9dce

Branch: refs/heads/master
Commit: a36c9dcef06f14d26850c2cf3f3a661aa303914b
Parents: f9cebd5
Author: Joe Meiring <jmeiring@tacc.utexas.edu>
Authored: Fri Nov 16 14:33:25 2018 -0600
Committer: Andrew Gaul <gaul@apache.org>
Committed: Fri Jan 4 14:17:00 2019 -0800

----------------------------------------------------------------------
 .../internal/FilesystemStorageStrategyImpl.java | 83 ++++++++++++++++----
 .../FilesystemContainerIntegrationTest.java     |  4 -
 .../internal/BaseContainerIntegrationTest.java  | 27 +++++--
 3 files changed, 88 insertions(+), 26 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/jclouds/blob/a36c9dce/apis/filesystem/src/main/java/org/jclouds/filesystem/strategy/internal/FilesystemStorageStrategyImpl.java
----------------------------------------------------------------------
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.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);
@@ -859,6 +891,18 @@ public class FilesystemStorageStrategyImpl implements LocalStorageStrategy
{
    }
 
    /**
+    * 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;
          }
       }
    }

http://git-wip-us.apache.org/repos/asf/jclouds/blob/a36c9dce/apis/filesystem/src/test/java/org/jclouds/filesystem/integration/FilesystemContainerIntegrationTest.java
----------------------------------------------------------------------
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 @@ 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");
-   }
 }

http://git-wip-us.apache.org/repos/asf/jclouds/blob/a36c9dce/blobstore/src/test/java/org/jclouds/blobstore/integration/internal/BaseContainerIntegrationTest.java
----------------------------------------------------------------------
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 @@ 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);
       }


Mime
View raw message