Revert "[FEAT]: Add mem caching to the API to create and retrieve IntegritySignature" (#27936)
This reverts commit c6192b8b40656c44ba0a89cdd569fb0c0e4f87c4. The caching does little to save performance and in the case of whole course interation, it has a netgative performance impact.
This commit is contained in:
@@ -5,11 +5,9 @@ Agreements API
|
||||
import logging
|
||||
|
||||
from django.contrib.auth import get_user_model
|
||||
from django.core.cache import cache
|
||||
from django.core.exceptions import ObjectDoesNotExist
|
||||
from opaque_keys.edx.keys import CourseKey
|
||||
|
||||
from openedx.core.djangoapps.agreements.cache import get_integrity_signature_cache_key
|
||||
from openedx.core.djangoapps.agreements.models import IntegritySignature
|
||||
|
||||
log = logging.getLogger(__name__)
|
||||
@@ -35,9 +33,6 @@ def create_integrity_signature(username, course_id):
|
||||
'Integrity signature already exists for user_id={user_id} and '
|
||||
'course_id={course_id}'.format(user_id=user.id, course_id=course_id)
|
||||
)
|
||||
cache_key = get_integrity_signature_cache_key(username, course_id)
|
||||
# Write into the cache for future retrieval
|
||||
cache.set(cache_key, signature)
|
||||
return signature
|
||||
|
||||
|
||||
@@ -53,17 +48,10 @@ def get_integrity_signature(username, course_id):
|
||||
* An IntegritySignature object, or None if one does not exist for the
|
||||
user + course combination.
|
||||
"""
|
||||
cache_key = get_integrity_signature_cache_key(username, course_id)
|
||||
cached_integrity_signature = cache.get(cache_key)
|
||||
if cached_integrity_signature:
|
||||
return cached_integrity_signature
|
||||
|
||||
user = User.objects.get(username=username)
|
||||
course_key = CourseKey.from_string(course_id)
|
||||
try:
|
||||
signature = IntegritySignature.objects.get(user=user, course_key=course_key)
|
||||
cache.set(cache_key, signature)
|
||||
return signature
|
||||
return IntegritySignature.objects.get(user=user, course_key=course_key)
|
||||
except ObjectDoesNotExist:
|
||||
return None
|
||||
|
||||
@@ -78,12 +66,5 @@ def get_integrity_signatures_for_course(course_id):
|
||||
Returns:
|
||||
* QuerySet of IntegritySignature objects (can be empty).
|
||||
"""
|
||||
|
||||
course_key = CourseKey.from_string(course_id)
|
||||
course_integrity_signature = IntegritySignature.objects.filter(
|
||||
course_key=course_key
|
||||
).select_related('user')
|
||||
for signature in course_integrity_signature:
|
||||
cache_key = get_integrity_signature_cache_key(signature.user.username, course_id)
|
||||
cache.set(cache_key, signature)
|
||||
return course_integrity_signature
|
||||
return IntegritySignature.objects.filter(course_key=course_key)
|
||||
|
||||
@@ -1,13 +0,0 @@
|
||||
# lint-amnesty, pylint: disable=missing-module-docstring
|
||||
# Template used to create cache keys for Integrity Signatures
|
||||
INTEGRITY_SIGNATURE_CACHE_KEY_TPL = 'integrity-signature-{course_id}-{username}'
|
||||
|
||||
|
||||
def get_integrity_signature_cache_key(username, course_id):
|
||||
"""
|
||||
Util function to help form the cache key for integrity signature
|
||||
"""
|
||||
return INTEGRITY_SIGNATURE_CACHE_KEY_TPL.format(
|
||||
username=username,
|
||||
course_id=course_id,
|
||||
)
|
||||
@@ -3,14 +3,13 @@ Tests for the Agreements API
|
||||
"""
|
||||
import logging
|
||||
|
||||
from django.core.cache import cache
|
||||
from testfixtures import LogCapture
|
||||
|
||||
from common.djangoapps.student.tests.factories import UserFactory
|
||||
from openedx.core.djangoapps.agreements.api import (
|
||||
create_integrity_signature,
|
||||
get_integrity_signature,
|
||||
get_integrity_signatures_for_course
|
||||
get_integrity_signatures_for_course,
|
||||
)
|
||||
from openedx.core.djangolib.testing.utils import skip_unless_lms
|
||||
from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase
|
||||
@@ -63,17 +62,15 @@ class TestIntegritySignatureApi(SharedModuleStoreTestCase):
|
||||
Test to get an integrity signature
|
||||
"""
|
||||
create_integrity_signature(self.user.username, self.course_id)
|
||||
with self.assertNumQueries(0):
|
||||
signature = get_integrity_signature(self.user.username, self.course_id)
|
||||
self._assert_integrity_signature(signature)
|
||||
signature = get_integrity_signature(self.user.username, self.course_id)
|
||||
self._assert_integrity_signature(signature)
|
||||
|
||||
def test_get_nonexistent_integrity_signature(self):
|
||||
"""
|
||||
Test that None is returned if an integrity signature does not exist
|
||||
"""
|
||||
with self.assertNumQueries(2):
|
||||
signature = get_integrity_signature(self.user.username, self.course_id)
|
||||
self.assertIsNone(signature)
|
||||
signature = get_integrity_signature(self.user.username, self.course_id)
|
||||
self.assertIsNone(signature)
|
||||
|
||||
def test_get_integrity_signatures_for_course(self):
|
||||
"""
|
||||
@@ -101,21 +98,3 @@ class TestIntegritySignatureApi(SharedModuleStoreTestCase):
|
||||
"""
|
||||
self.assertEqual(signature.user, self.user)
|
||||
self.assertEqual(signature.course_key, self.course.id)
|
||||
|
||||
def test_get_integrity_signatures_for_course_cached(self):
|
||||
"""
|
||||
Test to ensure the integrity_signatures retrieved by course is also set into cache
|
||||
"""
|
||||
create_integrity_signature(self.user.username, self.course_id)
|
||||
second_user = UserFactory()
|
||||
create_integrity_signature(second_user.username, self.course_id)
|
||||
cache.clear()
|
||||
with self.assertNumQueries(1):
|
||||
get_integrity_signatures_for_course(self.course_id)
|
||||
|
||||
with self.assertNumQueries(0):
|
||||
signature = get_integrity_signature(self.user.username, self.course_id)
|
||||
self._assert_integrity_signature(signature)
|
||||
|
||||
with self.assertNumQueries(0):
|
||||
get_integrity_signature(second_user.username, self.course_id)
|
||||
|
||||
Reference in New Issue
Block a user