Move media operations to storages backend (#6478)

* [BUG] Inventree fiddles with files directly rather than using Django Storage api
Fixes #2585

* PEP fix

* clean diff

* move template discovery into central location

* more moving file operations

* fix paths

* and another path fixing

* more fixes

* fix typing

* switch config back to local

* revert locale stats

* remove dependency for template copy step

* fix typing

* log more

* log data

* more logging

* pass filenames as strings

* clean diff

InvenTree/InvenTree/files.py

@@ -0,0 +1,8 @@
+"""Helpers for file handling in InvenTree."""
+
+from pathlib import Path
+
+from django.conf import settings
+
+TEMPLATES_DIR = Path(__file__).parent.parent
+MEDIA_STORAGE_DIR = settings.MEDIA_ROOT

InvenTree/InvenTree/helpers.py

@@ -9,14 +9,15 @@ import os
 import os.path
 import re
 from decimal import Decimal, InvalidOperation
-from typing import TypeVar
+from pathlib import Path
+from typing import TypeVar, Union
 from wsgiref.util import FileWrapper
 
 import django.utils.timezone as timezone
 from django.conf import settings
 from django.contrib.staticfiles.storage import StaticFilesStorage
 from django.core.exceptions import FieldError, ValidationError
-from django.core.files.storage import default_storage
+from django.core.files.storage import Storage, default_storage
 from django.http import StreamingHttpResponse
 from django.utils.translation import gettext_lazy as _
 
@@ -861,9 +862,14 @@ def hash_barcode(barcode_data):
     return str(hash.hexdigest())
 
 
-def hash_file(filename: str):
+def hash_file(filename: Union[str, Path], storage: Union[Storage, None] = None):
     """Return the MD5 hash of a file."""
-    return hashlib.md5(open(filename, 'rb').read()).hexdigest()
+    content = (
+        open(filename, 'rb').read()
+        if storage is None
+        else storage.open(str(filename), 'rb').read()
+    )
+    return hashlib.md5(content).hexdigest()
 
 
 def server_timezone() -> str:

InvenTree/generic/templating/apps.py

@@ -1,11 +1,9 @@
 """Shared templating code."""
 
 import logging
-import os
 import warnings
 from pathlib import Path
 
-from django.conf import settings
 from django.core.exceptions import AppRegistryNotReady
 from django.core.files.storage import default_storage
 from django.db.utils import IntegrityError, OperationalError, ProgrammingError
@@ -18,9 +16,6 @@ from InvenTree.config import ensure_dir
 logger = logging.getLogger('inventree')
 
 
-MEDIA_STORAGE_DIR = Path(settings.MEDIA_ROOT)
-
-
 class TemplatingMixin:
     """Mixin that contains shared templating code."""
 
@@ -84,8 +79,7 @@ class TemplatingMixin:
 
         # Create root dir for templates
         src_dir = self.get_src_dir(ref_name)
-        dst_dir = MEDIA_STORAGE_DIR.joinpath(self.name, 'inventree', ref_name)
+        ensure_dir(Path(self.name, 'inventree', ref_name), default_storage)
-        ensure_dir(dst_dir, default_storage)
 
         # Copy each template across (if required)
         for entry in data:
@@ -94,29 +88,27 @@ class TemplatingMixin:
     def create_template_file(self, model, src_dir, data, ref_name):
         """Ensure a label template is in place."""
         # Destination filename
-        filename = os.path.join(self.name, 'inventree', ref_name, data['file'])
+        filename = Path(self.name, 'inventree', ref_name, data['file'])
-
         src_file = src_dir.joinpath(data['file'])
-        dst_file = MEDIA_STORAGE_DIR.joinpath(filename)
 
         do_copy = False
 
-        if not dst_file.exists():
+        if not default_storage.exists(filename):
             logger.info("%s template '%s' is not present", self.name, filename)
             do_copy = True
         else:
             # Check if the file contents are different
             src_hash = InvenTree.helpers.hash_file(src_file)
-            dst_hash = InvenTree.helpers.hash_file(dst_file)
+            dst_hash = InvenTree.helpers.hash_file(filename, default_storage)
 
             if src_hash != dst_hash:
                 logger.info("Hash differs for '%s'", filename)
                 do_copy = True
 
         if do_copy:
-            logger.info("Copying %s template '%s'", self.name, dst_file)
+            logger.info("Copying %s template '%s'", self.name, filename)
             # Ensure destination dir exists
-            dst_file.parent.mkdir(parents=True, exist_ok=True)
+            ensure_dir(filename.parent, default_storage)
 
             # Copy file
             default_storage.save(filename, src_file.open('rb'))
@@ -135,6 +127,8 @@ class TemplatingMixin:
         logger.info("Creating entry for %s '%s'", model, data.get('name'))
 
         try:
-            model.objects.create(**self.get_new_obj_data(data, filename))
+            model.objects.create(**self.get_new_obj_data(data, str(filename)))
-        except Exception:
+        except Exception as _e:
-            logger.warning("Failed to create %s '%s'", self.name, data['name'])
+            logger.warning(
+                "Failed to create %s '%s' with error '%s'", self.name, data['name'], _e
+            )

InvenTree/report/tests.py

@@ -3,7 +3,6 @@
 import os
 import shutil
 from io import StringIO
-from pathlib import Path
 
 from django.conf import settings
 from django.core.cache import cache
@@ -19,6 +18,7 @@ from PIL import Image
 import report.models as report_models
 from build.models import Build
 from common.models import InvenTreeSetting, InvenTreeUserSetting
+from InvenTree.files import MEDIA_STORAGE_DIR, TEMPLATES_DIR
 from InvenTree.unit_test import InvenTreeAPITestCase
 from report.templatetags import barcode as barcode_tags
 from report.templatetags import report as report_tags
@@ -249,11 +249,9 @@ class ReportTest(InvenTreeAPITestCase):
 
     def copyReportTemplate(self, filename, description):
         """Copy the provided report template into the required media directory."""
-        src_dir = Path(__file__).parent.joinpath('templates', 'report')
+        src_dir = TEMPLATES_DIR.joinpath('report', 'templates', 'report')
-
         template_dir = os.path.join('report', 'inventree', self.model.getSubdir())
-
+        dst_dir = MEDIA_STORAGE_DIR.joinpath(template_dir)
-        dst_dir = settings.MEDIA_ROOT.joinpath(template_dir)
 
         if not dst_dir.exists():  # pragma: no cover
             dst_dir.mkdir(parents=True, exist_ok=True)