Skip to content

Commit a388e2a

Browse files
committed
feat: update file storage access to support Django 5.0 storages registry
1 parent 447fd0b commit a388e2a

File tree

3 files changed

+145
-2
lines changed

3 files changed

+145
-2
lines changed

cms/djangoapps/contentstore/storage.py

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44

55

66
from django.conf import settings
7-
from django.core.files.storage import get_storage_class
7+
from common.djangoapps.util.storage import resolve_storage_backend
88
from storages.backends.s3boto3 import S3Boto3Storage
99
from storages.utils import setting
1010

@@ -19,4 +19,7 @@ def __init__(self):
1919
super().__init__(bucket_name=bucket, custom_domain=None, querystring_auth=True)
2020

2121
# pylint: disable=invalid-name
22-
course_import_export_storage = get_storage_class(settings.COURSE_IMPORT_EXPORT_STORAGE)()
22+
course_import_export_storage = resolve_storage_backend(
23+
storage_key="course_import_export",
24+
legacy_setting_key="COURSE_IMPORT_EXPORT_STORAGE"
25+
)

cms/djangoapps/contentstore/tests/test_import.py

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,9 @@
2121
from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
2222
from xmodule.modulestore.xml_importer import import_course_from_xml
2323

24+
from common.djangoapps.util.storage import resolve_storage_backend
25+
from storages.backends.s3boto3 import S3Boto3Storage
26+
2427
TEST_DATA_CONTENTSTORE = copy.deepcopy(settings.CONTENTSTORE)
2528
TEST_DATA_CONTENTSTORE['DOC_STORE_CONFIG']['db'] = 'test_xcontent_%s' % uuid4().hex
2629

@@ -275,3 +278,81 @@ def test_video_components_present_while_import(self):
275278

276279
video = module_store.get_item(vertical.children[1])
277280
self.assertEqual(video.display_name, 'default')
281+
282+
@override_settings(
283+
COURSE_IMPORT_EXPORT_STORAGE="cms.djangoapps.contentstore.storage.ImportExportS3Storage",
284+
DEFAULT_FILE_STORAGE="django.core.files.storage.FileSystemStorage"
285+
)
286+
def test_resolve_default_storage(self):
287+
""" Ensure the default storage is invoked, even if course export storage is configured """
288+
storage = resolve_storage_backend(
289+
storage_key="default",
290+
legacy_setting_key="DEFAULT_FILE_STORAGE"
291+
)
292+
self.assertEqual(storage.__class__.__name__, "FileSystemStorage")
293+
294+
@override_settings(
295+
COURSE_IMPORT_EXPORT_STORAGE="cms.djangoapps.contentstore.storage.ImportExportS3Storage",
296+
DEFAULT_FILE_STORAGE="django.core.files.storage.FileSystemStorage",
297+
COURSE_IMPORT_EXPORT_BUCKET="bucket_name_test"
298+
)
299+
def test_resolve_happy_path_storage(self):
300+
""" Make sure that the correct course export storage is being used """
301+
storage = resolve_storage_backend(
302+
storage_key="course_import_export",
303+
legacy_setting_key="COURSE_IMPORT_EXPORT_STORAGE"
304+
)
305+
self.assertEqual(storage.__class__.__name__, "ImportExportS3Storage")
306+
self.assertEqual(storage.bucket_name, "bucket_name_test")
307+
308+
@override_settings()
309+
def test_resolve_storage_with_no_config(self):
310+
""" If no storage setup is defined, we get FileSystemStorage by default """
311+
del settings.DEFAULT_FILE_STORAGE
312+
del settings.COURSE_IMPORT_EXPORT_STORAGE
313+
del settings.COURSE_IMPORT_EXPORT_BUCKET
314+
storage = resolve_storage_backend(
315+
storage_key="course_import_export",
316+
legacy_setting_key="COURSE_IMPORT_EXPORT_STORAGE"
317+
)
318+
self.assertEqual(storage.__class__.__name__, "FileSystemStorage")
319+
320+
@override_settings(
321+
COURSE_IMPORT_EXPORT_STORAGE=None,
322+
COURSE_IMPORT_EXPORT_BUCKET="bucket_name_test",
323+
STORAGES={
324+
'COURSE_IMPORT_EXPORT_STORAGE': {
325+
'BACKEND': 'cms.djangoapps.contentstore.storage.ImportExportS3Storage',
326+
'OPTIONS': {}
327+
}
328+
}
329+
)
330+
def test_resolve_storage_using_django5_settings(self):
331+
""" Simulating a Django 4 environment using Django 5 Storages configuration """
332+
storage = resolve_storage_backend(
333+
storage_key="course_import_export",
334+
legacy_setting_key="COURSE_IMPORT_EXPORT_STORAGE"
335+
)
336+
self.assertEqual(storage.__class__.__name__, "ImportExportS3Storage")
337+
self.assertEqual(storage.bucket_name, "bucket_name_test")
338+
339+
@override_settings(
340+
STORAGES={
341+
'course_import_export': {
342+
'BACKEND': 'storages.backends.s3boto3.S3Boto3Storage',
343+
'OPTIONS': {
344+
'bucket_name': 'bucket_name_test'
345+
}
346+
}
347+
}
348+
)
349+
def test_resolve_storage_using_django5_settings_with_options(self):
350+
""" Ensure we call the storage class with the correct parameters and Django 5 setup """
351+
del settings.COURSE_IMPORT_EXPORT_STORAGE
352+
del settings.COURSE_IMPORT_EXPORT_BUCKET
353+
storage = resolve_storage_backend(
354+
storage_key="course_import_export",
355+
legacy_setting_key="COURSE_IMPORT_EXPORT_STORAGE"
356+
)
357+
self.assertEqual(storage.__class__.__name__, S3Boto3Storage.__name__)
358+
self.assertEqual(storage.bucket_name, "bucket_name_test")

common/djangoapps/util/storage.py

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
""" Utility functions related to django storages """
2+
3+
from typing import Optional
4+
from django.conf import settings
5+
from django.core.files.storage import default_storage, storages
6+
from django.utils.module_loading import import_string
7+
8+
9+
def resolve_storage_backend(storage_key: str, legacy_setting_key: str, options: Optional[dict] = None):
10+
"""
11+
Configures and returns a Django `Storage` instance, compatible with both Django 4 and Django 5.
12+
Params:
13+
storage_key = The key name saved in Django storages settings.
14+
legacy_setting_key = The key name saved in Django settings.
15+
options = Kwargs for the storage class.
16+
Returns:
17+
An instance of the configured storage backend.
18+
Raises:
19+
ImportError: If the specified storage class cannot be imported.
20+
Main goal:
21+
Deprecate the use of `django.core.files.storage.get_storage_class`.
22+
How:
23+
Replace `get_storage_class` with direct configuration logic,
24+
ensuring backward compatibility with both Django 4 and Django 5 storage settings.
25+
"""
26+
27+
storage_path = getattr(settings, legacy_setting_key, None)
28+
storages_config = getattr(settings, 'STORAGES', {})
29+
30+
if options is None:
31+
options = {}
32+
33+
if storage_key == "default":
34+
# Use case 1: Default storage
35+
# Works consistently across Django 4.2 and 5.x.
36+
# In Django 4.2 and above, `default_storage` uses
37+
# either `DEFAULT_FILE_STORAGE` or `STORAGES['default']`.
38+
return default_storage
39+
40+
if storage_key in storages_config:
41+
# Use case 2: STORAGES is defined
42+
# If STORAGES is present, we retrieve it through the storages API
43+
# settings.py must define STORAGES like:
44+
# STORAGES = {
45+
# "default": {"BACKEND": "...", "OPTIONS": {...}},
46+
# "custom": {"BACKEND": "...", "OPTIONS": {...}},
47+
# }
48+
# See: https://docs.djangoproject.com/en/5.2/ref/settings/#std-setting-STORAGES
49+
return storages[storage_key]
50+
51+
if not storage_path:
52+
# Use case 3: No storage settings defined
53+
# If no storage settings are defined anywhere, use the default storage
54+
return default_storage
55+
56+
# Use case 4: Legacy settings
57+
# Fallback to import the storage_path (Obtained from django settings) manually
58+
StorageClass = import_string(storage_path)
59+
return StorageClass(**options)

0 commit comments

Comments
 (0)