diff --git a/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_course_index.py b/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_course_index.py index e12c3f6e7770..0ce51d4d57e6 100644 --- a/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_course_index.py +++ b/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_course_index.py @@ -161,7 +161,7 @@ def test_number_of_calls_to_db(self): """ Test to check number of queries made to mysql and mongo """ - with self.assertNumQueries(34, table_ignorelist=WAFFLE_TABLES): + with self.assertNumQueries(33, table_ignorelist=WAFFLE_TABLES): with check_mongo_calls(3): self.client.get(self.url) diff --git a/cms/djangoapps/contentstore/views/tests/test_course_index.py b/cms/djangoapps/contentstore/views/tests/test_course_index.py index 8c33ad3f891f..115c865296d0 100644 --- a/cms/djangoapps/contentstore/views/tests/test_course_index.py +++ b/cms/djangoapps/contentstore/views/tests/test_course_index.py @@ -227,7 +227,7 @@ def test_number_of_calls_to_db(self): """ Test to check number of queries made to mysql and mongo """ - with self.assertNumQueries(21, table_ignorelist=WAFFLE_TABLES): + with self.assertNumQueries(20, table_ignorelist=WAFFLE_TABLES): with check_mongo_calls(3): self.client.get(reverse_course_url('course_handler', self.course.id), content_type="application/json") diff --git a/lms/djangoapps/certificates/apis/v0/tests/test_views.py b/lms/djangoapps/certificates/apis/v0/tests/test_views.py index d08241b2cd79..d6be6bd8c7e3 100644 --- a/lms/djangoapps/certificates/apis/v0/tests/test_views.py +++ b/lms/djangoapps/certificates/apis/v0/tests/test_views.py @@ -333,7 +333,7 @@ def test_query_counts(self): assert len(resp.data) == 0 # Test student with 1 certificate - with self.assertNumQueries(13, table_ignorelist=WAFFLE_TABLES): + with self.assertNumQueries(12, table_ignorelist=WAFFLE_TABLES): resp = self.get_response( AuthType.jwt, requesting_user=self.global_staff, @@ -373,7 +373,7 @@ def test_query_counts(self): download_url='www.google.com', grade="0.88", ) - with self.assertNumQueries(13, table_ignorelist=WAFFLE_TABLES): + with self.assertNumQueries(12, table_ignorelist=WAFFLE_TABLES): resp = self.get_response( AuthType.jwt, requesting_user=self.global_staff, diff --git a/lms/djangoapps/course_api/tests/test_views.py b/lms/djangoapps/course_api/tests/test_views.py index 15d24f5120c4..9d9bbd7b6033 100644 --- a/lms/djangoapps/course_api/tests/test_views.py +++ b/lms/djangoapps/course_api/tests/test_views.py @@ -446,7 +446,7 @@ def test_too_many_courses(self): self.setup_user(self.audit_user) # These query counts were found empirically - query_counts = [57, 46, 46, 46, 46, 46, 46, 46, 46, 43, 12] + query_counts = [56, 44, 44, 44, 44, 44, 44, 44, 44, 41, 10] ordered_course_ids = sorted([str(cid) for cid in (course_ids + [c.id for c in self.courses])], key=str.lower) self.clear_caches() diff --git a/lms/djangoapps/courseware/tests/test_views.py b/lms/djangoapps/courseware/tests/test_views.py index 3c888011c16d..ec27b40c12f9 100644 --- a/lms/djangoapps/courseware/tests/test_views.py +++ b/lms/djangoapps/courseware/tests/test_views.py @@ -1280,8 +1280,8 @@ def test_view_certificate_link(self): self.assertContains(resp, "earned a certificate for this course.") @ddt.data( - (True, 56), - (False, 56), + (True, 54), + (False, 54), ) @ddt.unpack def test_progress_queries_paced_courses(self, self_paced, query_count): @@ -1296,13 +1296,13 @@ def test_progress_queries(self): ContentTypeGatingConfig.objects.create(enabled=True, enabled_as_of=datetime(2018, 1, 1)) self.setup_course() with self.assertNumQueries( - 56, table_ignorelist=QUERY_COUNT_TABLE_IGNORELIST + 54, table_ignorelist=QUERY_COUNT_TABLE_IGNORELIST ), check_mongo_calls(2): self._get_progress_page() for _ in range(2): with self.assertNumQueries( - 39, table_ignorelist=QUERY_COUNT_TABLE_IGNORELIST + 36, table_ignorelist=QUERY_COUNT_TABLE_IGNORELIST ), check_mongo_calls(2): self._get_progress_page() diff --git a/openedx/core/djangoapps/bookmarks/tests/test_views.py b/openedx/core/djangoapps/bookmarks/tests/test_views.py index 4816ddb26ea8..278541a1b6aa 100644 --- a/openedx/core/djangoapps/bookmarks/tests/test_views.py +++ b/openedx/core/djangoapps/bookmarks/tests/test_views.py @@ -268,7 +268,7 @@ def test_post_bookmark_with_invalid_data(self): assert response.data['developer_message'] == 'Parameter usage_id not provided.' # Send empty data dictionary. - with self.assertNumQueries(9): # No queries for bookmark table. + with self.assertNumQueries(7): # No queries for bookmark table. response = self.send_post( client=self.client, url=reverse('bookmarks'), diff --git a/openedx/core/djangoapps/content_libraries/tests/test_containers.py b/openedx/core/djangoapps/content_libraries/tests/test_containers.py index 3900234f1db5..1c44427d8620 100644 --- a/openedx/core/djangoapps/content_libraries/tests/test_containers.py +++ b/openedx/core/djangoapps/content_libraries/tests/test_containers.py @@ -801,7 +801,7 @@ def test_unit_collections(self) -> None: assert unit_as_read['collections'] == [{"title": col1.title, "key": col1.collection_code}] def test_section_hierarchy(self): - with self.assertNumQueries(126): + with self.assertNumQueries(124): hierarchy = self._get_container_hierarchy(self.section_with_subsections["id"]) assert hierarchy["object_key"] == self.section_with_subsections["id"] assert hierarchy["components"] == [ @@ -827,7 +827,7 @@ def test_section_hierarchy(self): ] def test_subsection_hierarchy(self): - with self.assertNumQueries(91): + with self.assertNumQueries(89): hierarchy = self._get_container_hierarchy(self.subsection_with_units["id"]) assert hierarchy["object_key"] == self.subsection_with_units["id"] assert hierarchy["components"] == [ @@ -850,7 +850,7 @@ def test_subsection_hierarchy(self): ] def test_units_hierarchy(self): - with self.assertNumQueries(56): + with self.assertNumQueries(54): hierarchy = self._get_container_hierarchy(self.unit_with_components["id"]) assert hierarchy["object_key"] == self.unit_with_components["id"] assert hierarchy["components"] == [ @@ -876,7 +876,7 @@ def test_container_hierarchy_not_found(self): ) def test_block_hierarchy(self): - with self.assertNumQueries(24): + with self.assertNumQueries(22): hierarchy = self._get_block_hierarchy(self.problem_block["id"]) assert hierarchy["object_key"] == self.problem_block["id"] assert hierarchy["components"] == [ diff --git a/openedx/core/djangoapps/content_tagging/rest_api/v1/tests/test_views.py b/openedx/core/djangoapps/content_tagging/rest_api/v1/tests/test_views.py index 0422845cc346..aaa9f72f1d36 100644 --- a/openedx/core/djangoapps/content_tagging/rest_api/v1/tests/test_views.py +++ b/openedx/core/djangoapps/content_tagging/rest_api/v1/tests/test_views.py @@ -520,13 +520,13 @@ def test_create_taxonomy(self, user_attr: str, expected_status: int) -> None: assert response.data["orgs"] == [self.orgA.short_name] @ddt.data( - ('staff', 11), - ("content_creatorA", 23), - ("library_staffA", 23), - ("library_userA", 23), - ("instructorA", 23), - ("course_instructorA", 23), - ("course_staffA", 23), + ('staff', 10), + ("content_creatorA", 22), + ("library_staffA", 22), + ("library_userA", 22), + ("instructorA", 22), + ("course_instructorA", 22), + ("course_staffA", 22), ) @ddt.unpack def test_list_taxonomy_query_count(self, user_attr: str, expected_queries: int): @@ -1980,19 +1980,19 @@ def test_get_copied_tags(self): assert response.data[str(object_id_2)]["taxonomies"] == expected_tags @ddt.data( - ('staff', 'courseA', 10), - ('staff', 'libraryA', 17), - ('staff', 'collection_key', 17), - ("content_creatorA", 'courseA', 20, False), - ("content_creatorA", 'libraryA', 23, False), - ("content_creatorA", 'collection_key', 23, False), - ("library_staffA", 'libraryA', 23, False), # Library users can only view objecttags, not change them? - ("library_staffA", 'collection_key', 23, False), - ("library_userA", 'libraryA', 23, False), - ("library_userA", 'collection_key', 23, False), - ("instructorA", 'courseA', 20), - ("course_instructorA", 'courseA', 20), - ("course_staffA", 'courseA', 20), + ('staff', 'courseA', 9), + ('staff', 'libraryA', 16), + ('staff', 'collection_key', 16), + ("content_creatorA", 'courseA', 19, False), + ("content_creatorA", 'libraryA', 22, False), + ("content_creatorA", 'collection_key', 22, False), + ("library_staffA", 'libraryA', 22, False), # Library users can only view objecttags, not change them? + ("library_staffA", 'collection_key', 22, False), + ("library_userA", 'libraryA', 22, False), + ("library_userA", 'collection_key', 22, False), + ("instructorA", 'courseA', 19), + ("course_instructorA", 'courseA', 19), + ("course_staffA", 'courseA', 19), ) @ddt.unpack def test_object_tags_query_count( @@ -2878,13 +2878,13 @@ class TestTaxonomyTagsViewSet(TestTaxonomyObjectsMixin, APITestCase): Test cases for TaxonomyTagsViewSet retrive action. """ @ddt.data( - ('staff', 11), - ("content_creatorA", 13), - ("library_staffA", 13), - ("library_userA", 13), - ("instructorA", 13), - ("course_instructorA", 13), - ("course_staffA", 13), + ('staff', 10), + ("content_creatorA", 12), + ("library_staffA", 12), + ("library_userA", 12), + ("instructorA", 12), + ("course_instructorA", 12), + ("course_staffA", 12), ) @ddt.unpack def test_taxonomy_tags_query_count(self, user_attr: str, expected_queries: int): diff --git a/openedx/core/djangoapps/lang_pref/tests/test_middleware.py b/openedx/core/djangoapps/lang_pref/tests/test_middleware.py index eba0467fdb6c..f96797556983 100644 --- a/openedx/core/djangoapps/lang_pref/tests/test_middleware.py +++ b/openedx/core/djangoapps/lang_pref/tests/test_middleware.py @@ -42,6 +42,7 @@ class TestUserPreferenceMiddleware(CacheIsolationTestCase): """ Tests to make sure user preferences are getting properly set in the middleware. """ + ENABLED_CACHES = ['default'] def setUp(self): super().setUp() @@ -228,7 +229,7 @@ def test_preference_update_noop(self): response = mock.Mock(spec=HttpResponse) - with self.assertNumQueries(1): + with self.assertNumQueries(0): self.middleware.process_response(self.request, response) # Preference is the same as the cookie, shouldn't write to the database @@ -240,7 +241,7 @@ def test_preference_update_noop(self): response = mock.Mock(spec=HttpResponse) - with self.assertNumQueries(1): + with self.assertNumQueries(0): self.middleware.process_response(self.request, response) # Cookie changed, should write to the database again @@ -249,7 +250,7 @@ def test_preference_update_noop(self): self.middleware.process_request(self.request) assert get_user_preference(self.user, LANGUAGE_KEY) == 'en' - with self.assertNumQueries(1): + with self.assertNumQueries(0): self.middleware.process_response(self.request, response) @mock.patch('openedx.core.djangoapps.lang_pref.middleware.is_request_from_mobile_app') 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 f3f37c2f1469..899696eb032f 100644 --- a/openedx/core/djangoapps/user_api/accounts/tests/test_views.py +++ b/openedx/core/djangoapps/user_api/accounts/tests/test_views.py @@ -235,7 +235,7 @@ def test_get_username(self): Test that a client (logged in) can get her own username. """ self.client.login(username=self.user.username, password=TEST_PASSWORD) - self._verify_get_own_username(18) + self._verify_get_own_username(17) def test_get_username_inactive(self): """ @@ -245,7 +245,7 @@ def test_get_username_inactive(self): self.client.login(username=self.user.username, password=TEST_PASSWORD) self.user.is_active = False self.user.save() - self._verify_get_own_username(18) + self._verify_get_own_username(17) def test_get_username_not_logged_in(self): """ @@ -361,7 +361,7 @@ class TestAccountsAPI(FilteredQueryCountMixin, CacheIsolationTestCase, UserAPITe """ ENABLED_CACHES = ['default'] - TOTAL_QUERY_COUNT = 25 + TOTAL_QUERY_COUNT = 24 FULL_RESPONSE_FIELD_COUNT = 28 def setUp(self): @@ -815,12 +815,12 @@ def verify_get_own_information(queries): assert data['time_zone'] is None self.client.login(username=self.user.username, password=TEST_PASSWORD) - verify_get_own_information(self._get_num_queries(23)) + verify_get_own_information(self._get_num_queries(22)) # Now make sure that the user can get the same information, even if not active self.user.is_active = False self.user.save() - verify_get_own_information(self._get_num_queries(15)) + verify_get_own_information(self._get_num_queries(13)) def test_get_account_empty_string(self): """ @@ -835,7 +835,7 @@ def test_get_account_empty_string(self): legacy_profile.save() self.client.login(username=self.user.username, password=TEST_PASSWORD) - with self.assertNumQueries(self._get_num_queries(23), table_ignorelist=WAFFLE_TABLES): + with self.assertNumQueries(self._get_num_queries(22), table_ignorelist=WAFFLE_TABLES): response = self.send_get(self.client) for empty_field in ("level_of_education", "gender", "country", "state", "bio",): assert response.data[empty_field] is None diff --git a/openedx/core/djangoapps/user_api/helpers.py b/openedx/core/djangoapps/user_api/helpers.py index c45970c98b56..ae1308869816 100644 --- a/openedx/core/djangoapps/user_api/helpers.py +++ b/openedx/core/djangoapps/user_api/helpers.py @@ -2,8 +2,6 @@ Helper functions for the account/profile Python APIs. This is NOT part of the public API. """ - - import json import logging import traceback @@ -12,12 +10,72 @@ from django import forms from django.conf import settings +from django.core.cache import cache from django.core.serializers.json import DjangoJSONEncoder from django.utils.encoding import force_str from django.utils.functional import Promise LOGGER = logging.getLogger(__name__) +USER_PREFERENCE_CACHE_TIMEOUT = getattr(settings, 'USER_PREFERENCE_CACHE_TIMEOUT', 5 * 60) +USER_PREFERENCE_CACHE_KEY_PREFIX = getattr(settings, 'USER_PREFERENCE_CACHE_KEY_PREFIX', 'user_preferences') + + +def user_preferences_cache_key(user_id): + """ + Generate a unique cache key for a specific user's preferences. + + Args: + user_id (int|str): The unique identifier of the user. + + Returns: + str: A formatted cache key string. + """ + return f"{USER_PREFERENCE_CACHE_KEY_PREFIX}.{user_id}" + + +def get_cached_preferences(user): + """ + Retrieve the cached preferences dictionary for a given user. + + Args: + user (User): The user object whose preferences are being requested. + + Returns: + dict|None: The cached preferences if found, otherwise None on a cache miss. + """ + return cache.get(user_preferences_cache_key(user.id)) + + +def set_cached_preferences(user, preferences): + """ + Store the user's preferences in the cache. + + Args: + user (User): The user object associated with the preferences. + preferences (dict): A dictionary containing the user's preference settings. + + Returns: + None + """ + cache.set(user_preferences_cache_key(user.id), preferences, USER_PREFERENCE_CACHE_TIMEOUT) + + +def invalidate_user_preferences_cache(user): + """ + Remove the cached preferences for a specific user. + + This should be called whenever user preferences are updated to ensure + cache consistency. + + Args: + user (User): The user object whose cache entry needs to be deleted. + + Returns: + None + """ + cache.delete(user_preferences_cache_key(user.id)) + def intercept_errors(api_error, ignore_errors=None): """ diff --git a/openedx/core/djangoapps/user_api/models.py b/openedx/core/djangoapps/user_api/models.py index 7fd08dcbe110..4f2dd0e5eb4a 100644 --- a/openedx/core/djangoapps/user_api/models.py +++ b/openedx/core/djangoapps/user_api/models.py @@ -16,8 +16,8 @@ # pylint: disable=unused-import from common.djangoapps.student.models import get_retired_email_by_email, get_retired_username_by_username from common.djangoapps.util.model_utils import emit_settings_changed_event, get_changed_fields_dict +from openedx.core.djangoapps.user_api.helpers import get_cached_preferences, set_cached_preferences from openedx.core.djangolib.model_mixins import DeletableByUserValue -from openedx.core.lib.cache_utils import request_cached # Currently, the "student" app is responsible for # accounts, profiles, enrollments, and the student dashboard. @@ -47,14 +47,19 @@ class Meta: unique_together = ("user", "key") @staticmethod - @request_cached() def get_all_preferences(user): """ Gets all preferences for a given user Returns: Set of (preference type, value) pairs for each of the user's preferences """ - return {pref.key: pref.value for pref in user.preferences.all()} + preferences = get_cached_preferences(user) + + if preferences is None: + preferences = {pref.key: pref.value for pref in user.preferences.all()} + set_cached_preferences(user, preferences) + + return preferences @classmethod def get_value(cls, user, preference_key, default=None): diff --git a/openedx/core/djangoapps/user_api/preferences/api.py b/openedx/core/djangoapps/user_api/preferences/api.py index 53ff77da9c29..e336cc41e60e 100644 --- a/openedx/core/djangoapps/user_api/preferences/api.py +++ b/openedx/core/djangoapps/user_api/preferences/api.py @@ -26,7 +26,7 @@ UserNotAuthorized, UserNotFound, ) -from ..helpers import intercept_errors, serializer_is_dirty +from ..helpers import intercept_errors, invalidate_user_preferences_cache, serializer_is_dirty from ..models import UserOrgTag, UserPreference from ..serializers import RawUserPreferenceSerializer @@ -55,7 +55,8 @@ def has_user_preference(requesting_user, preference_key, username=None): UserAPIInternalError: the operation failed due to an unexpected error. """ existing_user = _get_authorized_user(requesting_user, username, allow_staff=True) - return UserPreference.has_value(existing_user, preference_key) + preferences = UserPreference.get_all_preferences(existing_user) + return preference_key in preferences @intercept_errors(UserAPIInternalError, ignore_errors=[UserAPIRequestError]) @@ -80,7 +81,8 @@ def get_user_preference(requesting_user, preference_key, username=None): UserAPIInternalError: the operation failed due to an unexpected error. """ existing_user = _get_authorized_user(requesting_user, username, allow_staff=True) - return UserPreference.get_value(existing_user, preference_key) + preferences = UserPreference.get_all_preferences(existing_user) + return preferences.get(preference_key) @intercept_errors(UserAPIInternalError, ignore_errors=[UserAPIRequestError]) @@ -171,6 +173,7 @@ def update_user_preferences(requesting_user, update, user=None): raise _create_preference_update_error(preference_key, preference_value, error) # pylint: disable=raise-missing-from # noqa: B904 else: delete_user_preference(requesting_user, preference_key) + invalidate_user_preferences_cache(user) @intercept_errors(UserAPIInternalError, ignore_errors=[UserAPIRequestError]) @@ -209,6 +212,7 @@ def set_user_preference(requesting_user, preference_key, preference_value, usern serializer.save() except Exception as error: raise _create_preference_update_error(preference_key, preference_value, error) # pylint: disable=raise-missing-from # noqa: B904 + invalidate_user_preferences_cache(existing_user) @intercept_errors(UserAPIInternalError, ignore_errors=[UserAPIRequestError]) @@ -254,6 +258,7 @@ def delete_user_preference(requesting_user, preference_key, username=None): preference_key=preference_key ), ) + invalidate_user_preferences_cache(existing_user) return True diff --git a/openedx/features/course_experience/tests/views/test_course_updates.py b/openedx/features/course_experience/tests/views/test_course_updates.py index f35e2a792a37..91db449c2403 100644 --- a/openedx/features/course_experience/tests/views/test_course_updates.py +++ b/openedx/features/course_experience/tests/views/test_course_updates.py @@ -50,7 +50,7 @@ def test_queries(self): # Fetch the view and verify that the query counts haven't changed # TODO: decrease query count as part of REVO-28 - with self.assertNumQueries(54, table_ignorelist=QUERY_COUNT_TABLE_IGNORELIST): + with self.assertNumQueries(52, table_ignorelist=QUERY_COUNT_TABLE_IGNORELIST): with check_mongo_calls(3): url = course_updates_url(self.course) self.client.get(url)