From f1f5a8f12f005e906e24eac8656af98f77a88bf5 Mon Sep 17 00:00:00 2001 From: Nimisha Asthagiri Date: Thu, 17 Nov 2016 09:17:59 -0500 Subject: [PATCH] Optimize max_score by moving from XModule to Descriptor TNL-5939 --- common/lib/capa/capa/capa_problem.py | 10 ++++-- common/lib/xmodule/xmodule/capa_base.py | 6 ---- common/lib/xmodule/xmodule/capa_module.py | 32 +++++++++++++++++++ common/lib/xmodule/xmodule/lti_module.py | 7 ++-- .../xmodule/xmodule/tests/test_lti_unit.py | 1 + common/lib/xmodule/xmodule/x_module.py | 2 +- lms/djangoapps/grades/transformer.py | 21 +++--------- 7 files changed, 50 insertions(+), 29 deletions(-) diff --git a/common/lib/capa/capa/capa_problem.py b/common/lib/capa/capa/capa_problem.py index 312a6fcfc3..555b3ee6c7 100644 --- a/common/lib/capa/capa/capa_problem.py +++ b/common/lib/capa/capa/capa_problem.py @@ -129,7 +129,7 @@ class LoncapaProblem(object): Main class for capa Problems. """ def __init__(self, problem_text, id, capa_system, capa_module, # pylint: disable=redefined-builtin - state=None, seed=None): + state=None, seed=None, minimal_init=False): """ Initializes capa Problem. @@ -186,7 +186,10 @@ class LoncapaProblem(object): self._process_includes() # construct script processor context (eg for customresponse problems) - self.context = self._extract_context(self.tree) + if minimal_init: + self.context = {'script_code': ""} + else: + self.context = self._extract_context(self.tree) # Pre-parse the XML tree: modifies it to add ID's and perform some in-place # transformations. This also creates the dict (self.responders) of Response @@ -209,7 +212,8 @@ class LoncapaProblem(object): if hasattr(response, 'late_transforms'): response.late_transforms(self) - self.extracted_tree = self._extract_html(self.tree) + if not minimal_init: + self.extracted_tree = self._extract_html(self.tree) def make_xml_compatible(self, tree): """ diff --git a/common/lib/xmodule/xmodule/capa_base.py b/common/lib/xmodule/xmodule/capa_base.py index b6a01b8161..a617b53439 100644 --- a/common/lib/xmodule/xmodule/capa_base.py +++ b/common/lib/xmodule/xmodule/capa_base.py @@ -357,12 +357,6 @@ class CapaMixin(CapaFields): """ return self.lcp.get_score() - def max_score(self): - """ - Access the problem's max score - """ - return self.lcp.get_max_score() - def get_progress(self): """ For now, just return score / max_score diff --git a/common/lib/xmodule/xmodule/capa_module.py b/common/lib/xmodule/xmodule/capa_module.py index f90604e511..4ca258474f 100644 --- a/common/lib/xmodule/xmodule/capa_module.py +++ b/common/lib/xmodule/xmodule/capa_module.py @@ -275,6 +275,38 @@ class CapaDescriptor(CapaFields, RawDescriptor): ) return False + def max_score(self): + """ + Return the problem's max score + """ + from capa.capa_problem import LoncapaProblem, LoncapaSystem + capa_system = LoncapaSystem( + ajax_url=None, + anonymous_student_id=None, + cache=None, + can_execute_unsafe_code=None, + get_python_lib_zip=None, + DEBUG=None, + filestore=self.runtime.resources_fs, + i18n=self.runtime.service(self, "i18n"), + node_path=None, + render_template=None, + seed=None, + STATIC_URL=None, + xqueue=None, + matlab_api_key=None, + ) + lcp = LoncapaProblem( + problem_text=self.data, + id=self.location.html_id(), + capa_system=capa_system, + capa_module=self, + state={}, + seed=1, + minimal_init=True, + ) + return lcp.get_max_score() + # Proxy to CapaModule for access to any of its attributes answer_available = module_attr('answer_available') submit_button_name = module_attr('submit_button_name') diff --git a/common/lib/xmodule/xmodule/lti_module.py b/common/lib/xmodule/xmodule/lti_module.py index 6c4ca34009..df02c6ce9c 100644 --- a/common/lib/xmodule/xmodule/lti_module.py +++ b/common/lib/xmodule/xmodule/lti_module.py @@ -649,9 +649,6 @@ oauth_consumer_key="", oauth_signature="frVp4JuvT1mVXlxktiAUjQ7%2F1cw%3D"'} params.update(body) return params - def max_score(self): - return self.weight if self.has_score else None - @XBlock.handler def grade_handler(self, request, suffix): # pylint: disable=unused-argument """ @@ -898,6 +895,10 @@ class LTIDescriptor(LTIFields, MetadataOnlyEditingDescriptor, EmptyDataRawDescri """ Descriptor for LTI Xmodule. """ + + def max_score(self): + return self.weight if self.has_score else None + module_class = LTIModule resources_dir = None grade_handler = module_attr('grade_handler') diff --git a/common/lib/xmodule/xmodule/tests/test_lti_unit.py b/common/lib/xmodule/xmodule/tests/test_lti_unit.py index 6c2cdb71b9..ce83fed603 100644 --- a/common/lib/xmodule/xmodule/tests/test_lti_unit.py +++ b/common/lib/xmodule/xmodule/tests/test_lti_unit.py @@ -72,6 +72,7 @@ class LTIModuleTest(LogicTest): self.xmodule.due = None self.xmodule.graceperiod = None + self.xmodule.descriptor = self.system.construct_xblock_from_class(self.descriptor_class, self.xmodule.scope_ids) def get_request_body(self, params=None): """Fetches the body of a request specified by params""" diff --git a/common/lib/xmodule/xmodule/x_module.py b/common/lib/xmodule/xmodule/x_module.py index 601477563f..965cce93fd 100644 --- a/common/lib/xmodule/xmodule/x_module.py +++ b/common/lib/xmodule/xmodule/x_module.py @@ -806,6 +806,7 @@ class XModule(HTMLSnippet, XModuleMixin): entry_point = "xmodule.v1" has_score = descriptor_attr('has_score') + max_score = descriptor_attr('max_score') show_in_read_only_mode = descriptor_attr('show_in_read_only_mode') _field_data_cache = descriptor_attr('_field_data_cache') _field_data = descriptor_attr('_field_data') @@ -1201,7 +1202,6 @@ class XModuleDescriptor(HTMLSnippet, ResourceTemplates, XModuleMixin): get_progress = module_attr('get_progress') get_score = module_attr('get_score') handle_ajax = module_attr('handle_ajax') - max_score = module_attr('max_score') student_view = module_attr(STUDENT_VIEW) get_child_descriptors = module_attr('get_child_descriptors') xmodule_handler = module_attr('xmodule_handler') diff --git a/lms/djangoapps/grades/transformer.py b/lms/djangoapps/grades/transformer.py index cacaf0b78d..b812c617ed 100644 --- a/lms/djangoapps/grades/transformer.py +++ b/lms/djangoapps/grades/transformer.py @@ -118,8 +118,10 @@ class GradesTransformer(BlockStructureTransformer): """ Collect the `max_score` for every block in the provided `block_structure`. """ - for module in cls._iter_scorable_xmodules(block_structure): - cls._collect_max_score(block_structure, module) + for block_locator in block_structure.post_order_traversal(): + block = block_structure.get_xblock(block_locator) + if getattr(block, 'has_score', False): + cls._collect_max_score(block_structure, block) @classmethod def _collect_max_score(cls, block_structure, module): @@ -171,20 +173,7 @@ class GradesTransformer(BlockStructureTransformer): XModule, even though the data is not user specific. Here we bind the data to a SystemUser. """ - request = RequestFactory().get('/dummy-collect-max-grades') - user = SystemUser() - request.user = user - request.session = {} - root_block = block_structure.get_xblock(block_structure.root_block_usage_key) - course_key = block_structure.root_block_usage_key.course_key - cache = FieldDataCache.cache_for_descriptor_descendents( - course_id=course_key, - user=request.user, - descriptor=root_block, - descriptor_filter=lambda descriptor: descriptor.has_score, - ) for block_locator in block_structure.post_order_traversal(): block = block_structure.get_xblock(block_locator) if getattr(block, 'has_score', False): - module = courseware.module_render.get_module_for_descriptor(user, request, block, cache, course_key) - yield module + yield block