diff --git a/openedx/core/djangoapps/user_api/accounts/image_helpers.py b/openedx/core/djangoapps/user_api/accounts/image_helpers.py index 43aa4a60aeb7..eff2ad272b28 100644 --- a/openedx/core/djangoapps/user_api/accounts/image_helpers.py +++ b/openedx/core/djangoapps/user_api/accounts/image_helpers.py @@ -8,10 +8,11 @@ from django.conf import settings from django.contrib.staticfiles.storage import staticfiles_storage from django.core.exceptions import ObjectDoesNotExist -from django.core.files.storage import get_storage_class +from django.core.files.storage import default_storage, storages +from django.utils.module_loading import import_string -from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers from common.djangoapps.student.models import UserProfile +from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers from ..errors import UserNotFound @@ -22,12 +23,42 @@ def get_profile_image_storage(): """ - Configures and returns a django Storage instance that can be used - to physically locate, read and write profile images. + Returns an instance of the configured storage backend for profile images. + + This function prioritizes different settings in the following order to determine + which storage class to use: + + 1. Use 'profile_image' storage from Django's STORAGES if defined (Django 4.2+). + 2. If not available, check the legacy PROFILE_IMAGE_BACKEND setting. + 3. If still undefined, fall back to Django's default_storage. + + Note: + - Starting in Django 5+, `DEFAULT_FILE_STORAGE` and the `STORAGES` setting + are mutually exclusive. Only one of them should be used to avoid + `ImproperlyConfigured` errors. + + Returns: + An instance of the configured storage backend for handling profile images. + + Raises: + ImportError: If the specified storage class cannot be imported. """ - config = settings.PROFILE_IMAGE_BACKEND - storage_class = get_storage_class(config['class']) - return storage_class(**config['options']) + # Prefer new-style Django 4.2+ STORAGES + storages_config = getattr(settings, 'STORAGES', {}) + + if 'profile_image' in storages_config: + return storages['profile_image'] + + # Legacy fallback: PROFILE_IMAGE_BACKEND + config = getattr(settings, 'PROFILE_IMAGE_BACKEND', {}) + storage_class_path = config.get('class') + options = config.get('options', {}) + + if not storage_class_path: + return default_storage + + storage_class = import_string(storage_class_path) + return storage_class(**options) def _make_profile_image_name(username): diff --git a/openedx/core/djangoapps/user_api/accounts/tests/test_views.py b/openedx/core/djangoapps/user_api/accounts/tests/test_views.py index ff0fb7abe4eb..1c92aa22c706 100644 --- a/openedx/core/djangoapps/user_api/accounts/tests/test_views.py +++ b/openedx/core/djangoapps/user_api/accounts/tests/test_views.py @@ -12,11 +12,13 @@ import ddt import pytz from django.conf import settings +from django.core.files.storage import FileSystemStorage from django.test.testcases import TransactionTestCase from django.test.utils import override_settings from django.urls import reverse from rest_framework import status from rest_framework.test import APIClient, APITestCase +from storages.backends.s3boto3 import S3Boto3Storage from common.djangoapps.student.models import PendingEmailChange, UserProfile from common.djangoapps.student.models_api import do_name_change_request, get_pending_name_change @@ -33,6 +35,7 @@ RetirementStateFactory, UserRetirementStatusFactory ) +from openedx.core.djangoapps.user_api.accounts.image_helpers import get_profile_image_storage from openedx.core.djangoapps.user_api.models import UserPreference, UserRetirementStatus from openedx.core.djangoapps.user_api.preferences.api import set_user_preference from openedx.core.djangoapps.waffle_utils.testutils import WAFFLE_TABLES @@ -1156,6 +1159,37 @@ def test_patch_serializer_save_fails(self, serializer_save): assert "Error thrown when saving account updates: 'bummer'" == error_response.data['developer_message'] assert error_response.data['user_message'] is None + def test_profile_image_backend(self): + # settings file contains the `VIDEO_IMAGE_SETTINGS` but dont'have STORAGE_CLASS + # so it returns the default storage. + storage = get_profile_image_storage() + storage_class = storage.__class__ + self.assertEqual( + settings.PROFILE_IMAGE_BACKEND['class'], + f"{storage_class.__module__}.{storage_class.__name__}", + ) + self.assertEqual(storage.base_url, settings.PROFILE_IMAGE_BACKEND['options']['base_url']) + + @override_settings(PROFILE_IMAGE_BACKEND={ + 'class': 'storages.backends.s3boto3.S3Boto3Storage', + 'options': { + 'bucket_name': 'test', + 'default_acl': 'public', + 'location': 'abc/def' + } + }) + def test_profile_backend_with_params(self): + storage = get_profile_image_storage() + self.assertIsInstance(storage, S3Boto3Storage) + self.assertEqual(storage.bucket_name, "test") + self.assertEqual(storage.default_acl, 'public') + self.assertEqual(storage.location, "abc/def") + + @override_settings(PROFILE_IMAGE_BACKEND={'class': None, 'options': {}}) + def test_profile_backend_without_backend(self): + storage = get_profile_image_storage() + self.assertIsInstance(storage, FileSystemStorage) + @override_settings(PROFILE_IMAGE_BACKEND=TEST_PROFILE_IMAGE_BACKEND) def test_convert_relative_profile_url(self): """ @@ -1170,6 +1204,37 @@ def test_convert_relative_profile_url(self): 'image_url_full': 'http://testserver/static/default_50.png', 'image_url_small': 'http://testserver/static/default_10.png'} + @override_settings( + PROFILE_IMAGE_BACKEND={}, + STORAGES={ + 'profile_image': { + 'BACKEND': 'storages.backends.s3boto3.S3Boto3Storage', + 'OPTIONS': { + 'bucket_name': 'profiles', + 'default_acl': 'public', + 'location': 'profile/images', + } + } + } + ) + def test_profile_backend_with_profile_image_settings(self): + """ It will use the storages dict with profile_images backend""" + storage = get_profile_image_storage() + self.assertIsInstance(storage, S3Boto3Storage) + self.assertEqual(storage.bucket_name, "profiles") + self.assertEqual(storage.default_acl, 'public') + self.assertEqual(storage.location, "profile/images") + + @override_settings( + PROFILE_IMAGE_BACKEND={}, + ) + def test_profile_backend_with_default_hardcoded_backend(self): + """ In case of empty storages scenario uses the hardcoded backend.""" + del settings.DEFAULT_FILE_STORAGE + del settings.STORAGES + storage = get_profile_image_storage() + self.assertIsInstance(storage, FileSystemStorage) + @ddt.data( ("client", "user", True), ("different_client", "different_user", False),