Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions binary_database_files/apps.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
from django.apps import AppConfig
from django.conf import settings
from django.core.checks import Error, register


class DatabaseFilesAppConfig(AppConfig):
Expand All @@ -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
18 changes: 18 additions & 0 deletions binary_database_files/tests/tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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")
19 changes: 12 additions & 7 deletions binary_database_files/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)