From ddaf070ad7e8819f539d2d9dedc73f4e26a19848 Mon Sep 17 00:00:00 2001 From: Jorge Araya Navarro Date: Fri, 22 Jun 2012 15:15:57 -0600 Subject: [PATCH 1/3] Bug 255 fixed --- mediagoblin/storage/filestorage.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/mediagoblin/storage/filestorage.py b/mediagoblin/storage/filestorage.py index 00d6335e..1252a3f5 100644 --- a/mediagoblin/storage/filestorage.py +++ b/mediagoblin/storage/filestorage.py @@ -64,7 +64,12 @@ class BasicFileStorage(StorageInterface): def delete_file(self, filepath): # TODO: Also delete unused directories if empty (safely, with # checks to avoid race conditions). - os.remove(self._resolve_filepath(filepath)) + try: + os.remove(self._resolve_filepath(filepath)) + except OSError: + # the file do not exists! + # This should fix bug #255 + pass def file_url(self, filepath): if not self.base_url: From fb2fbe2c0a0f6944696e3f1994a94f1be3ac0535 Mon Sep 17 00:00:00 2001 From: Jorge Araya Navarro Date: Thu, 28 Jun 2012 22:13:26 -0600 Subject: [PATCH 2/3] 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( From 6d539afda68c5c3b14b744225b6037ce4ea11423 Mon Sep 17 00:00:00 2001 From: Jorge Araya Navarro Date: Thu, 5 Jul 2012 22:07:44 -0600 Subject: [PATCH 3/3] changing NoSuchFiles for no_such_files --- mediagoblin/tools/files.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/mediagoblin/tools/files.py b/mediagoblin/tools/files.py index 31a7a972..2b4ad4e4 100644 --- a/mediagoblin/tools/files.py +++ b/mediagoblin/tools/files.py @@ -36,21 +36,21 @@ def delete_media_files(media): Arguments: - media: A MediaEntry document """ - noSuchFiles = [] + no_such_files = [] for listpath in media.media_files.itervalues(): try: mg_globals.public_store.delete_file( listpath) except OSError: - noSuchFiles.append(_jointhat(listpath)) + no_such_files.append(_jointhat(listpath)) for attachment in media.attachment_files: try: mg_globals.public_store.delete_file( attachment['filepath']) except OSError: - noSuchFiles.append(_jointhat(attachment)) + no_such_files.append(_jointhat(attachment)) - if noSuchFiles: + if no_such_files: # This breaks pep8 as far as I know raise OSError, ", ".join(noSuchFiles)