From 86a51c6fa56df2ac206ffad2b62f2f5d785c3877 Mon Sep 17 00:00:00 2001
From: Wolf-Bastian Poettner <bastian@poettner.de>
Date: Sat, 1 Feb 2020 20:03:20 +0000
Subject: [PATCH] Refactored delete_empty_directory into
 try_delete_empty_directories and implemented feature to ensure, that all
 created and now empty directories are really deleted

---
 src/documents/models.py                   | 37 +++++++++++++----------
 src/documents/tests/test_file_handling.py | 31 +++++++++++++++++++
 2 files changed, 52 insertions(+), 16 deletions(-)

diff --git a/src/documents/models.py b/src/documents/models.py
index d016c32e7..58206ba11 100644
--- a/src/documents/models.py
+++ b/src/documents/models.py
@@ -294,7 +294,7 @@ class Document(models.Model):
             key = t.name[:delimeter]
             value = t.name[delimeter+1:]
 
-            mydictionary[key] = value
+            mydictionary[key] = slugify(value)
 
         return mydictionary
 
@@ -302,10 +302,10 @@ class Document(models.Model):
         # Create directory name based on configured format
         if settings.PAPERLESS_DIRECTORY_FORMAT is not None:
             directory = settings.PAPERLESS_DIRECTORY_FORMAT.format(
-                        correspondent=self.correspondent,
-                        title=self.title,
-                        created=self.created,
-                        added=self.added,
+                        correspondent=slugify(self.correspondent),
+                        title=slugify(self.title),
+                        created=slugify(self.created),
+                        added=slugify(self.added),
                         tags=defaultdict(str,
                                          self.many_to_dictionary(self.tags)))
         else:
@@ -314,16 +314,16 @@ class Document(models.Model):
         # Create filename based on configured format
         if settings.PAPERLESS_FILENAME_FORMAT is not None:
             filename = settings.PAPERLESS_FILENAME_FORMAT.format(
-                        correspondent=self.correspondent,
-                        title=self.title,
-                        created=self.created,
-                        added=self.added,
+                        correspondent=slugify(self.correspondent),
+                        title=slugify(self.title),
+                        created=slugify(self.created),
+                        added=slugify(self.added),
                         tags=defaultdict(str,
                                          self.many_to_dictionary(self.tags)))
         else:
             filename = ""
 
-        path = os.path.join(slugify(directory), slugify(filename))
+        path = os.path.join(directory, filename)
 
         # Always append the primary key to guarantee uniqueness of filename
         if len(path) > 0:
@@ -398,13 +398,18 @@ class Document(models.Model):
             self.filename = filename
 
 
-def delete_empty_directory(directory):
-    if len(os.listdir(directory)) == 0:
+def try_delete_empty_directories(directory):
+    # Go up in the directory hierarchy and try to delete all directories
+    while directory != Document.filename_to_path(""):
+        # Try to delete the current directory
         try:
             os.rmdir(directory)
         except os.error:
-            # Directory not empty
-            pass
+            # Directory not empty, no need to go further up
+            return
+
+        # Cut off actual directory and go one level up
+        directory, tmp = os.path.split(directory)
 
 
 @receiver(models.signals.m2m_changed, sender=Document.tags.through)
@@ -441,7 +446,7 @@ def update_filename(sender, instance, **kwargs):
     # Delete empty directory
     old_dir = os.path.dirname(instance.filename)
     old_path = instance.filename_to_path(old_dir)
-    delete_empty_directory(old_path)
+    try_delete_empty_directories(old_path)
 
     instance.filename = new_filename
 
@@ -469,7 +474,7 @@ def delete_files(sender, instance, **kwargs):
     # And remove the directory (if applicable)
     old_dir = os.path.dirname(instance.filename)
     old_path = instance.filename_to_path(old_dir)
-    delete_empty_directory(old_path)
+    try_delete_empty_directories(old_path)
 
 
 class Log(models.Model):
diff --git a/src/documents/tests/test_file_handling.py b/src/documents/tests/test_file_handling.py
index a4d842236..46f5bbc9d 100644
--- a/src/documents/tests/test_file_handling.py
+++ b/src/documents/tests/test_file_handling.py
@@ -145,6 +145,37 @@ class TestDate(TestCase):
                   "/documents/originals/none/none-0000001.pdftest")
         os.rmdir(settings.MEDIA_ROOT + "/documents/originals/none")
 
+
+    @override_settings(MEDIA_ROOT="/tmp/paperless-tests-{}".
+                       format(str(uuid4())[:8]))
+    @override_settings(PAPERLESS_DIRECTORY_FORMAT="{correspondent}/{correspondent}")
+    @override_settings(PAPERLESS_FILENAME_FORMAT="{correspondent}")
+    def test_nested_directory_cleanup(self):
+        document = Document()
+        document.file_type = "pdf"
+        document.storage_type = Document.STORAGE_TYPE_UNENCRYPTED
+        document.save()
+
+        # Ensure that filename is properly generated
+        tmp = document.source_filename
+        self.assertEqual(document.generate_source_filename(),
+                         "none/none/none-0000001.pdf")
+        document.create_source_directory()
+        Path(document.source_path).touch()
+
+        # Check proper handling of files
+        self.assertEqual(os.path.isdir(settings.MEDIA_ROOT +
+                         "/documents/originals/none/none"), True)
+
+        document.delete()
+
+        self.assertEqual(os.path.isfile(settings.MEDIA_ROOT +
+                         "/documents/originals/none/none/none-0000001.pdf"), False)
+        self.assertEqual(os.path.isdir(settings.MEDIA_ROOT +
+                         "/documents/originals/none/none"), False)
+        self.assertEqual(os.path.isdir(settings.MEDIA_ROOT +
+                         "/documents/originals/none"), False)
+
     @override_settings(MEDIA_ROOT="/tmp/paperless-tests-{}".
                        format(str(uuid4())[:8]))
     @override_settings(PAPERLESS_DIRECTORY_FORMAT=None)