From 2b4da4d836cf67f1241b83550a26de4a8de0cc0f Mon Sep 17 00:00:00 2001 From: Chris Rossi Date: Fri, 19 Dec 2014 11:58:37 -0500 Subject: [PATCH] MIT: CCX. Address performance issues. Use client's session to set POC for test, now that we have the capability to do that. Use a middleware to set the current POC one time per request, and avoid having to look up the stack for the current request in 'get_current_poc'. Fetch all overrides for a block at one time. Speed up the lineage computation by doing some caching. Unused import. Get field overrides once per user per block. Streamline configuration. Fix poc config tuples --- lms/djangoapps/courseware/field_overrides.py | 21 ++++- .../courseware/student_field_overrides.py | 37 +++++--- lms/djangoapps/pocs/overrides.py | 93 ++++++++++++------- lms/djangoapps/pocs/tests/test_views.py | 26 +++--- lms/djangoapps/pocs/views.py | 33 +++---- lms/envs/aws.py | 14 +++ lms/envs/test.py | 1 + 7 files changed, 145 insertions(+), 80 deletions(-) diff --git a/lms/djangoapps/courseware/field_overrides.py b/lms/djangoapps/courseware/field_overrides.py index 2901d81953..a84dc001b4 100644 --- a/lms/djangoapps/courseware/field_overrides.py +++ b/lms/djangoapps/courseware/field_overrides.py @@ -199,7 +199,20 @@ def _lineage(block): Returns an iterator over all ancestors of the given block, starting with its immediate parent and ending at the root of the block tree. """ - location = modulestore().get_parent_location(block.location) - while location: - yield modulestore().get_item(location) - location = modulestore().get_parent_location(location) + parent = _get_parent(block) + while parent: + yield parent + parent = _get_parent(parent) + + +def _get_parent(block): + """ + Gets parent of a block, trying to cache result. + """ + if not hasattr(block, '_parent'): + store = modulestore() + # Call to get_parent_location is fairly expensive. Is there a more + # performant way to get at the ancestors of a block? + location = store.get_parent_location(block.location) + block._parent = store.get_item(location) if location else None + return block._parent diff --git a/lms/djangoapps/courseware/student_field_overrides.py b/lms/djangoapps/courseware/student_field_overrides.py index 1e22efa6bc..9b9134b2aa 100644 --- a/lms/djangoapps/courseware/student_field_overrides.py +++ b/lms/djangoapps/courseware/student_field_overrides.py @@ -24,18 +24,31 @@ def get_override_for_user(user, block, name, default=None): specify the block and the name of the field. If the field is not overridden for the given user, returns `default`. """ - try: - override = StudentFieldOverride.objects.get( - course_id=block.runtime.course_id, - location=block.location, - student_id=user.id, - field=name - ) - field = block.fields[name] - return field.from_json(json.loads(override.value)) - except StudentFieldOverride.DoesNotExist: - pass - return default + if not hasattr(block, '_student_overrides'): + block._student_overrides = {} + overrides = block._student_overrides.get(user.id) + if overrides is None: + overrides = _get_overrides_for_user(user, block) + block._student_overrides[user.id] = overrides + return overrides.get(name, default) + + +def _get_overrides_for_user(user, block): + """ + Gets all of the individual student overrides for given user and block. + Returns a dictionary of field override values keyed by field name. + """ + query = StudentFieldOverride.objects.filter( + course_id=block.runtime.course_id, + location=block.location, + student_id=user.id, + ) + overrides = {} + for override in query: + field = block.fields[override.field] + value = field.from_json(json.loads(override.value)) + overrides[override.field] = value + return overrides def override_field_for_user(user, block, name, value): diff --git a/lms/djangoapps/pocs/overrides.py b/lms/djangoapps/pocs/overrides.py index 08ed7a30a6..bc2506165d 100644 --- a/lms/djangoapps/pocs/overrides.py +++ b/lms/djangoapps/pocs/overrides.py @@ -7,7 +7,6 @@ import threading from contextlib import contextmanager -from courseware.courses import get_request_for_thread from courseware.field_overrides import FieldOverrideProvider from pocs import ACTIVE_POC_KEY @@ -54,34 +53,12 @@ def poc_context(poc): def get_current_poc(user): """ - Return the poc that is active for this user - - The user's session data is used to look up the active poc by id - If no poc is active, None is returned and MOOC view will take precedence - Active poc can be overridden by context manager (see `poc_context`) + Return the poc that is active for this request. """ - # If poc context is explicitly set, that takes precedence over the user's - # session. poc = _POC_CONTEXT.poc if poc: return poc - request = get_request_for_thread() - if request is None: - return None - - poc = None - poc_id = request.session.get(ACTIVE_POC_KEY, None) - if poc_id is not None: - try: - membership = PocMembership.objects.get( - student=user, active=True, poc__id__exact=poc_id - ) - poc = membership.poc - except PocMembership.DoesNotExist: - pass - return poc - def get_override_for_poc(poc, block, name, default=None): """ @@ -89,16 +66,30 @@ def get_override_for_poc(poc, block, name, default=None): specify the block and the name of the field. If the field is not overridden for the given poc, returns `default`. """ - try: - override = PocFieldOverride.objects.get( - poc=poc, - location=block.location, - field=name) - field = block.fields[name] - return field.from_json(json.loads(override.value)) - except PocFieldOverride.DoesNotExist: - pass - return default + if not hasattr(block, '_poc_overrides'): + block._poc_overrides = {} + overrides = block._poc_overrides.get(poc.id) + if overrides is None: + overrides = _get_overrides_for_poc(poc, block) + block._poc_overrides[poc.id] = overrides + return overrides.get(name, default) + + +def _get_overrides_for_poc(poc, block): + """ + Returns a dictionary mapping field name to overriden value for any + overrides set on this block for this POC. + """ + overrides = {} + query = PocFieldOverride.objects.filter( + poc=poc, + location=block.location + ) + for override in query: + field = block.fields[override.field] + value = field.from_json(json.loads(override.value)) + overrides[override.field] = value + return overrides def override_field_for_poc(poc, block, name, value): @@ -115,6 +106,9 @@ def override_field_for_poc(poc, block, name, value): override.value = json.dumps(field.to_json(value)) override.save() + if hasattr(block, '_poc_overrides'): + del block._poc_overrides[poc.id] + def clear_override_for_poc(poc, block, name): """ @@ -128,5 +122,36 @@ def clear_override_for_poc(poc, block, name): poc=poc, location=block.location, field=name).delete() + + if hasattr(block, '_poc_overrides'): + del block._poc_overrides[poc.id] + except PocFieldOverride.DoesNotExist: pass + + +class PocMiddleware(object): + """ + Checks to see if current session is examining a POC and sets the POC as + the current POC for the override machinery if so. + """ + def process_request(self, request): + """ + Do the check. + """ + poc_id = request.session.get(ACTIVE_POC_KEY, None) + if poc_id is not None: + try: + membership = PocMembership.objects.get( + student=request.user, active=True, poc__id__exact=poc_id + ) + _POC_CONTEXT.poc = membership.poc + except PocMembership.DoesNotExist: + pass + + def process_response(self, request, response): + """ + Clean up afterwards. + """ + _POC_CONTEXT.poc = None + return response diff --git a/lms/djangoapps/pocs/tests/test_views.py b/lms/djangoapps/pocs/tests/test_views.py index 779ed46b76..bd6693328f 100644 --- a/lms/djangoapps/pocs/tests/test_views.py +++ b/lms/djangoapps/pocs/tests/test_views.py @@ -29,11 +29,8 @@ from ..models import ( PocMembership, PocFutureMembership, ) -from ..overrides import ( - get_override_for_poc, - override_field_for_poc, - poc_context, -) +from ..overrides import get_override_for_poc, override_field_for_poc +from .. import ACTIVE_POC_KEY from .factories import ( PocFactory, PocMembershipFactory, @@ -525,19 +522,20 @@ class TestPocGrades(ModuleStoreTestCase, LoginEnrollmentTestCase): self.addCleanup(patch_context.stop) self.client.login(username=self.student.username, password="test") + session = self.client.session + session[ACTIVE_POC_KEY] = self.poc.id + session.save() + self.client.session.get(ACTIVE_POC_KEY) url = reverse( 'progress', kwargs={'course_id': self.course.id.to_deprecated_string()} ) - - with poc_context(self.poc): - - response = self.client.get(url) - self.assertEqual(response.status_code, 200) - grades = response.mako_context['grade_summary'] - self.assertEqual(grades['percent'], 0.5) - self.assertEqual(grades['grade_breakdown'][0]['percent'], 0.5) - self.assertEqual(len(grades['section_breakdown']), 4) + response = self.client.get(url) + self.assertEqual(response.status_code, 200) + grades = response.mako_context['grade_summary'] + self.assertEqual(grades['percent'], 0.5) + self.assertEqual(grades['grade_breakdown'][0]['percent'], 0.5) + self.assertEqual(len(grades['section_breakdown']), 4) class TestSwitchActivePoc(ModuleStoreTestCase, LoginEnrollmentTestCase): diff --git a/lms/djangoapps/pocs/views.py b/lms/djangoapps/pocs/views.py index 4f9eb50a4c..964a79bc1d 100644 --- a/lms/djangoapps/pocs/views.py +++ b/lms/djangoapps/pocs/views.py @@ -26,7 +26,6 @@ from django_future.csrf import ensure_csrf_cookie from django.contrib.auth.decorators import login_required from django.contrib.auth.models import User -from courseware.courses import get_course from courseware.courses import get_course_by_id from courseware.field_overrides import disable_overrides from courseware.grades import iterate_grades_for @@ -50,7 +49,6 @@ from .overrides import ( from .utils import ( enroll_email, unenroll_email, - get_all_pocs_for_user, ) from pocs import ACTIVE_POC_KEY @@ -89,24 +87,27 @@ def dashboard(request, course): Display the POC Coach Dashboard. """ poc = get_poc_for_coach(course, request.user) - schedule = get_poc_schedule(course, poc) - grading_policy = get_override_for_poc(poc, course, 'grading_policy', - course.grading_policy) context = { 'course': course, 'poc': poc, - 'schedule': json.dumps(schedule, indent=4), - 'save_url': reverse('save_poc', kwargs={'course_id': course.id}), - 'poc_members': PocMembership.objects.filter(poc=poc), - 'gradebook_url': reverse('poc_gradebook', - kwargs={'course_id': course.id}), - 'grades_csv_url': reverse('poc_grades_csv', - kwargs={'course_id': course.id}), - 'grading_policy': json.dumps(grading_policy, indent=4), - 'grading_policy_url': reverse('poc_set_grading_policy', - kwargs={'course_id': course.id}), } - if not poc: + + if poc: + schedule = get_poc_schedule(course, poc) + grading_policy = get_override_for_poc( + poc, course, 'grading_policy', course.grading_policy) + context['schedule'] = json.dumps(schedule, indent=4) + context['save_url'] = reverse( + 'save_poc', kwargs={'course_id': course.id}) + context['poc_members'] = PocMembership.objects.filter(poc=poc) + context['gradebook_url'] = reverse( + 'poc_gradebook', kwargs={'course_id': course.id}) + context['grades_csv_url'] = reverse( + 'poc_grades_csv', kwargs={'course_id': course.id}) + context['grading_policy'] = json.dumps(grading_policy, indent=4) + context['grading_policy_url'] = reverse( + 'poc_set_grading_policy', kwargs={'course_id': course.id}) + else: context['create_poc_url'] = reverse( 'create_poc', kwargs={'course_id': course.id}) return render_to_response('pocs/coach_dashboard.html', context) diff --git a/lms/envs/aws.py b/lms/envs/aws.py index b3892c62a7..76d9c77f93 100644 --- a/lms/envs/aws.py +++ b/lms/envs/aws.py @@ -576,3 +576,17 @@ ONLOAD_BEACON_SAMPLE_RATE = ENV_TOKENS.get('ONLOAD_BEACON_SAMPLE_RATE', ONLOAD_B ECOMMERCE_API_URL = ENV_TOKENS.get('ECOMMERCE_API_URL', ECOMMERCE_API_URL) ECOMMERCE_API_SIGNING_KEY = AUTH_TOKENS.get('ECOMMERCE_API_SIGNING_KEY', ECOMMERCE_API_SIGNING_KEY) ECOMMERCE_API_TIMEOUT = ENV_TOKENS.get('ECOMMERCE_API_TIMEOUT', ECOMMERCE_API_TIMEOUT) + +##### Personal Online Courses ##### +if FEATURES.get('PERSONAL_ONLINE_COURSES'): + INSTALLED_APPS += ('pocs',) + MIDDLEWARE_CLASSES += ('pocs.overrides.PocMiddleware',) + FIELD_OVERRIDE_PROVIDERS += ( + 'pocs.overrides.PersonalOnlineCoursesOverrideProvider', + ) + +##### Individual Due Date Extensions ##### +if FEATURES.get('INDIVIDUAL_DUE_DATES'): + FIELD_OVERRIDE_PROVIDERS += ( + 'courseware.student_field_overrides.IndividualStudentOverrideProvider', + ) diff --git a/lms/envs/test.py b/lms/envs/test.py index 35d1c5285c..6a2e26109a 100644 --- a/lms/envs/test.py +++ b/lms/envs/test.py @@ -471,3 +471,4 @@ FEATURES['CERTIFICATES_HTML_VIEW'] = True ######### personal online courses ######### INSTALLED_APPS += ('pocs',) +MIDDLEWARE_CLASSES += ('pocs.overrides.PocMiddleware',)