Skip to content

Commit 021462a

Browse files
committed
perf(user_api): implement centralized caching for user preferences
Introduces a Django-based caching mechanism at the Model layer for UserPreference lookups. This mitigates severe database load caused by redundant queries, stabilizes the DB, and centralizes cache logic. Updated affected tests with LocMemCache to assert the new, reduced query counts.
1 parent 22512ba commit 021462a

14 files changed

Lines changed: 128 additions & 59 deletions

File tree

cms/djangoapps/contentstore/rest_api/v1/views/tests/test_course_index.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -161,7 +161,7 @@ def test_number_of_calls_to_db(self):
161161
"""
162162
Test to check number of queries made to mysql and mongo
163163
"""
164-
with self.assertNumQueries(34, table_ignorelist=WAFFLE_TABLES):
164+
with self.assertNumQueries(33, table_ignorelist=WAFFLE_TABLES):
165165
with check_mongo_calls(3):
166166
self.client.get(self.url)
167167

cms/djangoapps/contentstore/views/tests/test_course_index.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -227,7 +227,7 @@ def test_number_of_calls_to_db(self):
227227
"""
228228
Test to check number of queries made to mysql and mongo
229229
"""
230-
with self.assertNumQueries(21, table_ignorelist=WAFFLE_TABLES):
230+
with self.assertNumQueries(20, table_ignorelist=WAFFLE_TABLES):
231231
with check_mongo_calls(3):
232232
self.client.get(reverse_course_url('course_handler', self.course.id), content_type="application/json")
233233

lms/djangoapps/certificates/apis/v0/tests/test_views.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -333,7 +333,7 @@ def test_query_counts(self):
333333
assert len(resp.data) == 0
334334

335335
# Test student with 1 certificate
336-
with self.assertNumQueries(13, table_ignorelist=WAFFLE_TABLES):
336+
with self.assertNumQueries(12, table_ignorelist=WAFFLE_TABLES):
337337
resp = self.get_response(
338338
AuthType.jwt,
339339
requesting_user=self.global_staff,
@@ -373,7 +373,7 @@ def test_query_counts(self):
373373
download_url='www.google.com',
374374
grade="0.88",
375375
)
376-
with self.assertNumQueries(13, table_ignorelist=WAFFLE_TABLES):
376+
with self.assertNumQueries(12, table_ignorelist=WAFFLE_TABLES):
377377
resp = self.get_response(
378378
AuthType.jwt,
379379
requesting_user=self.global_staff,

lms/djangoapps/course_api/tests/test_views.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -446,7 +446,7 @@ def test_too_many_courses(self):
446446
self.setup_user(self.audit_user)
447447

448448
# These query counts were found empirically
449-
query_counts = [57, 46, 46, 46, 46, 46, 46, 46, 46, 43, 12]
449+
query_counts = [56, 44, 44, 44, 44, 44, 44, 44, 44, 41, 10]
450450
ordered_course_ids = sorted([str(cid) for cid in (course_ids + [c.id for c in self.courses])], key=str.lower)
451451

452452
self.clear_caches()

lms/djangoapps/courseware/tests/test_views.py

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1280,8 +1280,8 @@ def test_view_certificate_link(self):
12801280
self.assertContains(resp, "earned a certificate for this course.")
12811281

12821282
@ddt.data(
1283-
(True, 56),
1284-
(False, 56),
1283+
(True, 54),
1284+
(False, 54),
12851285
)
12861286
@ddt.unpack
12871287
def test_progress_queries_paced_courses(self, self_paced, query_count):
@@ -1296,13 +1296,13 @@ def test_progress_queries(self):
12961296
ContentTypeGatingConfig.objects.create(enabled=True, enabled_as_of=datetime(2018, 1, 1))
12971297
self.setup_course()
12981298
with self.assertNumQueries(
1299-
56, table_ignorelist=QUERY_COUNT_TABLE_IGNORELIST
1299+
54, table_ignorelist=QUERY_COUNT_TABLE_IGNORELIST
13001300
), check_mongo_calls(2):
13011301
self._get_progress_page()
13021302

13031303
for _ in range(2):
13041304
with self.assertNumQueries(
1305-
39, table_ignorelist=QUERY_COUNT_TABLE_IGNORELIST
1305+
36, table_ignorelist=QUERY_COUNT_TABLE_IGNORELIST
13061306
), check_mongo_calls(2):
13071307
self._get_progress_page()
13081308

openedx/core/djangoapps/bookmarks/tests/test_views.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -268,7 +268,7 @@ def test_post_bookmark_with_invalid_data(self):
268268
assert response.data['developer_message'] == 'Parameter usage_id not provided.'
269269

270270
# Send empty data dictionary.
271-
with self.assertNumQueries(9): # No queries for bookmark table.
271+
with self.assertNumQueries(7): # No queries for bookmark table.
272272
response = self.send_post(
273273
client=self.client,
274274
url=reverse('bookmarks'),

openedx/core/djangoapps/content_libraries/tests/test_containers.py

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -801,7 +801,7 @@ def test_unit_collections(self) -> None:
801801
assert unit_as_read['collections'] == [{"title": col1.title, "key": col1.collection_code}]
802802

803803
def test_section_hierarchy(self):
804-
with self.assertNumQueries(126):
804+
with self.assertNumQueries(124):
805805
hierarchy = self._get_container_hierarchy(self.section_with_subsections["id"])
806806
assert hierarchy["object_key"] == self.section_with_subsections["id"]
807807
assert hierarchy["components"] == [
@@ -827,7 +827,7 @@ def test_section_hierarchy(self):
827827
]
828828

829829
def test_subsection_hierarchy(self):
830-
with self.assertNumQueries(91):
830+
with self.assertNumQueries(89):
831831
hierarchy = self._get_container_hierarchy(self.subsection_with_units["id"])
832832
assert hierarchy["object_key"] == self.subsection_with_units["id"]
833833
assert hierarchy["components"] == [
@@ -850,7 +850,7 @@ def test_subsection_hierarchy(self):
850850
]
851851

852852
def test_units_hierarchy(self):
853-
with self.assertNumQueries(56):
853+
with self.assertNumQueries(54):
854854
hierarchy = self._get_container_hierarchy(self.unit_with_components["id"])
855855
assert hierarchy["object_key"] == self.unit_with_components["id"]
856856
assert hierarchy["components"] == [
@@ -876,7 +876,7 @@ def test_container_hierarchy_not_found(self):
876876
)
877877

878878
def test_block_hierarchy(self):
879-
with self.assertNumQueries(24):
879+
with self.assertNumQueries(22):
880880
hierarchy = self._get_block_hierarchy(self.problem_block["id"])
881881
assert hierarchy["object_key"] == self.problem_block["id"]
882882
assert hierarchy["components"] == [

openedx/core/djangoapps/content_tagging/rest_api/v1/tests/test_views.py

Lines changed: 27 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -520,13 +520,13 @@ def test_create_taxonomy(self, user_attr: str, expected_status: int) -> None:
520520
assert response.data["orgs"] == [self.orgA.short_name]
521521

522522
@ddt.data(
523-
('staff', 11),
524-
("content_creatorA", 23),
525-
("library_staffA", 23),
526-
("library_userA", 23),
527-
("instructorA", 23),
528-
("course_instructorA", 23),
529-
("course_staffA", 23),
523+
('staff', 10),
524+
("content_creatorA", 22),
525+
("library_staffA", 22),
526+
("library_userA", 22),
527+
("instructorA", 22),
528+
("course_instructorA", 22),
529+
("course_staffA", 22),
530530
)
531531
@ddt.unpack
532532
def test_list_taxonomy_query_count(self, user_attr: str, expected_queries: int):
@@ -1980,19 +1980,19 @@ def test_get_copied_tags(self):
19801980
assert response.data[str(object_id_2)]["taxonomies"] == expected_tags
19811981

19821982
@ddt.data(
1983-
('staff', 'courseA', 10),
1984-
('staff', 'libraryA', 17),
1985-
('staff', 'collection_key', 17),
1986-
("content_creatorA", 'courseA', 20, False),
1987-
("content_creatorA", 'libraryA', 23, False),
1988-
("content_creatorA", 'collection_key', 23, False),
1989-
("library_staffA", 'libraryA', 23, False), # Library users can only view objecttags, not change them?
1990-
("library_staffA", 'collection_key', 23, False),
1991-
("library_userA", 'libraryA', 23, False),
1992-
("library_userA", 'collection_key', 23, False),
1993-
("instructorA", 'courseA', 20),
1994-
("course_instructorA", 'courseA', 20),
1995-
("course_staffA", 'courseA', 20),
1983+
('staff', 'courseA', 9),
1984+
('staff', 'libraryA', 16),
1985+
('staff', 'collection_key', 16),
1986+
("content_creatorA", 'courseA', 19, False),
1987+
("content_creatorA", 'libraryA', 22, False),
1988+
("content_creatorA", 'collection_key', 22, False),
1989+
("library_staffA", 'libraryA', 22, False), # Library users can only view objecttags, not change them?
1990+
("library_staffA", 'collection_key', 22, False),
1991+
("library_userA", 'libraryA', 22, False),
1992+
("library_userA", 'collection_key', 22, False),
1993+
("instructorA", 'courseA', 19),
1994+
("course_instructorA", 'courseA', 19),
1995+
("course_staffA", 'courseA', 19),
19961996
)
19971997
@ddt.unpack
19981998
def test_object_tags_query_count(
@@ -2878,13 +2878,13 @@ class TestTaxonomyTagsViewSet(TestTaxonomyObjectsMixin, APITestCase):
28782878
Test cases for TaxonomyTagsViewSet retrive action.
28792879
"""
28802880
@ddt.data(
2881-
('staff', 11),
2882-
("content_creatorA", 13),
2883-
("library_staffA", 13),
2884-
("library_userA", 13),
2885-
("instructorA", 13),
2886-
("course_instructorA", 13),
2887-
("course_staffA", 13),
2881+
('staff', 10),
2882+
("content_creatorA", 12),
2883+
("library_staffA", 12),
2884+
("library_userA", 12),
2885+
("instructorA", 12),
2886+
("course_instructorA", 12),
2887+
("course_staffA", 12),
28882888
)
28892889
@ddt.unpack
28902890
def test_taxonomy_tags_query_count(self, user_attr: str, expected_queries: int):

openedx/core/djangoapps/lang_pref/tests/test_middleware.py

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ class TestUserPreferenceMiddleware(CacheIsolationTestCase):
4242
"""
4343
Tests to make sure user preferences are getting properly set in the middleware.
4444
"""
45+
ENABLED_CACHES = ['default']
4546

4647
def setUp(self):
4748
super().setUp()
@@ -228,7 +229,7 @@ def test_preference_update_noop(self):
228229

229230
response = mock.Mock(spec=HttpResponse)
230231

231-
with self.assertNumQueries(1):
232+
with self.assertNumQueries(0):
232233
self.middleware.process_response(self.request, response)
233234

234235
# Preference is the same as the cookie, shouldn't write to the database
@@ -240,7 +241,7 @@ def test_preference_update_noop(self):
240241

241242
response = mock.Mock(spec=HttpResponse)
242243

243-
with self.assertNumQueries(1):
244+
with self.assertNumQueries(0):
244245
self.middleware.process_response(self.request, response)
245246

246247
# Cookie changed, should write to the database again
@@ -249,7 +250,7 @@ def test_preference_update_noop(self):
249250
self.middleware.process_request(self.request)
250251
assert get_user_preference(self.user, LANGUAGE_KEY) == 'en'
251252

252-
with self.assertNumQueries(1):
253+
with self.assertNumQueries(0):
253254
self.middleware.process_response(self.request, response)
254255

255256
@mock.patch('openedx.core.djangoapps.lang_pref.middleware.is_request_from_mobile_app')

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

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -235,7 +235,7 @@ def test_get_username(self):
235235
Test that a client (logged in) can get her own username.
236236
"""
237237
self.client.login(username=self.user.username, password=TEST_PASSWORD)
238-
self._verify_get_own_username(18)
238+
self._verify_get_own_username(17)
239239

240240
def test_get_username_inactive(self):
241241
"""
@@ -245,7 +245,7 @@ def test_get_username_inactive(self):
245245
self.client.login(username=self.user.username, password=TEST_PASSWORD)
246246
self.user.is_active = False
247247
self.user.save()
248-
self._verify_get_own_username(18)
248+
self._verify_get_own_username(17)
249249

250250
def test_get_username_not_logged_in(self):
251251
"""
@@ -361,7 +361,7 @@ class TestAccountsAPI(FilteredQueryCountMixin, CacheIsolationTestCase, UserAPITe
361361
"""
362362

363363
ENABLED_CACHES = ['default']
364-
TOTAL_QUERY_COUNT = 25
364+
TOTAL_QUERY_COUNT = 24
365365
FULL_RESPONSE_FIELD_COUNT = 28
366366

367367
def setUp(self):
@@ -815,12 +815,12 @@ def verify_get_own_information(queries):
815815
assert data['time_zone'] is None
816816

817817
self.client.login(username=self.user.username, password=TEST_PASSWORD)
818-
verify_get_own_information(self._get_num_queries(23))
818+
verify_get_own_information(self._get_num_queries(22))
819819

820820
# Now make sure that the user can get the same information, even if not active
821821
self.user.is_active = False
822822
self.user.save()
823-
verify_get_own_information(self._get_num_queries(15))
823+
verify_get_own_information(self._get_num_queries(13))
824824

825825
def test_get_account_empty_string(self):
826826
"""
@@ -835,7 +835,7 @@ def test_get_account_empty_string(self):
835835
legacy_profile.save()
836836

837837
self.client.login(username=self.user.username, password=TEST_PASSWORD)
838-
with self.assertNumQueries(self._get_num_queries(23), table_ignorelist=WAFFLE_TABLES):
838+
with self.assertNumQueries(self._get_num_queries(22), table_ignorelist=WAFFLE_TABLES):
839839
response = self.send_get(self.client)
840840
for empty_field in ("level_of_education", "gender", "country", "state", "bio",):
841841
assert response.data[empty_field] is None

0 commit comments

Comments
 (0)