Implement delete_dir in the FileStorage

plus options for deleting only empty directories and deleting
them recursively.

Not sure how cloudfile storage is or should be handled here. Are
things such as a "directory" even a concept there?

diff --git a/mediagoblin/storage/__init__.py b/mediagoblin/storage/__init__.py
index 2db4c37db71c5c835f24f260fffa500bffd02e9c..77e4212ec2e83c2ce4a00e7c8dcd953abf0359d6 100644 (file)
--- a/mediagoblin/storage/__init__.py
+++ b/mediagoblin/storage/__init__.py
@@ -101,14 +101,28 @@ class StorageInterface(object):
 
     def delete_file(self, filepath):
         """
-        Delete or dereference the file at filepath.
+        Delete or dereference the file (not directory) at filepath.
 
+        TODO: is the below comment correct? AFAIK, we won't clean up empty directories...
         This might need to delete directories, buckets, whatever, for
         cleanliness.  (Be sure to avoid race conditions on that though)
         """
         # Subclasses should override this method.
         self.__raise_not_implemented()
 
+    def delete_dir(self, dirpath, recursive=False):
+        """Delete the directory at dirpath
+
+        :param recursive: Usually, a directory must not contain any
+            files for the delete to succeed. If True, containing files
+            and subdirectories within dirpath will be recursively
+            deleted.
+
+        :returns: True in case of success, False otherwise.
+        """
+        # Subclasses should override this method.
+        self.__raise_not_implemented()
+
     def file_url(self, filepath):
         """
         Get the URL for this file.  This assumes our storage has been

diff --git a/mediagoblin/storage/filestorage.py b/mediagoblin/storage/filestorage.py
index 00d6335ef8ece3a9a3429a6035117ba6fd5b2879..c86315f176e7f759580cc44300e5eefd91a85d46 100644 (file)
--- a/mediagoblin/storage/filestorage.py
+++ b/mediagoblin/storage/filestorage.py
@@ -62,10 +62,32 @@ class BasicFileStorage(StorageInterface):
         return open(self._resolve_filepath(filepath), mode)
 
     def delete_file(self, filepath):
-        # TODO: Also delete unused directories if empty (safely, with
-        # checks to avoid race conditions).
+        """Delete file at filepath
+
+        Raises OSError in case filepath is a directory."""
+        #TODO: log error
         os.remove(self._resolve_filepath(filepath))
 
+    def delete_dir(self, dirpath, recursive=False):
+        """returns True on succes, False on failure"""
+
+        dirpath = self._resolve_filepath(dirpath)
+
+        # Shortcut the default and simple case of nonempty=F, recursive=F
+        if recursive:
+            try:
+                shutil.rmtree(dirpath)
+            except OSError as e:
+                #TODO: log something here
+                return False
+        else: # recursively delete everything
+            try:
+                os.rmdir(dirpath)
+            except OSError as e:
+                #TODO: log something here
+                return False
+        return True
+
     def file_url(self, filepath):
         if not self.base_url:
             raise NoWebServing(