Skip to content

Commit 291c5c3

Browse files
authored
feat!: upgrade code and fix get_storage_class ( compatibility django42 and django52) (#36628)
* feat!: upgrade codebase for compatibility with Django 4.2 and 5.2.
1 parent e90bd04 commit 291c5c3

File tree

2 files changed

+103
-7
lines changed

2 files changed

+103
-7
lines changed

openedx/core/djangoapps/user_api/accounts/image_helpers.py

Lines changed: 38 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,11 @@
88
from django.conf import settings
99
from django.contrib.staticfiles.storage import staticfiles_storage
1010
from django.core.exceptions import ObjectDoesNotExist
11-
from django.core.files.storage import get_storage_class
11+
from django.core.files.storage import default_storage, storages
12+
from django.utils.module_loading import import_string
1213

13-
from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers
1414
from common.djangoapps.student.models import UserProfile
15+
from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers
1516

1617
from ..errors import UserNotFound
1718

@@ -22,12 +23,42 @@
2223

2324
def get_profile_image_storage():
2425
"""
25-
Configures and returns a django Storage instance that can be used
26-
to physically locate, read and write profile images.
26+
Returns an instance of the configured storage backend for profile images.
27+
28+
This function prioritizes different settings in the following order to determine
29+
which storage class to use:
30+
31+
1. Use 'profile_image' storage from Django's STORAGES if defined (Django 4.2+).
32+
2. If not available, check the legacy PROFILE_IMAGE_BACKEND setting.
33+
3. If still undefined, fall back to Django's default_storage.
34+
35+
Note:
36+
- Starting in Django 5+, `DEFAULT_FILE_STORAGE` and the `STORAGES` setting
37+
are mutually exclusive. Only one of them should be used to avoid
38+
`ImproperlyConfigured` errors.
39+
40+
Returns:
41+
An instance of the configured storage backend for handling profile images.
42+
43+
Raises:
44+
ImportError: If the specified storage class cannot be imported.
2745
"""
28-
config = settings.PROFILE_IMAGE_BACKEND
29-
storage_class = get_storage_class(config['class'])
30-
return storage_class(**config['options'])
46+
# Prefer new-style Django 4.2+ STORAGES
47+
storages_config = getattr(settings, 'STORAGES', {})
48+
49+
if 'profile_image' in storages_config:
50+
return storages['profile_image']
51+
52+
# Legacy fallback: PROFILE_IMAGE_BACKEND
53+
config = getattr(settings, 'PROFILE_IMAGE_BACKEND', {})
54+
storage_class_path = config.get('class')
55+
options = config.get('options', {})
56+
57+
if not storage_class_path:
58+
return default_storage
59+
60+
storage_class = import_string(storage_class_path)
61+
return storage_class(**options)
3162

3263

3364
def _make_profile_image_name(username):

openedx/core/djangoapps/user_api/accounts/tests/test_views.py

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,11 +12,13 @@
1212
import ddt
1313
import pytz
1414
from django.conf import settings
15+
from django.core.files.storage import FileSystemStorage
1516
from django.test.testcases import TransactionTestCase
1617
from django.test.utils import override_settings
1718
from django.urls import reverse
1819
from rest_framework import status
1920
from rest_framework.test import APIClient, APITestCase
21+
from storages.backends.s3boto3 import S3Boto3Storage
2022

2123
from common.djangoapps.student.models import PendingEmailChange, UserProfile
2224
from common.djangoapps.student.models_api import do_name_change_request, get_pending_name_change
@@ -33,6 +35,7 @@
3335
RetirementStateFactory,
3436
UserRetirementStatusFactory
3537
)
38+
from openedx.core.djangoapps.user_api.accounts.image_helpers import get_profile_image_storage
3639
from openedx.core.djangoapps.user_api.models import UserPreference, UserRetirementStatus
3740
from openedx.core.djangoapps.user_api.preferences.api import set_user_preference
3841
from openedx.core.djangoapps.waffle_utils.testutils import WAFFLE_TABLES
@@ -1156,6 +1159,37 @@ def test_patch_serializer_save_fails(self, serializer_save):
11561159
assert "Error thrown when saving account updates: 'bummer'" == error_response.data['developer_message']
11571160
assert error_response.data['user_message'] is None
11581161

1162+
def test_profile_image_backend(self):
1163+
# settings file contains the `VIDEO_IMAGE_SETTINGS` but dont'have STORAGE_CLASS
1164+
# so it returns the default storage.
1165+
storage = get_profile_image_storage()
1166+
storage_class = storage.__class__
1167+
self.assertEqual(
1168+
settings.PROFILE_IMAGE_BACKEND['class'],
1169+
f"{storage_class.__module__}.{storage_class.__name__}",
1170+
)
1171+
self.assertEqual(storage.base_url, settings.PROFILE_IMAGE_BACKEND['options']['base_url'])
1172+
1173+
@override_settings(PROFILE_IMAGE_BACKEND={
1174+
'class': 'storages.backends.s3boto3.S3Boto3Storage',
1175+
'options': {
1176+
'bucket_name': 'test',
1177+
'default_acl': 'public',
1178+
'location': 'abc/def'
1179+
}
1180+
})
1181+
def test_profile_backend_with_params(self):
1182+
storage = get_profile_image_storage()
1183+
self.assertIsInstance(storage, S3Boto3Storage)
1184+
self.assertEqual(storage.bucket_name, "test")
1185+
self.assertEqual(storage.default_acl, 'public')
1186+
self.assertEqual(storage.location, "abc/def")
1187+
1188+
@override_settings(PROFILE_IMAGE_BACKEND={'class': None, 'options': {}})
1189+
def test_profile_backend_without_backend(self):
1190+
storage = get_profile_image_storage()
1191+
self.assertIsInstance(storage, FileSystemStorage)
1192+
11591193
@override_settings(PROFILE_IMAGE_BACKEND=TEST_PROFILE_IMAGE_BACKEND)
11601194
def test_convert_relative_profile_url(self):
11611195
"""
@@ -1170,6 +1204,37 @@ def test_convert_relative_profile_url(self):
11701204
'image_url_full': 'http://testserver/static/default_50.png',
11711205
'image_url_small': 'http://testserver/static/default_10.png'}
11721206

1207+
@override_settings(
1208+
PROFILE_IMAGE_BACKEND={},
1209+
STORAGES={
1210+
'profile_image': {
1211+
'BACKEND': 'storages.backends.s3boto3.S3Boto3Storage',
1212+
'OPTIONS': {
1213+
'bucket_name': 'profiles',
1214+
'default_acl': 'public',
1215+
'location': 'profile/images',
1216+
}
1217+
}
1218+
}
1219+
)
1220+
def test_profile_backend_with_profile_image_settings(self):
1221+
""" It will use the storages dict with profile_images backend"""
1222+
storage = get_profile_image_storage()
1223+
self.assertIsInstance(storage, S3Boto3Storage)
1224+
self.assertEqual(storage.bucket_name, "profiles")
1225+
self.assertEqual(storage.default_acl, 'public')
1226+
self.assertEqual(storage.location, "profile/images")
1227+
1228+
@override_settings(
1229+
PROFILE_IMAGE_BACKEND={},
1230+
)
1231+
def test_profile_backend_with_default_hardcoded_backend(self):
1232+
""" In case of empty storages scenario uses the hardcoded backend."""
1233+
del settings.DEFAULT_FILE_STORAGE
1234+
del settings.STORAGES
1235+
storage = get_profile_image_storage()
1236+
self.assertIsInstance(storage, FileSystemStorage)
1237+
11731238
@ddt.data(
11741239
("client", "user", True),
11751240
("different_client", "different_user", False),

0 commit comments

Comments
 (0)