Merge remote-tracking branch 'cwebber/254_delete_queue_directories'

* cwebber/254_delete_queue_directories:
  Removing docstring bit about delete_file possibly deleting directories in the future
  Convert media processing backends to delete the queue directory (#254)
  Implement delete_dir in the FileStorage

Conflicts:
	mediagoblin/media_types/image/processing.py
	mediagoblin/media_types/video/processing.py

Conflicts, because those media_types already use the newer
proc_state.delete_queue_file() method (which needs
updating.

5 files changed, 57 insertions, 9 deletions

diff --git a/mediagoblin/media_types/ascii/processing.py b/mediagoblin/media_types/ascii/processing.py
index 382cd015..309aab0a 100644
--- a/mediagoblin/media_types/ascii/processing.py
+++ b/mediagoblin/media_types/ascii/processing.py
@@ -127,8 +127,14 @@ def process_ascii(proc_state):
                     'ascii',
                     'xmlcharrefreplace'))
 
-    mgg.queue_store.delete_file(queued_filepath)
+    # Remove queued media file from storage and database.
+    # queued_filepath is in the task_id directory which should
+    # be removed too, but fail if the directory is not empty to be on
+    # the super-safe side.
+    mgg.queue_store.delete_file(queued_filepath)      # rm file
+    mgg.queue_store.delete_dir(queued_filepath[:-1])  # rm dir
     entry.queued_media_file = []
+
     media_files_dict = entry.setdefault('media_files', {})
     media_files_dict['thumb'] = thumb_filepath
     media_files_dict['unicode'] = unicode_filepath
diff --git a/mediagoblin/media_types/audio/processing.py b/mediagoblin/media_types/audio/processing.py
index 5dffcaf9..101b83e5 100644
--- a/mediagoblin/media_types/audio/processing.py
+++ b/mediagoblin/media_types/audio/processing.py
@@ -147,4 +147,10 @@ def process_audio(proc_state):
     else:
         entry.media_files['thumb'] = ['fake', 'thumb', 'path.jpg']
 
-    mgg.queue_store.delete_file(queued_filepath)
+    # Remove queued media file from storage and database.
+    # queued_filepath is in the task_id directory which should
+    # be removed too, but fail if the directory is not empty to be on
+    # the super-safe side.
+    mgg.queue_store.delete_file(queued_filepath)      # rm file
+    mgg.queue_store.delete_dir(queued_filepath[:-1])  # rm dir
+    entry.queued_media_file = []
diff --git a/mediagoblin/media_types/stl/processing.py b/mediagoblin/media_types/stl/processing.py
index 77744ac5..e41df395 100644
--- a/mediagoblin/media_types/stl/processing.py
+++ b/mediagoblin/media_types/stl/processing.py
@@ -165,8 +165,12 @@ def process_stl(proc_state):
         with open(queued_filename, 'rb') as queued_file:
             model_file.write(queued_file.read())
 
-    # Remove queued media file from storage and database
-    mgg.queue_store.delete_file(queued_filepath)
+    # Remove queued media file from storage and database.
+    # queued_filepath is in the task_id directory which should
+    # be removed too, but fail if the directory is not empty to be on
+    # the super-safe side.
+    mgg.queue_store.delete_file(queued_filepath)      # rm file
+    mgg.queue_store.delete_dir(queued_filepath[:-1])  # rm dir
     entry.queued_media_file = []
 
     # Insert media file information into database
diff --git a/mediagoblin/storage/__init__.py b/mediagoblin/storage/__init__.py
index 5c1d7d36..bbe134a7 100644
--- a/mediagoblin/storage/__init__.py
+++ b/mediagoblin/storage/__init__.py
@@ -101,10 +101,20 @@ class StorageInterface(object):
 
     def delete_file(self, filepath):
         """
-        Delete or dereference the file at filepath.
+        Delete or dereference the file (not directory) at filepath.
+        """
+        # 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.
 
-        This might need to delete directories, buckets, whatever, for
-        cleanliness.  (Be sure to avoid race conditions on that though)
+        :returns: True in case of success, False otherwise.
         """
         # Subclasses should override this method.
         self.__raise_not_implemented()
diff --git a/mediagoblin/storage/filestorage.py b/mediagoblin/storage/filestorage.py
index ef786b61..3d6e0753 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(