From 8e15220caaaef4711e0cf67e1488aedf6f194706 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dominic=20Viehb=C3=B6ck?= <48133589+domvie@users.noreply.github.com> Date: Tue, 20 Aug 2024 23:24:58 +0200 Subject: [PATCH 1/2] fixes for Django 5.1, add support for new STORAGES setting (#524) Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> --- dbbackup/management/commands/mediabackup.py | 3 +- dbbackup/management/commands/mediarestore.py | 4 +- dbbackup/settings.py | 16 ++++-- dbbackup/storage.py | 28 ++++++++++- dbbackup/tests/commands/test_mediabackup.py | 3 +- dbbackup/tests/settings.py | 12 +++++ dbbackup/tests/test_storage.py | 51 +++++++++++++++++++- docs/changelog.rst | 2 + docs/storage.rst | 32 +++++++++++- tox.ini | 9 ++-- 10 files changed, 143 insertions(+), 17 deletions(-) diff --git a/dbbackup/management/commands/mediabackup.py b/dbbackup/management/commands/mediabackup.py index aea73201..8492da2f 100644 --- a/dbbackup/management/commands/mediabackup.py +++ b/dbbackup/management/commands/mediabackup.py @@ -5,11 +5,10 @@ import os import tarfile -from django.core.files.storage import get_storage_class from django.core.management.base import CommandError from ... import utils -from ...storage import StorageError, get_storage +from ...storage import StorageError, get_storage, get_storage_class from ._base import BaseDbBackupCommand, make_option diff --git a/dbbackup/management/commands/mediarestore.py b/dbbackup/management/commands/mediarestore.py index d353be6c..fb1355a2 100644 --- a/dbbackup/management/commands/mediarestore.py +++ b/dbbackup/management/commands/mediarestore.py @@ -4,10 +4,8 @@ import tarfile -from django.core.files.storage import get_storage_class - from ... import utils -from ...storage import get_storage +from ...storage import get_storage, get_storage_class from ._base import BaseDbBackupCommand, make_option diff --git a/dbbackup/settings.py b/dbbackup/settings.py index eec21734..0e5c1099 100644 --- a/dbbackup/settings.py +++ b/dbbackup/settings.py @@ -35,10 +35,20 @@ GPG_ALWAYS_TRUST = getattr(settings, "DBBACKUP_GPG_ALWAYS_TRUST", False) GPG_RECIPIENT = GPG_ALWAYS_TRUST = getattr(settings, "DBBACKUP_GPG_RECIPIENT", None) -STORAGE = getattr( - settings, "DBBACKUP_STORAGE", "django.core.files.storage.FileSystemStorage" -) +STORAGE = getattr(settings, "DBBACKUP_STORAGE", None) STORAGE_OPTIONS = getattr(settings, "DBBACKUP_STORAGE_OPTIONS", {}) +# https://docs.djangoproject.com/en/5.1/ref/settings/#std-setting-STORAGES +STORAGES_DBBACKUP_ALIAS = "dbbackup" +DJANGO_STORAGES = getattr(settings, "STORAGES", {}) +django_dbbackup_storage = DJANGO_STORAGES.get(STORAGES_DBBACKUP_ALIAS, {}) + +if not STORAGE: + STORAGE = ( + django_dbbackup_storage.get("BACKEND") + or "django.core.files.storage.FileSystemStorage" + ) +if not STORAGE_OPTIONS: + STORAGE_OPTIONS = django_dbbackup_storage.get("OPTIONS") or STORAGE_OPTIONS CONNECTORS = getattr(settings, "DBBACKUP_CONNECTORS", {}) diff --git a/dbbackup/storage.py b/dbbackup/storage.py index 52c46540..d4d71e71 100644 --- a/dbbackup/storage.py +++ b/dbbackup/storage.py @@ -5,7 +5,6 @@ import logging from django.core.exceptions import ImproperlyConfigured -from django.core.files.storage import get_storage_class from . import settings, utils @@ -285,3 +284,30 @@ def clean_old_backups( if keep_filter(filename): continue self.delete_file(filename) + + +def get_storage_class(path=None): + """ + Return the configured storage class. + + :param path: Path in Python dot style to module containing the storage + class. If empty, the default storage class will be used. + :type path: str or None + + :returns: Storage class + :rtype: :class:`django.core.files.storage.Storage` + """ + from django.utils.module_loading import import_string + + if path: + # this is a workaround to keep compatibility with Django >= 5.1 (django.core.files.storage.get_storage_class is removed) + return import_string(path) + + try: + from django.core.files.storage import DefaultStorage + + return DefaultStorage + except Exception: + from django.core.files.storage import get_storage_class + + return get_storage_class() diff --git a/dbbackup/tests/commands/test_mediabackup.py b/dbbackup/tests/commands/test_mediabackup.py index c3e9e5ab..63590bfb 100644 --- a/dbbackup/tests/commands/test_mediabackup.py +++ b/dbbackup/tests/commands/test_mediabackup.py @@ -6,11 +6,10 @@ import os import tempfile -from django.core.files.storage import get_storage_class from django.test import TestCase from dbbackup.management.commands.mediabackup import Command as DbbackupCommand -from dbbackup.storage import get_storage +from dbbackup.storage import get_storage, get_storage_class from dbbackup.tests.utils import DEV_NULL, HANDLED_FILES, add_public_gpg diff --git a/dbbackup/tests/settings.py b/dbbackup/tests/settings.py index b1e0c161..0e3501a7 100644 --- a/dbbackup/tests/settings.py +++ b/dbbackup/tests/settings.py @@ -63,6 +63,18 @@ ] ) +# For testing the new storages setting introduced in Django 4.2 +STORAGES = { + "default": { + "BACKEND": "django.core.files.storage.FileSystemStorage", + "OPTIONS": {}, + }, + "dbbackup": { + "BACKEND": DBBACKUP_STORAGE, + "OPTIONS": DBBACKUP_STORAGE_OPTIONS, + }, +} + LOGGING = { "version": 1, "disable_existing_loggers": False, diff --git a/dbbackup/tests/test_storage.py b/dbbackup/tests/test_storage.py index 2153f676..2edac76a 100644 --- a/dbbackup/tests/test_storage.py +++ b/dbbackup/tests/test_storage.py @@ -3,7 +3,7 @@ from django.test import TestCase from dbbackup import utils -from dbbackup.storage import Storage, get_storage +from dbbackup.storage import Storage, get_storage, get_storage_class from dbbackup.tests.utils import HANDLED_FILES, FakeStorage DEFAULT_STORAGE_PATH = "django.core.files.storage.FileSystemStorage" @@ -30,6 +30,55 @@ def test_set_options(self, *args): ("django.core.files.storage", "django.core.files.storage.filesystem"), ) + def test_get_storage_class(self): + storage_class = get_storage_class(DEFAULT_STORAGE_PATH) + self.assertIn( + storage_class.__module__, + ("django.core.files.storage", "django.core.files.storage.filesystem"), + ) + self.assertIn(storage_class.__name__, ("FileSystemStorage", "DefaultStorage")) + + storage_class = get_storage_class("dbbackup.tests.utils.FakeStorage") + self.assertEqual(storage_class.__module__, "dbbackup.tests.utils") + self.assertEqual(storage_class.__name__, "FakeStorage") + + def test_default_storage_class(self): + storage_class = get_storage_class() + self.assertIn( + storage_class.__module__, + ("django.core.files.storage", "django.core.files.storage.filesystem"), + ) + self.assertIn(storage_class.__name__, ("FileSystemStorage", "DefaultStorage")) + + def test_invalid_storage_class_path(self): + with self.assertRaises(ImportError): + get_storage_class("invalid.path.to.StorageClass") + + def test_storages_settings(self): + from .settings import STORAGES + + self.assertIsInstance(STORAGES, dict) + self.assertEqual( + STORAGES["dbbackup"]["BACKEND"], "dbbackup.tests.utils.FakeStorage" + ) + + from dbbackup.settings import DJANGO_STORAGES, STORAGE + + self.assertIsInstance(DJANGO_STORAGES, dict) + self.assertEqual(DJANGO_STORAGES, STORAGES) + self.assertEqual(STORAGES["dbbackup"]["BACKEND"], STORAGE) + + storage = get_storage() + self.assertEqual(storage.storage.__class__.__module__, "dbbackup.tests.utils") + self.assertEqual(storage.storage.__class__.__name__, "FakeStorage") + + def test_storages_settings_options(self): + from dbbackup.settings import STORAGE_OPTIONS + + from .settings import STORAGES + + self.assertEqual(STORAGES["dbbackup"]["OPTIONS"], STORAGE_OPTIONS) + class StorageTest(TestCase): def setUp(self): diff --git a/docs/changelog.rst b/docs/changelog.rst index 3e698605..4a12daf4 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -8,6 +8,8 @@ Unreleased * Add PostgreSQL Schema support by @angryfoxx in https://github.com/jazzband/django-dbbackup/pull/507 * Fix restore of database from S3 storage by reintroducing inputfile.seek(0) to utils.uncompress_file * Fix bug where dbbackup management command would not respect settings.py:DBBACKUP_DATABASES +* Remove usage of deprecated 'get_storage_class' function in newer Django versions +* Add support for new STORAGES (Django 4.2+) setting under the 'dbbackup' alias 4.1.0 (2024-01-14) ------------------ diff --git a/docs/storage.rst b/docs/storage.rst index 2238b11c..040b2cab 100644 --- a/docs/storage.rst +++ b/docs/storage.rst @@ -7,12 +7,32 @@ mainly based on Django Storage API and extends its possibilities. You can choose your backup storage backend by setting ``settings.DBBACKUP_STORAGE``, it must be the full path of a storage class. For example: -``django.core.files.storage.FileSystemStorage`` to use file system storage. +``django.core.files.storage.FileSystemStorage`` to use file system storage. Below, we'll list some of the available solutions and their options. + The storage's option are gathered in ``settings.DBBACKUP_STORAGE_OPTIONS`` which is a dictionary of keywords representing how to configure it. +Since Django 4.2 there is a new way to configure storages using the STORAGES setting. E.g.::: + + STORAGES = { + "default": { + "BACKEND": "django.core.files.storage.FileSystemStorage", + "OPTIONS": { + ...your_options_here + }, + }, + "staticfiles": { + ... + }, + } + +instead of the previous DEFAULT_FILE_STORAGE and STATICFILES_STORAGE settings. +If ``settings.DBBACKUP_STORAGE`` is not set, django-dbbackup will look for the ``dbbackup`` key in the ``STORAGES`` dictionary setting. +Otherwise, the default storage will be used. + + .. warning:: Do not configure backup storage with the same configuration as your media @@ -43,6 +63,16 @@ settings below. :: DBBACKUP_STORAGE = 'django.core.files.storage.FileSystemStorage' DBBACKUP_STORAGE_OPTIONS = {'location': '/my/backup/dir/'} +Alternatively, starting from Django 4.2, you can use the STORAGES setting: :: + + STORAGES = { + "dbbackup": { + "BACKEND": "django.core.files.storage.FileSystemStorage", + "OPTIONS": { + "location": "/my/backup/dir/", + }, + }, + } Available settings ~~~~~~~~~~~~~~~~~~ diff --git a/tox.ini b/tox.ini index bafe8999..a57f86be 100644 --- a/tox.ini +++ b/tox.ini @@ -1,5 +1,5 @@ [tox] -envlist = py{37,38,39,310,311,312}-django{32,42,50,master},lint,docs,functional +envlist = py{37,38,39,310,311,312}-django{32,42,50,51,master},lint,docs,functional [testenv] passenv = * @@ -10,6 +10,7 @@ deps = django32: django>=3.2,<3.3 django42: django>=4.2,<4.3 django50: django>=5.0,<5.1 + django51: django>=5.1,<5.2 djangomaster: https://github.com/django/django/archive/master.zip commands = {posargs:coverage run runtests.py} @@ -19,9 +20,9 @@ python = 3.7: py37-django{32},functional 3.8: py38-django{32,42},functional 3.9: py39-django{32,42},functional - 3.10: py310-django{32,42,50},functional - 3.11: py311-django{42,50},functional - 3.12: py312-django{42,50},functional + 3.10: py310-django{32,42,50,51},functional + 3.11: py311-django{42,50,51},functional + 3.12: py312-django{42,50,51},functional [testenv:lint] basepython = python From b0fdd2414bc35c801ed415f2f6ea20a65a383751 Mon Sep 17 00:00:00 2001 From: Jerin Peter George Date: Fri, 23 Aug 2024 03:24:38 +0530 Subject: [PATCH 2/2] Add check for filename template values (#484) Co-authored-by: Mark Bakhit Co-authored-by: Archmonger <16909269+Archmonger@users.noreply.github.com> --- dbbackup/checks.py | 43 +++++++++++++++++++++++++++++++++++ dbbackup/tests/test_checks.py | 24 +++++++++++++++++++ docs/changelog.rst | 1 + 3 files changed, 68 insertions(+) diff --git a/dbbackup/checks.py b/dbbackup/checks.py index 9fbf76c4..20de12c4 100644 --- a/dbbackup/checks.py +++ b/dbbackup/checks.py @@ -1,4 +1,5 @@ import re +from datetime import datetime from django.core.checks import Tags, Warning, register @@ -35,6 +36,46 @@ "settings.DBBACKUP_ADMINS", id="dbbackup.W006", ) +W007 = Warning( + "Invalid FILENAME_TEMPLATE parameter", + hint="settings.DBBACKUP_FILENAME_TEMPLATE must not contain slashes ('/'). " + "Did you mean to change the value for 'location'?", + id="dbbackup.W007", +) +W008 = Warning( + "Invalid MEDIA_FILENAME_TEMPLATE parameter", + hint="settings.DBBACKUP_MEDIA_FILENAME_TEMPLATE must not contain slashes ('/')" + "Did you mean to change the value for 'location'?", + id="dbbackup.W007", +) + + +def check_filename_templates(): + return _check_filename_template( + settings.FILENAME_TEMPLATE, + W007, + "db", + ) + _check_filename_template( + settings.MEDIA_FILENAME_TEMPLATE, + W008, + "media", + ) + + +def _check_filename_template(filename_template, check_code, content_type) -> list: + if callable(filename_template): + params = { + "servername": "localhost", + "datetime": datetime.now().strftime(settings.DATE_FORMAT), + "databasename": "default", + "extension": "dump", + "content_type": content_type, + } + filename_template = filename_template(params) + + if "/" in filename_template: + return [check_code] + return [] @register(Tags.compatibility) @@ -64,4 +105,6 @@ def check_settings(app_configs, **kwargs): if getattr(settings, "FAILURE_RECIPIENTS", None) is not None: errors.append(W006) + errors += check_filename_templates() + return errors diff --git a/dbbackup/tests/test_checks.py b/dbbackup/tests/test_checks.py index 2a58332a..279abea4 100644 --- a/dbbackup/tests/test_checks.py +++ b/dbbackup/tests/test_checks.py @@ -72,3 +72,27 @@ def test_Failure_recipients_warning(self): expected_errors = [checks.W006] errors = checks.check_settings(DbbackupConfig) self.assertEqual(expected_errors, errors) + + @patch("dbbackup.checks.settings.FILENAME_TEMPLATE", "foo/bar-{datetime}.ext") + def test_db_filename_template_with_slash(self): + expected_errors = [checks.W007] + errors = checks.check_settings(DbbackupConfig) + self.assertEqual(expected_errors, errors) + + @patch("dbbackup.checks.settings.FILENAME_TEMPLATE", lambda _: "foo/bar") + def test_db_filename_template_callable_with_slash(self): + expected_errors = [checks.W007] + errors = checks.check_settings(DbbackupConfig) + self.assertEqual(expected_errors, errors) + + @patch("dbbackup.checks.settings.MEDIA_FILENAME_TEMPLATE", "foo/bar-{datetime}.ext") + def test_media_filename_template_with_slash(self): + expected_errors = [checks.W008] + errors = checks.check_settings(DbbackupConfig) + self.assertEqual(expected_errors, errors) + + @patch("dbbackup.checks.settings.MEDIA_FILENAME_TEMPLATE", lambda _: "foo/bar") + def test_media_filename_template_callable_with_slash(self): + expected_errors = [checks.W008] + errors = checks.check_settings(DbbackupConfig) + self.assertEqual(expected_errors, errors) diff --git a/docs/changelog.rst b/docs/changelog.rst index 4a12daf4..b0384b96 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -7,6 +7,7 @@ Unreleased * Default HOST to localhost for postgres databases. https://github.com/jazzband/django-dbbackup/issues/520 * Add PostgreSQL Schema support by @angryfoxx in https://github.com/jazzband/django-dbbackup/pull/507 * Fix restore of database from S3 storage by reintroducing inputfile.seek(0) to utils.uncompress_file +* Add warning for filenames with slashes in them * Fix bug where dbbackup management command would not respect settings.py:DBBACKUP_DATABASES * Remove usage of deprecated 'get_storage_class' function in newer Django versions * Add support for new STORAGES (Django 4.2+) setting under the 'dbbackup' alias