From df5b142ab9bfc590f17768079104f6cfa2cd7bba Mon Sep 17 00:00:00 2001 From: Elrond Date: Mon, 18 Feb 2013 14:46:28 +0100 Subject: [PATCH] Fix deleting media with attachments. If one deletes a media with attachments, there have been various problems: 1) If the file in the storage did not exist any more (maybe because due to a previous deletion attempt?), the error propagation failed, because the wrong thing was gathered. 2) The attachment database entries were not deleted. Using cascade for this, for now. Also add a simple unit test, that tests both by having a broken attachment on a media. --- mediagoblin/db/models.py | 1 + mediagoblin/tests/test_misc.py | 15 +++++++++++++++ mediagoblin/tools/files.py | 2 +- 3 files changed, 17 insertions(+), 1 deletion(-) diff --git a/mediagoblin/db/models.py b/mediagoblin/db/models.py index 10e0c33f..2f58503f 100644 --- a/mediagoblin/db/models.py +++ b/mediagoblin/db/models.py @@ -145,6 +145,7 @@ class MediaEntry(Base, MediaEntryMixin): ) attachment_files_helper = relationship("MediaAttachmentFile", + cascade="all, delete-orphan", order_by="MediaAttachmentFile.created" ) attachment_files = association_proxy("attachment_files_helper", "dict_view", diff --git a/mediagoblin/tests/test_misc.py b/mediagoblin/tests/test_misc.py index b48b8762..776affc6 100644 --- a/mediagoblin/tests/test_misc.py +++ b/mediagoblin/tests/test_misc.py @@ -78,3 +78,18 @@ def test_user_deletes_other_comments(): assert_equal(med_cnt2, med_cnt1 - 2) # All comments gone assert_equal(cmt_cnt2, cmt_cnt1 - 4) + + +def test_media_deletes_broken_attachment(): + user_a = fixture_add_user(u"chris_a") + + media = fixture_media_entry(uploader=user_a.id, save=False) + media.attachment_files.append(dict( + name=u"some name", + filepath=[u"does", u"not", u"exist"], + )) + Session.add(media) + Session.flush() + + MediaEntry.query.get(media.id).delete() + User.query.get(user_a.id).delete() diff --git a/mediagoblin/tools/files.py b/mediagoblin/tools/files.py index fd38f05e..848c86f2 100644 --- a/mediagoblin/tools/files.py +++ b/mediagoblin/tools/files.py @@ -37,7 +37,7 @@ def delete_media_files(media): mg_globals.public_store.delete_file( attachment['filepath']) except OSError: - no_such_files.append("/".join(attachment)) + no_such_files.append("/".join(attachment['filepath'])) if no_such_files: raise OSError(", ".join(no_such_files))