From 82f5955ae26388f75b1e6261aea20fad64dbbbb4 Mon Sep 17 00:00:00 2001 From: Daniel Wong Date: Thu, 17 Jul 2025 14:36:49 -0600 Subject: [PATCH] fix: handle errors in nested legacy settings in resolve_storage_backend --- common/djangoapps/util/storage.py | 25 +++++++-- .../tests/test_resolve_storage_backend.py | 56 +++++++++++++++++++ lms/djangoapps/verify_student/models.py | 2 +- 3 files changed, 77 insertions(+), 6 deletions(-) create mode 100644 common/djangoapps/util/tests/test_resolve_storage_backend.py diff --git a/common/djangoapps/util/storage.py b/common/djangoapps/util/storage.py index 7d419d8fb2..37f908cd27 100644 --- a/common/djangoapps/util/storage.py +++ b/common/djangoapps/util/storage.py @@ -1,4 +1,5 @@ """ Utility functions related to django storages """ +import logging from typing import Optional, List from django.conf import settings @@ -6,6 +7,9 @@ from django.core.files.storage import storages from django.utils.module_loading import import_string +logger = logging.getLogger(__name__) + + def resolve_storage_backend( storage_key: str, legacy_setting_key: str, @@ -19,12 +23,12 @@ def resolve_storage_backend( legacy_sec_setting_keys = List of keys to get the storage class. For legacy dict settings like settings.BLOCK_STRUCTURES_SETTINGS.get('STORAGE_CLASS'), it is necessary to access a second-level key or above to retrieve the class path. - legacy_options = Kwargs for the storage class. options = Kwargs for the storage class. Returns: An instance of the configured storage backend. Raises: ImportError: If the specified storage class cannot be imported. + KeyError: If the specified key is not found in the legacy settings. Main goal: Deprecate the use of `django.core.files.storage.get_storage_class`. How: @@ -33,10 +37,6 @@ def resolve_storage_backend( """ storage_path = getattr(settings, legacy_setting_key, None) - if isinstance(storage_path, dict) and legacy_sec_setting_keys: - for deep_setting_key in legacy_sec_setting_keys: - # For legacy dict settings like settings.CUSTOM_STORAGE = {"BACKEND": "cms.custom.."} - storage_path = storage_path.get(deep_setting_key) storages_config = getattr(settings, 'STORAGES', {}) if options is None: @@ -55,5 +55,20 @@ def resolve_storage_backend( # Use case 2: Legacy settings # Fallback to import the storage_path (Obtained from django settings) manually + if isinstance(storage_path, dict) and legacy_sec_setting_keys: + # If storage_path is a dict, we need to access the second-level keys + # to retrieve the storage class path. + # This is useful for legacy settings that store storage paths in a nested structure. + for deep_setting_key in legacy_sec_setting_keys: + # For legacy dict settings like settings.CUSTOM_STORAGE = {"BACKEND": "cms.custom.."} + if deep_setting_key not in storage_path: + logger.warning( + f"Key {legacy_setting_key} '{deep_setting_key}' not found in storage settings {storage_path}." + "Using default storage path." + ) + storage_path = None # We set it to None to use the default storage later + break + storage_path = storage_path.get(deep_setting_key) + StorageClass = import_string(storage_path or settings.DEFAULT_FILE_STORAGE) return StorageClass(**options) diff --git a/common/djangoapps/util/tests/test_resolve_storage_backend.py b/common/djangoapps/util/tests/test_resolve_storage_backend.py new file mode 100644 index 0000000000..d892243c14 --- /dev/null +++ b/common/djangoapps/util/tests/test_resolve_storage_backend.py @@ -0,0 +1,56 @@ +""" +Tests for the resolve_storage_backend function in common.djangoapps.util.storage. +""" + +from django.test import TestCase +from django.test.utils import override_settings + +from common.djangoapps.util.storage import resolve_storage_backend + + +DEFAULT_STORAGE_CLASS_NAME = "FileSystemStorage" + + +class ResolveStorageTest(TestCase): + """ + Tests for the resolve_storage_backend function. + """ + + @override_settings( + BLOCK_STRUCTURES_SETTINGS="cms.djangoapps.contentstore.storage.ImportExportS3Storage" + ) + def test_legacy_settings(self): + storage = resolve_storage_backend( + storage_key="block_structures_settings", + legacy_setting_key="BLOCK_STRUCTURES_SETTINGS", + options={} + ) + assert storage.__class__.__name__ == "ImportExportS3Storage" + + @override_settings( + BLOCK_STRUCTURES_SETTINGS={ + "STORAGE_CLASS": "cms.djangoapps.contentstore.storage.ImportExportS3Storage" + } + ) + def test_nested_legacy_settings(self): + storage = resolve_storage_backend( + storage_key="block_structures_settings", + legacy_setting_key="BLOCK_STRUCTURES_SETTINGS", + legacy_sec_setting_keys=["STORAGE_CLASS"], + options={} + ) + assert storage.__class__.__name__ == "ImportExportS3Storage" + + @override_settings( + BLOCK_STRUCTURES_SETTINGS={ + "OTHER_KEY": "cms.djangoapps.contentstore.storage.ImportExportS3Storage" + } + ) + def test_nested_legacy_settings_failed(self): + storage = resolve_storage_backend( + storage_key="block_structures_settings", + legacy_setting_key="BLOCK_STRUCTURES_SETTINGS", + legacy_sec_setting_keys=["STORAGE_CLASS"], + options={} + ) + assert storage.__class__.__name__ == DEFAULT_STORAGE_CLASS_NAME diff --git a/lms/djangoapps/verify_student/models.py b/lms/djangoapps/verify_student/models.py index a0bed6444d..f3b49329b8 100644 --- a/lms/djangoapps/verify_student/models.py +++ b/lms/djangoapps/verify_student/models.py @@ -44,7 +44,7 @@ from lms.djangoapps.verify_student.ssencrypt import ( rsa_decrypt, rsa_encrypt ) -from openedx.core.djangoapps.content.block_structure.models import resolve_storage_backend +from common.djangoapps.util.storage import resolve_storage_backend from openedx.core.djangoapps.signals.signals import LEARNER_SSO_VERIFIED, PHOTO_VERIFICATION_APPROVED from .utils import auto_verify_for_testing_enabled, earliest_allowed_verification_date, submit_request_to_ss