From b34d7e1d9b53f393aa673cc431a26bc640164c39 Mon Sep 17 00:00:00 2001
From: Sebastian Spaeth <Sebastian@SSpaeth.de>
Date: Wed, 12 Dec 2012 16:29:14 +0100
Subject: [PATCH] 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?
---
 mediagoblin/storage/__init__.py    | 16 +++++++++++++++-
 mediagoblin/storage/filestorage.py | 26 ++++++++++++++++++++++++--
 2 files changed, 39 insertions(+), 3 deletions(-)

diff --git a/mediagoblin/storage/__init__.py b/mediagoblin/storage/__init__.py
index 2db4c37d..77e4212e 100644
--- 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 00d6335e..c86315f1 100644
--- 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(
-- 
2.25.1