From f5daded930afdfa1075249093a9e1eeea9d27919 Mon Sep 17 00:00:00 2001
From: Daniel Quinn <code@danielquinn.org>
Date: Tue, 16 Aug 2016 19:13:37 +0100
Subject: [PATCH] Fix for #131: delete files on document.delete

---
 src/documents/apps.py                      | 11 +++++++++--
 src/documents/signals/handlers.py          | 13 +++++++++----
 src/documents/tests/test_consumer.py       |  2 +-
 src/documents/tests/test_document_model.py | 20 ++++++++++++++++++++
 4 files changed, 39 insertions(+), 7 deletions(-)
 create mode 100644 src/documents/tests/test_document_model.py

diff --git a/src/documents/apps.py b/src/documents/apps.py
index 363f29a07..dd065c0a5 100644
--- a/src/documents/apps.py
+++ b/src/documents/apps.py
@@ -1,4 +1,5 @@
 from django.apps import AppConfig
+from django.db.models.signals import post_delete
 
 
 class DocumentsConfig(AppConfig):
@@ -10,8 +11,12 @@ class DocumentsConfig(AppConfig):
         from .signals import document_consumption_started
         from .signals import document_consumption_finished
         from .signals.handlers import (
-            set_correspondent, set_tags, run_pre_consume_script,
-            run_post_consume_script)
+            set_correspondent,
+            set_tags,
+            run_pre_consume_script,
+            run_post_consume_script,
+            cleanup_document_deletion
+        )
 
         document_consumption_started.connect(run_pre_consume_script)
 
@@ -19,4 +24,6 @@ class DocumentsConfig(AppConfig):
         document_consumption_finished.connect(set_correspondent)
         document_consumption_finished.connect(run_post_consume_script)
 
+        post_delete.connect(cleanup_document_deletion)
+
         AppConfig.ready(self)
diff --git a/src/documents/signals/handlers.py b/src/documents/signals/handlers.py
index d9ed9090f..f5fbe8527 100644
--- a/src/documents/signals/handlers.py
+++ b/src/documents/signals/handlers.py
@@ -1,4 +1,5 @@
 import logging
+import os
 
 from subprocess import Popen
 
@@ -62,10 +63,7 @@ def run_pre_consume_script(sender, filename, **kwargs):
     if not settings.PRE_CONSUME_SCRIPT:
         return
 
-    Popen((
-        settings.PRE_CONSUME_SCRIPT,
-        filename
-    )).wait()
+    Popen((settings.PRE_CONSUME_SCRIPT, filename)).wait()
 
 
 def run_post_consume_script(sender, document, **kwargs):
@@ -84,3 +82,10 @@ def run_post_consume_script(sender, document, **kwargs):
         str(document.correspondent),
         str(",".join(document.tags.all().values_list("slug", flat=True)))
     )).wait()
+
+
+def cleanup_document_deletion(sender, instance, using, **kwargs):
+    try:
+        os.unlink(instance.source_path)
+    except FileNotFoundError:
+        pass  # The file's already gone, so we're cool with it.
diff --git a/src/documents/tests/test_consumer.py b/src/documents/tests/test_consumer.py
index 48407044d..7bfde733a 100644
--- a/src/documents/tests/test_consumer.py
+++ b/src/documents/tests/test_consumer.py
@@ -1,6 +1,6 @@
 from django.test import TestCase
 
-from ..models import Document, FileInfo
+from ..models import FileInfo
 
 
 class TestAttachment(TestCase):
diff --git a/src/documents/tests/test_document_model.py b/src/documents/tests/test_document_model.py
new file mode 100644
index 000000000..f531f50bb
--- /dev/null
+++ b/src/documents/tests/test_document_model.py
@@ -0,0 +1,20 @@
+from unittest import mock
+
+from django.test import TestCase
+
+from ..models import Document, Correspondent
+
+
+class TestDocument(TestCase):
+
+    def test_file_deletion(self):
+        document = Document.objects.create(
+            correspondent=Correspondent.objects.create(name="Test0"),
+            title="Title",
+            content="content",
+            checksum="checksum",
+        )
+        file_path = document.source_path
+        with mock.patch("documents.signals.handlers.os.unlink") as mock_unlink:
+            document.delete()
+            mock_unlink.assert_called_with(file_path)