From 4589dcba025f82ffc23726f7284856755268ab10 Mon Sep 17 00:00:00 2001 From: Jonas Metzener Date: Tue, 8 Aug 2023 18:05:32 +0200 Subject: [PATCH] fix: improve command to upload local template files to storage backend --- document_merge_service/api/data/__init__.py | 13 ++-- .../commands/upload_local_templates.py | 61 ++++++++++++++----- .../api/tests/test_upload_local_templates.py | 56 ++++++++++++++--- 3 files changed, 104 insertions(+), 26 deletions(-) diff --git a/document_merge_service/api/data/__init__.py b/document_merge_service/api/data/__init__.py index bedabf91..e2fd6ede 100644 --- a/document_merge_service/api/data/__init__.py +++ b/document_merge_service/api/data/__init__.py @@ -7,15 +7,20 @@ _data_path = os.path.dirname(os.path.realpath(__file__)) -def django_file(name, mode="rb"): +def django_file(name, mode="rb", new_path=None, new_name=None): abspath = os.path.join(_data_path, name) - new_path = f"{settings.MEDIA_ROOT}/attachments" + + if not new_path: + new_path = settings.MEDIA_ROOT + + if not new_name: + new_name = name try: os.makedirs(new_path) except FileExistsError: # pragma: no cover pass - shutil.copy(abspath, f"{new_path}/{name}") + shutil.copy(abspath, f"{new_path}/{new_name}") - return File(open(abspath, mode), name=f"attachments/{name}") + return File(open(abspath, mode), name=new_name) diff --git a/document_merge_service/api/management/commands/upload_local_templates.py b/document_merge_service/api/management/commands/upload_local_templates.py index dcfaaebc..0fd60408 100644 --- a/document_merge_service/api/management/commands/upload_local_templates.py +++ b/document_merge_service/api/management/commands/upload_local_templates.py @@ -1,6 +1,6 @@ +import glob import os -from django.conf import settings from django.core.files.storage import DefaultStorage from django.core.management.base import BaseCommand @@ -8,28 +8,59 @@ class Command(BaseCommand): - help = "Add filesystem files to configured storage backend" + help = "Upload local template files to configured storage backend" def add_arguments(self, parser): - parser.add_argument("--dry-run", dest="dry", action="store_true", default=False) + parser.add_argument( + "-s", + "--source", + help="Glob-style path to the template files that should be uploaded. E.g. `/tmp/templates/*.docx`", + dest="source", + type=str, + required=True, + ) + parser.add_argument( + "--dry-run", + help="Only show what files would be uploaded to the storage backend; don't actually upload them.", + dest="dry", + action="store_true", + default=False, + ) def handle(self, *args, **options): storage = DefaultStorage() - for template in Template.objects.all(): + + for path in glob.iglob(options["source"]): + filename = os.path.basename(path) + try: - with open( - os.path.join(settings.MEDIA_ROOT, template.template.name), "rb" - ) as file: - if not options.get("dry"): + template = Template.objects.get(template=filename) + except Template.DoesNotExist: + self.stdout.write( + self.style.WARNING(f'No template for filename "{filename}" found') + ) + continue + + if not options.get("dry"): + try: + with open(path, "rb") as file: + storage.delete(template.template.name) storage.save(template.template.name, file) - self.stdout.write( - self.style.SUCCESS(f"Uploaded file for '{template.pk}'") + + self.stdout.write( + self.style.SUCCESS( + f'Uploaded file for template "{template.pk}"' ) - else: - self.stdout.write( - self.style.WARNING(f"Would upload file for '{template.pk}'") + ) + except Exception as e: # pragma: no cover + self.stdout.write( + self.style.ERROR( + f'Could not upload file for template "{template.pk}": {str(e)}' ) - except Exception as e: # pragma: no cover + ) + else: self.stdout.write( - self.style.ERROR(f"Could not upload file '{template.pk}': {str(e)}") + self.style.WARNING( + f'Would upload file for template "{template.pk}"' + ) ) diff --git a/document_merge_service/api/tests/test_upload_local_templates.py b/document_merge_service/api/tests/test_upload_local_templates.py index e91cf70e..9760d380 100644 --- a/document_merge_service/api/tests/test_upload_local_templates.py +++ b/document_merge_service/api/tests/test_upload_local_templates.py @@ -1,21 +1,63 @@ +import hashlib import os +import shutil import pytest from django.core.management import call_command +from faker import Faker from document_merge_service.api.data import django_file +def sha256sum(path): + with open(path, "rb") as file: + return hashlib.sha256(file.read()).hexdigest() + + +def is_equal(a, b): + return sha256sum(a) == sha256sum(b) + + +@pytest.fixture +def tmp_path(settings): + path = os.path.join(settings.MEDIA_ROOT, f"local-templates-{Faker().uuid4()}") + + yield path + + shutil.rmtree(path, ignore_errors=True) + + @pytest.mark.parametrize("dry", [True, False]) -def test_upload_local_templates(db, dry, settings, template_factory): +def test_upload_local_templates(db, dry, template_factory, tmp_path): templates = [ - template_factory(template=django_file("docx-template.docx")), template_factory(template=django_file("docx-template-syntax.docx")), + template_factory(template=django_file("docx-template-syntax.docx")), + ] + + files = [ + django_file( + "docx-template.docx", + new_path=tmp_path, + new_name=templates[0].template.name, + ), + django_file( + "docx-template.docx", + new_path=tmp_path, + new_name=templates[1].template.name, + ), + django_file( + "docx-template.docx", + new_path=tmp_path, + new_name="some-file-without-template.docx", + ), ] - call_command("upload_local_templates", dry=dry) + paths = [os.path.join(tmp_path, file.name) for file in files] + + assert not is_equal(templates[0].template.path, paths[0]) + assert not is_equal(templates[1].template.path, paths[1]) + + call_command("upload_local_templates", dry=dry, source=f"{tmp_path}/*.docx") - assert ( - all([os.path.isfile(template.template.path) is True for template in templates]) - is True - ) + assert is_equal(templates[0].template.path, paths[0]) != dry + assert is_equal(templates[1].template.path, paths[1]) != dry