fix kimetrica#65: handle empty MEDIA_ROOT settings and add settings checks

Author: olivierdalang

binary_database_files/apps.py

@@ -1,4 +1,6 @@
 from django.apps import AppConfig
+from django.conf import settings
+from django.core.checks import Error, register
 
 
 class DatabaseFilesAppConfig(AppConfig):
@@ -7,3 +9,25 @@ class DatabaseFilesAppConfig(AppConfig):
     name = "binary_database_files"
     label = "binary_database_files"
     verbose_name = "django-binary-database-files"
+
+
+@register()
+def check_settings(app_configs, **kwargs):
+    errors = []
+    if not settings.MEDIA_ROOT and settings.DATABASE_FILES_URL_METHOD_NAME == "URL_METHOD_1":
+        errors.append(
+            Error(
+                "MEDIA_ROOT is not defined, yet you are using URL_METHOD_1 which serves media files from the filesystem.",
+                hint="If you intend to only serve files from the database, use URL_METHOD_2.",
+                id="binary_database_files.E001",
+            )
+        )
+    if not settings.MEDIA_ROOT and settings.DB_FILES_AUTO_EXPORT_DB_TO_FS:
+        errors.append(
+            Error(
+                "MEDIA_ROOT is not defined, yet you are using DB_FILES_AUTO_EXPORT_DB_TO_FS which copies media files from the filesystem.",
+                hint="If you intend to only serve files from the database, set DB_FILES_AUTO_EXPORT_DB_TO_FS to False.",
+                id="binary_database_files.E002",
+            )
+        )
+    return errors

binary_database_files/tests/tests.py

@@ -6,13 +6,15 @@
 from io import BytesIO
 from zipfile import ZipFile
 
+from django.conf import global_settings
 from django.conf import settings
 from django.core import files
 from django.core.files import File as DjangoFile
 from django.core.files.base import ContentFile
 from django.core.files.storage import default_storage
 from django.core.files.temp import NamedTemporaryFile
 from django.core.management import call_command
+from django.core.management.base import SystemCheckError
 from django.db import models
 from django.test import TestCase, override_settings
 
@@ -420,3 +422,19 @@ def test_serve_file_from_database(self):
         self.assertEqual(content, b"1234567890")
         self.assertEqual(response["content-type"], "text/plain")
         self.assertEqual(response["content-length"], "10")
+
+    @override_settings(
+            MEDIA_ROOT=global_settings.MEDIA_ROOT,
+            DATABASE_FILES_URL_METHOD_NAME = "URL_METHOD_1", # default
+            DB_FILES_AUTO_EXPORT_DB_TO_FS = True, # default
+    )
+    def test_refuse_unset_media_root(self):
+        # regression test for issue #65 where unset MEDIA_ROOT would result in serving the source code
+
+        message_a = "(binary_database_files.E001) MEDIA_ROOT is not defined, yet you are using URL_METHOD_1 which serves media files from the filesystem"
+        with self.assertRaisesMessage(SystemCheckError, message_a):
+                call_command("check")
+
+        message_b = "(binary_database_files.E002) MEDIA_ROOT is not defined, yet you are using DB_FILES_AUTO_EXPORT_DB_TO_FS which copies media files from the filesystem."
+        with self.assertRaisesMessage(SystemCheckError, message_b):
+                call_command("check")

binary_database_files/views.py

@@ -27,15 +27,20 @@ def serve(request, name):
 
 def serve_mixed(request, *args, **kwargs):
     """
-    First attempts to serve the file from the filesystem,
+    If document_root or MEDIA_ROOT is set, attempts to serve the file from the filesystem,
     then tries the database.
     """
     name = kwargs.get("name") or kwargs.get("path")
     document_root = kwargs.get("document_root")
     document_root = document_root or settings.MEDIA_ROOT
-    try:
-        # First attempt to serve from filesystem.
-        return django_serve(request, name, document_root)
-    except Http404:
-        # Then try serving from database.
-        return serve(request, name)
+
+    # empty document_root would result in serving the source code
+    if document_root:
+        try:
+            # First attempt to serve from filesystem.
+            return django_serve(request, name, document_root)
+        except Http404:
+            pass
+
+    # Then try serving from database.
+    return serve(request, name)