Cache CcxFieldOverrides on a per-request basis
This commit is contained in:
@@ -84,35 +84,40 @@ def get_override_for_ccx(ccx, block, name, default=None):
|
||||
specify the block and the name of the field. If the field is not
|
||||
overridden for the given ccx, returns `default`.
|
||||
"""
|
||||
if not hasattr(block, '_ccx_overrides'):
|
||||
block._ccx_overrides = {} # pylint: disable=protected-access
|
||||
overrides = block._ccx_overrides.get(ccx.id) # pylint: disable=protected-access
|
||||
if overrides is None:
|
||||
overrides = _get_overrides_for_ccx(ccx, block)
|
||||
block._ccx_overrides[ccx.id] = overrides # pylint: disable=protected-access
|
||||
return overrides.get(name, default)
|
||||
overrides = _get_overrides_for_ccx(ccx)
|
||||
|
||||
if isinstance(block.location, CCXBlockUsageLocator):
|
||||
non_ccx_key = block.location.to_block_locator()
|
||||
else:
|
||||
non_ccx_key = block.location
|
||||
|
||||
block_overrides = overrides.get(non_ccx_key, {})
|
||||
if name in block_overrides:
|
||||
return block.fields[name].from_json(block_overrides[name])
|
||||
else:
|
||||
return default
|
||||
|
||||
|
||||
def _get_overrides_for_ccx(ccx, block):
|
||||
def _get_overrides_for_ccx(ccx):
|
||||
"""
|
||||
Returns a dictionary mapping field name to overriden value for any
|
||||
overrides set on this block for this CCX.
|
||||
"""
|
||||
overrides = {}
|
||||
# block as passed in may have a location specific to a CCX, we must strip
|
||||
# that for this query
|
||||
location = block.location
|
||||
if isinstance(block.location, CCXBlockUsageLocator):
|
||||
location = block.location.to_block_locator()
|
||||
query = CcxFieldOverride.objects.filter(
|
||||
ccx=ccx,
|
||||
location=location
|
||||
)
|
||||
for override in query:
|
||||
field = block.fields[override.field]
|
||||
value = field.from_json(json.loads(override.value))
|
||||
overrides[override.field] = value
|
||||
return overrides
|
||||
overrides_cache = request_cache.get_cache('ccx-overrides')
|
||||
|
||||
if ccx not in overrides_cache:
|
||||
overrides = {}
|
||||
query = CcxFieldOverride.objects.filter(
|
||||
ccx=ccx,
|
||||
)
|
||||
|
||||
for override in query:
|
||||
block_overrides = overrides.setdefault(override.location, {})
|
||||
block_overrides[override.field] = json.loads(override.value)
|
||||
|
||||
overrides_cache[ccx] = overrides
|
||||
|
||||
return overrides_cache[ccx]
|
||||
|
||||
|
||||
@transaction.commit_on_success
|
||||
@@ -123,23 +128,25 @@ def override_field_for_ccx(ccx, block, name, value):
|
||||
value to set for the given field.
|
||||
"""
|
||||
field = block.fields[name]
|
||||
value = json.dumps(field.to_json(value))
|
||||
value_json = field.to_json(value)
|
||||
serialized_value = json.dumps(value_json)
|
||||
try:
|
||||
override = CcxFieldOverride.objects.create(
|
||||
ccx=ccx,
|
||||
location=block.location,
|
||||
field=name,
|
||||
value=value)
|
||||
value=serialized_value
|
||||
)
|
||||
except IntegrityError:
|
||||
transaction.commit()
|
||||
override = CcxFieldOverride.objects.get(
|
||||
ccx=ccx,
|
||||
location=block.location,
|
||||
field=name)
|
||||
override.value = value
|
||||
field=name
|
||||
)
|
||||
override.value = serialized_value
|
||||
override.save()
|
||||
if hasattr(block, '_ccx_overrides'):
|
||||
del block._ccx_overrides[ccx.id] # pylint: disable=protected-access
|
||||
_get_overrides_for_ccx(ccx).setdefault(block.location, {})[name] = value_json
|
||||
|
||||
|
||||
def clear_override_for_ccx(ccx, block, name):
|
||||
@@ -155,8 +162,7 @@ def clear_override_for_ccx(ccx, block, name):
|
||||
location=block.location,
|
||||
field=name).delete()
|
||||
|
||||
if hasattr(block, '_ccx_overrides'):
|
||||
del block._ccx_overrides[ccx.id] # pylint: disable=protected-access
|
||||
_get_overrides_for_ccx(ccx).setdefault(block.location, {}).pop(name)
|
||||
|
||||
except CcxFieldOverride.DoesNotExist:
|
||||
pass
|
||||
|
||||
@@ -250,9 +250,9 @@ class TestFieldOverrideSplitPerformance(FieldOverridePerformanceTestCase):
|
||||
('ccx', 1, True, False): (23, 4, 9),
|
||||
('ccx', 2, True, False): (68, 19, 54),
|
||||
('ccx', 3, True, False): (263, 84, 215),
|
||||
('ccx', 1, True, True): (33, 4, 13),
|
||||
('ccx', 2, True, True): (68, 19, 84),
|
||||
('ccx', 3, True, True): (263, 84, 335),
|
||||
('ccx', 1, True, True): (25, 4, 13),
|
||||
('ccx', 2, True, True): (70, 19, 84),
|
||||
('ccx', 3, True, True): (265, 84, 335),
|
||||
('no_overrides', 1, False, False): (23, 4, 9),
|
||||
('no_overrides', 2, False, False): (68, 19, 54),
|
||||
('no_overrides', 3, False, False): (263, 84, 215),
|
||||
|
||||
@@ -100,7 +100,7 @@ class TestFieldOverrides(ModuleStoreTestCase):
|
||||
"""
|
||||
ccx_start = datetime.datetime(2014, 12, 25, 00, 00, tzinfo=pytz.UTC)
|
||||
chapter = self.ccx.course.get_children()[0]
|
||||
with self.assertNumQueries(4):
|
||||
with self.assertNumQueries(3):
|
||||
override_field_for_ccx(self.ccx, chapter, 'start', ccx_start)
|
||||
dummy = chapter.start
|
||||
|
||||
@@ -110,7 +110,7 @@ class TestFieldOverrides(ModuleStoreTestCase):
|
||||
"""
|
||||
ccx_start = datetime.datetime(2014, 12, 25, 00, 00, tzinfo=pytz.UTC)
|
||||
chapter = self.ccx.course.get_children()[0]
|
||||
with self.assertNumQueries(4):
|
||||
with self.assertNumQueries(3):
|
||||
override_field_for_ccx(self.ccx, chapter, 'start', ccx_start)
|
||||
dummy1 = chapter.start
|
||||
dummy2 = chapter.start
|
||||
|
||||
@@ -21,6 +21,7 @@ from django.test.utils import override_settings
|
||||
from django.test import RequestFactory
|
||||
from edxmako.shortcuts import render_to_response # pylint: disable=import-error
|
||||
from student.models import CourseEnrollment
|
||||
from request_cache.middleware import RequestCache
|
||||
from student.roles import CourseCcxCoachRole # pylint: disable=import-error
|
||||
from student.tests.factories import ( # pylint: disable=import-error
|
||||
AdminFactory,
|
||||
@@ -503,26 +504,6 @@ class TestCCXGrades(ModuleStoreTestCase, LoginEnrollmentTestCase):
|
||||
role.add_users(coach)
|
||||
ccx = CcxFactory(course_id=course.id, coach=self.coach)
|
||||
|
||||
# Apparently the test harness doesn't use LmsFieldStorage, and I'm not
|
||||
# sure if there's a way to poke the test harness to do so. So, we'll
|
||||
# just inject the override field storage in this brute force manner.
|
||||
OverrideFieldData.provider_classes = None
|
||||
# pylint: disable=protected-access
|
||||
for block in iter_blocks(course):
|
||||
block._field_data = OverrideFieldData.wrap(coach, course, block._field_data)
|
||||
new_cache = {'tabs': [], 'discussion_topics': []}
|
||||
if 'grading_policy' in block._field_data_cache:
|
||||
new_cache['grading_policy'] = block._field_data_cache['grading_policy']
|
||||
block._field_data_cache = new_cache
|
||||
|
||||
def cleanup_provider_classes():
|
||||
"""
|
||||
After everything is done, clean up by un-doing the change to the
|
||||
OverrideFieldData object that is done during the wrap method.
|
||||
"""
|
||||
OverrideFieldData.provider_classes = None
|
||||
self.addCleanup(cleanup_provider_classes)
|
||||
|
||||
# override course grading policy and make last section invisible to students
|
||||
override_field_for_ccx(ccx, course, 'grading_policy', {
|
||||
'GRADER': [
|
||||
@@ -561,9 +542,13 @@ class TestCCXGrades(ModuleStoreTestCase, LoginEnrollmentTestCase):
|
||||
|
||||
self.client.login(username=coach.username, password="test")
|
||||
|
||||
self.addCleanup(RequestCache.clear_request_cache)
|
||||
|
||||
@patch('ccx.views.render_to_response', intercept_renderer)
|
||||
def test_gradebook(self):
|
||||
self.course.enable_ccx = True
|
||||
RequestCache.clear_request_cache()
|
||||
|
||||
url = reverse(
|
||||
'ccx_gradebook',
|
||||
kwargs={'course_id': self.ccx_key}
|
||||
@@ -580,6 +565,8 @@ class TestCCXGrades(ModuleStoreTestCase, LoginEnrollmentTestCase):
|
||||
|
||||
def test_grades_csv(self):
|
||||
self.course.enable_ccx = True
|
||||
RequestCache.clear_request_cache()
|
||||
|
||||
url = reverse(
|
||||
'ccx_grades_csv',
|
||||
kwargs={'course_id': self.ccx_key}
|
||||
@@ -591,11 +578,11 @@ class TestCCXGrades(ModuleStoreTestCase, LoginEnrollmentTestCase):
|
||||
response.content.strip().split('\n')
|
||||
)
|
||||
data = dict(zip(headers, row))
|
||||
self.assertTrue('HW 04' not in data)
|
||||
self.assertEqual(data['HW 01'], '0.75')
|
||||
self.assertEqual(data['HW 02'], '0.5')
|
||||
self.assertEqual(data['HW 03'], '0.25')
|
||||
self.assertEqual(data['HW Avg'], '0.5')
|
||||
self.assertTrue('HW 04' not in data)
|
||||
|
||||
@patch('courseware.views.render_to_response', intercept_renderer)
|
||||
def test_student_progress(self):
|
||||
|
||||
Reference in New Issue
Block a user