fix: handle errors in nested legacy settings in resolve_storage_backend
This commit is contained in:
@@ -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)
|
||||
|
||||
56
common/djangoapps/util/tests/test_resolve_storage_backend.py
Normal file
56
common/djangoapps/util/tests/test_resolve_storage_backend.py
Normal file
@@ -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
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user