fix(cleanup): delete old files when template is deleted or changed (#445

)

Resolves #437

document_merge_service/api/data/__init__.py

@@ -1,14 +1,21 @@
 import os.path
+import shutil
 
+from django.conf import settings
 from django.core.files import File
 
 _data_path = os.path.dirname(os.path.realpath(__file__))
 
 
-def path(name):
-    return os.path.join(_data_path, name)
+def django_file(name, mode="rb"):
+    abspath = os.path.join(_data_path, name)
+    new_path = f"{settings.MEDIA_ROOT}/attachments"
 
+    try:
+        os.makedirs(new_path)
+    except FileExistsError:  # pragma: no cover
+        pass
 
-def django_file(name, mode="rb"):
-    abspath = path(name)
-    return File(open(abspath, mode))
+    shutil.copy(abspath, f"{new_path}/{name}")
+
+    return File(open(abspath, mode), name=f"attachments/{name}")

document_merge_service/api/migrations/0004_cleanup_files.py

@@ -0,0 +1,28 @@
+import os
+import logging
+
+
+from django.db import migrations
+from django.conf import settings
+
+logger = logging.getLogger(__name__)
+logger.setLevel(logging.DEBUG)
+
+
+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):
+                os.remove(f.path)
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ("api", "0003_template_meta"),
+    ]
+
+    operations = [migrations.RunPython(cleanup_files, migrations.RunPython.noop)]

document_merge_service/api/models.py

@@ -1,4 +1,7 @@
+import os
+
 from django.db import models
+from django.dispatch import receiver
 
 
 class Template(models.Model):
@@ -22,3 +25,27 @@ class Template(models.Model):
     engine = models.CharField(max_length=20, choices=ENGINE_CHOICES_TUPLE)
     group = models.CharField(max_length=255, db_index=True, blank=True, null=True)
     meta = models.JSONField(default=dict)
+
+
+@receiver(models.signals.post_delete, sender=Template)
+def auto_delete_file_on_delete(sender, instance, **kwargs):
+    """Delete template file from filesystem when `Template` object is deleted."""
+
+    if os.path.isfile(instance.template.path):
+        os.remove(instance.template.path)
+
+
+@receiver(models.signals.pre_save, sender=Template)
+def auto_delete_file_on_change(sender, instance, **kwargs):
+    """Delete old template file from filesystem when `Template` is given a new template file."""
+
+    try:
+        old_file = Template.objects.get(pk=instance.pk).template
+    except Template.DoesNotExist:
+        return
+
+    if old_file:
+        new_file = instance.template
+        if not old_file == new_file:
+            if os.path.isfile(old_file.path):
+                os.remove(old_file.path)

document_merge_service/api/tests/snapshots/snap_test_template.py

@@ -691,7 +691,7 @@
 """
 
 snapshots[
-    "test_template_merge_jinja_filters_docx[docx-template-False-False-200] 1"
+    "test_template_merge_jinja_filters_docx[docx-template-template__template0-False-False-200] 1"
 ] = """<w:body xmlns:w="http://schemas.openxmlformats.org/wordprocessingml/2006/main" xmlns:o="urn:schemas-microsoft-com:office:office" xmlns:r="http://schemas.openxmlformats.org/officeDocument/2006/relationships" xmlns:v="urn:schemas-microsoft-com:vml" xmlns:w10="urn:schemas-microsoft-com:office:word" xmlns:wp="http://schemas.openxmlformats.org/drawingml/2006/wordprocessingDrawing" xmlns:wps="http://schemas.microsoft.com/office/word/2010/wordprocessingShape" xmlns:wpg="http://schemas.microsoft.com/office/word/2010/wordprocessingGroup" xmlns:mc="http://schemas.openxmlformats.org/markup-compatibility/2006" xmlns:wp14="http://schemas.microsoft.com/office/word/2010/wordprocessingDrawing" xmlns:w14="http://schemas.microsoft.com/office/word/2010/wordml">
   <w:p>
     <w:pPr>

document_merge_service/api/tests/test_template.py

@@ -1,5 +1,6 @@
 import io
 import json
+import os
 
 import pytest
 from django.urls import reverse
@@ -462,7 +463,10 @@ def test_template_create_with_available_placeholders(
         Document(io.BytesIO(response.content))
 
 
-@pytest.mark.parametrize("template__engine", [models.Template.DOCX_TEMPLATE])
+@pytest.mark.parametrize(
+    "template__engine,template__template",
+    [(models.Template.DOCX_TEMPLATE, django_file("docx-template-filters.docx"))],
+)
 @pytest.mark.parametrize(
     "template_name,status_code",
     [
@@ -479,15 +483,18 @@ def test_template_update(db, client, template, template_name, status_code):
     assert response.status_code == status_code
 
     if status_code == status.HTTP_200_OK:
+        assert os.path.isfile(template.template.path) is False
         template.refresh_from_db()
         assert template.description == "Test description"
 
 
+@pytest.mark.parametrize("template__template", [django_file("docx-template.docx")])
 def test_template_destroy(db, client, template):
     url = reverse("template-detail", args=[template.pk])
 
     response = client.delete(url)
     assert response.status_code == status.HTTP_204_NO_CONTENT
+    assert os.path.isfile(template.template.path) is False
 
 
 @pytest.mark.parametrize(
@@ -652,8 +659,8 @@ def test_template_merge_jinja_extensions_docx(
     ],
 )
 @pytest.mark.parametrize(
-    "template__engine",
-    [models.Template.DOCX_TEMPLATE],
+    "template__engine,template__template",
+    [(models.Template.DOCX_TEMPLATE, django_file("docx-template-filters.docx"))],
 )
 def test_template_merge_jinja_filters_docx(
     db,
@@ -670,11 +677,6 @@ def test_template_merge_jinja_filters_docx(
     settings.LANGUAGE_CODE = "de-ch"
     url = reverse("template-merge", args=[template.pk])
 
-    # Couldn't put this into `parametrize`. For some reason, in the second run, the
-    # template name is extended with a seemingly random string.
-    template.template = django_file("docx-template-filters.docx")
-    template.save()
-
     data = {
         "data": json.dumps(
             {
@@ -715,8 +717,8 @@ def test_template_merge_jinja_filters_docx(
 
 
 @pytest.mark.parametrize(
-    "template__engine",
-    [models.Template.DOCX_TEMPLATE],
+    "template__engine,template__template",
+    [(models.Template.DOCX_TEMPLATE, django_file("docx-template-filters.docx"))],
 )
 @pytest.mark.parametrize(
     "file_value",
@@ -733,11 +735,6 @@ def test_template_merge_file_reset(
     settings.LANGUAGE_CODE = "de-ch"
     url = reverse("template-merge", args=[template.pk])
 
-    # Couldn't put this into `parametrize`. For some reason, in the second run, the
-    # template name is extended with a seemingly random string.
-    template.template = django_file("docx-template-filters.docx")
-    template.save()
-
     data = {
         "data": {
             "test_date": "1984-09-15",

document_merge_service/conftest.py

@@ -76,10 +76,11 @@ def mock_filefield_name_validation(mocker):
 def docx_template_with_placeholder(admin_client, template):
     """Return a factory function to build a docx template with a given placeholder."""
     template.engine = models.Template.DOCX_TEMPLATE
+    template.template = django_file("docx-template.docx")
     template.save()
 
     def make_template(placeholder):
-        engine = engines.get_engine(template.engine, django_file("docx-template.docx"))
+        engine = engines.get_engine(template.engine, template.template)
         binary = BytesIO()
         engine.merge({"test": placeholder}, binary)
         binary.seek(0)

requirements-dev.txt

@@ -1,6 +1,5 @@
 -r requirements.txt
 black==21.10b0
-dj-inmemorystorage==2.1.0
 factory-boy==3.2.1
 flake8==4.0.1
 flake8-debugger==4.0.0

setup.cfg

@@ -49,7 +49,6 @@ filterwarnings =
     error::PendingDeprecationWarning
 env =
     ADMINS=Test Example <test@example.com>,Test2 <test2@example.com>
-    FILE_STORAGE=inmemorystorage.InMemoryStorage
     OIDC_USERINFO_ENDPOINT=mock://document-merge-service.github.com/openid/userinfo
     OIDC_BEARER_TOKEN_REVALIDATION_TIME=60