From 33052eed48dc01a311aa57462d3a64595b74e743 Mon Sep 17 00:00:00 2001
From: Jonas Metzener <jonas.metzener@adfinis.com>
Date: Fri, 21 Jan 2022 11:33:18 +0100
Subject: [PATCH] fix(cleanup): convert cleanup migration to command (#467)

The cleanup migration is too dangerous since it will possibly delete
production data if something goes wrong. This commit converts the
migration to a command which can be called manually and has a dry run
option to test it beforehand.
---
 .../api/management/__init__.py                |  0
 .../api/management/commands/__init__.py       |  0
 .../commands/clean_dangling_files.py          | 39 +++++++++++++++++++
 .../api/migrations/0004_cleanup_files.py      | 26 +------------
 .../api/tests/test_clean_dangling_files.py    | 34 ++++++++++++++++
 5 files changed, 75 insertions(+), 24 deletions(-)
 create mode 100644 document_merge_service/api/management/__init__.py
 create mode 100644 document_merge_service/api/management/commands/__init__.py
 create mode 100644 document_merge_service/api/management/commands/clean_dangling_files.py
 create mode 100644 document_merge_service/api/tests/test_clean_dangling_files.py

diff --git a/document_merge_service/api/management/__init__.py b/document_merge_service/api/management/__init__.py
new file mode 100644
index 00000000..e69de29b
diff --git a/document_merge_service/api/management/commands/__init__.py b/document_merge_service/api/management/commands/__init__.py
new file mode 100644
index 00000000..e69de29b
diff --git a/document_merge_service/api/management/commands/clean_dangling_files.py b/document_merge_service/api/management/commands/clean_dangling_files.py
new file mode 100644
index 00000000..4f7c8dab
--- /dev/null
+++ b/document_merge_service/api/management/commands/clean_dangling_files.py
@@ -0,0 +1,39 @@
+import os
+
+from django.conf import settings
+from django.core.management.base import BaseCommand
+
+from document_merge_service.api.models import Template
+
+
+class Command(BaseCommand):
+    help = "Remove dangling template files that are not attached to a template model anymore"
+
+    def add_arguments(self, parser):
+        parser.add_argument("--dry-run", dest="dry", action="store_true", default=False)
+
+    def handle(self, *args, **options):
+        used_files = [template.template.path for template in Template.objects.all()]
+
+        for subdir, dirs, files in os.walk(settings.MEDIA_ROOT):
+            for f in files:
+                path = os.path.join(subdir, f)
+                if path not in used_files and os.path.isfile(path):
+                    try:
+                        if not options.get("dry"):
+                            os.remove(path)
+                            self.stdout.write(
+                                self.style.SUCCESS(f"Deleted dangling file '{path}'")
+                            )
+                        else:
+                            self.stdout.write(
+                                self.style.WARNING(
+                                    f"Would delete dangling file '{path}'"
+                                )
+                            )
+                    except Exception as e:  # pragma: no cover
+                        self.stdout.write(
+                            self.style.ERROR(
+                                f"Could not delete dangling file '{path}': {str(e)}"
+                            )
+                        )
diff --git a/document_merge_service/api/migrations/0004_cleanup_files.py b/document_merge_service/api/migrations/0004_cleanup_files.py
index b163cc22..b5c2b12e 100644
--- a/document_merge_service/api/migrations/0004_cleanup_files.py
+++ b/document_merge_service/api/migrations/0004_cleanup_files.py
@@ -1,28 +1,6 @@
-import os
-import logging
-
-
 from django.db import migrations
-from django.conf import settings
-
-
-def cleanup_files(apps, schema_editor):
-    Template = apps.get_model("api", "Template")
-    used_files = [template.template.path for template in Template.objects.all()]
-
-    with os.scandir(settings.MEDIA_ROOT) as files:
-        for f in files:
-            if not f.path in used_files and os.path.isfile(f.path):
-                try:
-                    os.remove(f.path)
-                except Exception as e:
-                    pass  # ignore permission issues when running this in a  fresh container
 
 
 class Migration(migrations.Migration):
-
-    dependencies = [
-        ("api", "0003_template_meta"),
-    ]
-
-    operations = [migrations.RunPython(cleanup_files, migrations.RunPython.noop)]
+    dependencies = [("api", "0003_template_meta")]
+    operations = []
diff --git a/document_merge_service/api/tests/test_clean_dangling_files.py b/document_merge_service/api/tests/test_clean_dangling_files.py
new file mode 100644
index 00000000..4ee4716c
--- /dev/null
+++ b/document_merge_service/api/tests/test_clean_dangling_files.py
@@ -0,0 +1,34 @@
+import os
+
+import pytest
+from django.core.management import call_command
+
+from document_merge_service.api.data import django_file
+
+
+@pytest.mark.parametrize("dry", [True, False])
+def test_clean_dangling_files(db, dry, settings, template_factory):
+    templates = [
+        template_factory(template=django_file("docx-template.docx")),
+        template_factory(template=django_file("docx-template-syntax.docx")),
+    ]
+    dangling_files = [
+        django_file("docx-template-filters.docx"),
+        django_file("docx-template-loopcontrols.docx"),
+    ]
+
+    call_command("clean_dangling_files", dry=dry)
+
+    assert (
+        all(
+            [
+                os.path.isfile(os.path.join(settings.MEDIA_ROOT, file.name)) is dry
+                for file in dangling_files
+            ]
+        )
+        is True
+    )
+    assert (
+        all([os.path.isfile(template.template.path) is True for template in templates])
+        is True
+    )