From fb2fbe2c0a0f6944696e3f1994a94f1be3ac0535 Mon Sep 17 00:00:00 2001 From: Jorge Araya Navarro Date: Thu, 28 Jun 2012 22:13:26 -0600 Subject: [PATCH] fixing bug #255 as Joar and CWebber ask me to do :) --- mediagoblin/storage/filestorage.py | 7 +------ mediagoblin/tools/files.py | 32 ++++++++++++++++++++++++++---- mediagoblin/user_pages/views.py | 13 +++++++++++- 3 files changed, 41 insertions(+), 11 deletions(-) diff --git a/mediagoblin/storage/filestorage.py b/mediagoblin/storage/filestorage.py index 1252a3f5..00d6335e 100644 --- a/mediagoblin/storage/filestorage.py +++ b/mediagoblin/storage/filestorage.py @@ -64,12 +64,7 @@ class BasicFileStorage(StorageInterface): def delete_file(self, filepath): # TODO: Also delete unused directories if empty (safely, with # checks to avoid race conditions). - try: - os.remove(self._resolve_filepath(filepath)) - except OSError: - # the file do not exists! - # This should fix bug #255 - pass + os.remove(self._resolve_filepath(filepath)) def file_url(self, filepath): if not self.base_url: diff --git a/mediagoblin/tools/files.py b/mediagoblin/tools/files.py index 25c1a6e6..31a7a972 100644 --- a/mediagoblin/tools/files.py +++ b/mediagoblin/tools/files.py @@ -16,6 +16,19 @@ from mediagoblin import mg_globals +import os + +def _jointhat(thing): + if type(thing) == type(list()) or\ + type(thing) == type(tuple()): + filepath = "" + for item in thing: + filepath = os.path.join(filepath, item) + return filepath + else: + raise TypeError, "expecting a list or tuple, {0} received".format( + str(type(thing))) + def delete_media_files(media): """ Delete all files associated with a MediaEntry @@ -23,10 +36,21 @@ def delete_media_files(media): Arguments: - media: A MediaEntry document """ + noSuchFiles = [] for listpath in media.media_files.itervalues(): - mg_globals.public_store.delete_file( - listpath) + try: + mg_globals.public_store.delete_file( + listpath) + except OSError: + noSuchFiles.append(_jointhat(listpath)) for attachment in media.attachment_files: - mg_globals.public_store.delete_file( - attachment['filepath']) + try: + mg_globals.public_store.delete_file( + attachment['filepath']) + except OSError: + noSuchFiles.append(_jointhat(attachment)) + + if noSuchFiles: + # This breaks pep8 as far as I know + raise OSError, ", ".join(noSuchFiles) diff --git a/mediagoblin/user_pages/views.py b/mediagoblin/user_pages/views.py index dad68ba5..870e2155 100644 --- a/mediagoblin/user_pages/views.py +++ b/mediagoblin/user_pages/views.py @@ -15,6 +15,7 @@ # along with this program. If not, see . from webob import exc +import logging from mediagoblin import messages, mg_globals from mediagoblin.db.util import DESCENDING, ObjectId @@ -33,6 +34,9 @@ from werkzeug.contrib.atom import AtomFeed from mediagoblin.media_types import get_media_manager +_log = logging.getLogger(__name__) +_log.setLevel(logging.DEBUG) + @uses_pagination def user_home(request, page): """'Homepage' of a User()""" @@ -185,7 +189,14 @@ def media_confirm_delete(request, media): comment.delete() # Delete all files on the public storage - delete_media_files(media) + try: + delete_media_files(media) + except OSError, error: + _log.error('No such files from the user "{1}"' + ' to delete: {0}'.format(str(error), username)) + messages.add_message(request, messages.ERROR, + _('Some of the files with this entry seem' + ' to be missing. Deleting anyway.')) media.delete() messages.add_message( -- 2.25.1