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.

diff --git a/mediagoblin/db/models.py b/mediagoblin/db/models.py
index 10e0c33f4937fbb5d32ad33e3cfd440d49d70744..2f58503fca5487418b33d394e5070168e37b2351 100644 (file)
--- 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 b48b876213117594a7d067083a0d182a313c15c9..776affc65afaea8d227e52e58402c58cd5e2a866 100644 (file)
--- 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 fd38f05eda6c0fb59d5866d5feeb6f43a5746fd8..848c86f24fd467c3eb7789755c5edb26df56749c 100644 (file)
--- 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))