From 8201b1412ebae84bade3a6b3c932916a0e787a58 Mon Sep 17 00:00:00 2001 From: Calen Pennington <cale@edx.org> Date: Fri, 30 Aug 2013 07:30:18 -0400 Subject: [PATCH] Use XBlock 0.3 --- .../contentstore/module_info_model.py | 6 +- .../contentstore/tests/test_contentstore.py | 51 ++--- .../tests/test_course_settings.py | 12 +- .../contentstore/tests/test_crud.py | 14 +- .../tests/test_import_nostatic.py | 6 +- .../contentstore/tests/test_item.py | 12 +- .../contentstore/views/component.py | 51 +++-- cms/djangoapps/contentstore/views/item.py | 14 +- cms/djangoapps/contentstore/views/preview.py | 16 +- .../contentstore/views/session_kv_store.py | 25 +-- .../models/settings/course_grading.py | 28 +-- .../models/settings/course_metadata.py | 22 +- cms/envs/common.py | 11 + cms/startup.py | 4 +- cms/templates/edit_subsection.html | 16 +- cms/templates/overview.html | 12 +- cms/templates/widgets/sequence-edit.html | 2 +- cms/templates/widgets/units.html | 2 +- cms/xmodule_namespace.py | 12 +- common/djangoapps/external_auth/views.py | 4 +- common/djangoapps/tests.py | 2 +- common/djangoapps/xmodule_modifiers.py | 19 +- common/lib/xmodule/xmodule/abtest_module.py | 2 +- .../lib/xmodule/xmodule/annotatable_module.py | 2 +- common/lib/xmodule/xmodule/capa_module.py | 2 +- .../xmodule/combined_open_ended_module.py | 2 +- .../lib/xmodule/xmodule/conditional_module.py | 2 +- common/lib/xmodule/xmodule/course_module.py | 15 +- .../lib/xmodule/xmodule/crowdsource_hinter.py | 2 +- .../lib/xmodule/xmodule/discussion_module.py | 2 +- common/lib/xmodule/xmodule/editing_module.py | 2 +- common/lib/xmodule/xmodule/error_module.py | 16 +- common/lib/xmodule/xmodule/fields.py | 10 +- common/lib/xmodule/xmodule/foldit_module.py | 2 +- common/lib/xmodule/xmodule/gst_module.py | 2 +- common/lib/xmodule/xmodule/html_module.py | 2 +- common/lib/xmodule/xmodule/mako_module.py | 6 +- .../xmodule/xmodule/modulestore/__init__.py | 3 +- .../lib/xmodule/xmodule/modulestore/django.py | 1 + .../xmodule/modulestore/inheritance.py | 103 ++++++--- .../xmodule/xmodule/modulestore/locator.py | 27 ++- .../lib/xmodule/xmodule/modulestore/mixed.py | 6 +- .../xmodule/modulestore/mongo/__init__.py | 2 +- .../xmodule/xmodule/modulestore/mongo/base.py | 131 +++++------ .../xmodule/modulestore/mongo/draft.py | 10 +- .../split_mongo/caching_descriptor_system.py | 65 +++--- .../xmodule/modulestore/split_mongo/split.py | 49 ++-- .../split_mongo/split_mongo_kvs.py | 41 +--- .../xmodule/modulestore/store_utilities.py | 23 +- .../xmodule/modulestore/tests/factories.py | 4 +- .../xmodule/modulestore/tests/test_mongo.py | 10 +- .../tests/test_split_modulestore.py | 8 +- common/lib/xmodule/xmodule/modulestore/xml.py | 43 ++-- .../xmodule/modulestore/xml_importer.py | 81 ++++--- .../xmodule/xmodule/peer_grading_module.py | 10 +- common/lib/xmodule/xmodule/plugin.py | 64 ------ common/lib/xmodule/xmodule/poll_module.py | 4 +- .../lib/xmodule/xmodule/randomize_module.py | 2 +- common/lib/xmodule/xmodule/raw_module.py | 2 +- common/lib/xmodule/xmodule/seq_module.py | 6 +- common/lib/xmodule/xmodule/tests/__init__.py | 8 +- .../xmodule/tests/test_annotatable_module.py | 11 +- .../xmodule/xmodule/tests/test_capa_module.py | 29 ++- .../xmodule/tests/test_combined_open_ended.py | 13 +- .../xmodule/xmodule/tests/test_conditional.py | 11 +- .../xmodule/tests/test_course_module.py | 12 +- .../xmodule/tests/test_crowdsource_hinter.py | 30 +-- .../xmodule/tests/test_editing_module.py | 6 +- .../xmodule/tests/test_error_module.py | 4 +- .../lib/xmodule/xmodule/tests/test_export.py | 3 +- .../xmodule/xmodule/tests/test_html_module.py | 14 +- .../lib/xmodule/xmodule/tests/test_import.py | 52 +++-- common/lib/xmodule/xmodule/tests/test_poll.py | 2 +- .../xmodule/xmodule/tests/test_progress.py | 5 +- .../lib/xmodule/xmodule/tests/test_video.py | 30 ++- .../xmodule/xmodule/tests/test_word_cloud.py | 2 +- .../xmodule/tests/test_xblock_wrappers.py | 42 ++-- .../xmodule/xmodule/tests/test_xml_module.py | 26 +-- .../lib/xmodule/xmodule/timelimit_module.py | 2 +- common/lib/xmodule/xmodule/video_module.py | 41 ++-- .../lib/xmodule/xmodule/word_cloud_module.py | 6 +- common/lib/xmodule/xmodule/x_module.py | 212 ++++++++---------- common/lib/xmodule/xmodule/xml_module.py | 52 +++-- docs/source/conf.py | 3 + lms/djangoapps/analytics/basic.py | 2 +- lms/djangoapps/courseware/access.py | 18 +- lms/djangoapps/courseware/courses.py | 20 +- lms/djangoapps/courseware/features/common.py | 6 +- lms/djangoapps/courseware/grades.py | 52 ++--- .../management/commands/check_course.py | 4 +- ...ock_field_content_to_user_state_summary.py | 148 ++++++++++++ lms/djangoapps/courseware/model_data.py | 118 +++------- lms/djangoapps/courseware/models.py | 41 +--- lms/djangoapps/courseware/module_render.py | 67 +++--- lms/djangoapps/courseware/tabs.py | 8 +- lms/djangoapps/courseware/tests/__init__.py | 18 +- lms/djangoapps/courseware/tests/factories.py | 16 +- .../courseware/tests/test_model_data.py | 98 +++----- .../courseware/tests/test_module_render.py | 36 +-- .../tests/test_submitting_problems.py | 10 +- .../courseware/tests/test_video_mongo.py | 5 +- .../courseware/tests/test_video_xml.py | 8 +- .../tests/test_view_authentication.py | 2 +- lms/djangoapps/courseware/tests/tests.py | 1 - lms/djangoapps/courseware/views.py | 26 +-- lms/djangoapps/django_comment_client/utils.py | 4 +- lms/djangoapps/instructor/hint_manager.py | 28 +-- .../instructor/tests/test_hint_manager.py | 32 +-- lms/djangoapps/instructor/views/legacy.py | 14 +- .../instructor_task/tasks_helper.py | 6 +- .../open_ended_notifications.py | 4 +- .../staff_grading_service.py | 2 +- lms/djangoapps/open_ended_grading/tests.py | 12 +- lms/djangoapps/open_ended_grading/views.py | 2 +- lms/envs/common.py | 9 + lms/startup.py | 3 + lms/templates/staff_problem_info.html | 6 - lms/xblock/__init__.py | 0 lms/xblock/field_data.py | 26 +++ lms/xblock/mixin.py | 22 ++ lms/xmodule_namespace.py | 59 ----- requirements/edx/github.txt | 2 +- requirements/edx/local.txt | 1 - scripts/runone.py | 3 +- setup.cfg | 3 +- setup.py | 19 -- 126 files changed, 1385 insertions(+), 1297 deletions(-) delete mode 100644 common/lib/xmodule/xmodule/plugin.py create mode 100644 lms/djangoapps/courseware/migrations/0010_rename_xblock_field_content_to_user_state_summary.py create mode 100644 lms/xblock/__init__.py create mode 100644 lms/xblock/field_data.py create mode 100644 lms/xblock/mixin.py delete mode 100644 lms/xmodule_namespace.py delete mode 100644 setup.py diff --git a/cms/djangoapps/contentstore/module_info_model.py b/cms/djangoapps/contentstore/module_info_model.py index c0e1ff7207e..b43a32f635d 100644 --- a/cms/djangoapps/contentstore/module_info_model.py +++ b/cms/djangoapps/contentstore/module_info_model.py @@ -64,11 +64,11 @@ def set_module_info(store, location, post_data): if posted_metadata[metadata_key] is None: # remove both from passed in collection as well as the collection read in from the modulestore - if metadata_key in module._model_data: - del module._model_data[metadata_key] + if module._field_data.has(module, metadata_key): + module._field_data.delete(module, metadata_key) del posted_metadata[metadata_key] else: - module._model_data[metadata_key] = value + module._field_data.set(module, metadata_key, value) # commit to datastore # TODO (cpennington): This really shouldn't have to do this much reaching in to get the metadata diff --git a/cms/djangoapps/contentstore/tests/test_contentstore.py b/cms/djangoapps/contentstore/tests/test_contentstore.py index 696b60fbe59..f76b563d6a5 100644 --- a/cms/djangoapps/contentstore/tests/test_contentstore.py +++ b/cms/djangoapps/contentstore/tests/test_contentstore.py @@ -219,7 +219,7 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase): 'course', '2012_Fall', None]), depth=None) html_module = draft_store.get_item(['i4x', 'edX', 'simple', 'html', 'test_html', None]) - self.assertEqual(html_module.lms.graceperiod, course.lms.graceperiod) + self.assertEqual(html_module.graceperiod, course.graceperiod) self.assertNotIn('graceperiod', own_metadata(html_module)) draft_store.convert_to_draft(html_module.location) @@ -227,7 +227,7 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase): # refetch to check metadata html_module = draft_store.get_item(['i4x', 'edX', 'simple', 'html', 'test_html', None]) - self.assertEqual(html_module.lms.graceperiod, course.lms.graceperiod) + self.assertEqual(html_module.graceperiod, course.graceperiod) self.assertNotIn('graceperiod', own_metadata(html_module)) # publish module @@ -236,7 +236,7 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase): # refetch to check metadata html_module = draft_store.get_item(['i4x', 'edX', 'simple', 'html', 'test_html', None]) - self.assertEqual(html_module.lms.graceperiod, course.lms.graceperiod) + self.assertEqual(html_module.graceperiod, course.graceperiod) self.assertNotIn('graceperiod', own_metadata(html_module)) # put back in draft and change metadata and see if it's now marked as 'own_metadata' @@ -246,12 +246,12 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase): new_graceperiod = timedelta(hours=1) self.assertNotIn('graceperiod', own_metadata(html_module)) - html_module.lms.graceperiod = new_graceperiod + html_module.graceperiod = new_graceperiod # Save the data that we've just changed to the underlying # MongoKeyValueStore before we update the mongo datastore. html_module.save() self.assertIn('graceperiod', own_metadata(html_module)) - self.assertEqual(html_module.lms.graceperiod, new_graceperiod) + self.assertEqual(html_module.graceperiod, new_graceperiod) draft_store.update_metadata(html_module.location, own_metadata(html_module)) @@ -259,7 +259,7 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase): html_module = draft_store.get_item(['i4x', 'edX', 'simple', 'html', 'test_html', None]) self.assertIn('graceperiod', own_metadata(html_module)) - self.assertEqual(html_module.lms.graceperiod, new_graceperiod) + self.assertEqual(html_module.graceperiod, new_graceperiod) # republish draft_store.publish(html_module.location, 0) @@ -269,7 +269,7 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase): html_module = draft_store.get_item(['i4x', 'edX', 'simple', 'html', 'test_html', None]) self.assertIn('graceperiod', own_metadata(html_module)) - self.assertEqual(html_module.lms.graceperiod, new_graceperiod) + self.assertEqual(html_module.graceperiod, new_graceperiod) def test_get_depth_with_drafts(self): import_from_xml(modulestore('direct'), 'common/test/data/', ['simple']) @@ -696,7 +696,7 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase): # we want to assert equality between the objects, but we know the locations # differ, so just make them equal for testing purposes - source_item.location = new_loc + source_item.scope_ids = source_item.scope_ids._replace(def_id=new_loc, usage_id=new_loc) if hasattr(source_item, 'data') and hasattr(lookup_item, 'data'): self.assertEqual(source_item.data, lookup_item.data) @@ -877,7 +877,9 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase): depth=1 ) # We had a bug where orphaned draft nodes caused export to fail. This is here to cover that case. - vertical.location = mongo.draft.as_draft(vertical.location.replace(name='no_references')) + draft_loc = mongo.draft.as_draft(vertical.location.replace(name='no_references')) + vertical.scope_ids = vertical.scope_ids._replace(def_id=draft_loc, usage_id=draft_loc) + draft_store.save_xmodule(vertical) orphan_vertical = draft_store.get_item(vertical.location) self.assertEqual(orphan_vertical.location.name, 'no_references') @@ -894,7 +896,8 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase): root_dir = path(mkdtemp_clean()) # now create a new/different private (draft only) vertical - vertical.location = mongo.draft.as_draft(Location(['i4x', 'edX', 'toy', 'vertical', 'a_private_vertical', None])) + draft_loc = mongo.draft.as_draft(Location(['i4x', 'edX', 'toy', 'vertical', 'a_private_vertical', None])) + vertical.scope_ids = vertical.scope_ids._replace(def_id=draft_loc, usage_id=draft_loc) draft_store.save_xmodule(vertical) private_vertical = draft_store.get_item(vertical.location) vertical = None # blank out b/c i destructively manipulated its location 2 lines above @@ -965,7 +968,7 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase): self.assertTrue(getattr(vertical, 'is_draft', False)) self.assertNotIn('index_in_children_list', child.xml_attributes) self.assertNotIn('parent_sequential_url', vertical.xml_attributes) - + for child in vertical.get_children(): self.assertTrue(getattr(child, 'is_draft', False)) self.assertNotIn('index_in_children_list', child.xml_attributes) @@ -1628,8 +1631,8 @@ class ContentStoreTest(ModuleStoreTestCase): # let's assert on the metadata_inheritance on an existing vertical for vertical in verticals: - self.assertEqual(course.lms.xqa_key, vertical.lms.xqa_key) - self.assertEqual(course.start, vertical.lms.start) + self.assertEqual(course.xqa_key, vertical.xqa_key) + self.assertEqual(course.start, vertical.start) self.assertGreater(len(verticals), 0) @@ -1645,16 +1648,16 @@ class ContentStoreTest(ModuleStoreTestCase): new_module = module_store.get_item(new_component_location) # check for grace period definition which should be defined at the course level - self.assertEqual(parent.lms.graceperiod, new_module.lms.graceperiod) - self.assertEqual(parent.lms.start, new_module.lms.start) - self.assertEqual(course.start, new_module.lms.start) + self.assertEqual(parent.graceperiod, new_module.graceperiod) + self.assertEqual(parent.start, new_module.start) + self.assertEqual(course.start, new_module.start) - self.assertEqual(course.lms.xqa_key, new_module.lms.xqa_key) + self.assertEqual(course.xqa_key, new_module.xqa_key) # # now let's define an override at the leaf node level # - new_module.lms.graceperiod = timedelta(1) + new_module.graceperiod = timedelta(1) new_module.save() module_store.update_metadata(new_module.location, own_metadata(new_module)) @@ -1662,7 +1665,7 @@ class ContentStoreTest(ModuleStoreTestCase): module_store.refresh_cached_metadata_inheritance_tree(new_component_location) new_module = module_store.get_item(new_component_location) - self.assertEqual(timedelta(1), new_module.lms.graceperiod) + self.assertEqual(timedelta(1), new_module.graceperiod) def test_default_metadata_inheritance(self): course = CourseFactory.create() @@ -1670,7 +1673,7 @@ class ContentStoreTest(ModuleStoreTestCase): course.children.append(vertical) # in memory self.assertIsNotNone(course.start) - self.assertEqual(course.start, vertical.lms.start) + self.assertEqual(course.start, vertical.start) self.assertEqual(course.textbooks, []) self.assertIn('GRADER', course.grading_policy) self.assertIn('GRADE_CUTOFFS', course.grading_policy) @@ -1682,7 +1685,7 @@ class ContentStoreTest(ModuleStoreTestCase): fetched_item = module_store.get_item(vertical.location) self.assertIsNotNone(fetched_course.start) self.assertEqual(course.start, fetched_course.start) - self.assertEqual(fetched_course.start, fetched_item.lms.start) + self.assertEqual(fetched_course.start, fetched_item.start) self.assertEqual(course.textbooks, fetched_course.textbooks) # is this test too strict? i.e., it requires the dicts to be == self.assertEqual(course.checklists, fetched_course.checklists) @@ -1755,12 +1758,10 @@ class MetadataSaveTestCase(ModuleStoreTestCase): 'track' } - fields = self.video_descriptor.fields location = self.video_descriptor.location - for field in fields: - if field.name in attrs_to_strip: - field.delete_from(self.video_descriptor) + for field_name in attrs_to_strip: + delattr(self.video_descriptor, field_name) self.assertNotIn('html5_sources', own_metadata(self.video_descriptor)) get_modulestore(location).update_metadata( diff --git a/cms/djangoapps/contentstore/tests/test_course_settings.py b/cms/djangoapps/contentstore/tests/test_course_settings.py index 286477228b3..b27d1494e9c 100644 --- a/cms/djangoapps/contentstore/tests/test_course_settings.py +++ b/cms/djangoapps/contentstore/tests/test_course_settings.py @@ -343,8 +343,8 @@ class CourseGradingTest(CourseTestCase): section_grader_type = CourseGradingModel.get_section_grader_type(self.course.location) self.assertEqual('Not Graded', section_grader_type['graderType']) - self.assertEqual(None, descriptor.lms.format) - self.assertEqual(False, descriptor.lms.graded) + self.assertEqual(None, descriptor.format) + self.assertEqual(False, descriptor.graded) # Change the default grader type to Homework, which should also mark the section as graded CourseGradingModel.update_section_grader_type(self.course.location, {'graderType': 'Homework'}) @@ -352,8 +352,8 @@ class CourseGradingTest(CourseTestCase): section_grader_type = CourseGradingModel.get_section_grader_type(self.course.location) self.assertEqual('Homework', section_grader_type['graderType']) - self.assertEqual('Homework', descriptor.lms.format) - self.assertEqual(True, descriptor.lms.graded) + self.assertEqual('Homework', descriptor.format) + self.assertEqual(True, descriptor.graded) # Change the grader type back to Not Graded, which should also unmark the section as graded CourseGradingModel.update_section_grader_type(self.course.location, {'graderType': 'Not Graded'}) @@ -361,8 +361,8 @@ class CourseGradingTest(CourseTestCase): section_grader_type = CourseGradingModel.get_section_grader_type(self.course.location) self.assertEqual('Not Graded', section_grader_type['graderType']) - self.assertEqual(None, descriptor.lms.format) - self.assertEqual(False, descriptor.lms.graded) + self.assertEqual(None, descriptor.format) + self.assertEqual(False, descriptor.graded) class CourseMetadataEditingTest(CourseTestCase): diff --git a/cms/djangoapps/contentstore/tests/test_crud.py b/cms/djangoapps/contentstore/tests/test_crud.py index e543b7b5171..3a8e0b0222d 100644 --- a/cms/djangoapps/contentstore/tests/test_crud.py +++ b/cms/djangoapps/contentstore/tests/test_crud.py @@ -220,18 +220,14 @@ class TemplateTests(unittest.TestCase): if not '_inherited_settings' in json_data and parent_xblock is not None: json_data['_inherited_settings'] = parent_xblock.xblock_kvs.get_inherited_settings().copy() json_fields = json_data.get('fields', {}) - for field in inheritance.INHERITABLE_METADATA: - if field in json_fields: - json_data['_inherited_settings'][field] = json_fields[field] + for field_name in inheritance.InheritanceMixin.fields: + if field_name in json_fields: + json_data['_inherited_settings'][field_name] = json_fields[field_name] new_block = system.xblock_from_json(class_, usage_id, json_data) if parent_xblock is not None: - children = parent_xblock.children - children.append(new_block) - # trigger setter method by using top level field access - parent_xblock.children = children - # decache pending children field settings (Note, truly persisting at this point would break b/c - # persistence assumes children is a list of ids not actual xblocks) + parent_xblock.children.append(new_block.scope_ids.usage_id) + # decache pending children field settings parent_xblock.save() return new_block diff --git a/cms/djangoapps/contentstore/tests/test_import_nostatic.py b/cms/djangoapps/contentstore/tests/test_import_nostatic.py index f0f65c9b076..275f9b6b1f9 100644 --- a/cms/djangoapps/contentstore/tests/test_import_nostatic.py +++ b/cms/djangoapps/contentstore/tests/test_import_nostatic.py @@ -97,9 +97,9 @@ class ContentStoreImportNoStaticTest(ModuleStoreTestCase): self.assertIsNotNone(content) - # make sure course.lms.static_asset_path is correct - print "static_asset_path = {0}".format(course.lms.static_asset_path) - self.assertEqual(course.lms.static_asset_path, 'test_import_course') + # make sure course.static_asset_path is correct + print "static_asset_path = {0}".format(course.static_asset_path) + self.assertEqual(course.static_asset_path, 'test_import_course') def test_asset_import_nostatic(self): ''' diff --git a/cms/djangoapps/contentstore/tests/test_item.py b/cms/djangoapps/contentstore/tests/test_item.py index e5ff992cb8e..dcd94838c8b 100644 --- a/cms/djangoapps/contentstore/tests/test_item.py +++ b/cms/djangoapps/contentstore/tests/test_item.py @@ -1,4 +1,4 @@ -from contentstore.tests.test_course_settings import CourseTestCase +from contentstore.tests.utils import CourseTestCase from xmodule.modulestore.tests.factories import CourseFactory from django.core.urlresolvers import reverse from xmodule.capa_module import CapaDescriptor @@ -69,7 +69,7 @@ class TestCreateItem(CourseTestCase): # get the new item and check its category and display_name chap_location = self.response_id(resp) new_obj = modulestore().get_item(chap_location) - self.assertEqual(new_obj.category, 'chapter') + self.assertEqual(new_obj.scope_ids.block_type, 'chapter') self.assertEqual(new_obj.display_name, display_name) self.assertEqual(new_obj.location.org, self.course.location.org) self.assertEqual(new_obj.location.course, self.course.location.course) @@ -226,7 +226,7 @@ class TestEditItem(CourseTestCase): Test setting due & start dates on sequential """ sequential = modulestore().get_item(self.seq_location) - self.assertIsNone(sequential.lms.due) + self.assertIsNone(sequential.due) self.client.post( reverse('save_item'), json.dumps({ @@ -236,7 +236,7 @@ class TestEditItem(CourseTestCase): content_type="application/json" ) sequential = modulestore().get_item(self.seq_location) - self.assertEqual(sequential.lms.due, datetime.datetime(2010, 11, 22, 4, 0, tzinfo=UTC)) + self.assertEqual(sequential.due, datetime.datetime(2010, 11, 22, 4, 0, tzinfo=UTC)) self.client.post( reverse('save_item'), json.dumps({ @@ -246,5 +246,5 @@ class TestEditItem(CourseTestCase): content_type="application/json" ) sequential = modulestore().get_item(self.seq_location) - self.assertEqual(sequential.lms.due, datetime.datetime(2010, 11, 22, 4, 0, tzinfo=UTC)) - self.assertEqual(sequential.lms.start, datetime.datetime(2010, 9, 12, 14, 0, tzinfo=UTC)) + self.assertEqual(sequential.due, datetime.datetime(2010, 11, 22, 4, 0, tzinfo=UTC)) + self.assertEqual(sequential.start, datetime.datetime(2010, 9, 12, 14, 0, tzinfo=UTC)) diff --git a/cms/djangoapps/contentstore/views/component.py b/cms/djangoapps/contentstore/views/component.py index a5fec7c033b..0a09bd8a042 100644 --- a/cms/djangoapps/contentstore/views/component.py +++ b/cms/djangoapps/contentstore/views/component.py @@ -2,14 +2,14 @@ import json import logging from collections import defaultdict -from django.http import ( HttpResponse, HttpResponseBadRequest, +from django.http import ( HttpResponse, HttpResponseBadRequest, HttpResponseForbidden ) from django.contrib.auth.decorators import login_required from django.views.decorators.http import require_http_methods from django.core.exceptions import PermissionDenied from django_future.csrf import ensure_csrf_cookie from django.conf import settings -from xmodule.modulestore.exceptions import ( ItemNotFoundError, +from xmodule.modulestore.exceptions import ( ItemNotFoundError, InvalidLocationError ) from mitxmako.shortcuts import render_to_response @@ -17,11 +17,11 @@ from xmodule.modulestore import Location from xmodule.modulestore.django import modulestore from xmodule.util.date_utils import get_default_time_display -from xblock.core import Scope +from xblock.fields import Scope from util.json_request import expect_json, JsonResponse from contentstore.module_info_model import get_module_info, set_module_info -from contentstore.utils import ( get_modulestore, get_lms_link_for_item, +from contentstore.utils import ( get_modulestore, get_lms_link_for_item, compute_unit_state, UnitState, get_course_for_item ) from models.settings.course_grading import CourseGradingModel @@ -91,7 +91,7 @@ def edit_subsection(request, location): # we're for now assuming a single parent if len(parent_locs) != 1: logging.error( - 'Multiple (or none) parents have been found for %', + 'Multiple (or none) parents have been found for %s', location ) @@ -99,12 +99,14 @@ def edit_subsection(request, location): parent = modulestore().get_item(parent_locs[0]) # remove all metadata from the generic dictionary that is presented in a - # more normalized UI + # more normalized UI. We only want to display the XBlocks fields, not + # the fields from any mixins that have been added + fields = getattr(item, 'unmixed_class', item.__class__).fields policy_metadata = dict( (field.name, field.read_from(item)) for field - in item.fields + in fields.values() if field.name not in ['display_name', 'start', 'due', 'format'] and field.scope == Scope.settings ) @@ -165,20 +167,27 @@ def edit_unit(request, location): for category in COMPONENT_TYPES: component_class = XModuleDescriptor.load_class(category) # add the default template + # TODO: Once mixins are defined per-application, rather than per-runtime, + # this should use a cms mixed-in class. (cpennington) + if hasattr(component_class, 'display_name'): + display_name = component_class.display_name.default or 'Blank' + else: + display_name = 'Blank' component_templates[category].append(( - component_class.display_name.default or 'Blank', + display_name, category, False, # No defaults have markdown (hardcoded current default) None # no boilerplate for overrides )) # add boilerplates - for template in component_class.templates(): - component_templates[category].append(( - template['metadata'].get('display_name'), - category, - template['metadata'].get('markdown') is not None, - template.get('template_id') - )) + if hasattr(component_class, 'templates'): + for template in component_class.templates(): + component_templates[category].append(( + template['metadata'].get('display_name'), + category, + template['metadata'].get('markdown') is not None, + template.get('template_id') + )) # Check if there are any advanced modules specified in the course policy. # These modules should be specified as a list of strings, where the strings @@ -272,13 +281,17 @@ def edit_unit(request, location): 'draft_preview_link': preview_lms_link, 'published_preview_link': lms_link, 'subsection': containing_subsection, - 'release_date': get_default_time_display(containing_subsection.lms.start) - if containing_subsection.lms.start is not None else None, + 'release_date': ( + get_default_time_display(containing_subsection.start) + if containing_subsection.start is not None else None + ), 'section': containing_section, 'new_unit_category': 'vertical', 'unit_state': unit_state, - 'published_date': get_default_time_display(item.cms.published_date) - if item.cms.published_date is not None else None + 'published_date': ( + get_default_time_display(item.published_date) + if item.published_date is not None else None + ), }) diff --git a/cms/djangoapps/contentstore/views/item.py b/cms/djangoapps/contentstore/views/item.py index bbd95dba843..84a199d71da 100644 --- a/cms/djangoapps/contentstore/views/item.py +++ b/cms/djangoapps/contentstore/views/item.py @@ -58,13 +58,13 @@ def save_item(request): # 'apply' the submitted metadata, so we don't end up deleting system metadata existing_item = modulestore().get_item(item_location) for metadata_key in request.POST.get('nullout', []): - _get_xblock_field(existing_item, metadata_key).write_to(existing_item, None) + setattr(existing_item, metadata_key, None) # update existing metadata with submitted metadata (which can be partial) # IMPORTANT NOTE: if the client passed 'null' (None) for a piece of metadata that means 'remove it'. If # the intent is to make it None, use the nullout field for metadata_key, value in request.POST.get('metadata', {}).items(): - field = _get_xblock_field(existing_item, metadata_key) + field = existing_item.fields[metadata_key] if value is None: field.delete_from(existing_item) @@ -80,16 +80,6 @@ def save_item(request): return JsonResponse() -def _get_xblock_field(xblock, field_name): - """ - A temporary function to get the xblock field either from the xblock or one of its namespaces by name. - :param xblock: - :param field_name: - """ - for field in xblock.iterfields(): - if field.name == field_name: - return field - @login_required @expect_json def create_item(request): diff --git a/cms/djangoapps/contentstore/views/preview.py b/cms/djangoapps/contentstore/views/preview.py index 7a3a224d868..7fc669888e5 100644 --- a/cms/djangoapps/contentstore/views/preview.py +++ b/cms/djangoapps/contentstore/views/preview.py @@ -11,12 +11,12 @@ from xmodule_modifiers import replace_static_urls, wrap_xmodule, save_module # from xmodule.error_module import ErrorDescriptor from xmodule.errortracker import exc_info_to_str from xmodule.exceptions import NotFoundError, ProcessingError -from xmodule.modulestore import Location from xmodule.modulestore.django import modulestore -from xmodule.modulestore.mongo import MongoUsage from xmodule.x_module import ModuleSystem from xblock.runtime import DbModel +from lms.xblock.field_data import lms_field_data + from util.sandboxing import can_execute_unsafe_code import static_replace @@ -97,14 +97,10 @@ def preview_module_system(request, preview_id, descriptor): descriptor: An XModuleDescriptor """ - def preview_model_data(descriptor): + def preview_field_data(descriptor): "Helper method to create a DbModel from a descriptor" - return DbModel( - SessionKeyValueStore(request, descriptor._model_data), - descriptor.module_class, - preview_id, - MongoUsage(preview_id, descriptor.location.url()), - ) + student_data = DbModel(SessionKeyValueStore(request)) + return lms_field_data(descriptor._field_data, student_data) course_id = get_course_for_item(descriptor.location).location.course_id @@ -118,7 +114,7 @@ def preview_module_system(request, preview_id, descriptor): debug=True, replace_urls=partial(static_replace.replace_static_urls, data_directory=None, course_id=course_id), user=request.user, - xblock_model_data=preview_model_data, + xblock_field_data=preview_field_data, can_execute_unsafe_code=(lambda: can_execute_unsafe_code(course_id)), ) diff --git a/cms/djangoapps/contentstore/views/session_kv_store.py b/cms/djangoapps/contentstore/views/session_kv_store.py index 87a92a9e2e4..ddee2c1dbc5 100644 --- a/cms/djangoapps/contentstore/views/session_kv_store.py +++ b/cms/djangoapps/contentstore/views/session_kv_store.py @@ -1,28 +1,21 @@ -from xblock.runtime import KeyValueStore, InvalidScopeError +""" +An :class:`~xblock.runtime.KeyValueStore` that stores data in the django session +""" +from xblock.runtime import KeyValueStore class SessionKeyValueStore(KeyValueStore): - def __init__(self, request, descriptor_model_data): - self._descriptor_model_data = descriptor_model_data + def __init__(self, request): self._session = request.session def get(self, key): - try: - return self._descriptor_model_data[key.field_name] - except (KeyError, InvalidScopeError): - return self._session[tuple(key)] + return self._session[tuple(key)] def set(self, key, value): - try: - self._descriptor_model_data[key.field_name] = value - except (KeyError, InvalidScopeError): - self._session[tuple(key)] = value + self._session[tuple(key)] = value def delete(self, key): - try: - del self._descriptor_model_data[key.field_name] - except (KeyError, InvalidScopeError): - del self._session[tuple(key)] + del self._session[tuple(key)] def has(self, key): - return key.field_name in self._descriptor_model_data or tuple(key) in self._session + return tuple(key) in self._session diff --git a/cms/djangoapps/models/settings/course_grading.py b/cms/djangoapps/models/settings/course_grading.py index 0746fc7a902..578961fad6c 100644 --- a/cms/djangoapps/models/settings/course_grading.py +++ b/cms/djangoapps/models/settings/course_grading.py @@ -125,7 +125,7 @@ class CourseGradingModel(object): # Save the data that we've just changed to the underlying # MongoKeyValueStore before we update the mongo datastore. descriptor.save() - get_modulestore(course_location).update_item(course_location, descriptor._model_data._kvs._data) + get_modulestore(course_location).update_item(course_location, descriptor._field_data._kvs._data) return CourseGradingModel.jsonize_grader(index, descriptor.raw_grader[index]) @@ -144,7 +144,7 @@ class CourseGradingModel(object): # Save the data that we've just changed to the underlying # MongoKeyValueStore before we update the mongo datastore. descriptor.save() - get_modulestore(course_location).update_item(course_location, descriptor._model_data._kvs._data) + get_modulestore(course_location).update_item(course_location, descriptor._field_data._kvs._data) return cutoffs @@ -168,12 +168,12 @@ class CourseGradingModel(object): grace_timedelta = timedelta(**graceperiodjson) descriptor = get_modulestore(course_location).get_item(course_location) - descriptor.lms.graceperiod = grace_timedelta + descriptor.graceperiod = grace_timedelta # Save the data that we've just changed to the underlying # MongoKeyValueStore before we update the mongo datastore. descriptor.save() - get_modulestore(course_location).update_metadata(course_location, descriptor._model_data._kvs._metadata) + get_modulestore(course_location).update_metadata(course_location, descriptor._field_data._kvs._metadata) @staticmethod def delete_grader(course_location, index): @@ -193,7 +193,7 @@ class CourseGradingModel(object): # Save the data that we've just changed to the underlying # MongoKeyValueStore before we update the mongo datastore. descriptor.save() - get_modulestore(course_location).update_item(course_location, descriptor._model_data._kvs._data) + get_modulestore(course_location).update_item(course_location, descriptor._field_data._kvs._data) @staticmethod def delete_grace_period(course_location): @@ -204,12 +204,12 @@ class CourseGradingModel(object): course_location = Location(course_location) descriptor = get_modulestore(course_location).get_item(course_location) - del descriptor.lms.graceperiod + del descriptor.graceperiod # Save the data that we've just changed to the underlying # MongoKeyValueStore before we update the mongo datastore. descriptor.save() - get_modulestore(course_location).update_metadata(course_location, descriptor._model_data._kvs._metadata) + get_modulestore(course_location).update_metadata(course_location, descriptor._field_data._kvs._metadata) @staticmethod def get_section_grader_type(location): @@ -217,7 +217,7 @@ class CourseGradingModel(object): location = Location(location) descriptor = get_modulestore(location).get_item(location) - return {"graderType": descriptor.lms.format if descriptor.lms.format is not None else 'Not Graded', + return {"graderType": descriptor.format if descriptor.format is not None else 'Not Graded', "location": location, "id": 99 # just an arbitrary value to } @@ -229,21 +229,21 @@ class CourseGradingModel(object): descriptor = get_modulestore(location).get_item(location) if 'graderType' in jsondict and jsondict['graderType'] != u"Not Graded": - descriptor.lms.format = jsondict.get('graderType') - descriptor.lms.graded = True + descriptor.format = jsondict.get('graderType') + descriptor.graded = True else: - del descriptor.lms.format - del descriptor.lms.graded + del descriptor.format + del descriptor.graded # Save the data that we've just changed to the underlying # MongoKeyValueStore before we update the mongo datastore. descriptor.save() - get_modulestore(location).update_metadata(location, descriptor._model_data._kvs._metadata) + get_modulestore(location).update_metadata(location, descriptor._field_data._kvs._metadata) @staticmethod def convert_set_grace_period(descriptor): # 5 hours 59 minutes 59 seconds => converted to iso format - rawgrace = descriptor.lms.graceperiod + rawgrace = descriptor.graceperiod if rawgrace: hours_from_days = rawgrace.days * 24 seconds = rawgrace.seconds diff --git a/cms/djangoapps/models/settings/course_metadata.py b/cms/djangoapps/models/settings/course_metadata.py index adbbf06c646..f186fbf1164 100644 --- a/cms/djangoapps/models/settings/course_metadata.py +++ b/cms/djangoapps/models/settings/course_metadata.py @@ -1,8 +1,9 @@ from xmodule.modulestore import Location from contentstore.utils import get_modulestore from xmodule.modulestore.inheritance import own_metadata -from xblock.core import Scope +from xblock.fields import Scope from xmodule.course_module import CourseDescriptor +from cms.xmodule_namespace import CmsBlockMixin class CourseMetadata(object): @@ -34,12 +35,17 @@ class CourseMetadata(object): descriptor = get_modulestore(course_location).get_item(course_location) - for field in descriptor.fields + descriptor.lms.fields: + for field in descriptor.fields.values(): + if field.name in CmsBlockMixin.fields: + continue + if field.scope != Scope.settings: continue - if field.name not in cls.FILTERED_LIST: - course[field.name] = field.read_json(descriptor) + if field.name in cls.FILTERED_LIST: + continue + + course[field.name] = field.read_json(descriptor) return course @@ -67,12 +73,8 @@ class CourseMetadata(object): if hasattr(descriptor, key) and getattr(descriptor, key) != val: dirty = True - value = getattr(CourseDescriptor, key).from_json(val) + value = descriptor.fields[key].from_json(val) setattr(descriptor, key, value) - elif hasattr(descriptor.lms, key) and getattr(descriptor.lms, key) != key: - dirty = True - value = getattr(CourseDescriptor.lms, key).from_json(val) - setattr(descriptor.lms, key, value) if dirty: # Save the data that we've just changed to the underlying @@ -96,8 +98,6 @@ class CourseMetadata(object): for key in payload['deleteKeys']: if hasattr(descriptor, key): delattr(descriptor, key) - elif hasattr(descriptor.lms, key): - delattr(descriptor.lms, key) # Save the data that we've just changed to the underlying # MongoKeyValueStore before we update the mongo datastore. diff --git a/cms/envs/common.py b/cms/envs/common.py index d0650e9ed24..ecb85a05e02 100644 --- a/cms/envs/common.py +++ b/cms/envs/common.py @@ -28,6 +28,10 @@ import lms.envs.common from lms.envs.common import USE_TZ, TECH_SUPPORT_EMAIL, PLATFORM_NAME, BUGS_EMAIL from path import path +from lms.xblock.mixin import LmsBlockMixin +from cms.xmodule_namespace import CmsBlockMixin +from xmodule.modulestore.inheritance import InheritanceMixin + ############################ FEATURE CONFIGURATION ############################# MITX_FEATURES = { @@ -160,6 +164,13 @@ MIDDLEWARE_CLASSES = ( 'ratelimitbackend.middleware.RateLimitMiddleware', ) +############# XBlock Configuration ########## + +# This should be moved into an XBlock Runtime/Application object +# once the responsibility of XBlock creation is moved out of modulestore - cpennington +XBLOCK_MIXINS = (LmsBlockMixin, CmsBlockMixin, InheritanceMixin) + + ############################ SIGNAL HANDLERS ################################ # This is imported to register the exception signal handling that logs exceptions import monitoring.exceptions # noqa diff --git a/cms/startup.py b/cms/startup.py index eb1098a7074..111cb9f96ae 100644 --- a/cms/startup.py +++ b/cms/startup.py @@ -1,6 +1,7 @@ """ Module with code executed during Studio startup """ +import logging from django.conf import settings # Force settings to run so that the python path is modified @@ -8,6 +9,8 @@ settings.INSTALLED_APPS # pylint: disable=W0104 from django_startup import autostartup +log = logging.getLogger(__name__) + # TODO: Remove this code once Studio/CMS runs via wsgi in all environments INITIALIZED = False @@ -22,4 +25,3 @@ def run(): INITIALIZED = True autostartup() - diff --git a/cms/templates/edit_subsection.html b/cms/templates/edit_subsection.html index 5b03643f3b9..b1be834709e 100644 --- a/cms/templates/edit_subsection.html +++ b/cms/templates/edit_subsection.html @@ -37,21 +37,21 @@ <div class="field field-start-date"> <label for="start_date">${_("Release Day")}</label> <input type="text" id="start_date" name="start_date" - value="${subsection.lms.start.strftime('%m/%d/%Y') if subsection.lms.start else ''}" + value="${subsection.start.strftime('%m/%d/%Y') if subsection.start else ''}" placeholder="MM/DD/YYYY" class="date" size='15' autocomplete="off"/> </div> <div class="field field-start-time"> <label for="start_time">${_("Release Time")} (<abbr title="${_("Coordinated Universal Time")}">${_("UTC")}</abbr>)</label> <input type="text" id="start_time" name="start_time" - value="${subsection.lms.start.strftime('%H:%M') if subsection.lms.start else ''}" + value="${subsection.start.strftime('%H:%M') if subsection.start else ''}" placeholder="HH:MM" class="time" size='10' autocomplete="off"/> </div> </div> - % if subsection.lms.start and not almost_same_datetime(subsection.lms.start, parent_item.lms.start): - % if parent_item.lms.start is None: + % if subsection.start and not almost_same_datetime(subsection.start, parent_item.start): + % if parent_item.start is None: <p class="notice">${_("The date above differs from the release date of {name}, which is unset.").format(name=parent_item.display_name_with_default)} % else: - <p class="notice">${_("The date above differs from the release date of {name} - {start_time}").format(name=parent_item.display_name_with_default, start_time=get_default_time_display(parent_item.lms.start))}. + <p class="notice">${_("The date above differs from the release date of {name} - {start_time}").format(name=parent_item.display_name_with_default, start_time=get_default_time_display(parent_item.start))}. % endif <a href="#" class="sync-date no-spinner">${_("Sync to {name}.").format(name=parent_item.display_name_with_default)}</a></p> % endif @@ -60,7 +60,7 @@ <div class="row gradable"> <label>${_("Graded as:")}</label> - <div class="gradable-status" data-initial-status="${subsection.lms.format if subsection.lms.format is not None else _('Not Graded')}"> + <div class="gradable-status" data-initial-status="${subsection.format if subsection.format is not None else _('Not Graded')}"> </div> <div class="due-date-input row"> @@ -69,13 +69,13 @@ <div class="field field-start-date"> <label for="due_date">${_("Due Day")}</label> <input type="text" id="due_date" name="due_date" - value="${subsection.lms.due.strftime('%m/%d/%Y') if subsection.lms.due else ''}" + value="${subsection.due.strftime('%m/%d/%Y') if subsection.due else ''}" placeholder="MM/DD/YYYY" class="date" size='15' autocomplete="off"/> </div> <div class="field field-start-time"> <label for="due_time">${_("Due Time")} (<abbr title="${_('Coordinated Universal Time')}">UTC</abbr>)</label> <input type="text" id="due_time" name="due_time" - value="${subsection.lms.due.strftime('%H:%M') if subsection.lms.due else ''}" + value="${subsection.due.strftime('%H:%M') if subsection.due else ''}" placeholder="HH:MM" class="time" size='10' autocomplete="off"/> </div> <a href="#" class="remove-date">${_("Remove due date")}</a> diff --git a/cms/templates/overview.html b/cms/templates/overview.html index 2c42df187ae..a8cdfff5541 100644 --- a/cms/templates/overview.html +++ b/cms/templates/overview.html @@ -157,19 +157,19 @@ <h3 class="section-name" data-name="${section.display_name_with_default | h}"></h3> <div class="section-published-date"> <% - if section.lms.start is not None: - start_date_str = section.lms.start.strftime('%m/%d/%Y') - start_time_str = section.lms.start.strftime('%H:%M') + if section.start is not None: + start_date_str = section.start.strftime('%m/%d/%Y') + start_time_str = section.start.strftime('%H:%M') else: start_date_str = '' start_time_str = '' %> - %if section.lms.start is None: + %if section.start is None: <span class="published-status">${_("This section has not been released.")}</span> <a href="#" class="schedule-button" data-date="" data-time="" data-id="${section.location}">${_("Schedule")}</a> %else: <span class="published-status"><strong>${_("Will Release:")}</strong> - ${date_utils.get_default_time_display(section.lms.start)}</span> + ${date_utils.get_default_time_display(section.start)}</span> <a href="#" class="edit-button" data-date="${start_date_str}" data-time="${start_time_str}" data-id="${section.location}">${_("Edit")}</a> %endif @@ -199,7 +199,7 @@ </a> </div> - <div class="gradable-status" data-initial-status="${subsection.lms.format if subsection.lms.format is not None else _('Not Graded')}"> + <div class="gradable-status" data-initial-status="${subsection.format if subsection.format is not None else _('Not Graded')}"> </div> <div class="item-actions"> diff --git a/cms/templates/widgets/sequence-edit.html b/cms/templates/widgets/sequence-edit.html index 7956fbfbafe..c729f28950f 100644 --- a/cms/templates/widgets/sequence-edit.html +++ b/cms/templates/widgets/sequence-edit.html @@ -35,7 +35,7 @@ <li> <ol id="sortable"> % for child in module.get_children(): - <li class="${module.category}"> + <li class="${module.scope_ids.block_type}"> <a href="#" class="module-edit" data-id="${child.location.url()}" data-type="${child.js_module_name}" diff --git a/cms/templates/widgets/units.html b/cms/templates/widgets/units.html index 62c1fb62d7c..4e971736f7e 100644 --- a/cms/templates/widgets/units.html +++ b/cms/templates/widgets/units.html @@ -21,7 +21,7 @@ This def will enumerate through a passed in subsection and list all of the units %> <div class="section-item ${selected_class}"> <a href="${reverse('edit_unit', args=[unit.location])}" class="${unit_state}-item"> - <span class="${unit.category}-icon"></span> + <span class="${unit.scope_ids.block_type}-icon"></span> <span class="unit-name">${unit.display_name_with_default}</span> </a> % if actions: diff --git a/cms/xmodule_namespace.py b/cms/xmodule_namespace.py index eef4b41f378..c028ab37106 100644 --- a/cms/xmodule_namespace.py +++ b/cms/xmodule_namespace.py @@ -4,12 +4,12 @@ Namespace defining common fields used by Studio for all blocks import datetime -from xblock.core import Namespace, Scope, ModelType, String +from xblock.fields import Scope, Field, Integer, XBlockMixin -class DateTuple(ModelType): +class DateTuple(Field): """ - ModelType that stores datetime objects as time tuples + Field that stores datetime objects as time tuples """ def from_json(self, value): return datetime.datetime(*value[0:6]) @@ -21,9 +21,9 @@ class DateTuple(ModelType): return list(value.timetuple()) -class CmsNamespace(Namespace): +class CmsBlockMixin(XBlockMixin): """ - Namespace with fields common to all blocks in Studio + Mixin with fields common to all blocks in Studio """ published_date = DateTuple(help="Date when the module was published", scope=Scope.settings) - published_by = String(help="Id of the user who published this module", scope=Scope.settings) + published_by = Integer(help="Id of the user who published this module", scope=Scope.settings) diff --git a/common/djangoapps/external_auth/views.py b/common/djangoapps/external_auth/views.py index 37dcd5313a9..a4d6752276b 100644 --- a/common/djangoapps/external_auth/views.py +++ b/common/djangoapps/external_auth/views.py @@ -44,7 +44,7 @@ from ratelimitbackend.exceptions import RateLimitException import student.views as student_views # Required for Pearson from courseware.views import get_module_for_descriptor, jump_to -from courseware.model_data import ModelDataCache +from courseware.model_data import FieldDataCache from xmodule.modulestore.django import modulestore from xmodule.course_module import CourseDescriptor from xmodule.modulestore import Location @@ -915,7 +915,7 @@ def test_center_login(request): log.error("cand {} on exam {} for course {}: descriptor not found for location {}".format(client_candidate_id, exam_series_code, course_id, location)) return HttpResponseRedirect(makeErrorURL(error_url, "missingClientProgram")) - timelimit_module_cache = ModelDataCache.cache_for_descriptor_descendents(course_id, testcenteruser.user, + timelimit_module_cache = FieldDataCache.cache_for_descriptor_descendents(course_id, testcenteruser.user, timelimit_descriptor, depth=None) timelimit_module = get_module_for_descriptor(request.user, request, timelimit_descriptor, timelimit_module_cache, course_id, position=None) diff --git a/common/djangoapps/tests.py b/common/djangoapps/tests.py index 4a61a106d44..17e9c07f6a6 100644 --- a/common/djangoapps/tests.py +++ b/common/djangoapps/tests.py @@ -32,7 +32,7 @@ class TestXmoduleModfiers(ModuleStoreTestCase): late_problem = ItemFactory.create( parent_location=section.location, display_name='problem hist 2', category='problem') - late_problem.lms.start = datetime.datetime.now(UTC) + datetime.timedelta(days=32) + late_problem.start = datetime.datetime.now(UTC) + datetime.timedelta(days=32) late_problem.has_score = False diff --git a/common/djangoapps/xmodule_modifiers.py b/common/djangoapps/xmodule_modifiers.py index 16fe12371bb..14f80c0a70e 100644 --- a/common/djangoapps/xmodule_modifiers.py +++ b/common/djangoapps/xmodule_modifiers.py @@ -29,12 +29,16 @@ def wrap_xmodule(get_html, module, template, context=None): if context is None: context = {} + # If XBlock generated this class, then use the first baseclass + # as the name (since that's the original, unmixed class) + class_name = getattr(module, 'unmixed_class', module.__class__).__name__ + @wraps(get_html) def _get_html(): context.update({ 'content': get_html(), 'display_name': module.display_name, - 'class_': module.__class__.__name__, + 'class_': class_name, 'module_name': module.js_module_name }) @@ -157,7 +161,7 @@ def add_histogram(get_html, module, user): # doesn't like symlinks) filepath = filename data_dir = osfs.root_path.rsplit('/')[-1] - giturl = module.lms.giturl or 'https://github.com/MITx' + giturl = module.giturl or 'https://github.com/MITx' edit_link = "%s/%s/tree/master/%s" % (giturl, data_dir, filepath) else: edit_link = False @@ -165,22 +169,21 @@ def add_histogram(get_html, module, user): giturl = "" data_dir = "" - source_file = module.lms.source_file # source used to generate the problem XML, eg latex or word + source_file = module.source_file # source used to generate the problem XML, eg latex or word # useful to indicate to staff if problem has been released or not # TODO (ichuang): use _has_access_descriptor.can_load in lms.courseware.access, instead of now>mstart comparison here now = datetime.datetime.now(UTC()) is_released = "unknown" - mstart = module.descriptor.lms.start + mstart = module.descriptor.start if mstart is not None: is_released = "<font color='red'>Yes!</font>" if (now > mstart) else "<font color='green'>Not yet</font>" - staff_context = {'fields': [(field.name, getattr(module, field.name)) for field in module.fields], - 'lms_fields': [(field.name, getattr(module.lms, field.name)) for field in module.lms.fields], - 'xml_attributes' : getattr(module.descriptor, 'xml_attributes', {}), + staff_context = {'fields': [(name, field.read_from(module)) for name, field in module.fields.items()], + 'xml_attributes': getattr(module.descriptor, 'xml_attributes', {}), 'location': module.location, - 'xqa_key': module.lms.xqa_key, + 'xqa_key': module.xqa_key, 'source_file': source_file, 'source_url': '%s/%s/tree/master/%s' % (giturl, data_dir, source_file), 'category': str(module.__class__.__name__), diff --git a/common/lib/xmodule/xmodule/abtest_module.py b/common/lib/xmodule/xmodule/abtest_module.py index 53f080eb3a8..c0a53d048f4 100644 --- a/common/lib/xmodule/xmodule/abtest_module.py +++ b/common/lib/xmodule/xmodule/abtest_module.py @@ -6,7 +6,7 @@ from xmodule.x_module import XModule from xmodule.raw_module import RawDescriptor from xmodule.xml_module import XmlDescriptor from xmodule.exceptions import InvalidDefinitionError -from xblock.core import String, Scope, Dict +from xblock.fields import String, Scope, Dict DEFAULT = "_DEFAULT_GROUP" diff --git a/common/lib/xmodule/xmodule/annotatable_module.py b/common/lib/xmodule/xmodule/annotatable_module.py index f80e3e488e6..ca85065577e 100644 --- a/common/lib/xmodule/xmodule/annotatable_module.py +++ b/common/lib/xmodule/xmodule/annotatable_module.py @@ -5,7 +5,7 @@ from pkg_resources import resource_string from xmodule.x_module import XModule from xmodule.raw_module import RawDescriptor -from xblock.core import Scope, String +from xblock.fields import Scope, String import textwrap log = logging.getLogger(__name__) diff --git a/common/lib/xmodule/xmodule/capa_module.py b/common/lib/xmodule/xmodule/capa_module.py index dbd535a471b..8de38022b73 100644 --- a/common/lib/xmodule/xmodule/capa_module.py +++ b/common/lib/xmodule/xmodule/capa_module.py @@ -18,7 +18,7 @@ from .progress import Progress from xmodule.x_module import XModule from xmodule.raw_module import RawDescriptor from xmodule.exceptions import NotFoundError, ProcessingError -from xblock.core import Scope, String, Boolean, Dict, Integer, Float +from xblock.fields import Scope, String, Boolean, Dict, Integer, Float from .fields import Timedelta, Date from django.utils.timezone import UTC diff --git a/common/lib/xmodule/xmodule/combined_open_ended_module.py b/common/lib/xmodule/xmodule/combined_open_ended_module.py index 5354915b860..c6dc882bb6c 100644 --- a/common/lib/xmodule/xmodule/combined_open_ended_module.py +++ b/common/lib/xmodule/xmodule/combined_open_ended_module.py @@ -5,7 +5,7 @@ from pkg_resources import resource_string from xmodule.raw_module import RawDescriptor from .x_module import XModule -from xblock.core import Integer, Scope, String, List, Float, Boolean +from xblock.fields import Integer, Scope, String, List, Float, Boolean from xmodule.open_ended_grading_classes.combined_open_ended_modulev1 import CombinedOpenEndedV1Module, CombinedOpenEndedV1Descriptor from collections import namedtuple from .fields import Date, Timedelta diff --git a/common/lib/xmodule/xmodule/conditional_module.py b/common/lib/xmodule/xmodule/conditional_module.py index 5bdc8e77976..82416d48d1b 100644 --- a/common/lib/xmodule/xmodule/conditional_module.py +++ b/common/lib/xmodule/xmodule/conditional_module.py @@ -10,7 +10,7 @@ from pkg_resources import resource_string from xmodule.x_module import XModule from xmodule.modulestore import Location from xmodule.seq_module import SequenceDescriptor -from xblock.core import Scope, List +from xblock.fields import Scope, List from xmodule.modulestore.exceptions import ItemNotFoundError diff --git a/common/lib/xmodule/xmodule/course_module.py b/common/lib/xmodule/xmodule/course_module.py index c583aec3d12..aca804d5e2e 100644 --- a/common/lib/xmodule/xmodule/course_module.py +++ b/common/lib/xmodule/xmodule/course_module.py @@ -13,7 +13,7 @@ from xmodule.util.decorators import lazyproperty from xmodule.graders import grader_from_conf import json -from xblock.core import Scope, List, String, Dict, Boolean +from xblock.fields import Scope, List, String, Dict, Boolean from .fields import Date from xmodule.modulestore.locator import CourseLocator from django.utils.timezone import UTC @@ -118,6 +118,13 @@ class Textbook(object): return table_of_contents + def __eq__(self, other): + return (self.title == other.title and + self.book_url == other.book_url) + + def __ne__(self, other): + return not self == other + class TextbookList(List): def from_json(self, values): @@ -737,7 +744,7 @@ class CourseDescriptor(CourseFields, SequenceDescriptor): all_descriptors - This contains a list of all xmodules that can effect grading a student. This is used to efficiently fetch - all the xmodule state for a ModelDataCache without walking + all the xmodule state for a FieldDataCache without walking the descriptor tree again. @@ -754,14 +761,14 @@ class CourseDescriptor(CourseFields, SequenceDescriptor): for c in self.get_children(): for s in c.get_children(): - if s.lms.graded: + if s.graded: xmoduledescriptors = list(yield_descriptor_descendents(s)) xmoduledescriptors.append(s) # The xmoduledescriptors included here are only the ones that have scores. section_description = {'section_descriptor': s, 'xmoduledescriptors': filter(lambda child: child.has_score, xmoduledescriptors)} - section_format = s.lms.format if s.lms.format is not None else '' + section_format = s.format if s.format is not None else '' graded_sections[section_format] = graded_sections.get(section_format, []) + [section_description] all_descriptors.extend(xmoduledescriptors) diff --git a/common/lib/xmodule/xmodule/crowdsource_hinter.py b/common/lib/xmodule/xmodule/crowdsource_hinter.py index 7e538efa24f..fcc74d5cb9a 100644 --- a/common/lib/xmodule/xmodule/crowdsource_hinter.py +++ b/common/lib/xmodule/xmodule/crowdsource_hinter.py @@ -15,7 +15,7 @@ from lxml import etree from xmodule.x_module import XModule from xmodule.raw_module import RawDescriptor -from xblock.core import Scope, String, Integer, Boolean, Dict, List +from xblock.fields import Scope, String, Integer, Boolean, Dict, List from capa.responsetypes import FormulaResponse diff --git a/common/lib/xmodule/xmodule/discussion_module.py b/common/lib/xmodule/xmodule/discussion_module.py index 2c6cc9f4ddb..8051cb23a37 100644 --- a/common/lib/xmodule/xmodule/discussion_module.py +++ b/common/lib/xmodule/xmodule/discussion_module.py @@ -3,7 +3,7 @@ from pkg_resources import resource_string from xmodule.x_module import XModule from xmodule.raw_module import RawDescriptor from xmodule.editing_module import MetadataOnlyEditingDescriptor -from xblock.core import String, Scope +from xblock.fields import String, Scope from uuid import uuid4 diff --git a/common/lib/xmodule/xmodule/editing_module.py b/common/lib/xmodule/xmodule/editing_module.py index eed8fb310f5..35e3e4c6d10 100644 --- a/common/lib/xmodule/xmodule/editing_module.py +++ b/common/lib/xmodule/xmodule/editing_module.py @@ -2,7 +2,7 @@ from pkg_resources import resource_string from xmodule.mako_module import MakoModuleDescriptor -from xblock.core import Scope, String +from xblock.fields import Scope, String import logging log = logging.getLogger(__name__) diff --git a/common/lib/xmodule/xmodule/error_module.py b/common/lib/xmodule/xmodule/error_module.py index 027067f4c09..2013ee2003d 100644 --- a/common/lib/xmodule/xmodule/error_module.py +++ b/common/lib/xmodule/xmodule/error_module.py @@ -12,7 +12,8 @@ from lxml import etree from xmodule.x_module import XModule, XModuleDescriptor from xmodule.errortracker import exc_info_to_str from xmodule.modulestore import Location -from xblock.core import String, Scope +from xblock.fields import String, Scope, ScopeIds +from xblock.field_data import DictFieldData log = logging.getLogger(__name__) @@ -95,16 +96,19 @@ class ErrorDescriptor(ErrorFields, XModuleDescriptor): ) # real metadata stays in the content, but add a display name - model_data = { + field_data = DictFieldData({ 'error_msg': str(error_msg), 'contents': contents, 'display_name': 'Error: ' + location.url(), 'location': location, 'category': 'error' - } - return cls( - system, - model_data, + }) + return system.construct_xblock_from_class( + cls, + field_data, + # The error module doesn't use scoped data, and thus doesn't need + # real scope keys + ScopeIds('error', None, location, location) ) def get_context(self): diff --git a/common/lib/xmodule/xmodule/fields.py b/common/lib/xmodule/xmodule/fields.py index b7094203c41..a0b53859ce7 100644 --- a/common/lib/xmodule/xmodule/fields.py +++ b/common/lib/xmodule/xmodule/fields.py @@ -2,7 +2,7 @@ import time import logging import re -from xblock.core import ModelType +from xblock.fields import Field import datetime import dateutil.parser @@ -11,7 +11,7 @@ from pytz import UTC log = logging.getLogger(__name__) -class Date(ModelType): +class Date(Field): ''' Date fields know how to parse and produce json (iso) compatible formats. Converts to tz aware datetimes. ''' @@ -20,6 +20,8 @@ class Date(ModelType): PREVENT_DEFAULT_DAY_MON_SEED1 = datetime.datetime(CURRENT_YEAR, 1, 1, tzinfo=UTC) PREVENT_DEFAULT_DAY_MON_SEED2 = datetime.datetime(CURRENT_YEAR, 2, 2, tzinfo=UTC) + MUTABLE = False + def _parse_date_wo_default_month_day(self, field): """ Parse the field as an iso string but prevent dateutils from defaulting the day or month while @@ -76,12 +78,12 @@ class Date(ModelType): else: return value.isoformat() else: - raise TypeError("Cannot convert {} to json".format(value)) + raise TypeError("Cannot convert {!r} to json".format(value)) TIMEDELTA_REGEX = re.compile(r'^((?P<days>\d+?) day(?:s?))?(\s)?((?P<hours>\d+?) hour(?:s?))?(\s)?((?P<minutes>\d+?) minute(?:s)?)?(\s)?((?P<seconds>\d+?) second(?:s)?)?$') -class Timedelta(ModelType): +class Timedelta(Field): # Timedeltas are immutable, see http://docs.python.org/2/library/datetime.html#available-types MUTABLE = False diff --git a/common/lib/xmodule/xmodule/foldit_module.py b/common/lib/xmodule/xmodule/foldit_module.py index cadf6cef0bc..e1714ff96b6 100644 --- a/common/lib/xmodule/xmodule/foldit_module.py +++ b/common/lib/xmodule/xmodule/foldit_module.py @@ -6,7 +6,7 @@ from pkg_resources import resource_string from xmodule.editing_module import EditingDescriptor from xmodule.x_module import XModule from xmodule.xml_module import XmlDescriptor -from xblock.core import Scope, Integer, String +from xblock.fields import Scope, Integer, String from .fields import Date diff --git a/common/lib/xmodule/xmodule/gst_module.py b/common/lib/xmodule/xmodule/gst_module.py index d6516d92ef9..ef69130f84b 100644 --- a/common/lib/xmodule/xmodule/gst_module.py +++ b/common/lib/xmodule/xmodule/gst_module.py @@ -14,7 +14,7 @@ from xmodule.xml_module import XmlDescriptor from xmodule.x_module import XModule from xmodule.stringify import stringify_children from pkg_resources import resource_string -from xblock.core import String, Scope +from xblock.fields import String, Scope log = logging.getLogger(__name__) diff --git a/common/lib/xmodule/xmodule/html_module.py b/common/lib/xmodule/xmodule/html_module.py index 7a68c42ac93..5b3ca5661ab 100644 --- a/common/lib/xmodule/xmodule/html_module.py +++ b/common/lib/xmodule/xmodule/html_module.py @@ -7,7 +7,7 @@ from lxml import etree from path import path from pkg_resources import resource_string -from xblock.core import Scope, String +from xblock.fields import Scope, String from xmodule.editing_module import EditingDescriptor from xmodule.html_checker import check_html from xmodule.stringify import stringify_children diff --git a/common/lib/xmodule/xmodule/mako_module.py b/common/lib/xmodule/xmodule/mako_module.py index 526fc1a0eb2..3d37bd615ed 100644 --- a/common/lib/xmodule/xmodule/mako_module.py +++ b/common/lib/xmodule/xmodule/mako_module.py @@ -2,10 +2,8 @@ from .x_module import XModuleDescriptor, DescriptorSystem class MakoDescriptorSystem(DescriptorSystem): - def __init__(self, load_item, resources_fs, error_tracker, - render_template, **kwargs): - super(MakoDescriptorSystem, self).__init__( - load_item, resources_fs, error_tracker, **kwargs) + def __init__(self, render_template, **kwargs): + super(MakoDescriptorSystem, self).__init__(**kwargs) self.render_template = render_template diff --git a/common/lib/xmodule/xmodule/modulestore/__init__.py b/common/lib/xmodule/xmodule/modulestore/__init__.py index f2b70ad3659..e7bcc171f76 100644 --- a/common/lib/xmodule/xmodule/modulestore/__init__.py +++ b/common/lib/xmodule/xmodule/modulestore/__init__.py @@ -398,7 +398,7 @@ class ModuleStoreBase(ModuleStore): ''' Implement interface functionality that can be shared. ''' - def __init__(self, metadata_inheritance_cache_subsystem=None, request_cache=None, modulestore_update_signal=None): + def __init__(self, metadata_inheritance_cache_subsystem=None, request_cache=None, modulestore_update_signal=None, xblock_mixins=()): ''' Set up the error-tracking logic. ''' @@ -406,6 +406,7 @@ class ModuleStoreBase(ModuleStore): self.metadata_inheritance_cache_subsystem = metadata_inheritance_cache_subsystem self.modulestore_update_signal = modulestore_update_signal self.request_cache = request_cache + self.xblock_mixins = xblock_mixins def _get_errorlog(self, location): """ diff --git a/common/lib/xmodule/xmodule/modulestore/django.py b/common/lib/xmodule/xmodule/modulestore/django.py index 9ff82e11374..93416cae17e 100644 --- a/common/lib/xmodule/xmodule/modulestore/django.py +++ b/common/lib/xmodule/xmodule/modulestore/django.py @@ -62,6 +62,7 @@ def create_modulestore_instance(engine, options): metadata_inheritance_cache_subsystem=metadata_inheritance_cache, request_cache=request_cache, modulestore_update_signal=Signal(providing_args=['modulestore', 'course_id', 'location']), + xblock_mixins=getattr(settings, 'XBLOCK_MIXINS', ()), **_options ) diff --git a/common/lib/xmodule/xmodule/modulestore/inheritance.py b/common/lib/xmodule/xmodule/modulestore/inheritance.py index aeec53cc294..a9de875128c 100644 --- a/common/lib/xmodule/xmodule/modulestore/inheritance.py +++ b/common/lib/xmodule/xmodule/modulestore/inheritance.py @@ -1,17 +1,47 @@ -from xblock.core import Scope - -# A list of metadata that this module can inherit from its parent module -INHERITABLE_METADATA = ( - 'graded', 'start', 'due', 'graceperiod', 'showanswer', 'rerandomize', - # TODO (ichuang): used for Fall 2012 xqa server access - 'xqa_key', - # How many days early to show a course element to beta testers (float) - # intended to be set per-course, but can be overridden in for specific - # elements. Can be a float. - 'days_early_for_beta', - 'giturl', # for git edit link - 'static_asset_path', # for static assets placed outside xcontent contentstore -) +from datetime import datetime +from pytz import UTC + +from xblock.fields import Scope, Boolean, String, Float, XBlockMixin +from xmodule.fields import Date, Timedelta + + +class InheritanceMixin(XBlockMixin): + """Field definitions for inheritable fields""" + + graded = Boolean( + help="Whether this module contributes to the final course grade", + default=False, + scope=Scope.settings + ) + + start = Date( + help="Start time when this module is visible", + default=datetime.fromtimestamp(0, UTC), + scope=Scope.settings + ) + due = Date(help="Date that this problem is due by", scope=Scope.settings) + giturl = String(help="url root for course data git repository", scope=Scope.settings) + xqa_key = String(help="DO NOT USE", scope=Scope.settings) + graceperiod = Timedelta( + help="Amount of time after the due date that submissions will be accepted", + scope=Scope.settings + ) + showanswer = String( + help="When to show the problem answer to the student", + scope=Scope.settings, + default="finished" + ) + rerandomize = String( + help="When to rerandomize the problem", + default="never", + scope=Scope.settings + ) + days_early_for_beta = Float( + help="Number of days early to show content to beta users", + default=None, + scope=Scope.settings + ) + static_asset_path = String(help="Path to use for static assets - overrides Studio c4x://", scope=Scope.settings, default='') def compute_inherited_metadata(descriptor): @@ -22,32 +52,47 @@ def compute_inherited_metadata(descriptor): NOTE: This means that there is no such thing as lazy loading at the moment--this accesses all the children.""" for child in descriptor.get_children(): - inherit_metadata(child, descriptor._model_data) + inherit_metadata( + child, + { + name: field.read_from(descriptor) + for name, field in InheritanceMixin.fields.items() + if field.is_set_on(descriptor) + } + ) compute_inherited_metadata(child) -def inherit_metadata(descriptor, model_data): +def inherit_metadata(descriptor, inherited_data): """ Updates this module with metadata inherited from a containing module. Only metadata specified in self.inheritable_metadata will be inherited + + `inherited_data`: A dictionary mapping field names to the values that + they should inherit """ # The inherited values that are actually being used. if not hasattr(descriptor, '_inherited_metadata'): setattr(descriptor, '_inherited_metadata', {}) - # All inheritable metadata values (for which a value exists in model_data). + # All inheritable metadata values (for which a value exists in field_data). if not hasattr(descriptor, '_inheritable_metadata'): setattr(descriptor, '_inheritable_metadata', {}) # Set all inheritable metadata from kwargs that are # in self.inheritable_metadata and aren't already set in metadata - for attr in INHERITABLE_METADATA: - if attr in model_data: - descriptor._inheritable_metadata[attr] = model_data[attr] - if attr not in descriptor._model_data: - descriptor._inherited_metadata[attr] = model_data[attr] - descriptor._model_data[attr] = model_data[attr] + for name, field in InheritanceMixin.fields.items(): + if name not in inherited_data: + continue + inherited_value = inherited_data[name] + + descriptor._inheritable_metadata[name] = inherited_value + if not field.is_set_on(descriptor): + descriptor._inherited_metadata[name] = inherited_value + field.write_to(descriptor, inherited_value) + # We've updated the fields on the descriptor, so we need to save it + descriptor.save() def own_metadata(module): @@ -55,25 +100,25 @@ def own_metadata(module): # FIXME move into kvs? will that work for xml mongo? """ Return a dictionary that contains only non-inherited field keys, - mapped to their values + mapped to their serialized values """ inherited_metadata = getattr(module, '_inherited_metadata', {}) metadata = {} - for field in module.fields + module.lms.fields: + for name, field in module.fields.items(): # Only save metadata that wasn't inherited if field.scope != Scope.settings: continue - if field.name in inherited_metadata and module._model_data.get(field.name) == inherited_metadata.get(field.name): + if not field.is_set_on(module): continue - if field.name not in module._model_data: + if name in inherited_metadata and field.read_from(module) == inherited_metadata.get(name): continue try: - metadata[field.name] = module._model_data[field.name] + metadata[name] = field.read_json(module) except KeyError: - # Ignore any missing keys in _model_data + # Ignore any missing keys in _field_data pass return metadata diff --git a/common/lib/xmodule/xmodule/modulestore/locator.py b/common/lib/xmodule/xmodule/modulestore/locator.py index bdc9e61fce7..916a970ad0d 100644 --- a/common/lib/xmodule/xmodule/modulestore/locator.py +++ b/common/lib/xmodule/xmodule/modulestore/locator.py @@ -6,7 +6,6 @@ from __future__ import absolute_import import logging import inspect from abc import ABCMeta, abstractmethod -from urllib import quote from bson.objectid import ObjectId from bson.errors import InvalidId @@ -19,6 +18,15 @@ from .parsers import BRANCH_PREFIX, BLOCK_PREFIX, URL_VERSION_PREFIX log = logging.getLogger(__name__) +class LocalId(object): + """ + Class for local ids for non-persisted xblocks + + Should be hashable and distinguishable, but nothing else + """ + pass + + class Locator(object): """ A locator is like a URL, it refers to a course resource. @@ -386,9 +394,12 @@ class BlockUsageLocator(CourseLocator): self.set_property('usage_id', new) def init_block_ref(self, block_ref): - parse = parse_block_ref(block_ref) - assert parse, 'Could not parse "%s" as a block_ref' % block_ref - self.set_usage_id(parse['block']) + if isinstance(block_ref, LocalId): + self.set_usage_id(block_ref) + else: + parse = parse_block_ref(block_ref) + assert parse, 'Could not parse "%s" as a block_ref' % block_ref + self.set_usage_id(parse['block']) def init_block_ref_from_url(self, url): if isinstance(url, Locator): @@ -409,12 +420,8 @@ class BlockUsageLocator(CourseLocator): """ Return a string representing this location. """ - rep = CourseLocator.__unicode__(self) - if self.usage_id is None: - # usage_id has not been initialized - return rep + BLOCK_PREFIX + 'NONE' - else: - return rep + BLOCK_PREFIX + self.usage_id + rep = super(BlockUsageLocator, self).__unicode__() + return rep + BLOCK_PREFIX + unicode(self.usage_id) class DescriptionLocator(Locator): diff --git a/common/lib/xmodule/xmodule/modulestore/mixed.py b/common/lib/xmodule/xmodule/modulestore/mixed.py index 8827f333604..6a1c3b37c9f 100644 --- a/common/lib/xmodule/xmodule/modulestore/mixed.py +++ b/common/lib/xmodule/xmodule/modulestore/mixed.py @@ -29,9 +29,9 @@ class MixedModuleStore(ModuleStoreBase): if 'default' not in stores: raise Exception('Missing a default modulestore in the MixedModuleStore __init__ method.') - for key in stores: - self.modulestores[key] = create_modulestore_instance(stores[key]['ENGINE'], - stores[key]['OPTIONS']) + for key, store in stores.items(): + self.modulestores[key] = create_modulestore_instance(store['ENGINE'], + store['OPTIONS']) def _get_modulestore_for_courseid(self, course_id): """ diff --git a/common/lib/xmodule/xmodule/modulestore/mongo/__init__.py b/common/lib/xmodule/xmodule/modulestore/mongo/__init__.py index bdeda76d321..c20ca43e84c 100644 --- a/common/lib/xmodule/xmodule/modulestore/mongo/__init__.py +++ b/common/lib/xmodule/xmodule/modulestore/mongo/__init__.py @@ -2,7 +2,7 @@ Provide names as exported by older mongo.py module """ -from xmodule.modulestore.mongo.base import MongoModuleStore, MongoKeyValueStore, MongoUsage +from xmodule.modulestore.mongo.base import MongoModuleStore, MongoKeyValueStore # Backwards compatibility for prod systems that refererence # xmodule.modulestore.mongo.DraftMongoModuleStore diff --git a/common/lib/xmodule/xmodule/modulestore/mongo/base.py b/common/lib/xmodule/xmodule/modulestore/mongo/base.py index b1fecec120d..10516764848 100644 --- a/common/lib/xmodule/xmodule/modulestore/mongo/base.py +++ b/common/lib/xmodule/xmodule/modulestore/mongo/base.py @@ -29,12 +29,13 @@ from xmodule.errortracker import null_error_tracker, exc_info_to_str from xmodule.mako_module import MakoDescriptorSystem from xmodule.x_module import XModuleDescriptor from xmodule.error_module import ErrorDescriptor -from xblock.runtime import DbModel, KeyValueStore, InvalidScopeError -from xblock.core import Scope +from xblock.runtime import DbModel, KeyValueStore +from xblock.exceptions import InvalidScopeError +from xblock.fields import Scope, ScopeIds from xmodule.modulestore import ModuleStoreBase, Location, namedtuple_to_son, MONGO_MODULESTORE_TYPE from xmodule.modulestore.exceptions import ItemNotFoundError -from xmodule.modulestore.inheritance import own_metadata, INHERITABLE_METADATA, inherit_metadata +from xmodule.modulestore.inheritance import own_metadata, InheritanceMixin, inherit_metadata log = logging.getLogger(__name__) @@ -62,12 +63,10 @@ class MongoKeyValueStore(KeyValueStore): A KeyValueStore that maps keyed data access to one of the 3 data areas known to the MongoModuleStore (data, children, and metadata) """ - def __init__(self, data, children, metadata, location, category): + def __init__(self, data, children, metadata): self._data = data self._children = children self._metadata = metadata - self._location = location - self._category = category def get(self, key): if key.scope == Scope.children: @@ -77,11 +76,7 @@ class MongoKeyValueStore(KeyValueStore): elif key.scope == Scope.settings: return self._metadata[key.field_name] elif key.scope == Scope.content: - if key.field_name == 'location': - return self._location - elif key.field_name == 'category': - return self._category - elif key.field_name == 'data' and not isinstance(self._data, dict): + if key.field_name == 'data' and not isinstance(self._data, dict): return self._data else: return self._data[key.field_name] @@ -94,11 +89,7 @@ class MongoKeyValueStore(KeyValueStore): elif key.scope == Scope.settings: self._metadata[key.field_name] = value elif key.scope == Scope.content: - if key.field_name == 'location': - self._location = value - elif key.field_name == 'category': - self._category = value - elif key.field_name == 'data' and not isinstance(self._data, dict): + if key.field_name == 'data' and not isinstance(self._data, dict): self._data = value else: self._data[key.field_name] = value @@ -112,11 +103,7 @@ class MongoKeyValueStore(KeyValueStore): if key.field_name in self._metadata: del self._metadata[key.field_name] elif key.scope == Scope.content: - if key.field_name == 'location': - self._location = Location(None) - elif key.field_name == 'category': - self._category = None - elif key.field_name == 'data' and not isinstance(self._data, dict): + if key.field_name == 'data' and not isinstance(self._data, dict): self._data = None else: del self._data[key.field_name] @@ -129,12 +116,7 @@ class MongoKeyValueStore(KeyValueStore): elif key.scope == Scope.settings: return key.field_name in self._metadata elif key.scope == Scope.content: - if key.field_name == 'location': - # WHY TRUE? if it's been deleted should it be False? - return True - elif key.field_name == 'category': - return self._category is not None - elif key.field_name == 'data' and not isinstance(self._data, dict): + if key.field_name == 'data' and not isinstance(self._data, dict): return True else: return key.field_name in self._data @@ -142,9 +124,6 @@ class MongoKeyValueStore(KeyValueStore): return False -MongoUsage = namedtuple('MongoUsage', 'id, def_id') - - class CachingDescriptorSystem(MakoDescriptorSystem): """ A system that has a cache of module json that it will use to load modules @@ -152,8 +131,7 @@ class CachingDescriptorSystem(MakoDescriptorSystem): TODO (cdodge) when the 'split module store' work has been completed we can remove all references to metadata_inheritance_tree """ - def __init__(self, modulestore, module_data, default_class, resources_fs, - error_tracker, render_template, cached_metadata=None): + def __init__(self, modulestore, module_data, default_class, cached_metadata, **kwargs): """ modulestore: the module store that can be used to retrieve additional modules @@ -170,8 +148,8 @@ class CachingDescriptorSystem(MakoDescriptorSystem): render_template: a function for rendering templates, as per MakoDescriptorSystem """ - super(CachingDescriptorSystem, self).__init__(self.load_item, resources_fs, - error_tracker, render_template) + super(CachingDescriptorSystem, self).__init__(load_item=self.load_item, **kwargs) + self.modulestore = modulestore self.module_data = module_data self.default_class = default_class @@ -190,7 +168,7 @@ class CachingDescriptorSystem(MakoDescriptorSystem): module = self.modulestore.get_item(location) if module is not None: # update our own cache after going to the DB to get cache miss - self.module_data.update(module.system.module_data) + self.module_data.update(module.runtime.module_data) return module else: # load the module and apply the inherited metadata @@ -202,7 +180,7 @@ class CachingDescriptorSystem(MakoDescriptorSystem): ) definition = json_data.get('definition', {}) metadata = json_data.get('metadata', {}) - for old_name, new_name in class_.metadata_translations.items(): + for old_name, new_name in getattr(class_, 'metadata_translations', {}).items(): if old_name in metadata: metadata[new_name] = metadata[old_name] del metadata[old_name] @@ -211,19 +189,22 @@ class CachingDescriptorSystem(MakoDescriptorSystem): definition.get('data', {}), definition.get('children', []), metadata, - location, - category ) - model_data = DbModel(kvs, class_, None, MongoUsage(self.course_id, location)) - model_data['category'] = category - model_data['location'] = location - module = class_(self, model_data) + field_data = DbModel(kvs) + scope_ids = ScopeIds(None, category, location, location) + module = self.construct_xblock_from_class(class_, field_data, scope_ids) if self.cached_metadata is not None: # parent container pointers don't differentiate between draft and non-draft # so when we do the lookup, we should do so with a non-draft location non_draft_loc = location.replace(revision=None) - metadata_to_inherit = self.cached_metadata.get(non_draft_loc.url(), {}) + + # Convert the serialized fields values in self.cached_metadata + # to python values + metadata_to_inherit = { + key: module.fields[key].from_json(value) + for key, value in self.cached_metadata.get(non_draft_loc.url(), {}).items() + } inherit_metadata(module, metadata_to_inherit) # decache any computed pending field settings module.save() @@ -323,8 +304,8 @@ class MongoModuleStore(ModuleStoreBase): # just get the inheritable metadata since that is all we need for the computation # this minimizes both data pushed over the wire - for attr in INHERITABLE_METADATA: - record_filter['metadata.{0}'.format(attr)] = 1 + for field_name in InheritanceMixin.fields: + record_filter['metadata.{0}'.format(field_name)] = 1 # call out to the DB resultset = self.collection.find(query, record_filter) @@ -496,13 +477,14 @@ class MongoModuleStore(ModuleStoreBase): # TODO (cdodge): When the 'split module store' work has been completed, we should remove # the 'metadata_inheritance_tree' parameter system = CachingDescriptorSystem( - self, - data_cache, - self.default_class, - resource_fs, - self.error_tracker, - self.render_template, - cached_metadata, + modulestore=self, + module_data=data_cache, + default_class=self.default_class, + resources_fs=resource_fs, + error_tracker=self.error_tracker, + render_template=self.render_template, + cached_metadata=cached_metadata, + mixins=self.xblock_mixins, ) return system.load_item(item['location']) @@ -606,7 +588,7 @@ class MongoModuleStore(ModuleStoreBase): :param location: a Location--must have a category :param definition_data: can be empty. The initial definition_data for the kvs :param metadata: can be empty, the initial metadata for the kvs - :param system: if you already have an xmodule from the course, the xmodule.system value + :param system: if you already have an xblock from the course, the xblock.runtime value """ if not isinstance(location, Location): location = Location(location) @@ -616,13 +598,14 @@ class MongoModuleStore(ModuleStoreBase): metadata = {} if system is None: system = CachingDescriptorSystem( - self, - {}, - self.default_class, - None, - self.error_tracker, - self.render_template, - {} + modulestore=self, + module_data={}, + default_class=self.default_class, + resources_fs=None, + error_tracker=self.error_tracker, + render_template=self.render_template, + cached_metadata={}, + mixins=self.xblock_mixins, ) xblock_class = XModuleDescriptor.load_class(location.category, self.default_class) if definition_data is None: @@ -630,8 +613,16 @@ class MongoModuleStore(ModuleStoreBase): definition_data = getattr(xblock_class, 'data').default else: definition_data = {} - dbmodel = self._create_new_model_data(location.category, location, definition_data, metadata) - xmodule = xblock_class(system, dbmodel) + dbmodel = self._create_new_field_data(location.category, location, definition_data, metadata) + xmodule = system.construct_xblock_from_class( + xblock_class, + dbmodel, + + # We're loading a descriptor, so student_id is meaningless + # We also don't have separate notions of definition and usage ids yet, + # so we use the location for both. + ScopeIds(None, location.category, location, location) + ) # decache any pending field settings from init xmodule.save() return xmodule @@ -668,7 +659,7 @@ class MongoModuleStore(ModuleStoreBase): :param location: a Location--must have a category :param definition_data: can be empty. The initial definition_data for the kvs :param metadata: can be empty, the initial metadata for the kvs - :param system: if you already have an xmodule from the course, the xmodule.system value + :param system: if you already have an xblock from the course, the xblock.runtime value """ # differs from split mongo in that I believe most of this logic should be above the persistence # layer but added it here to enable quick conversion. I'll need to reconcile these. @@ -848,7 +839,7 @@ class MongoModuleStore(ModuleStoreBase): """ return MONGO_MODULESTORE_TYPE - def _create_new_model_data(self, category, location, definition_data, metadata): + def _create_new_field_data(self, category, location, definition_data, metadata): """ To instantiate a new xmodule which will be saved latter, set up the dbModel and kvs """ @@ -856,15 +847,7 @@ class MongoModuleStore(ModuleStoreBase): definition_data, [], metadata, - location, - category ) - class_ = XModuleDescriptor.load_class( - category, - self.default_class - ) - model_data = DbModel(kvs, class_, None, MongoUsage(None, location)) - model_data['category'] = category - model_data['location'] = location - return model_data + field_data = DbModel(kvs) + return field_data diff --git a/common/lib/xmodule/xmodule/modulestore/mongo/draft.py b/common/lib/xmodule/xmodule/modulestore/mongo/draft.py index d289e037399..610f7b1b8a9 100644 --- a/common/lib/xmodule/xmodule/modulestore/mongo/draft.py +++ b/common/lib/xmodule/xmodule/modulestore/mongo/draft.py @@ -42,7 +42,7 @@ def wrap_draft(item): non-draft location in either case """ setattr(item, 'is_draft', item.location.revision == DRAFT) - item.location = item.location.replace(revision=None) + item.scope_ids = item.scope_ids._replace(usage_id=item.location.replace(revision=None)) return item @@ -235,10 +235,10 @@ class DraftModuleStore(MongoModuleStore): """ draft = self.get_item(location) - draft.cms.published_date = datetime.now(UTC) - draft.cms.published_by = published_by_id - super(DraftModuleStore, self).update_item(location, draft._model_data._kvs._data) - super(DraftModuleStore, self).update_children(location, draft._model_data._kvs._children) + draft.published_date = datetime.now(UTC) + draft.published_by = published_by_id + super(DraftModuleStore, self).update_item(location, draft._field_data._kvs._data) + super(DraftModuleStore, self).update_children(location, draft._field_data._kvs._children) super(DraftModuleStore, self).update_metadata(location, own_metadata(draft)) self.delete_item(location) diff --git a/common/lib/xmodule/xmodule/modulestore/split_mongo/caching_descriptor_system.py b/common/lib/xmodule/xmodule/modulestore/split_mongo/caching_descriptor_system.py index 73dcabfa695..020ebdbbfe5 100644 --- a/common/lib/xmodule/xmodule/modulestore/split_mongo/caching_descriptor_system.py +++ b/common/lib/xmodule/xmodule/modulestore/split_mongo/caching_descriptor_system.py @@ -2,15 +2,17 @@ import sys import logging from xmodule.mako_module import MakoDescriptorSystem from xmodule.x_module import XModuleDescriptor -from xmodule.modulestore.locator import BlockUsageLocator +from xmodule.modulestore.locator import BlockUsageLocator, LocalId from xmodule.error_module import ErrorDescriptor from xmodule.errortracker import exc_info_to_str from xblock.runtime import DbModel from ..exceptions import ItemNotFoundError -from .split_mongo_kvs import SplitMongoKVS, SplitMongoKVSid +from .split_mongo_kvs import SplitMongoKVS +from xblock.fields import ScopeIds log = logging.getLogger(__name__) + class CachingDescriptorSystem(MakoDescriptorSystem): """ A system that has a cache of a course version's json that it will use to load modules @@ -18,8 +20,7 @@ class CachingDescriptorSystem(MakoDescriptorSystem): Computes the settings (nee 'metadata') inheritance upon creation. """ - def __init__(self, modulestore, course_entry, module_data, lazy, - default_class, error_tracker, render_template): + def __init__(self, modulestore, course_entry, default_class, module_data, lazy, **kwargs): """ Computes the settings inheritance and sets up the cache. @@ -28,34 +29,31 @@ class CachingDescriptorSystem(MakoDescriptorSystem): module_data: a dict mapping Location -> json that was cached from the underlying modulestore - - default_class: The default_class to use when loading an - XModuleDescriptor from the module_data - - resources_fs: a filesystem, as per MakoDescriptorSystem - - error_tracker: a function that logs errors for later display to users - - render_template: a function for rendering templates, as per - MakoDescriptorSystem """ # TODO find all references to resources_fs and make handle None - super(CachingDescriptorSystem, self).__init__( - self._load_item, None, error_tracker, render_template) + super(CachingDescriptorSystem, self).__init__(load_item=self._load_item, **kwargs) self.modulestore = modulestore self.course_entry = course_entry self.lazy = lazy self.module_data = module_data - self.default_class = default_class # TODO see if self.course_id is needed: is already in course_entry but could be > 1 value # Compute inheritance modulestore.inherit_settings( course_entry.get('blocks', {}), course_entry.get('blocks', {}).get(course_entry.get('root')) ) + self.default_class = default_class + self.local_modules = {} def _load_item(self, usage_id, course_entry_override=None): # TODO ensure all callers of system.load_item pass just the id + + if isinstance(usage_id, BlockUsageLocator) and isinstance(usage_id.usage_id, LocalId): + try: + return self.local_modules[usage_id] + except KeyError: + raise ItemNotFoundError + json_data = self.module_data.get(usage_id) if json_data is None: # deeper than initial descendant fetch or doesn't exist @@ -75,6 +73,11 @@ class CachingDescriptorSystem(MakoDescriptorSystem): course_entry_override = self.course_entry # most likely a lazy loader or the id directly definition = json_data.get('definition', {}) + definition_id = self.modulestore.definition_locator(definition) + + # If no usage id is provided, generate an in-memory id + if usage_id is None: + usage_id = LocalId() block_locator = BlockUsageLocator( version_guid=course_entry_override['_id'], @@ -87,25 +90,24 @@ class CachingDescriptorSystem(MakoDescriptorSystem): definition, json_data.get('fields', {}), json_data.get('_inherited_settings'), - block_locator, - json_data.get('category')) - model_data = DbModel(kvs, class_, None, - SplitMongoKVSid( - # DbModel req's that these support .url() - block_locator, - self.modulestore.definition_locator(definition))) + ) + field_data = DbModel(kvs) try: - module = class_(self, model_data) + module = self.construct_xblock_from_class( + class_, + field_data, + ScopeIds(None, json_data.get('category'), definition_id, block_locator) + ) except Exception: log.warning("Failed to load descriptor", exc_info=True) - if usage_id is None: - usage_id = "MISSING" return ErrorDescriptor.from_json( json_data, self, - BlockUsageLocator(version_guid=course_entry_override['_id'], - usage_id=usage_id), + BlockUsageLocator( + version_guid=course_entry_override['_id'], + usage_id=usage_id + ), error_msg=exc_info_to_str(sys.exc_info()) ) @@ -117,4 +119,9 @@ class CachingDescriptorSystem(MakoDescriptorSystem): module.definition_locator = self.modulestore.definition_locator(definition) # decache any pending field settings module.save() + + # If this is an in-memory block, store it in this system + if isinstance(block_locator.usage_id, LocalId): + self.local_modules[block_locator] = module + return module diff --git a/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py b/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py index c5f4c2404c1..d9cf2245cd9 100644 --- a/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py +++ b/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py @@ -8,14 +8,15 @@ from path import path from xmodule.errortracker import null_error_tracker from xmodule.x_module import XModuleDescriptor -from xmodule.modulestore.locator import BlockUsageLocator, DescriptionLocator, CourseLocator, VersionTree +from xmodule.modulestore.locator import BlockUsageLocator, DescriptionLocator, CourseLocator, VersionTree, LocalId from xmodule.modulestore.exceptions import InsufficientSpecificationError, VersionConflictError from xmodule.modulestore import inheritance, ModuleStoreBase from ..exceptions import ItemNotFoundError from .definition_lazy_loader import DefinitionLazyLoader from .caching_descriptor_system import CachingDescriptorSystem -from xblock.core import Scope +from xblock.fields import Scope +from xblock.runtime import Mixologist from pytz import UTC import collections @@ -41,6 +42,8 @@ log = logging.getLogger(__name__) #============================================================================== + + class SplitMongoModuleStore(ModuleStoreBase): """ A Mongodb backed ModuleStore supporting versions, inheritance, @@ -53,7 +56,7 @@ class SplitMongoModuleStore(ModuleStoreBase): mongo_options=None, **kwargs): - ModuleStoreBase.__init__(self) + super(SplitMongoModuleStore, self).__init__(**kwargs) if mongo_options is None: mongo_options = {} @@ -93,6 +96,11 @@ class SplitMongoModuleStore(ModuleStoreBase): self.error_tracker = error_tracker self.render_template = render_template + # TODO: Don't have a runtime just to generate the appropriate mixin classes (cpennington) + # This is only used by _partition_fields_by_scope, which is only needed because + # the split mongo store is used for item creation as well as item persistence + self.mixologist = Mixologist(self.xblock_mixins) + def cache_items(self, system, base_usage_ids, depth=0, lazy=True): ''' Handles caching of items once inheritance and any other one time @@ -144,13 +152,15 @@ class SplitMongoModuleStore(ModuleStoreBase): system = self._get_cache(course_entry['_id']) if system is None: system = CachingDescriptorSystem( - self, - course_entry, - {}, - lazy, - self.default_class, - self.error_tracker, - self.render_template + modulestore=self, + course_entry=course_entry, + module_data={}, + lazy=lazy, + default_class=self.default_class, + error_tracker=self.error_tracker, + render_template=self.render_template, + resources_fs=None, + mixins=self.xblock_mixins ) self._add_cache(course_entry['_id'], system) self.cache_items(system, usage_ids, depth, lazy) @@ -943,12 +953,12 @@ class SplitMongoModuleStore(ModuleStoreBase): xblock.definition_locator, is_updated = self.update_definition_from_data( xblock.definition_locator, new_def_data, user_id) - if xblock.location.usage_id is None: + if isinstance(xblock.scope_ids.usage_id.usage_id, LocalId): # generate an id is_new = True is_updated = True usage_id = self._generate_usage_id(structure_blocks, xblock.category) - xblock.location.usage_id = usage_id + xblock.scope_ids.usage_id.usage_id = usage_id else: is_new = False usage_id = xblock.location.usage_id @@ -960,9 +970,10 @@ class SplitMongoModuleStore(ModuleStoreBase): updated_blocks = [] if xblock.has_children: for child in xblock.children: - if isinstance(child, XModuleDescriptor): - updated_blocks += self._persist_subdag(child, user_id, structure_blocks) - children.append(child.location.usage_id) + if isinstance(child.usage_id, LocalId): + child_block = xblock.system.get_block(child) + updated_blocks += self._persist_subdag(child_block, user_id, structure_blocks) + children.append(child_block.location.usage_id) else: children.append(child) @@ -1137,9 +1148,9 @@ class SplitMongoModuleStore(ModuleStoreBase): # update the inheriting w/ what should pass to children inheriting_settings = block['_inherited_settings'].copy() block_fields = block['fields'] - for field in inheritance.INHERITABLE_METADATA: - if field in block_fields: - inheriting_settings[field] = block_fields[field] + for field_name in inheritance.InheritanceMixin.fields: + if field_name in block_fields: + inheriting_settings[field_name] = block_fields[field_name] for child in block_fields.get('children', []): self.inherit_settings(block_map, block_map[child], inheriting_settings) @@ -1308,7 +1319,7 @@ class SplitMongoModuleStore(ModuleStoreBase): """ if fields is None: return {} - cls = XModuleDescriptor.load_class(category) + cls = self.mixologist.mix(XModuleDescriptor.load_class(category)) result = collections.defaultdict(dict) for field_name, value in fields.iteritems(): field = getattr(cls, field_name) diff --git a/common/lib/xmodule/xmodule/modulestore/split_mongo/split_mongo_kvs.py b/common/lib/xmodule/xmodule/modulestore/split_mongo/split_mongo_kvs.py index e4a4b920275..361ffff85d0 100644 --- a/common/lib/xmodule/xmodule/modulestore/split_mongo/split_mongo_kvs.py +++ b/common/lib/xmodule/xmodule/modulestore/split_mongo/split_mongo_kvs.py @@ -1,7 +1,8 @@ import copy -from xblock.core import Scope +from xblock.fields import Scope from collections import namedtuple -from xblock.runtime import KeyValueStore, InvalidScopeError +from xblock.runtime import KeyValueStore +from xblock.exceptions import InvalidScopeError from .definition_lazy_loader import DefinitionLazyLoader # id is a BlockUsageLocator, def_id is the definition's guid @@ -18,7 +19,7 @@ class SplitMongoKVS(KeyValueStore): known to the MongoModuleStore (data, children, and metadata) """ - def __init__(self, definition, fields, _inherited_settings, location, category): + def __init__(self, definition, fields, _inherited_settings): """ :param definition: either a lazyloader or definition id for the definition @@ -34,8 +35,6 @@ class SplitMongoKVS(KeyValueStore): # if the db id, then the definition is presumed to be loaded into _fields self._fields = copy.copy(fields) self._inherited_settings = _inherited_settings - self._location = location - self._category = category def get(self, key): # simplest case, field is directly set @@ -57,11 +56,7 @@ class SplitMongoKVS(KeyValueStore): # or get default raise KeyError() elif key.scope == Scope.content: - if key.field_name == 'location': - return self._location - elif key.field_name == 'category': - return self._category - elif isinstance(self._definition, DefinitionLazyLoader): + if isinstance(self._definition, DefinitionLazyLoader): self._load_definition() if key.field_name in self._fields: return self._fields[key.field_name] @@ -75,14 +70,7 @@ class SplitMongoKVS(KeyValueStore): if key.scope not in [Scope.children, Scope.settings, Scope.content]: raise InvalidScopeError(key.scope) if key.scope == Scope.content: - if key.field_name == 'location': - self._location = value # is changing this legal? - return - elif key.field_name == 'category': - # TODO should this raise an exception? category is not changeable. - return - else: - self._load_definition() + self._load_definition() # set the field self._fields[key.field_name] = value @@ -99,13 +87,7 @@ class SplitMongoKVS(KeyValueStore): if key.scope not in [Scope.children, Scope.settings, Scope.content]: raise InvalidScopeError(key.scope) if key.scope == Scope.content: - if key.field_name == 'location': - return # noop - elif key.field_name == 'category': - # TODO should this raise an exception? category is not deleteable. - return # noop - else: - self._load_definition() + self._load_definition() # delete the field value if key.field_name in self._fields: @@ -123,17 +105,12 @@ class SplitMongoKVS(KeyValueStore): """ # handle any special cases if key.scope == Scope.content: - if key.field_name == 'location': - return True - elif key.field_name == 'category': - return self._category is not None - else: - self._load_definition() + self._load_definition() elif key.scope == Scope.parent: return True # it's not clear whether inherited values should return True. Right now they don't - # if someone changes it so that they do, then change any tests of field.name in xx._model_data + # if someone changes it so that they do, then change any tests of field.name in xx._field_data return key.field_name in self._fields # would like to just take a key, but there's a bunch of magic in DbModel for constructing the key via diff --git a/common/lib/xmodule/xmodule/modulestore/store_utilities.py b/common/lib/xmodule/xmodule/modulestore/store_utilities.py index 725d4aebb74..a025eb7c827 100644 --- a/common/lib/xmodule/xmodule/modulestore/store_utilities.py +++ b/common/lib/xmodule/xmodule/modulestore/store_utilities.py @@ -110,12 +110,27 @@ def _clone_modules(modulestore, modules, source_location, dest_location): original_loc = Location(module.location) if original_loc.category != 'course': - module.location = module.location._replace( - tag=dest_location.tag, org=dest_location.org, course=dest_location.course) + new_location = module.location._replace( + tag=dest_location.tag, + org=dest_location.org, + course=dest_location.course + ) + module.scope_ids = module.scope_ids._replace( + def_id=new_location, + usage_id=new_location + ) else: # on the course module we also have to update the module name - module.location = module.location._replace( - tag=dest_location.tag, org=dest_location.org, course=dest_location.course, name=dest_location.name) + new_location = module.location._replace( + tag=dest_location.tag, + org=dest_location.org, + course=dest_location.course, + name=dest_location.name + ) + module.scope_ids = module.scope_ids._replace( + def_id=new_location, + usage_id=new_location + ) print "Cloning module {0} to {1}....".format(original_loc, module.location) diff --git a/common/lib/xmodule/xmodule/modulestore/tests/factories.py b/common/lib/xmodule/xmodule/modulestore/tests/factories.py index 7913434086c..4ad801aef84 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/factories.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/factories.py @@ -6,8 +6,6 @@ from pytz import UTC from xmodule.modulestore import Location from xmodule.modulestore.django import editable_modulestore -from xmodule.course_module import CourseDescriptor -from xblock.core import Scope from xmodule.x_module import XModuleDescriptor @@ -35,7 +33,7 @@ class XModuleCourseFactory(Factory): if display_name is not None: new_course.display_name = display_name - new_course.lms.start = datetime.datetime.now(UTC).replace(microsecond=0) + new_course.start = datetime.datetime.now(UTC).replace(microsecond=0) # The rest of kwargs become attributes on the course: for k, v in kwargs.iteritems(): diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py b/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py index 25ad2586688..6f3276bf446 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py @@ -6,8 +6,9 @@ from nose.tools import assert_equals, assert_raises, \ import pymongo from uuid import uuid4 -from xblock.core import Scope -from xblock.runtime import KeyValueStore, InvalidScopeError +from xblock.fields import Scope +from xblock.runtime import KeyValueStore +from xblock.exceptions import InvalidScopeError from xmodule.tests import DATA_DIR from xmodule.modulestore import Location @@ -181,7 +182,7 @@ class TestMongoKeyValueStore(object): self.location = Location('i4x://org/course/category/name@version') self.children = ['i4x://org/course/child/a', 'i4x://org/course/child/b'] self.metadata = {'meta': 'meta_val'} - self.kvs = MongoKeyValueStore(self.data, self.children, self.metadata, self.location, 'category') + self.kvs = MongoKeyValueStore(self.data, self.children, self.metadata) def _check_read(self, key, expected_value): """ @@ -192,7 +193,6 @@ class TestMongoKeyValueStore(object): def test_read(self): assert_equals(self.data['foo'], self.kvs.get(KeyValueStore.Key(Scope.content, None, None, 'foo'))) - assert_equals(self.location, self.kvs.get(KeyValueStore.Key(Scope.content, None, None, 'location'))) assert_equals(self.children, self.kvs.get(KeyValueStore.Key(Scope.children, None, None, 'children'))) assert_equals(self.metadata['meta'], self.kvs.get(KeyValueStore.Key(Scope.settings, None, None, 'meta'))) assert_equals(None, self.kvs.get(KeyValueStore.Key(Scope.parent, None, None, 'parent'))) @@ -214,7 +214,6 @@ class TestMongoKeyValueStore(object): def test_write(self): yield (self._check_write, KeyValueStore.Key(Scope.content, None, None, 'foo'), 'new_data') - yield (self._check_write, KeyValueStore.Key(Scope.content, None, None, 'location'), Location('i4x://org/course/category/name@new_version')) yield (self._check_write, KeyValueStore.Key(Scope.children, None, None, 'children'), []) yield (self._check_write, KeyValueStore.Key(Scope.settings, None, None, 'meta'), 'new_settings') @@ -240,7 +239,6 @@ class TestMongoKeyValueStore(object): def test_delete(self): yield (self._check_delete_key_error, KeyValueStore.Key(Scope.content, None, None, 'foo')) - yield (self._check_delete_default, KeyValueStore.Key(Scope.content, None, None, 'location'), Location(None)) yield (self._check_delete_default, KeyValueStore.Key(Scope.children, None, None, 'children'), []) yield (self._check_delete_key_error, KeyValueStore.Key(Scope.settings, None, None, 'meta')) diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_split_modulestore.py b/common/lib/xmodule/xmodule/modulestore/tests/test_split_modulestore.py index d90c3020b44..b299f56711e 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_split_modulestore.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_split_modulestore.py @@ -9,10 +9,11 @@ import unittest import uuid from importlib import import_module -from xblock.core import Scope +from xblock.fields import Scope from xmodule.course_module import CourseDescriptor from xmodule.modulestore.exceptions import InsufficientSpecificationError, ItemNotFoundError, VersionConflictError from xmodule.modulestore.locator import CourseLocator, BlockUsageLocator, VersionTree, DescriptionLocator +from xmodule.modulestore.inheritance import InheritanceMixin from pytz import UTC from path import path import re @@ -31,6 +32,7 @@ class SplitModuleTest(unittest.TestCase): 'db': 'test_xmodule', 'collection': 'modulestore{0}'.format(uuid.uuid4().hex), 'fs_root': '', + 'xblock_mixins': (InheritanceMixin,) } MODULESTORE = { @@ -187,7 +189,7 @@ class SplitModuleCourseTests(SplitModuleTest): self.assertEqual(course.category, 'course') self.assertEqual(len(course.tabs), 6) self.assertEqual(course.display_name, "The Ancient Greek Hero") - self.assertEqual(course.lms.graceperiod, datetime.timedelta(hours=2)) + self.assertEqual(course.graceperiod, datetime.timedelta(hours=2)) self.assertIsNone(course.advertised_start) self.assertEqual(len(course.children), 0) self.assertEqual(course.definition_locator.definition_id, "head12345_11") @@ -893,7 +895,7 @@ class TestCourseCreation(SplitModuleTest): original = modulestore().get_course(original_locator) original_index = modulestore().get_course_index_info(original_locator) fields = {} - for field in original.fields: + for field in original.fields.values(): if field.scope == Scope.content and field.name != 'location': fields[field.name] = getattr(original, field.name) elif field.scope == Scope.settings: diff --git a/common/lib/xmodule/xmodule/modulestore/xml.py b/common/lib/xmodule/xmodule/modulestore/xml.py index 6527a5e34ab..dd7888179b3 100644 --- a/common/lib/xmodule/xmodule/modulestore/xml.py +++ b/common/lib/xmodule/xmodule/modulestore/xml.py @@ -20,8 +20,11 @@ from xmodule.mako_module import MakoDescriptorSystem from xmodule.x_module import XModuleDescriptor, XMLParsingSystem from xmodule.html_module import HtmlDescriptor +from xblock.fields import ScopeIds +from xblock.field_data import DictFieldData from . import ModuleStoreBase, Location, XML_MODULESTORE_TYPE + from .exceptions import ItemNotFoundError from .inheritance import compute_inherited_metadata @@ -44,7 +47,7 @@ def clean_out_mako_templating(xml_string): class ImportSystem(XMLParsingSystem, MakoDescriptorSystem): def __init__(self, xmlstore, course_id, course_dir, - policy, error_tracker, parent_tracker, + error_tracker, parent_tracker, load_error_modules=True, **kwargs): """ A class that handles loading from xml. Does some munging to ensure that @@ -206,11 +209,14 @@ class ImportSystem(XMLParsingSystem, MakoDescriptorSystem): # policy to be loaded. For now, just add the course_id here... load_item = lambda location: xmlstore.get_instance(course_id, location) resources_fs = OSFS(xmlstore.data_dir / course_dir) - - MakoDescriptorSystem.__init__(self, load_item, resources_fs, - error_tracker, render_template, **kwargs) - XMLParsingSystem.__init__(self, load_item, resources_fs, - error_tracker, process_xml, policy, **kwargs) + super(ImportSystem, self).__init__( + load_item=load_item, + resources_fs=resources_fs, + render_template=render_template, + error_tracker=error_tracker, + process_xml=process_xml, + **kwargs + ) class ParentTracker(object): @@ -412,13 +418,14 @@ class XMLModuleStore(ModuleStoreBase): course_id = CourseDescriptor.make_id(org, course, url_name) system = ImportSystem( - self, - course_id, - course_dir, - policy, - tracker, - self.parent_trackers[course_id], - self.load_error_modules, + xmlstore=self, + course_id=course_id, + course_dir=course_dir, + error_tracker=tracker, + parent_tracker=self.parent_trackers[course_id], + load_error_modules=self.load_error_modules, + policy=policy, + mixins=self.xblock_mixins, ) course_descriptor = system.process_xml(etree.tostring(course_data, encoding='unicode')) @@ -467,9 +474,13 @@ class XMLModuleStore(ModuleStoreBase): # tabs are referenced in policy.json through a 'slug' which is just the filename without the .html suffix slug = os.path.splitext(os.path.basename(filepath))[0] loc = Location('i4x', course_descriptor.location.org, course_descriptor.location.course, category, slug) - module = HtmlDescriptor( - system, - {'data': html, 'location': loc, 'category': category} + module = system.construct_xblock_from_class( + HtmlDescriptor, + DictFieldData({'data': html, 'location': loc, 'category': category}), + # We're loading a descriptor, so student_id is meaningless + # We also don't have separate notions of definition and usage ids yet, + # so we use the location for both + ScopeIds(None, category, loc, loc), ) # VS[compat]: # Hack because we need to pull in the 'display_name' for static tabs (because we need to edit them) diff --git a/common/lib/xmodule/xmodule/modulestore/xml_importer.py b/common/lib/xmodule/xmodule/modulestore/xml_importer.py index beb255fb147..cd34d294a78 100644 --- a/common/lib/xmodule/xmodule/modulestore/xml_importer.py +++ b/common/lib/xmodule/xmodule/modulestore/xml_importer.py @@ -3,7 +3,7 @@ import os import mimetypes from path import path -from xblock.core import Scope +from xblock.fields import Scope from .xml import XMLModuleStore, ImportSystem, ParentTracker from xmodule.modulestore import Location @@ -91,7 +91,8 @@ def import_from_xml(store, data_dir, course_dirs=None, data_dir, default_class=default_class, course_dirs=course_dirs, - load_error_modules=load_error_modules + load_error_modules=load_error_modules, + xblock_mixins=store.xblock_mixins, ) # NOTE: the XmlModuleStore does not implement get_items() which would be a preferable means @@ -120,7 +121,7 @@ def import_from_xml(store, data_dir, course_dirs=None, # Quick scan to get course module as we need some info from there. Also we need to make sure that the # course module is committed first into the store for module in xml_module_store.modules[course_id].itervalues(): - if module.category == 'course': + if module.scope_ids.block_type == 'course': course_data_path = path(data_dir) / module.data_dir course_location = module.location @@ -129,9 +130,9 @@ def import_from_xml(store, data_dir, course_dirs=None, module = remap_namespace(module, target_location_namespace) if not do_import_static: - module.lms.static_asset_path = module.data_dir # for old-style xblock where this was actually linked to kvs - module._model_data['static_asset_path'] = module.data_dir - log.debug('course static_asset_path={0}'.format(module.lms.static_asset_path)) + module.static_asset_path = module.data_dir # for old-style xblock where this was actually linked to kvs + module.save() + log.debug('course static_asset_path={0}'.format(module.static_asset_path)) log.debug('course data_dir={0}'.format(module.data_dir)) @@ -177,7 +178,7 @@ def import_from_xml(store, data_dir, course_dirs=None, # finally loop through all the modules for module in xml_module_store.modules[course_id].itervalues(): - if module.category == 'course': + if module.scope_ids.block_type == 'course': # we've already saved the course module up at the top of the loop # so just skip over it in the inner loop continue @@ -195,9 +196,15 @@ def import_from_xml(store, data_dir, course_dirs=None, # now import any 'draft' items if draft_store is not None: - import_course_draft(xml_module_store, store, draft_store, course_data_path, - static_content_store, course_location, target_location_namespace if target_location_namespace - else course_location) + import_course_draft( + xml_module_store, + store, + draft_store, + course_data_path, + static_content_store, + course_location, + target_location_namespace if target_location_namespace else course_location + ) finally: # turn back on all write signalling @@ -217,13 +224,13 @@ def import_module(module, store, course_data_path, static_content_store, logging.debug('processing import of module {0}...'.format(module.location.url())) content = {} - for field in module.fields: + for field in module.fields.values(): if field.scope != Scope.content: continue try: - content[field.name] = module._model_data[field.name] + content[field.name] = module._field_data.get(module, field.name) except KeyError: - # Ignore any missing keys in _model_data + # Ignore any missing keys in _field_data pass module_data = {} @@ -274,13 +281,13 @@ def import_course_draft(xml_module_store, store, draft_store, course_data_path, # create a new 'System' object which will manage the importing errorlog = make_error_tracker() system = ImportSystem( - xml_module_store, - target_location_namespace.course_id, - draft_dir, - {}, - errorlog.tracker, - ParentTracker(), - None, + xmlstore=xml_module_store, + course_id=target_location_namespace.course_id, + course_dir=draft_dir, + policy={}, + error_tracker=errorlog.tracker, + parent_tracker=ParentTracker(), + load_error_modules=False, ) # now walk the /vertical directory where each file in there will be a draft copy of the Vertical @@ -368,15 +375,30 @@ def remap_namespace(module, target_location_namespace): # This looks a bit wonky as we need to also change the 'name' of the imported course to be what # the caller passed in if module.location.category != 'course': - module.location = module.location._replace(tag=target_location_namespace.tag, org=target_location_namespace.org, - course=target_location_namespace.course) + new_location = module.location._replace( + tag=target_location_namespace.tag, + org=target_location_namespace.org, + course=target_location_namespace.course + ) + module.scope_ids = module.scope_ids._replace( + def_id=new_location, + usage_id=new_location + ) else: original_location = module.location # # module is a course module # - module.location = module.location._replace(tag=target_location_namespace.tag, org=target_location_namespace.org, - course=target_location_namespace.course, name=target_location_namespace.name) + new_location = module.location._replace( + tag=target_location_namespace.tag, + org=target_location_namespace.org, + course=target_location_namespace.course, + name=target_location_namespace.name + ) + module.scope_ids = module.scope_ids._replace( + def_id=new_location, + usage_id=new_location + ) # # There is more re-namespacing work we have to do when importing course modules # @@ -401,8 +423,11 @@ def remap_namespace(module, target_location_namespace): new_locs = [] for child in children_locs: child_loc = Location(child) - new_child_loc = child_loc._replace(tag=target_location_namespace.tag, org=target_location_namespace.org, - course=target_location_namespace.course) + new_child_loc = child_loc._replace( + tag=target_location_namespace.tag, + org=target_location_namespace.org, + course=target_location_namespace.course + ) new_locs.append(new_child_loc.url()) @@ -501,10 +526,10 @@ def validate_course_policy(module_store, course_id): warn_cnt = 0 for module in module_store.modules[course_id].itervalues(): if module.location.category == 'course': - if not 'rerandomize' in module._model_data: + if not module._field_data.has(module, 'rerandomize'): warn_cnt += 1 print 'WARN: course policy does not specify value for "rerandomize" whose default is now "never". The behavior of your course may change.' - if not 'showanswer' in module._model_data: + if not module._field_data.has(module, 'showanswer'): warn_cnt += 1 print 'WARN: course policy does not specify value for "showanswer" whose default is now "finished". The behavior of your course may change.' return warn_cnt diff --git a/common/lib/xmodule/xmodule/peer_grading_module.py b/common/lib/xmodule/xmodule/peer_grading_module.py index ae4a8b87de4..3a4bbdb9f65 100644 --- a/common/lib/xmodule/xmodule/peer_grading_module.py +++ b/common/lib/xmodule/xmodule/peer_grading_module.py @@ -10,7 +10,7 @@ from .x_module import XModule from xmodule.raw_module import RawDescriptor from xmodule.modulestore.exceptions import ItemNotFoundError from .timeinfo import TimeInfo -from xblock.core import Dict, String, Scope, Boolean, Float +from xblock.fields import Dict, String, Scope, Boolean, Float from xmodule.fields import Date, Timedelta from xmodule.open_ended_grading_classes.peer_grading_service import PeerGradingService, GradingServiceError, MockPeerGradingService @@ -108,9 +108,9 @@ class PeerGradingModule(PeerGradingFields, XModule): log.error("Linked location {0} for peer grading module {1} does not exist".format( self.link_to_location, self.location)) raise - due_date = self.linked_problem.lms.due + due_date = self.linked_problem.due if due_date: - self.lms.due = due_date + self.due = due_date try: self.timeinfo = TimeInfo(self.due, self.graceperiod) @@ -532,8 +532,8 @@ class PeerGradingModule(PeerGradingFields, XModule): except Exception: continue if descriptor: - problem['due'] = descriptor.lms.due - grace_period = descriptor.lms.graceperiod + problem['due'] = descriptor.due + grace_period = descriptor.graceperiod try: problem_timeinfo = TimeInfo(problem['due'], grace_period) except Exception: diff --git a/common/lib/xmodule/xmodule/plugin.py b/common/lib/xmodule/xmodule/plugin.py deleted file mode 100644 index 5cf9c647aa1..00000000000 --- a/common/lib/xmodule/xmodule/plugin.py +++ /dev/null @@ -1,64 +0,0 @@ -import pkg_resources -import logging - -log = logging.getLogger(__name__) - -class PluginNotFoundError(Exception): - pass - - -class Plugin(object): - """ - Base class for a system that uses entry_points to load plugins. - - Implementing classes are expected to have the following attributes: - - entry_point: The name of the entry point to load plugins from - """ - - _plugin_cache = None - - @classmethod - def load_class(cls, identifier, default=None): - """ - Loads a single class instance specified by identifier. If identifier - specifies more than a single class, then logs a warning and returns the - first class identified. - - If default is not None, will return default if no entry_point matching - identifier is found. Otherwise, will raise a ModuleMissingError - """ - if cls._plugin_cache is None: - cls._plugin_cache = {} - - if identifier not in cls._plugin_cache: - identifier = identifier.lower() - classes = list(pkg_resources.iter_entry_points( - cls.entry_point, name=identifier)) - - if len(classes) > 1: - log.warning("Found multiple classes for {entry_point} with " - "identifier {id}: {classes}. " - "Returning the first one.".format( - entry_point=cls.entry_point, - id=identifier, - classes=", ".join( - class_.module_name for class_ in classes))) - - if len(classes) == 0: - if default is not None: - return default - raise PluginNotFoundError(identifier) - - cls._plugin_cache[identifier] = classes[0].load() - return cls._plugin_cache[identifier] - - @classmethod - def load_classes(cls): - """ - Returns a list of containing the identifiers and their corresponding classes for all - of the available instances of this plugin - """ - return [(class_.name, class_.load()) - for class_ - in pkg_resources.iter_entry_points(cls.entry_point)] diff --git a/common/lib/xmodule/xmodule/poll_module.py b/common/lib/xmodule/xmodule/poll_module.py index 8e7407752a0..86852cd698e 100644 --- a/common/lib/xmodule/xmodule/poll_module.py +++ b/common/lib/xmodule/xmodule/poll_module.py @@ -19,7 +19,7 @@ from xmodule.x_module import XModule from xmodule.stringify import stringify_children from xmodule.mako_module import MakoModuleDescriptor from xmodule.xml_module import XmlDescriptor -from xblock.core import Scope, String, Dict, Boolean, List +from xblock.fields import Scope, String, Dict, Boolean, List log = logging.getLogger(__name__) @@ -30,7 +30,7 @@ class PollFields(object): voted = Boolean(help="Whether this student has voted on the poll", scope=Scope.user_state, default=False) poll_answer = String(help="Student answer", scope=Scope.user_state, default='') - poll_answers = Dict(help="All possible answers for the poll fro other students", scope=Scope.content) + poll_answers = Dict(help="All possible answers for the poll fro other students", scope=Scope.user_state_summary) answers = List(help="Poll answers from xml", scope=Scope.content, default=[]) question = String(help="Poll question", scope=Scope.content, default='') diff --git a/common/lib/xmodule/xmodule/randomize_module.py b/common/lib/xmodule/xmodule/randomize_module.py index 04b9a6e2154..38535d38bb2 100644 --- a/common/lib/xmodule/xmodule/randomize_module.py +++ b/common/lib/xmodule/xmodule/randomize_module.py @@ -6,7 +6,7 @@ from xmodule.seq_module import SequenceDescriptor from lxml import etree -from xblock.core import Scope, Integer +from xblock.fields import Scope, Integer log = logging.getLogger('mitx.' + __name__) diff --git a/common/lib/xmodule/xmodule/raw_module.py b/common/lib/xmodule/xmodule/raw_module.py index 4c6ddb5b512..42c20344b68 100644 --- a/common/lib/xmodule/xmodule/raw_module.py +++ b/common/lib/xmodule/xmodule/raw_module.py @@ -3,7 +3,7 @@ from xmodule.editing_module import XMLEditingDescriptor from xmodule.xml_module import XmlDescriptor import logging import sys -from xblock.core import String, Scope +from xblock.fields import String, Scope from exceptions import SerializationError log = logging.getLogger(__name__) diff --git a/common/lib/xmodule/xmodule/seq_module.py b/common/lib/xmodule/xmodule/seq_module.py index 580475e1ae7..ebd49d75cf7 100644 --- a/common/lib/xmodule/xmodule/seq_module.py +++ b/common/lib/xmodule/xmodule/seq_module.py @@ -8,7 +8,7 @@ from xmodule.xml_module import XmlDescriptor from xmodule.x_module import XModule from xmodule.progress import Progress from xmodule.exceptions import NotFoundError -from xblock.core import Integer, Scope +from xblock.fields import Integer, Scope from pkg_resources import resource_string log = logging.getLogger(__name__) @@ -40,8 +40,8 @@ class SequenceModule(SequenceFields, XModule): XModule.__init__(self, *args, **kwargs) # if position is specified in system, then use that instead - if self.system.get('position'): - self.position = int(self.system.get('position')) + if getattr(self.system, 'position', None) is not None: + self.position = int(self.system.position) self.rendered = False diff --git a/common/lib/xmodule/xmodule/tests/__init__.py b/common/lib/xmodule/xmodule/tests/__init__.py index df197edb870..ad5fa9c3cba 100644 --- a/common/lib/xmodule/xmodule/tests/__init__.py +++ b/common/lib/xmodule/xmodule/tests/__init__.py @@ -18,6 +18,7 @@ from mock import Mock from path import path import calc +from xblock.field_data import DictFieldData from xmodule.x_module import ModuleSystem, XModuleDescriptor @@ -61,7 +62,7 @@ def get_test_system(): debug=True, xqueue={'interface': None, 'callback_url': '/', 'default_queuename': 'testqueue', 'waittime': 10, 'construct_callback' : Mock(side_effect="/")}, node_path=os.environ.get("NODE_PATH", "/usr/local/lib/node_modules"), - xblock_model_data=lambda descriptor: descriptor._model_data, + xblock_field_data=lambda descriptor: descriptor._field_data, anonymous_student_id='student', open_ended_grading_interface= open_ended_grading_interface ) @@ -89,7 +90,7 @@ class PostData(object): class LogicTest(unittest.TestCase): """Base class for testing xmodule logic.""" descriptor_class = None - raw_model_data = {} + raw_field_data = {} def setUp(self): class EmptyClass: @@ -102,7 +103,8 @@ class LogicTest(unittest.TestCase): self.xmodule_class = self.descriptor_class.module_class self.xmodule = self.xmodule_class( - self.system, self.descriptor, self.raw_model_data) + self.descriptor, self.system, DictFieldData(self.raw_field_data), Mock() + ) def ajax_request(self, dispatch, data): """Call Xmodule.handle_ajax.""" diff --git a/common/lib/xmodule/xmodule/tests/test_annotatable_module.py b/common/lib/xmodule/xmodule/tests/test_annotatable_module.py index 6993841ab27..2b454ff45b6 100644 --- a/common/lib/xmodule/xmodule/tests/test_annotatable_module.py +++ b/common/lib/xmodule/xmodule/tests/test_annotatable_module.py @@ -5,6 +5,8 @@ import unittest from lxml import etree from mock import Mock +from xblock.field_data import DictFieldData +from xblock.fields import ScopeIds from xmodule.annotatable_module import AnnotatableModule from xmodule.modulestore import Location @@ -29,10 +31,15 @@ class AnnotatableModuleTestCase(unittest.TestCase): </annotatable> ''' descriptor = Mock() - module_data = {'data': sample_xml, 'location': location} + field_data = DictFieldData({'data': sample_xml}) def setUp(self): - self.annotatable = AnnotatableModule(get_test_system(), self.descriptor, self.module_data) + self.annotatable = AnnotatableModule( + self.descriptor, + get_test_system(), + self.field_data, + ScopeIds(None, None, None, None) + ) def test_annotation_data_attr(self): el = etree.fromstring('<annotation title="bar" body="foo" problem="0">test</annotation>') diff --git a/common/lib/xmodule/xmodule/tests/test_capa_module.py b/common/lib/xmodule/xmodule/tests/test_capa_module.py index 92d30fac8ca..c47cfc2ca85 100644 --- a/common/lib/xmodule/xmodule/tests/test_capa_module.py +++ b/common/lib/xmodule/xmodule/tests/test_capa_module.py @@ -18,6 +18,8 @@ from capa.responsetypes import (StudentInputError, LoncapaProblemError, ResponseError) from xmodule.capa_module import CapaModule, ComplexEncoder from xmodule.modulestore import Location +from xblock.field_data import DictFieldData +from xblock.fields import ScopeIds from django.http import QueryDict @@ -95,34 +97,39 @@ class CapaFactory(object): """ location = Location(["i4x", "edX", "capa_test", "problem", "SampleProblem{0}".format(CapaFactory.next_num())]) - model_data = {'data': CapaFactory.sample_problem_xml, 'location': location} + field_data = {'data': CapaFactory.sample_problem_xml} if graceperiod is not None: - model_data['graceperiod'] = graceperiod + field_data['graceperiod'] = graceperiod if due is not None: - model_data['due'] = due + field_data['due'] = due if max_attempts is not None: - model_data['max_attempts'] = max_attempts + field_data['max_attempts'] = max_attempts if showanswer is not None: - model_data['showanswer'] = showanswer + field_data['showanswer'] = showanswer if force_save_button is not None: - model_data['force_save_button'] = force_save_button + field_data['force_save_button'] = force_save_button if rerandomize is not None: - model_data['rerandomize'] = rerandomize + field_data['rerandomize'] = rerandomize if done is not None: - model_data['done'] = done + field_data['done'] = done descriptor = Mock(weight="1") if problem_state is not None: - model_data.update(problem_state) + field_data.update(problem_state) if attempts is not None: # converting to int here because I keep putting "0" and "1" in the tests # since everything else is a string. - model_data['attempts'] = int(attempts) + field_data['attempts'] = int(attempts) system = get_test_system() system.render_template = Mock(return_value="<div>Test Template HTML</div>") - module = CapaModule(system, descriptor, model_data) + module = CapaModule( + descriptor, + system, + DictFieldData(field_data), + ScopeIds(None, None, location, location), + ) if correct: # TODO: probably better to actually set the internal state properly, but... diff --git a/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py b/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py index 4eadca110c9..1b3fd36f458 100644 --- a/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py +++ b/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py @@ -28,6 +28,9 @@ from xmodule.tests.test_util_open_ended import ( MOCK_INSTANCE_STATE, TEST_STATE_SA, TEST_STATE_AI, TEST_STATE_AI2, TEST_STATE_AI2_INVALID, TEST_STATE_SINGLE, TEST_STATE_PE_SINGLE ) + +from xblock.field_data import DictFieldData +from xblock.fields import ScopeIds import capa.xqueue_interface as xqueue_interface @@ -418,13 +421,13 @@ class CombinedOpenEndedModuleTest(unittest.TestCase): test_system = get_test_system() test_system.open_ended_grading_interface = None combinedoe_container = CombinedOpenEndedModule( - test_system, - descriptor, - model_data={ + descriptor=descriptor, + runtime=test_system, + field_data=DictFieldData({ 'data': full_definition, 'weight': '1', - 'location': location - } + }), + scope_ids=ScopeIds(None, None, None, None), ) def setUp(self): diff --git a/common/lib/xmodule/xmodule/tests/test_conditional.py b/common/lib/xmodule/xmodule/tests/test_conditional.py index 95472a62ede..040af7a2303 100644 --- a/common/lib/xmodule/xmodule/tests/test_conditional.py +++ b/common/lib/xmodule/xmodule/tests/test_conditional.py @@ -6,6 +6,8 @@ import unittest from fs.memoryfs import MemoryFS from mock import Mock, patch +from xblock.field_data import DictFieldData +from xblock.fields import ScopeIds from xmodule.error_module import NonStaffErrorDescriptor from xmodule.modulestore import Location from xmodule.modulestore.xml import ImportSystem, XMLModuleStore @@ -87,8 +89,13 @@ class ConditionalFactory(object): # construct conditional module: cond_location = Location(["i4x", "edX", "conditional_test", "conditional", "SampleConditional"]) - model_data = {'data': '<conditional/>', 'location': cond_location} - cond_module = ConditionalModule(system, cond_descriptor, model_data) + field_data = DictFieldData({'data': '<conditional/>', 'location': cond_location}) + cond_module = ConditionalModule( + cond_descriptor, + system, + field_data, + ScopeIds(None, None, cond_location, cond_location) + ) # return dict: return {'cond_module': cond_module, diff --git a/common/lib/xmodule/xmodule/tests/test_course_module.py b/common/lib/xmodule/xmodule/tests/test_course_module.py index 48690a8b257..bf83fbd17d8 100644 --- a/common/lib/xmodule/xmodule/tests/test_course_module.py +++ b/common/lib/xmodule/xmodule/tests/test_course_module.py @@ -29,12 +29,12 @@ class DummySystem(ImportSystem): parent_tracker = Mock() super(DummySystem, self).__init__( - xmlstore, - course_id, - course_dir, - policy, - error_tracker, - parent_tracker, + xmlstore=xmlstore, + course_id=course_id, + course_dir=course_dir, + policy=policy, + error_tracker=error_tracker, + parent_tracker=parent_tracker, load_error_modules=load_error_modules, ) diff --git a/common/lib/xmodule/xmodule/tests/test_crowdsource_hinter.py b/common/lib/xmodule/xmodule/tests/test_crowdsource_hinter.py index 8347b71076e..0369fffe8e6 100644 --- a/common/lib/xmodule/xmodule/tests/test_crowdsource_hinter.py +++ b/common/lib/xmodule/xmodule/tests/test_crowdsource_hinter.py @@ -8,6 +8,7 @@ import copy from xmodule.crowdsource_hinter import CrowdsourceHinterModule from xmodule.vertical_module import VerticalModule, VerticalDescriptor +from xblock.field_data import DictFieldData from . import get_test_system @@ -60,12 +61,12 @@ class CHModuleFactory(object): """ A factory method for making CHM's """ - model_data = {'data': CHModuleFactory.sample_problem_xml} + field_data = {'data': CHModuleFactory.sample_problem_xml} if hints is not None: - model_data['hints'] = hints + field_data['hints'] = hints else: - model_data['hints'] = { + field_data['hints'] = { '24.0': {'0': ['Best hint', 40], '3': ['Another hint', 30], '4': ['A third hint', 20], @@ -74,31 +75,31 @@ class CHModuleFactory(object): } if mod_queue is not None: - model_data['mod_queue'] = mod_queue + field_data['mod_queue'] = mod_queue else: - model_data['mod_queue'] = { + field_data['mod_queue'] = { '24.0': {'2': ['A non-approved hint']}, '26.0': {'5': ['Another non-approved hint']} } if previous_answers is not None: - model_data['previous_answers'] = previous_answers + field_data['previous_answers'] = previous_answers else: - model_data['previous_answers'] = [ + field_data['previous_answers'] = [ ['24.0', [0, 3, 4]], ['29.0', []] ] if user_submissions is not None: - model_data['user_submissions'] = user_submissions + field_data['user_submissions'] = user_submissions else: - model_data['user_submissions'] = ['24.0', '29.0'] + field_data['user_submissions'] = ['24.0', '29.0'] if user_voted is not None: - model_data['user_voted'] = user_voted + field_data['user_voted'] = user_voted if moderate is not None: - model_data['moderate'] = moderate + field_data['moderate'] = moderate descriptor = Mock(weight='1') # Make the descriptor have a capa problem child. @@ -138,8 +139,7 @@ class CHModuleFactory(object): if descriptor.name == 'capa': return capa_module system.get_module = fake_get_module - - module = CrowdsourceHinterModule(system, descriptor, model_data) + module = CrowdsourceHinterModule(descriptor, system, DictFieldData(field_data), Mock()) return module @@ -196,10 +196,10 @@ class VerticalWithModulesFactory(object): @staticmethod def create(): """Make a vertical.""" - model_data = {'data': VerticalWithModulesFactory.sample_problem_xml} + field_data = {'data': VerticalWithModulesFactory.sample_problem_xml} system = get_test_system() descriptor = VerticalDescriptor.from_xml(VerticalWithModulesFactory.sample_problem_xml, system) - module = VerticalModule(system, descriptor, model_data) + module = VerticalModule(system, descriptor, field_data) return module diff --git a/common/lib/xmodule/xmodule/tests/test_editing_module.py b/common/lib/xmodule/xmodule/tests/test_editing_module.py index 838a4f9adaa..515e367ee0b 100644 --- a/common/lib/xmodule/xmodule/tests/test_editing_module.py +++ b/common/lib/xmodule/xmodule/tests/test_editing_module.py @@ -6,6 +6,8 @@ import logging from mock import Mock from pkg_resources import resource_string from xmodule.editing_module import TabsEditingDescriptor +from xblock.field_data import DictFieldData +from xblock.fields import ScopeIds from .import get_test_system @@ -44,7 +46,9 @@ class TabsEditingDescriptorTestCase(unittest.TestCase): TabsEditingDescriptor.tabs = self.tabs self.descriptor = TabsEditingDescriptor( runtime=system, - model_data={}) + field_data=DictFieldData({}), + scope_ids=ScopeIds(None, None, None, None), + ) def test_get_css(self): """test get_css""" diff --git a/common/lib/xmodule/xmodule/tests/test_error_module.py b/common/lib/xmodule/xmodule/tests/test_error_module.py index f91bf7cb372..186754e16ce 100644 --- a/common/lib/xmodule/xmodule/tests/test_error_module.py +++ b/common/lib/xmodule/xmodule/tests/test_error_module.py @@ -39,7 +39,7 @@ class TestErrorModule(unittest.TestCase, SetupTestErrorModules): descriptor = MagicMock([XModuleDescriptor], system=self.system, location=self.location, - _model_data=self.valid_xml) + _field_data=self.valid_xml) error_descriptor = error_module.ErrorDescriptor.from_descriptor( descriptor, self.error_msg) @@ -74,7 +74,7 @@ class TestNonStaffErrorModule(unittest.TestCase, SetupTestErrorModules): descriptor = MagicMock([XModuleDescriptor], system=self.system, location=self.location, - _model_data=self.valid_xml) + _field_data=self.valid_xml) error_descriptor = error_module.NonStaffErrorDescriptor.from_descriptor( descriptor, self.error_msg) diff --git a/common/lib/xmodule/xmodule/tests/test_export.py b/common/lib/xmodule/xmodule/tests/test_export.py index b0c1617580e..1ee059f9feb 100644 --- a/common/lib/xmodule/xmodule/tests/test_export.py +++ b/common/lib/xmodule/xmodule/tests/test_export.py @@ -24,7 +24,8 @@ def strip_filenames(descriptor): Recursively strips 'filename' from all children's definitions. """ print("strip filename from {desc}".format(desc=descriptor.location.url())) - descriptor._model_data.pop('filename', None) + if descriptor._field_data.has(descriptor, 'filename'): + descriptor._field_data.delete(descriptor, 'filename') if hasattr(descriptor, 'xml_attributes'): if 'filename' in descriptor.xml_attributes: diff --git a/common/lib/xmodule/xmodule/tests/test_html_module.py b/common/lib/xmodule/xmodule/tests/test_html_module.py index 4fe02423787..2f2e9114c6f 100644 --- a/common/lib/xmodule/xmodule/tests/test_html_module.py +++ b/common/lib/xmodule/xmodule/tests/test_html_module.py @@ -2,6 +2,7 @@ import unittest from mock import Mock +from xblock.field_data import DictFieldData from xmodule.html_module import HtmlModule from . import get_test_system @@ -11,9 +12,9 @@ class HtmlModuleSubstitutionTestCase(unittest.TestCase): def test_substitution_works(self): sample_xml = '''%%USER_ID%%''' - module_data = {'data': sample_xml} + field_data = DictFieldData({'data': sample_xml}) module_system = get_test_system() - module = HtmlModule(module_system, self.descriptor, module_data) + module = HtmlModule(self.descriptor, module_system, field_data, Mock()) self.assertEqual(module.get_html(), str(module_system.anonymous_student_id)) @@ -23,16 +24,17 @@ class HtmlModuleSubstitutionTestCase(unittest.TestCase): <p>Hi USER_ID!11!</p> </html> ''' - module_data = {'data': sample_xml} - module = HtmlModule(get_test_system(), self.descriptor, module_data) + field_data = DictFieldData({'data': sample_xml}) + module_system = get_test_system() + module = HtmlModule(self.descriptor, module_system, field_data, Mock()) self.assertEqual(module.get_html(), sample_xml) def test_substitution_without_anonymous_student_id(self): sample_xml = '''%%USER_ID%%''' - module_data = {'data': sample_xml} + field_data = DictFieldData({'data': sample_xml}) module_system = get_test_system() module_system.anonymous_student_id = None - module = HtmlModule(module_system, self.descriptor, module_data) + module = HtmlModule(self.descriptor, module_system, field_data, Mock()) self.assertEqual(module.get_html(), sample_xml) diff --git a/common/lib/xmodule/xmodule/tests/test_import.py b/common/lib/xmodule/xmodule/tests/test_import.py index b6758dc9179..6ab5e29ad10 100644 --- a/common/lib/xmodule/xmodule/tests/test_import.py +++ b/common/lib/xmodule/xmodule/tests/test_import.py @@ -15,6 +15,7 @@ from xmodule.modulestore.xml import ImportSystem, XMLModuleStore from xmodule.modulestore.inheritance import compute_inherited_metadata from xmodule.fields import Date from xmodule.tests import DATA_DIR +from xmodule.modulestore.inheritance import InheritanceMixin ORG = 'test_org' @@ -34,13 +35,14 @@ class DummySystem(ImportSystem): parent_tracker = Mock() super(DummySystem, self).__init__( - xmlstore, - course_id, - course_dir, - policy, - error_tracker, - parent_tracker, + xmlstore=xmlstore, + course_id=course_id, + course_dir=course_dir, + policy=policy, + error_tracker=error_tracker, + parent_tracker=parent_tracker, load_error_modules=load_error_modules, + mixins=(InheritanceMixin,) ) def render_template(self, _template, _context): @@ -58,7 +60,7 @@ class BaseCourseTestCase(unittest.TestCase): """Get a test course by directory name. If there's more than one, error.""" print("Importing {0}".format(name)) - modulestore = XMLModuleStore(DATA_DIR, course_dirs=[name]) + modulestore = XMLModuleStore(DATA_DIR, course_dirs=[name], xblock_mixins=(InheritanceMixin,)) courses = modulestore.get_courses() self.assertEquals(len(courses), 1) return courses[0] @@ -76,7 +78,7 @@ class ImportTestCase(BaseCourseTestCase): descriptor = system.process_xml(bad_xml) - self.assertEqual(descriptor.__class__.__name__, 'ErrorDescriptor') + self.assertEqual(descriptor.__class__.__name__, 'ErrorDescriptorWithMixins') def test_unique_url_names(self): '''Check that each error gets its very own url_name''' @@ -102,7 +104,7 @@ class ImportTestCase(BaseCourseTestCase): re_import_descriptor = system.process_xml(tag_xml) self.assertEqual(re_import_descriptor.__class__.__name__, - 'ErrorDescriptor') + 'ErrorDescriptorWithMixins') self.assertEqual(descriptor.contents, re_import_descriptor.contents) @@ -150,15 +152,18 @@ class ImportTestCase(BaseCourseTestCase): compute_inherited_metadata(descriptor) # pylint: disable=W0212 - print(descriptor, descriptor._model_data) - self.assertEqual(descriptor.lms.due, ImportTestCase.date.from_json(v)) + print(descriptor, descriptor._field_data) + self.assertEqual(descriptor.due, ImportTestCase.date.from_json(v)) # Check that the child inherits due correctly child = descriptor.get_children()[0] - self.assertEqual(child.lms.due, ImportTestCase.date.from_json(v)) + self.assertEqual(child.due, ImportTestCase.date.from_json(v)) self.assertEqual(child._inheritable_metadata, child._inherited_metadata) self.assertEqual(1, len(child._inherited_metadata)) - self.assertEqual(v, child._inherited_metadata['due']) + self.assertEqual( + datetime.datetime(2013, 3, 20, 17, 0, tzinfo=UTC()), + child._inherited_metadata['due'] + ) # Now export and check things resource_fs = MemoryFS() @@ -208,15 +213,15 @@ class ImportTestCase(BaseCourseTestCase): descriptor = system.process_xml(start_xml) compute_inherited_metadata(descriptor) - self.assertEqual(descriptor.lms.due, None) + self.assertEqual(descriptor.due, None) # Check that the child does not inherit a value for due child = descriptor.get_children()[0] - self.assertEqual(child.lms.due, None) + self.assertEqual(child.due, None) # pylint: disable=W0212 self.assertEqual(child._inheritable_metadata, child._inherited_metadata) self.assertLessEqual( - child.lms.start, + child.start, datetime.datetime.now(UTC()) ) @@ -238,14 +243,17 @@ class ImportTestCase(BaseCourseTestCase): descriptor = system.process_xml(start_xml) child = descriptor.get_children()[0] # pylint: disable=W0212 - child._model_data['due'] = child_due + child._field_data.set(child, 'due', child_due) compute_inherited_metadata(descriptor) - self.assertEqual(descriptor.lms.due, ImportTestCase.date.from_json(course_due)) - self.assertEqual(child.lms.due, ImportTestCase.date.from_json(child_due)) + self.assertEqual(descriptor.due, ImportTestCase.date.from_json(course_due)) + self.assertEqual(child.due, ImportTestCase.date.from_json(child_due)) # Test inherited metadata. Due does not appear here (because explicitly set on child). self.assertEqual(1, len(child._inheritable_metadata)) - self.assertEqual(course_due, child._inheritable_metadata['due']) + self.assertEqual( + datetime.datetime(2013, 3, 20, 17, 0, tzinfo=UTC()), + child._inheritable_metadata['due'] + ) def test_is_pointer_tag(self): """ @@ -283,7 +291,7 @@ class ImportTestCase(BaseCourseTestCase): def check_for_key(key, node): "recursive check for presence of key" print("Checking {0}".format(node.location.url())) - self.assertTrue(key in node._model_data) + self.assertTrue(node._field_data.has(node, key)) for c in node.get_children(): check_for_key(key, c) @@ -310,7 +318,7 @@ class ImportTestCase(BaseCourseTestCase): # Also check that keys from policy are run through the # appropriate attribute maps -- 'graded' should be True, not 'true' - self.assertEqual(toy.lms.graded, True) + self.assertEqual(toy.graded, True) def test_definition_loading(self): """When two courses share the same org and course name and diff --git a/common/lib/xmodule/xmodule/tests/test_poll.py b/common/lib/xmodule/xmodule/tests/test_poll.py index bbd0c3bb12a..ea9fba79486 100644 --- a/common/lib/xmodule/xmodule/tests/test_poll.py +++ b/common/lib/xmodule/xmodule/tests/test_poll.py @@ -7,7 +7,7 @@ from . import LogicTest class PollModuleTest(LogicTest): """Logic tests for Poll Xmodule.""" descriptor_class = PollDescriptor - raw_model_data = { + raw_field_data = { 'poll_answers': {'Yes': 1, 'Dont_know': 0, 'No': 0}, 'voted': False, 'poll_answer': '' diff --git a/common/lib/xmodule/xmodule/tests/test_progress.py b/common/lib/xmodule/xmodule/tests/test_progress.py index e7af7a0c098..053768a0076 100644 --- a/common/lib/xmodule/xmodule/tests/test_progress.py +++ b/common/lib/xmodule/xmodule/tests/test_progress.py @@ -1,6 +1,9 @@ """Module progress tests""" import unittest +from mock import Mock + +from xblock.field_data import DictFieldData from xmodule.progress import Progress from xmodule import x_module @@ -134,6 +137,6 @@ class ModuleProgressTest(unittest.TestCase): ''' def test_xmodule_default(self): '''Make sure default get_progress exists, returns None''' - xm = x_module.XModule(get_test_system(), None, {'location': 'a://b/c/d/e'}) + xm = x_module.XModule(None, get_test_system(), DictFieldData({'location': 'a://b/c/d/e'}), Mock()) p = xm.get_progress() self.assertEqual(p, None) diff --git a/common/lib/xmodule/xmodule/tests/test_video.py b/common/lib/xmodule/xmodule/tests/test_video.py index a6a7d86510b..1001ae81809 100644 --- a/common/lib/xmodule/xmodule/tests/test_video.py +++ b/common/lib/xmodule/xmodule/tests/test_video.py @@ -14,12 +14,16 @@ the course, section, subsection, unit, etc. """ import unittest +from mock import Mock + from . import LogicTest from lxml import etree from .import get_test_system from xmodule.modulestore import Location from xmodule.video_module import VideoDescriptor, _create_youtube_string from .test_import import DummySystem +from xblock.field_data import DictFieldData +from xblock.fields import ScopeIds from textwrap import dedent @@ -28,7 +32,7 @@ class VideoModuleTest(LogicTest): """Logic tests for Video Xmodule.""" descriptor_class = VideoDescriptor - raw_model_data = { + raw_field_data = { 'data': '<video />' } @@ -123,7 +127,9 @@ class VideoDescriptorTest(unittest.TestCase): system = get_test_system() self.descriptor = VideoDescriptor( runtime=system, - model_data={}) + field_data=DictFieldData({}), + scope_ids=ScopeIds(None, None, None, None), + ) def test_get_context(self): """"test get_context""" @@ -144,8 +150,8 @@ class VideoDescriptorTest(unittest.TestCase): """ system = DummySystem(load_error_modules=True) location = Location(["i4x", "edX", "video", "default", "SampleProblem1"]) - model_data = {'location': location} - descriptor = VideoDescriptor(system, model_data) + field_data = DictFieldData({'location': location}) + descriptor = VideoDescriptor(system, field_data, Mock()) descriptor.youtube_id_0_75 = 'izygArpw-Qo' descriptor.youtube_id_1_0 = 'p2Q6BrNhdh8' descriptor.youtube_id_1_25 = '1EeWXzPdhSA' @@ -160,8 +166,8 @@ class VideoDescriptorTest(unittest.TestCase): """ system = DummySystem(load_error_modules=True) location = Location(["i4x", "edX", "video", "default", "SampleProblem1"]) - model_data = {'location': location} - descriptor = VideoDescriptor(system, model_data) + field_data = DictFieldData({'location': location}) + descriptor = VideoDescriptor(system, field_data, Mock()) descriptor.youtube_id_0_75 = 'izygArpw-Qo' descriptor.youtube_id_1_0 = 'p2Q6BrNhdh8' descriptor.youtube_id_1_25 = '1EeWXzPdhSA' @@ -196,10 +202,12 @@ class VideoDescriptorImportTestCase(unittest.TestCase): ''' location = Location(["i4x", "edX", "video", "default", "SampleProblem1"]) - model_data = {'data': sample_xml, - 'location': location} + field_data = DictFieldData({ + 'data': sample_xml, + 'location': location + }) system = DummySystem(load_error_modules=True) - descriptor = VideoDescriptor(system, model_data) + descriptor = VideoDescriptor(system, field_data, Mock()) self.assert_attributes_equal(descriptor, { 'youtube_id_0_75': 'izygArpw-Qo', 'youtube_id_1_0': 'p2Q6BrNhdh8', @@ -415,7 +423,7 @@ class VideoExportTestCase(unittest.TestCase): """Test that we write the correct XML on export.""" module_system = DummySystem(load_error_modules=True) location = Location(["i4x", "edX", "video", "default", "SampleProblem1"]) - desc = VideoDescriptor(module_system, {'location': location}) + desc = VideoDescriptor(module_system, DictFieldData({}), ScopeIds(None, None, location, location)) desc.youtube_id_0_75 = 'izygArpw-Qo' desc.youtube_id_1_0 = 'p2Q6BrNhdh8' @@ -442,7 +450,7 @@ class VideoExportTestCase(unittest.TestCase): """Test XML export with defaults.""" module_system = DummySystem(load_error_modules=True) location = Location(["i4x", "edX", "video", "default", "SampleProblem1"]) - desc = VideoDescriptor(module_system, {'location': location}) + desc = VideoDescriptor(module_system, DictFieldData({}), ScopeIds(None, None, location, location)) xml = desc.definition_to_xml(None) expected = '<video url_name="SampleProblem1"/>\n' diff --git a/common/lib/xmodule/xmodule/tests/test_word_cloud.py b/common/lib/xmodule/xmodule/tests/test_word_cloud.py index c0e00941c8b..0db5caceeaa 100644 --- a/common/lib/xmodule/xmodule/tests/test_word_cloud.py +++ b/common/lib/xmodule/xmodule/tests/test_word_cloud.py @@ -8,7 +8,7 @@ from . import PostData, LogicTest class WordCloudModuleTest(LogicTest): """Logic tests for Word Cloud Xmodule.""" descriptor_class = WordCloudDescriptor - raw_model_data = { + raw_field_data = { 'all_words': {'cat': 10, 'dog': 5, 'mom': 1, 'dad': 2}, 'top_words': {'cat': 10, 'dog': 5, 'dad': 2}, 'submitted': False diff --git a/common/lib/xmodule/xmodule/tests/test_xblock_wrappers.py b/common/lib/xmodule/xmodule/tests/test_xblock_wrappers.py index 0a97728ec7b..1333cf8cb43 100644 --- a/common/lib/xmodule/xmodule/tests/test_xblock_wrappers.py +++ b/common/lib/xmodule/xmodule/tests/test_xblock_wrappers.py @@ -7,6 +7,11 @@ from nose.tools import assert_equal # pylint: disable=E0611 from unittest.case import SkipTest from mock import Mock +from xblock.field_data import DictFieldData +from xblock.fields import ScopeIds + +from xmodule.x_module import ModuleSystem +from xmodule.mako_module import MakoDescriptorSystem from xmodule.annotatable_module import AnnotatableDescriptor from xmodule.capa_module import CapaDescriptor from xmodule.course_module import CourseDescriptor @@ -63,26 +68,34 @@ class TestXBlockWrapper(object): @property def leaf_module_runtime(self): - runtime = Mock() - runtime.render_template = lambda *args, **kwargs: u'{!r}, {!r}'.format(args, kwargs) - runtime.anonymous_student_id = 'dummy_anonymous_student_id' - runtime.open_ended_grading_interface = {} - runtime.seed = 5 - runtime.get = lambda x: getattr(runtime, x) - runtime.ajax_url = 'dummy_ajax_url' - runtime.xblock_model_data = lambda d: d._model_data + runtime = ModuleSystem( + render_template=lambda *args, **kwargs: u'{!r}, {!r}'.format(args, kwargs), + anonymous_student_id='dummy_anonymous_student_id', + open_ended_grading_interface={}, + ajax_url='dummy_ajax_url', + xblock_field_data=lambda d: d._field_data, + get_module=Mock(), + replace_urls=Mock(), + track_function=Mock(), + ) return runtime @property def leaf_descriptor_runtime(self): - runtime = Mock() - runtime.render_template = lambda *args, **kwargs: u'{!r}, {!r}'.format(args, kwargs) + runtime = MakoDescriptorSystem( + load_item=Mock(), + resources_fs=Mock(), + error_tracker=Mock(), + render_template=(lambda *args, **kwargs: u'{!r}, {!r}'.format(args, kwargs)), + ) return runtime def leaf_descriptor(self, descriptor_cls): + location = 'i4x://org/course/category/name' return descriptor_cls( self.leaf_descriptor_runtime, - {'location': 'i4x://org/course/category/name'} + DictFieldData({}), + ScopeIds(None, descriptor_cls.__name__, location, location) ) def leaf_module(self, descriptor_cls): @@ -104,12 +117,13 @@ class TestXBlockWrapper(object): return runtime def container_descriptor(self, descriptor_cls): + location = 'i4x://org/course/category/name' return descriptor_cls( self.container_descriptor_runtime, - { - 'location': 'i4x://org/course/category/name', + DictFieldData({ 'children': range(3) - } + }), + ScopeIds(None, descriptor_cls.__name__, location, location) ) def container_module(self, descriptor_cls, depth): diff --git a/common/lib/xmodule/xmodule/tests/test_xml_module.py b/common/lib/xmodule/xmodule/tests/test_xml_module.py index 34631405556..a7bce08fd82 100644 --- a/common/lib/xmodule/xmodule/tests/test_xml_module.py +++ b/common/lib/xmodule/xmodule/tests/test_xml_module.py @@ -2,7 +2,8 @@ #pylint: disable=C0111 from xmodule.x_module import XModuleFields -from xblock.core import Scope, String, Dict, Boolean, Integer, Float, Any, List +from xblock.fields import Scope, String, Dict, Boolean, Integer, Float, Any, List +from xblock.field_data import DictFieldData from xmodule.fields import Date, Timedelta from xmodule.xml_module import XmlDescriptor, serialize_field, deserialize_field import unittest @@ -44,7 +45,7 @@ class TestFields(object): class EditableMetadataFieldsTest(unittest.TestCase): def test_display_name_field(self): - editable_fields = self.get_xml_editable_fields({}) + editable_fields = self.get_xml_editable_fields(DictFieldData({})) # Tests that the xblock fields (currently tags and name) get filtered out. # Also tests that xml_attributes is filtered out of XmlDescriptor. self.assertEqual(1, len(editable_fields), "Expected only 1 editable field for xml descriptor.") @@ -55,14 +56,14 @@ class EditableMetadataFieldsTest(unittest.TestCase): def test_override_default(self): # Tests that explicitly_set is correct when a value overrides the default (not inheritable). - editable_fields = self.get_xml_editable_fields({'display_name': 'foo'}) + editable_fields = self.get_xml_editable_fields(DictFieldData({'display_name': 'foo'})) self.assert_field_values( editable_fields, 'display_name', XModuleFields.display_name, explicitly_set=True, inheritable=False, value='foo', default_value=None ) def test_integer_field(self): - descriptor = self.get_descriptor({'max_attempts': '7'}) + descriptor = self.get_descriptor(DictFieldData({'max_attempts': '7'})) editable_fields = descriptor.editable_metadata_fields self.assertEqual(7, len(editable_fields)) self.assert_field_values( @@ -75,7 +76,7 @@ class EditableMetadataFieldsTest(unittest.TestCase): explicitly_set=False, inheritable=False, value='local default', default_value='local default' ) - editable_fields = self.get_descriptor({}).editable_metadata_fields + editable_fields = self.get_descriptor(DictFieldData({})).editable_metadata_fields self.assert_field_values( editable_fields, 'max_attempts', TestFields.max_attempts, explicitly_set=False, inheritable=False, value=1000, default_value=1000, type='Integer', @@ -84,7 +85,7 @@ class EditableMetadataFieldsTest(unittest.TestCase): def test_inherited_field(self): model_val = {'display_name': 'inherited'} - descriptor = self.get_descriptor(model_val) + descriptor = self.get_descriptor(DictFieldData(model_val)) # Mimic an inherited value for display_name (inherited and inheritable are the same in this case). descriptor._inherited_metadata = model_val descriptor._inheritable_metadata = model_val @@ -94,7 +95,7 @@ class EditableMetadataFieldsTest(unittest.TestCase): explicitly_set=False, inheritable=True, value='inherited', default_value='inherited' ) - descriptor = self.get_descriptor({'display_name': 'explicit'}) + descriptor = self.get_descriptor(DictFieldData({'display_name': 'explicit'})) # Mimic the case where display_name WOULD have been inherited, except we explicitly set it. descriptor._inheritable_metadata = {'display_name': 'inheritable value'} descriptor._inherited_metadata = {} @@ -108,7 +109,7 @@ class EditableMetadataFieldsTest(unittest.TestCase): # test_display_name_field verifies that a String field is of type "Generic". # test_integer_field verifies that a Integer field is of type "Integer". - descriptor = self.get_descriptor({}) + descriptor = self.get_descriptor(DictFieldData({})) editable_fields = descriptor.editable_metadata_fields # Tests for select @@ -145,13 +146,12 @@ class EditableMetadataFieldsTest(unittest.TestCase): ) # Start of helper methods - def get_xml_editable_fields(self, model_data): + def get_xml_editable_fields(self, field_data): system = get_test_system() system.render_template = Mock(return_value="<div>Test Template HTML</div>") - model_data['category'] = 'test' - return XmlDescriptor(runtime=system, model_data=model_data).editable_metadata_fields + return XmlDescriptor(runtime=system, field_data=field_data, scope_ids=Mock()).editable_metadata_fields - def get_descriptor(self, model_data): + def get_descriptor(self, field_data): class TestModuleDescriptor(TestFields, XmlDescriptor): @property def non_editable_metadata_fields(self): @@ -161,7 +161,7 @@ class EditableMetadataFieldsTest(unittest.TestCase): system = get_test_system() system.render_template = Mock(return_value="<div>Test Template HTML</div>") - return TestModuleDescriptor(runtime=system, model_data=model_data) + return TestModuleDescriptor(runtime=system, field_data=field_data, scope_ids=Mock()) def assert_field_values(self, editable_fields, name, field, explicitly_set, inheritable, value, default_value, type='Generic', options=[]): diff --git a/common/lib/xmodule/xmodule/timelimit_module.py b/common/lib/xmodule/xmodule/timelimit_module.py index 3f52ae0baa1..a5de35cfc29 100644 --- a/common/lib/xmodule/xmodule/timelimit_module.py +++ b/common/lib/xmodule/xmodule/timelimit_module.py @@ -8,7 +8,7 @@ from xmodule.xml_module import XmlDescriptor from xmodule.x_module import XModule from xmodule.progress import Progress from xmodule.exceptions import NotFoundError -from xblock.core import Float, String, Boolean, Scope +from xblock.fields import Float, String, Boolean, Scope log = logging.getLogger(__name__) diff --git a/common/lib/xmodule/xmodule/video_module.py b/common/lib/xmodule/xmodule/video_module.py index bbdef454f7e..f76a8278c7a 100644 --- a/common/lib/xmodule/xmodule/video_module.py +++ b/common/lib/xmodule/xmodule/video_module.py @@ -24,7 +24,9 @@ from xmodule.editing_module import TabsEditingDescriptor from xmodule.raw_module import EmptyDataRawDescriptor from xmodule.xml_module import is_pointer_tag, name_to_pathname from xmodule.modulestore import Location -from xblock.core import Scope, String, Boolean, Float, List, Integer +from xblock.fields import Scope, String, Boolean, Float, List, Integer, ScopeIds + +from xblock.field_data import DictFieldData import datetime import time @@ -98,7 +100,6 @@ class VideoFields(object): help="A list of filenames to be used with HTML5 video. The first supported filetype will be displayed.", display_name="Video Sources", scope=Scope.settings, - default=[] ) track = String( help="The external URL to download the timed transcript track. This appears as a link beneath the video.", @@ -213,8 +214,8 @@ class VideoDescriptor(VideoFields, TabsEditingDescriptor, EmptyDataRawDescriptor # For backwards compatibility -- if we've got XML data, parse # it out and set the metadata fields if self.data: - model_data = VideoDescriptor._parse_video_xml(self.data) - self._model_data.update(model_data) + field_data = VideoDescriptor._parse_video_xml(self.data) + self._field_data.set_many(self, field_data) del self.data @classmethod @@ -237,9 +238,17 @@ class VideoDescriptor(VideoFields, TabsEditingDescriptor, EmptyDataRawDescriptor if is_pointer_tag(xml_object): filepath = cls._format_filepath(xml_object.tag, name_to_pathname(url_name)) xml_data = etree.tostring(cls.load_file(filepath, system.resources_fs, location)) - model_data = VideoDescriptor._parse_video_xml(xml_data) - model_data['location'] = location - video = cls(system, model_data) + field_data = VideoDescriptor._parse_video_xml(xml_data) + field_data['location'] = location + video = system.construct_xblock_from_class( + cls, + DictFieldData(field_data), + + # We're loading a descriptor, so student_id is meaningless + # We also don't have separate notions of definition and usage ids yet, + # so we use the location for both + ScopeIds(None, location.category, location, location) + ) return video def definition_to_xml(self, resource_fs): @@ -261,7 +270,7 @@ class VideoDescriptor(VideoFields, TabsEditingDescriptor, EmptyDataRawDescriptor 'sub': self.sub, 'url_name': self.url_name } - fields = {field.name: field for field in self.fields} + fields = {field.name: field for field in self.fields.values()} for key, value in attrs.items(): # Mild workaround to ensure that tests pass -- if a field # is set to its default value, we don't need to write it out. @@ -312,7 +321,7 @@ class VideoDescriptor(VideoFields, TabsEditingDescriptor, EmptyDataRawDescriptor present in the XML. """ xml = etree.fromstring(xml_data) - model_data = {} + field_data = {} conversions = { 'start_time': VideoDescriptor._parse_time, @@ -328,12 +337,12 @@ class VideoDescriptor(VideoFields, TabsEditingDescriptor, EmptyDataRawDescriptor sources = xml.findall('source') if sources: - model_data['html5_sources'] = [ele.get('src') for ele in sources] - model_data['source'] = model_data['html5_sources'][0] + field_data['html5_sources'] = [ele.get('src') for ele in sources] + field_data['source'] = field_data['html5_sources'][0] track = xml.find('track') if track is not None: - model_data['track'] = track.get('src') + field_data['track'] = track.get('src') for attr, value in xml.items(): if attr in compat_keys: @@ -347,8 +356,8 @@ class VideoDescriptor(VideoFields, TabsEditingDescriptor, EmptyDataRawDescriptor # cleanliness, but hindsight doesn't need glasses normalized_speed = speed[:-1] if speed.endswith('0') else speed # If the user has specified html5 sources, make sure we don't use the default video - if youtube_id != '' or 'html5_sources' in model_data: - model_data['youtube_id_{0}'.format(normalized_speed.replace('.', '_'))] = youtube_id + if youtube_id != '' or 'html5_sources' in field_data: + field_data['youtube_id_{0}'.format(normalized_speed.replace('.', '_'))] = youtube_id else: # Convert XML attrs into Python values. if attr in conversions: @@ -357,9 +366,9 @@ class VideoDescriptor(VideoFields, TabsEditingDescriptor, EmptyDataRawDescriptor # We export values with json.dumps (well, except for Strings, but # for about a month we did it for Strings also). value = VideoDescriptor._deserialize(attr, value) - model_data[attr] = value + field_data[attr] = value - return model_data + return field_data @classmethod def _deserialize(cls, attr, value): diff --git a/common/lib/xmodule/xmodule/word_cloud_module.py b/common/lib/xmodule/xmodule/word_cloud_module.py index 900bded760f..fd644c0f1f4 100644 --- a/common/lib/xmodule/xmodule/word_cloud_module.py +++ b/common/lib/xmodule/xmodule/word_cloud_module.py @@ -14,7 +14,7 @@ from xmodule.raw_module import EmptyDataRawDescriptor from xmodule.editing_module import MetadataOnlyEditingDescriptor from xmodule.x_module import XModule -from xblock.core import Scope, Dict, Boolean, List, Integer, String +from xblock.fields import Scope, Dict, Boolean, List, Integer, String log = logging.getLogger(__name__) @@ -71,11 +71,11 @@ class WordCloudFields(object): ) all_words = Dict( help="All possible words from all students.", - scope=Scope.content + scope=Scope.user_state_summary ) top_words = Dict( help="Top num_top_words words for word cloud.", - scope=Scope.content + scope=Scope.user_state_summary ) diff --git a/common/lib/xmodule/xmodule/x_module.py b/common/lib/xmodule/xmodule/x_module.py index e459d6bf52c..2ae3a5ae4f9 100644 --- a/common/lib/xmodule/xmodule/x_module.py +++ b/common/lib/xmodule/xmodule/x_module.py @@ -10,7 +10,8 @@ from pkg_resources import resource_listdir, resource_string, resource_isdir from xmodule.modulestore import Location from xmodule.modulestore.exceptions import ItemNotFoundError, InsufficientSpecificationError, InvalidLocationError -from xblock.core import XBlock, Scope, String, Integer, Float, List, ModelType +from xblock.core import XBlock +from xblock.fields import Scope, String, Integer, Float, List from xblock.fragment import Fragment from xblock.runtime import Runtime from xmodule.modulestore.locator import BlockUsageLocator @@ -22,29 +23,6 @@ def dummy_track(_event_type, _event): pass -class LocationField(ModelType): - """ - XBlock field for storing Location values - """ - def from_json(self, value): - """ - Parse the json value as a Location - """ - try: - return Location(value) - except InvalidLocationError: - if isinstance(value, BlockUsageLocator): - return value - else: - return BlockUsageLocator(value) - - def to_json(self, value): - """ - Store the Location as a url string in json - """ - return value.url() - - class HTMLSnippet(object): """ A base class defining an interface for an object that is able to present an @@ -115,24 +93,6 @@ class XModuleFields(object): default=None ) - # Please note that in order to be compatible with XBlocks more generally, - # the LMS and CMS shouldn't be using this field. It's only for internal - # consumption by the XModules themselves - location = LocationField( - display_name="Location", - help="This is the location id for the XModule.", - scope=Scope.content, - default=Location(None), - ) - # Please note that in order to be compatible with XBlocks more generally, - # the LMS and CMS shouldn't be using this field. It's only for internal - # consumption by the XModules themselves - category = String( - display_name="xmodule category", - help="This is the category id for the XModule. It's for internal use only", - scope=Scope.content, - ) - class XModule(XModuleFields, HTMLSnippet, XBlock): ''' Implements a generic learning module. @@ -152,7 +112,7 @@ class XModule(XModuleFields, HTMLSnippet, XBlock): icon_class = 'other' - def __init__(self, runtime, descriptor, model_data): + def __init__(self, descriptor, *args, **kwargs): ''' Construct a new xmodule @@ -160,33 +120,44 @@ class XModule(XModuleFields, HTMLSnippet, XBlock): descriptor: the XModuleDescriptor that this module is an instance of. - model_data: A dictionary-like object that maps field names to values + field_data: A dictionary-like object that maps field names to values for those fields. ''' - super(XModule, self).__init__(runtime, model_data) - self._model_data = model_data - self.system = runtime + super(XModule, self).__init__(*args, **kwargs) + self.system = self.runtime self.descriptor = descriptor - # LMS tests don't require descriptor but really it's required - if descriptor: - self.url_name = descriptor.url_name - # don't need to set category as it will automatically get from descriptor - elif isinstance(self.location, Location): - self.url_name = self.location.name - if getattr(self, 'category', None) is None: - self.category = self.location.category - elif isinstance(self.location, BlockUsageLocator): - self.url_name = self.location.usage_id - if getattr(self, 'category', None) is None: - raise InsufficientSpecificationError() - else: - raise InsufficientSpecificationError() self._loaded_children = None @property def id(self): return self.location.url() + @property + def category(self): + return self.scope_ids.block_type + + @property + def location(self): + try: + return Location(self.scope_ids.usage_id) + except InvalidLocationError: + if isinstance(self.scope_ids.usage_id, BlockUsageLocator): + return self.scope_ids.usage_id + else: + return BlockUsageLocator(self.scope_ids.usage_id) + + @property + def url_name(self): + if self.descriptor: + return self.descriptor.url_name + elif isinstance(self.location, Location): + return self.location.name + elif isinstance(self.location, BlockUsageLocator): + return self.location.usage_id + else: + raise InsufficientSpecificationError() + + @property def display_name_with_default(self): ''' @@ -424,10 +395,6 @@ class XModuleDescriptor(XModuleFields, HTMLSnippet, ResourceTemplates, XBlock): # (like a practice problem). has_score = False - # A list of descriptor attributes that must be equal for the descriptors to - # be equal - equality_attributes = ('_model_data', 'location') - # Class level variable # True if this descriptor always requires recalculation of grades, for @@ -458,23 +425,13 @@ class XModuleDescriptor(XModuleFields, HTMLSnippet, ResourceTemplates, XBlock): runtime: A DescriptorSystem for interacting with external resources - model_data: A dictionary-like object that maps field names to values + field_data: A dictionary-like object that maps field names to values for those fields. XModuleDescriptor.__init__ takes the same arguments as xblock.core:XBlock.__init__ """ super(XModuleDescriptor, self).__init__(*args, **kwargs) self.system = self.runtime - if isinstance(self.location, Location): - self.url_name = self.location.name - if getattr(self, 'category', None) is None: - self.category = self.location.category - elif isinstance(self.location, BlockUsageLocator): - self.url_name = self.location.usage_id - if getattr(self, 'category', None) is None: - raise InsufficientSpecificationError() - else: - raise InsufficientSpecificationError() # update_version is the version which last updated this xblock v prev being the penultimate updater # leaving off original_version since it complicates creation w/o any obv value yet and is computable # by following previous until None @@ -486,6 +443,30 @@ class XModuleDescriptor(XModuleFields, HTMLSnippet, ResourceTemplates, XBlock): def id(self): return self.location.url() + @property + def category(self): + return self.scope_ids.block_type + + @property + def location(self): + try: + return Location(self.scope_ids.usage_id) + except InvalidLocationError: + if isinstance(self.scope_ids.usage_id, BlockUsageLocator): + return self.scope_ids.usage_id + else: + return BlockUsageLocator(self.scope_ids.usage_id) + + @property + def url_name(self): + if isinstance(self.location, Location): + return self.location.name + elif isinstance(self.location, BlockUsageLocator): + return self.location.usage_id + else: + raise InsufficientSpecificationError() + + @property def display_name_with_default(self): ''' @@ -539,10 +520,11 @@ class XModuleDescriptor(XModuleFields, HTMLSnippet, ResourceTemplates, XBlock): system: Module system """ # save any field changes - module = self.module_class( - system, - self, - system.xblock_model_data(self), + module = system.construct_xblock_from_class( + self.module_class, + descriptor=self, + field_data=system.xblock_field_data(self), + scope_ids=self.scope_ids, ) module.save() return module @@ -638,36 +620,24 @@ class XModuleDescriptor(XModuleFields, HTMLSnippet, ResourceTemplates, XBlock): """ # if caller wants kvs, caller's assuming it's up to date; so, decache it self.save() - return self._model_data._kvs + return self._field_data._kvs # =============================== BUILTIN METHODS ========================== def __eq__(self, other): - return (self.__class__ == other.__class__ and - all(getattr(self, attr, None) == getattr(other, attr, None) - for attr in self.equality_attributes)) + return (self.scope_ids == other.scope_ids and + self.fields.keys() == other.fields.keys() and + all(getattr(self, field.name) == getattr(other, field.name) + for field in self.fields.values())) def __repr__(self): return ( - "{class_}({system!r}, location={location!r}," - " model_data={model_data!r})".format( - class_=self.__class__.__name__, - system=self.system, - location=self.location, - model_data=self._model_data, - ) + "{0.__class__.__name__}(" + "{0.runtime!r}, " + "{0._field_data!r}, " + "{0.scope_ids!r}" + ")".format(self) ) - def iterfields(self): - """ - A generator for iterate over the fields of this xblock (including the ones in namespaces). - Example usage: [field.name for field in module.iterfields()] - """ - for field in self.fields: - yield field - for namespace in self.namespaces: - for field in getattr(self, namespace).fields: - yield field - @property def non_editable_metadata_fields(self): """ @@ -686,17 +656,17 @@ class XModuleDescriptor(XModuleFields, HTMLSnippet, ResourceTemplates, XBlock): if scope == Scope.settings and hasattr(self, '_inherited_metadata'): inherited_metadata = getattr(self, '_inherited_metadata') result = {} - for field in self.iterfields(): + for field in self.fields.values(): if (field.scope == scope and - field.name in self._model_data and + self._field_data.has(self, field.name) and field.name not in inherited_metadata): - result[field.name] = self._model_data[field.name] + result[field.name] = self._field_data.get(self, field.name) return result else: result = {} - for field in self.iterfields(): - if (field.scope == scope and field.name in self._model_data): - result[field.name] = self._model_data[field.name] + for field in self.fields.values(): + if (field.scope == scope and self._field_data.has(self, field.name)): + result[field.name] = self._field_data.get(self, field.name) return result @property @@ -709,7 +679,11 @@ class XModuleDescriptor(XModuleFields, HTMLSnippet, ResourceTemplates, XBlock): inherited_metadata = getattr(self, '_inherited_metadata', {}) inheritable_metadata = getattr(self, '_inheritable_metadata', {}) metadata_fields = {} - for field in self.fields: + + # Only use the fields from this class, not mixins + fields = getattr(self, 'unmixed_class', self.__class__).fields + + for field in fields.values(): if field.scope != Scope.settings or field in self.non_editable_metadata_fields: continue @@ -717,7 +691,7 @@ class XModuleDescriptor(XModuleFields, HTMLSnippet, ResourceTemplates, XBlock): inheritable = False value = getattr(self, field.name) default_value = field.default - explicitly_set = field.name in self._model_data + explicitly_set = self._field_data.has(self, field.name) if field.name in inheritable_metadata: inheritable = True default_value = field.from_json(inheritable_metadata.get(field.name)) @@ -813,6 +787,8 @@ class DescriptorSystem(Runtime): that you're about to re-raise---let the caller track them. """ + super(DescriptorSystem, self).__init__(**kwargs) + self.load_item = load_item self.resources_fs = resources_fs self.error_tracker = error_tracker @@ -823,7 +799,7 @@ class DescriptorSystem(Runtime): class XMLParsingSystem(DescriptorSystem): - def __init__(self, load_item, resources_fs, error_tracker, process_xml, policy, **kwargs): + def __init__(self, process_xml, policy, **kwargs): """ load_item, resources_fs, error_tracker: see DescriptorSystem @@ -832,8 +808,8 @@ class XMLParsingSystem(DescriptorSystem): process_xml: Takes an xml string, and returns a XModuleDescriptor created from that xml """ - DescriptorSystem.__init__(self, load_item, resources_fs, error_tracker, - **kwargs) + + super(XMLParsingSystem, self).__init__(**kwargs) self.process_xml = process_xml self.policy = policy @@ -852,12 +828,12 @@ class ModuleSystem(Runtime): ''' def __init__( self, ajax_url, track_function, get_module, render_template, - replace_urls, xblock_model_data, user=None, filestore=None, + replace_urls, xblock_field_data, user=None, filestore=None, debug=False, xqueue=None, publish=None, node_path="", anonymous_student_id='', course_id=None, open_ended_grading_interface=None, s3_interface=None, cache=None, can_execute_unsafe_code=None, replace_course_urls=None, - replace_jump_to_id_urls=None): + replace_jump_to_id_urls=None, **kwargs): ''' Create a closure around the system environment. @@ -897,7 +873,7 @@ class ModuleSystem(Runtime): publish(event) - A function that allows XModules to publish events (such as grade changes) - xblock_model_data - A function that constructs a model_data for an xblock from its + xblock_field_data - A function that constructs a field_data for an xblock from its corresponding descriptor cache - A cache object with two methods: @@ -908,6 +884,8 @@ class ModuleSystem(Runtime): not to allow the execution of unsafe, unsandboxed code. ''' + super(ModuleSystem, self).__init__(**kwargs) + self.ajax_url = ajax_url self.xqueue = xqueue self.track_function = track_function @@ -921,7 +899,7 @@ class ModuleSystem(Runtime): self.anonymous_student_id = anonymous_student_id self.course_id = course_id self.user_is_staff = user is not None and user.is_staff - self.xblock_model_data = xblock_model_data + self.xblock_field_data = xblock_field_data if publish is None: publish = lambda e: None diff --git a/common/lib/xmodule/xmodule/xml_module.py b/common/lib/xmodule/xmodule/xml_module.py index b0b7f300ed7..174dce03e6f 100644 --- a/common/lib/xmodule/xmodule/xml_module.py +++ b/common/lib/xmodule/xmodule/xml_module.py @@ -6,12 +6,14 @@ import sys from collections import namedtuple from lxml import etree -from xblock.core import Dict, Scope +from xblock.fields import Dict, Scope, ScopeIds from xmodule.x_module import (XModuleDescriptor, policy_key) from xmodule.modulestore import Location from xmodule.modulestore.inheritance import own_metadata from xmodule.modulestore.xml_exporter import EdxJSONEncoder +from xblock.field_data import DictFieldData + log = logging.getLogger(__name__) # assume all XML files are persisted as utf-8. @@ -172,13 +174,12 @@ class XmlDescriptor(XModuleDescriptor): Searches through fields defined by cls to find one named attr. """ - for field in set(cls.fields + cls.lms.fields): - if field.name == attr: - from_xml = lambda val: deserialize_field(field, val) - to_xml = lambda val: serialize_field(val) - return AttrMap(from_xml, to_xml) - - return AttrMap() + if attr in cls.fields: + from_xml = lambda val: deserialize_field(cls.fields[attr], val) + to_xml = lambda val: serialize_field(val) + return AttrMap(from_xml, to_xml) + else: + return AttrMap() @classmethod def definition_from_xml(cls, xml_object, system): @@ -352,22 +353,27 @@ class XmlDescriptor(XModuleDescriptor): if k in system.policy: cls.apply_policy(metadata, system.policy[k]) - model_data = {} - model_data.update(metadata) - model_data.update(definition) - model_data['children'] = children + field_data = {} + field_data.update(metadata) + field_data.update(definition) + field_data['children'] = children - model_data['xml_attributes'] = {} - model_data['xml_attributes']['filename'] = definition.get('filename', ['', None]) # for git link + field_data['xml_attributes'] = {} + field_data['xml_attributes']['filename'] = definition.get('filename', ['', None]) # for git link for key, value in metadata.items(): - if key not in set(f.name for f in cls.fields + cls.lms.fields): - model_data['xml_attributes'][key] = value - model_data['location'] = location - model_data['category'] = xml_object.tag - - return cls( - system, - model_data, + if key not in cls.fields: + field_data['xml_attributes'][key] = value + field_data['location'] = location + field_data['category'] = xml_object.tag + + return system.construct_xblock_from_class( + cls, + DictFieldData(field_data), + + # We're loading a descriptor, so student_id is meaningless + # We also don't have separate notions of definition and usage ids yet, + # so we use the location for both + ScopeIds(None, location.category, location, location) ) @classmethod @@ -413,7 +419,7 @@ class XmlDescriptor(XModuleDescriptor): (Possible format conversion through an AttrMap). """ attr_map = self.get_map_for_field(attr) - return attr_map.to_xml(self._model_data[attr]) + return attr_map.to_xml(self._field_data.get(self, attr)) # Add the non-inherited metadata for attr in sorted(own_metadata(self)): diff --git a/docs/source/conf.py b/docs/source/conf.py index aa62613370c..40fe28538e0 100644 --- a/docs/source/conf.py +++ b/docs/source/conf.py @@ -100,6 +100,9 @@ pygments_style = 'sphinx' # A list of ignored prefixes for module index sorting. #modindex_common_prefix = [] +# When auto-doc'ing a class, write the class' docstring and the __init__ docstring +# into the class docs. +autoclass_content = "both" # -- Options for HTML output --------------------------------------------------- diff --git a/lms/djangoapps/analytics/basic.py b/lms/djangoapps/analytics/basic.py index 3600d0e3938..c4682f05944 100644 --- a/lms/djangoapps/analytics/basic.py +++ b/lms/djangoapps/analytics/basic.py @@ -80,7 +80,7 @@ def dump_grading_context(course): msg += "--> Section %s:\n" % (gsomething) for sec in gsvals: sdesc = sec['section_descriptor'] - frmat = getattr(sdesc.lms, 'format', None) + frmat = getattr(sdesc, 'format', None) aname = '' if frmat in graders: gform = graders[frmat] diff --git a/lms/djangoapps/courseware/access.py b/lms/djangoapps/courseware/access.py index 8fa40ec3922..7836ab8bbce 100644 --- a/lms/djangoapps/courseware/access.py +++ b/lms/djangoapps/courseware/access.py @@ -143,12 +143,12 @@ def _has_access_course_desc(user, course, action): """ First check if restriction of enrollment by login method is enabled, both globally and by the course. - If it is, then the user must pass the criterion set by the course, e.g. that ExternalAuthMap + If it is, then the user must pass the criterion set by the course, e.g. that ExternalAuthMap was set by 'shib:https://idp.stanford.edu/", in addition to requirements below. Rest of requirements: Enrollment can only happen in the course enrollment period, if one exists. or - + (CourseEnrollmentAllowed always overrides) (staff can always enroll) """ @@ -195,7 +195,7 @@ def _has_access_course_desc(user, course, action): if settings.MITX_FEATURES.get('ACCESS_REQUIRE_STAFF_FOR_COURSE'): # if this feature is on, only allow courses that have ispublic set to be # seen by non-staff - if course.lms.ispublic: + if course.ispublic: debug("Allow: ACCESS_REQUIRE_STAFF_FOR_COURSE and ispublic") return True return _has_staff_access_to_descriptor(user, course) @@ -272,7 +272,7 @@ def _has_access_descriptor(user, descriptor, action, course_context=None): return True # Check start date - if descriptor.lms.start is not None: + if descriptor.start is not None: now = datetime.now(UTC()) effective_start = _adjust_start_date_for_beta_testers(user, descriptor) if now > effective_start: @@ -526,20 +526,20 @@ def _adjust_start_date_for_beta_testers(user, descriptor): NOTE: If testing manually, make sure MITX_FEATURES['DISABLE_START_DATES'] = False in envs/dev.py! """ - if descriptor.lms.days_early_for_beta is None: + if descriptor.days_early_for_beta is None: # bail early if no beta testing is set up - return descriptor.lms.start + return descriptor.start user_groups = [g.name for g in user.groups.all()] beta_group = course_beta_test_group_name(descriptor.location) if beta_group in user_groups: debug("Adjust start time: user in group %s", beta_group) - delta = timedelta(descriptor.lms.days_early_for_beta) - effective = descriptor.lms.start - delta + delta = timedelta(descriptor.days_early_for_beta) + effective = descriptor.start - delta return effective - return descriptor.lms.start + return descriptor.start def _has_instructor_access_to_location(user, location, course_context=None): diff --git a/lms/djangoapps/courseware/courses.py b/lms/djangoapps/courseware/courses.py index b39e2d6315c..78e1d51cbc3 100644 --- a/lms/djangoapps/courseware/courses.py +++ b/lms/djangoapps/courseware/courses.py @@ -13,7 +13,7 @@ from xmodule.modulestore import Location, XML_MODULESTORE_TYPE from xmodule.modulestore.django import modulestore from xmodule.contentstore.content import StaticContent from xmodule.modulestore.exceptions import ItemNotFoundError, InvalidLocationError -from courseware.model_data import ModelDataCache +from courseware.model_data import FieldDataCache from static_replace import replace_static_urls from courseware.access import has_access import branding @@ -82,8 +82,8 @@ def get_opt_course_with_access(user, course_id, action): def course_image_url(course): """Try to look up the image url for the course. If it's not found, log an error and return the dead link""" - if course.lms.static_asset_path or modulestore().get_modulestore_type(course.location.course_id) == XML_MODULESTORE_TYPE: - return '/static/' + (course.lms.static_asset_path or getattr(course, 'data_dir', '')) + "/images/course_image.jpg" + if course.static_asset_path or modulestore().get_modulestore_type(course.location.course_id) == XML_MODULESTORE_TYPE: + return '/static/' + (course.static_asset_path or getattr(course, 'data_dir', '')) + "/images/course_image.jpg" else: loc = course.location._replace(tag='c4x', category='asset', name=course.course_image) _path = StaticContent.get_url_path_from_location(loc) @@ -149,16 +149,16 @@ def get_course_about_section(course, section_key): loc = course.location._replace(category='about', name=section_key) # Use an empty cache - model_data_cache = ModelDataCache([], course.id, request.user) + field_data_cache = FieldDataCache([], course.id, request.user) about_module = get_module( request.user, request, loc, - model_data_cache, + field_data_cache, course.id, not_found_ok=True, wrap_xmodule_display=False, - static_asset_path=course.lms.static_asset_path + static_asset_path=course.static_asset_path ) html = '' @@ -199,15 +199,15 @@ def get_course_info_section(request, course, section_key): loc = Location(course.location.tag, course.location.org, course.location.course, 'course_info', section_key) # Use an empty cache - model_data_cache = ModelDataCache([], course.id, request.user) + field_data_cache = FieldDataCache([], course.id, request.user) info_module = get_module( request.user, request, loc, - model_data_cache, + field_data_cache, course.id, wrap_xmodule_display=False, - static_asset_path=course.lms.static_asset_path + static_asset_path=course.static_asset_path ) html = '' @@ -246,7 +246,7 @@ def get_course_syllabus_section(course, section_key): htmlFile.read().decode('utf-8'), getattr(course, 'data_dir', None), course_id=course.location.course_id, - static_asset_path=course.lms.static_asset_path, + static_asset_path=course.static_asset_path, ) except ResourceNotFoundError: log.exception("Missing syllabus section {key} in course {url}".format( diff --git a/lms/djangoapps/courseware/features/common.py b/lms/djangoapps/courseware/features/common.py index 349ad7e79e1..d0a777eb1e4 100644 --- a/lms/djangoapps/courseware/features/common.py +++ b/lms/djangoapps/courseware/features/common.py @@ -111,7 +111,7 @@ def get_courseware_with_tabs(course_id): the tabs on the right hand main navigation page. This hides the appropriate courseware as defined by the hide_from_toc field: - chapter.lms.hide_from_toc + chapter.hide_from_toc Example: @@ -164,14 +164,14 @@ def get_courseware_with_tabs(course_id): """ course = get_course_by_id(course_id) - chapters = [chapter for chapter in course.get_children() if not chapter.lms.hide_from_toc] + chapters = [chapter for chapter in course.get_children() if not chapter.hide_from_toc] courseware = [{'chapter_name': c.display_name_with_default, 'sections': [{'section_name': s.display_name_with_default, 'clickable_tab_count': len(s.get_children()) if (type(s) == seq_module.SequenceDescriptor) else 0, 'tabs': [{'children_count': len(t.get_children()) if (type(t) == vertical_module.VerticalDescriptor) else 0, 'class': t.__class__.__name__} for t in s.get_children()]} - for s in c.get_children() if not s.lms.hide_from_toc]} + for s in c.get_children() if not s.hide_from_toc]} for c in chapters] return courseware diff --git a/lms/djangoapps/courseware/grades.py b/lms/djangoapps/courseware/grades.py index 8874a5686c3..b8fde4276b2 100644 --- a/lms/djangoapps/courseware/grades.py +++ b/lms/djangoapps/courseware/grades.py @@ -8,8 +8,8 @@ from collections import defaultdict from django.conf import settings from django.contrib.auth.models import User -from .model_data import ModelDataCache, LmsKeyValueStore -from xblock.core import Scope +from courseware.model_data import FieldDataCache, DjangoKeyValueStore +from xblock.fields import Scope from .module_render import get_module, get_module_for_descriptor from xmodule import graders from xmodule.capa_module import CapaModule @@ -75,10 +75,10 @@ def yield_problems(request, course, student): sections_to_list.append(section_descriptor) break - model_data_cache = ModelDataCache(sections_to_list, course.id, student) + field_data_cache = FieldDataCache(sections_to_list, course.id, student) for section_descriptor in sections_to_list: section_module = get_module(student, request, - section_descriptor.location, model_data_cache, + section_descriptor.location, field_data_cache, course.id) if section_module is None: # student doesn't have access to this module, or something else @@ -119,7 +119,7 @@ def answer_distributions(request, course): return counts -def grade(student, request, course, model_data_cache=None, keep_raw_scores=False): +def grade(student, request, course, field_data_cache=None, keep_raw_scores=False): """ This grades a student as quickly as possible. It returns the output from the course grader, augmented with the final letter @@ -141,8 +141,8 @@ def grade(student, request, course, model_data_cache=None, keep_raw_scores=False grading_context = course.grading_context raw_scores = [] - if model_data_cache is None: - model_data_cache = ModelDataCache(grading_context['all_descriptors'], course.id, student) + if field_data_cache is None: + field_data_cache = FieldDataCache(grading_context['all_descriptors'], course.id, student) totaled_scores = {} # This next complicated loop is just to collect the totaled_scores, which is @@ -162,15 +162,15 @@ def grade(student, request, course, model_data_cache=None, keep_raw_scores=False should_grade_section = True break - # Create a fake key to pull out a StudentModule object from the ModelDataCache + # Create a fake key to pull out a StudentModule object from the FieldDataCache - key = LmsKeyValueStore.Key( + key = DjangoKeyValueStore.Key( Scope.user_state, student.id, moduledescriptor.location, None ) - if model_data_cache.find(key): + if field_data_cache.find(key): should_grade_section = True break @@ -181,11 +181,11 @@ def grade(student, request, course, model_data_cache=None, keep_raw_scores=False '''creates an XModule instance given a descriptor''' # TODO: We need the request to pass into here. If we could forego that, our arguments # would be simpler - return get_module_for_descriptor(student, request, descriptor, model_data_cache, course.id) + return get_module_for_descriptor(student, request, descriptor, field_data_cache, course.id) for module_descriptor in yield_dynamic_descriptor_descendents(section_descriptor, create_module): - (correct, total) = get_score(course.id, student, module_descriptor, create_module, model_data_cache) + (correct, total) = get_score(course.id, student, module_descriptor, create_module, field_data_cache) if correct is None and total is None: continue @@ -195,7 +195,7 @@ def grade(student, request, course, model_data_cache=None, keep_raw_scores=False else: correct = total - graded = module_descriptor.lms.graded + graded = module_descriptor.graded if not total > 0: #We simply cannot grade a problem that is 12/0, because we might need it as a percentage graded = False @@ -257,7 +257,7 @@ def grade_for_percentage(grade_cutoffs, percentage): # TODO: This method is not very good. It was written in the old course style and # then converted over and performance is not good. Once the progress page is redesigned # to not have the progress summary this method should be deleted (so it won't be copied). -def progress_summary(student, request, course, model_data_cache): +def progress_summary(student, request, course, field_data_cache): """ This pulls a summary of all problems in the course. @@ -271,7 +271,7 @@ def progress_summary(student, request, course, model_data_cache): Arguments: student: A User object for the student to grade course: A Descriptor containing the course to grade - model_data_cache: A ModelDataCache initialized with all + field_data_cache: A FieldDataCache initialized with all instance_modules for the student If the student does not have access to load the course module, this function @@ -281,7 +281,7 @@ def progress_summary(student, request, course, model_data_cache): # TODO: We need the request to pass into here. If we could forego that, our arguments # would be simpler - course_module = get_module(student, request, course.location, model_data_cache, course.id, depth=None) + course_module = get_module(student, request, course.location, field_data_cache, course.id, depth=None) if not course_module: # This student must not have access to the course. return None @@ -290,17 +290,17 @@ def progress_summary(student, request, course, model_data_cache): # Don't include chapters that aren't displayable (e.g. due to error) for chapter_module in course_module.get_display_items(): # Skip if the chapter is hidden - if chapter_module.lms.hide_from_toc: + if chapter_module.hide_from_toc: continue sections = [] for section_module in chapter_module.get_display_items(): # Skip if the section is hidden - if section_module.lms.hide_from_toc: + if section_module.hide_from_toc: continue # Same for sections - graded = section_module.lms.graded + graded = section_module.graded scores = [] module_creator = section_module.system.get_module @@ -308,7 +308,7 @@ def progress_summary(student, request, course, model_data_cache): for module_descriptor in yield_dynamic_descriptor_descendents(section_module.descriptor, module_creator): course_id = course.id - (correct, total) = get_score(course_id, student, module_descriptor, module_creator, model_data_cache) + (correct, total) = get_score(course_id, student, module_descriptor, module_creator, field_data_cache) if correct is None and total is None: continue @@ -318,14 +318,14 @@ def progress_summary(student, request, course, model_data_cache): section_total, _ = graders.aggregate_scores( scores, section_module.display_name_with_default) - module_format = section_module.lms.format if section_module.lms.format is not None else '' + module_format = section_module.format if section_module.format is not None else '' sections.append({ 'display_name': section_module.display_name_with_default, 'url_name': section_module.url_name, 'scores': scores, 'section_total': section_total, 'format': module_format, - 'due': section_module.lms.due, + 'due': section_module.due, 'graded': graded, }) @@ -337,7 +337,7 @@ def progress_summary(student, request, course, model_data_cache): return chapters -def get_score(course_id, user, problem_descriptor, module_creator, model_data_cache): +def get_score(course_id, user, problem_descriptor, module_creator, field_data_cache): """ Return the score for a user on a problem, as a tuple (correct, total). e.g. (5,7) if you got 5 out of 7 points. @@ -349,7 +349,7 @@ def get_score(course_id, user, problem_descriptor, module_creator, model_data_ca problem_descriptor: an XModuleDescriptor module_creator: a function that takes a descriptor, and returns the corresponding XModule for this user. Can return None if user doesn't have access, or if something else went wrong. - cache: A ModelDataCache + cache: A FieldDataCache """ if not user.is_authenticated(): return (None, None) @@ -371,14 +371,14 @@ def get_score(course_id, user, problem_descriptor, module_creator, model_data_ca return (None, None) # Create a fake KeyValueStore key to pull out the StudentModule - key = LmsKeyValueStore.Key( + key = DjangoKeyValueStore.Key( Scope.user_state, user.id, problem_descriptor.location, None ) - student_module = model_data_cache.find(key) + student_module = field_data_cache.find(key) if student_module is not None and student_module.max_grade is not None: correct = student_module.grade if student_module.grade is not None else 0 diff --git a/lms/djangoapps/courseware/management/commands/check_course.py b/lms/djangoapps/courseware/management/commands/check_course.py index a4f2bcc4757..9bc9dd409c5 100644 --- a/lms/djangoapps/courseware/management/commands/check_course.py +++ b/lms/djangoapps/courseware/management/commands/check_course.py @@ -11,7 +11,7 @@ from django.contrib.auth.models import User import xmodule from xmodule.modulestore.django import modulestore -from courseware.model_data import ModelDataCache +from courseware.model_data import FieldDataCache from courseware.module_render import get_module @@ -81,7 +81,7 @@ class Command(BaseCommand): # TODO (cpennington): Get coursename in a legitimate way course_location = 'i4x://edx/6002xs12/course/6.002_Spring_2012' - student_module_cache = ModelDataCache.cache_for_descriptor_descendents( + student_module_cache = FieldDataCache.cache_for_descriptor_descendents( course_id, sample_user, modulestore().get_item(course_location)) course = get_module(sample_user, None, course_location, student_module_cache) diff --git a/lms/djangoapps/courseware/migrations/0010_rename_xblock_field_content_to_user_state_summary.py b/lms/djangoapps/courseware/migrations/0010_rename_xblock_field_content_to_user_state_summary.py new file mode 100644 index 00000000000..51b2e9d0fd4 --- /dev/null +++ b/lms/djangoapps/courseware/migrations/0010_rename_xblock_field_content_to_user_state_summary.py @@ -0,0 +1,148 @@ +# -*- coding: utf-8 -*- +import datetime +from south.db import db +from south.v2 import SchemaMigration +from django.db import models + + +class Migration(SchemaMigration): + + def forwards(self, orm): + # Removing unique constraint on 'XModuleSettingsField', fields ['usage_id', 'field_name'] + db.delete_unique('courseware_xmodulesettingsfield', ['usage_id', 'field_name']) + + # Deleting model 'XModuleSettingsField' + db.delete_table('courseware_xmodulesettingsfield') + + # Move all content currently stored as Scope.content to Scope.user_state_summary + db.rename_table('courseware_xmodulecontentfield', 'courseware_xmoduleuserstatesummaryfield') + db.rename_column('courseware_xmoduleuserstatesummaryfield', 'definition_id', 'usage_id') + + + def backwards(self, orm): + # Adding model 'XModuleSettingsField' + db.create_table('courseware_xmodulesettingsfield', ( + ('created', self.gf('django.db.models.fields.DateTimeField')(auto_now_add=True, blank=True, db_index=True)), + ('modified', self.gf('django.db.models.fields.DateTimeField')(auto_now=True, blank=True, db_index=True)), + ('value', self.gf('django.db.models.fields.TextField')(default='null')), + ('field_name', self.gf('django.db.models.fields.CharField')(max_length=64, db_index=True)), + ('id', self.gf('django.db.models.fields.AutoField')(primary_key=True)), + ('usage_id', self.gf('django.db.models.fields.CharField')(max_length=255, db_index=True)), + )) + db.send_create_signal('courseware', ['XModuleSettingsField']) + + # Adding unique constraint on 'XModuleSettingsField', fields ['usage_id', 'field_name'] + db.create_unique('courseware_xmodulesettingsfield', ['usage_id', 'field_name']) + + db.rename_table('courseware_xmoduleuserstatesummaryfield', 'courseware_xmodulecontentfield') + db.rename_column('courseware_xmodulecontentfield', 'usage_id', 'definition_id') + + models = { + 'auth.group': { + 'Meta': {'object_name': 'Group'}, + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'name': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '80'}), + 'permissions': ('django.db.models.fields.related.ManyToManyField', [], {'to': "orm['auth.Permission']", 'symmetrical': 'False', 'blank': 'True'}) + }, + 'auth.permission': { + 'Meta': {'ordering': "('content_type__app_label', 'content_type__model', 'codename')", 'unique_together': "(('content_type', 'codename'),)", 'object_name': 'Permission'}, + 'codename': ('django.db.models.fields.CharField', [], {'max_length': '100'}), + 'content_type': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['contenttypes.ContentType']"}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'name': ('django.db.models.fields.CharField', [], {'max_length': '50'}) + }, + 'auth.user': { + 'Meta': {'object_name': 'User'}, + 'date_joined': ('django.db.models.fields.DateTimeField', [], {'default': 'datetime.datetime.now'}), + 'email': ('django.db.models.fields.EmailField', [], {'max_length': '75', 'blank': 'True'}), + 'first_name': ('django.db.models.fields.CharField', [], {'max_length': '30', 'blank': 'True'}), + 'groups': ('django.db.models.fields.related.ManyToManyField', [], {'to': "orm['auth.Group']", 'symmetrical': 'False', 'blank': 'True'}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'is_active': ('django.db.models.fields.BooleanField', [], {'default': 'True'}), + 'is_staff': ('django.db.models.fields.BooleanField', [], {'default': 'False'}), + 'is_superuser': ('django.db.models.fields.BooleanField', [], {'default': 'False'}), + 'last_login': ('django.db.models.fields.DateTimeField', [], {'default': 'datetime.datetime.now'}), + 'last_name': ('django.db.models.fields.CharField', [], {'max_length': '30', 'blank': 'True'}), + 'password': ('django.db.models.fields.CharField', [], {'max_length': '128'}), + 'user_permissions': ('django.db.models.fields.related.ManyToManyField', [], {'to': "orm['auth.Permission']", 'symmetrical': 'False', 'blank': 'True'}), + 'username': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '30'}) + }, + 'contenttypes.contenttype': { + 'Meta': {'ordering': "('name',)", 'unique_together': "(('app_label', 'model'),)", 'object_name': 'ContentType', 'db_table': "'django_content_type'"}, + 'app_label': ('django.db.models.fields.CharField', [], {'max_length': '100'}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'model': ('django.db.models.fields.CharField', [], {'max_length': '100'}), + 'name': ('django.db.models.fields.CharField', [], {'max_length': '100'}) + }, + 'courseware.offlinecomputedgrade': { + 'Meta': {'unique_together': "(('user', 'course_id'),)", 'object_name': 'OfflineComputedGrade'}, + 'course_id': ('django.db.models.fields.CharField', [], {'max_length': '255', 'db_index': 'True'}), + 'created': ('django.db.models.fields.DateTimeField', [], {'auto_now_add': 'True', 'null': 'True', 'db_index': 'True', 'blank': 'True'}), + 'gradeset': ('django.db.models.fields.TextField', [], {'null': 'True', 'blank': 'True'}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'updated': ('django.db.models.fields.DateTimeField', [], {'auto_now': 'True', 'db_index': 'True', 'blank': 'True'}), + 'user': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['auth.User']"}) + }, + 'courseware.offlinecomputedgradelog': { + 'Meta': {'ordering': "['-created']", 'object_name': 'OfflineComputedGradeLog'}, + 'course_id': ('django.db.models.fields.CharField', [], {'max_length': '255', 'db_index': 'True'}), + 'created': ('django.db.models.fields.DateTimeField', [], {'auto_now_add': 'True', 'null': 'True', 'db_index': 'True', 'blank': 'True'}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'nstudents': ('django.db.models.fields.IntegerField', [], {'default': '0'}), + 'seconds': ('django.db.models.fields.IntegerField', [], {'default': '0'}) + }, + 'courseware.studentmodule': { + 'Meta': {'unique_together': "(('student', 'module_state_key', 'course_id'),)", 'object_name': 'StudentModule'}, + 'course_id': ('django.db.models.fields.CharField', [], {'max_length': '255', 'db_index': 'True'}), + 'created': ('django.db.models.fields.DateTimeField', [], {'auto_now_add': 'True', 'db_index': 'True', 'blank': 'True'}), + 'done': ('django.db.models.fields.CharField', [], {'default': "'na'", 'max_length': '8', 'db_index': 'True'}), + 'grade': ('django.db.models.fields.FloatField', [], {'db_index': 'True', 'null': 'True', 'blank': 'True'}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'max_grade': ('django.db.models.fields.FloatField', [], {'null': 'True', 'blank': 'True'}), + 'modified': ('django.db.models.fields.DateTimeField', [], {'auto_now': 'True', 'db_index': 'True', 'blank': 'True'}), + 'module_state_key': ('django.db.models.fields.CharField', [], {'max_length': '255', 'db_column': "'module_id'", 'db_index': 'True'}), + 'module_type': ('django.db.models.fields.CharField', [], {'default': "'problem'", 'max_length': '32', 'db_index': 'True'}), + 'state': ('django.db.models.fields.TextField', [], {'null': 'True', 'blank': 'True'}), + 'student': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['auth.User']"}) + }, + 'courseware.studentmodulehistory': { + 'Meta': {'object_name': 'StudentModuleHistory'}, + 'created': ('django.db.models.fields.DateTimeField', [], {'db_index': 'True'}), + 'grade': ('django.db.models.fields.FloatField', [], {'null': 'True', 'blank': 'True'}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'max_grade': ('django.db.models.fields.FloatField', [], {'null': 'True', 'blank': 'True'}), + 'state': ('django.db.models.fields.TextField', [], {'null': 'True', 'blank': 'True'}), + 'student_module': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['courseware.StudentModule']"}), + 'version': ('django.db.models.fields.CharField', [], {'db_index': 'True', 'max_length': '255', 'null': 'True', 'blank': 'True'}) + }, + 'courseware.xmodulestudentinfofield': { + 'Meta': {'unique_together': "(('student', 'field_name'),)", 'object_name': 'XModuleStudentInfoField'}, + 'created': ('django.db.models.fields.DateTimeField', [], {'auto_now_add': 'True', 'db_index': 'True', 'blank': 'True'}), + 'field_name': ('django.db.models.fields.CharField', [], {'max_length': '64', 'db_index': 'True'}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'modified': ('django.db.models.fields.DateTimeField', [], {'auto_now': 'True', 'db_index': 'True', 'blank': 'True'}), + 'student': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['auth.User']"}), + 'value': ('django.db.models.fields.TextField', [], {'default': "'null'"}) + }, + 'courseware.xmodulestudentprefsfield': { + 'Meta': {'unique_together': "(('student', 'module_type', 'field_name'),)", 'object_name': 'XModuleStudentPrefsField'}, + 'created': ('django.db.models.fields.DateTimeField', [], {'auto_now_add': 'True', 'db_index': 'True', 'blank': 'True'}), + 'field_name': ('django.db.models.fields.CharField', [], {'max_length': '64', 'db_index': 'True'}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'modified': ('django.db.models.fields.DateTimeField', [], {'auto_now': 'True', 'db_index': 'True', 'blank': 'True'}), + 'module_type': ('django.db.models.fields.CharField', [], {'max_length': '64', 'db_index': 'True'}), + 'student': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['auth.User']"}), + 'value': ('django.db.models.fields.TextField', [], {'default': "'null'"}) + }, + 'courseware.xmoduleuserstatesummary': { + 'Meta': {'unique_together': "(('usage_id', 'field_name'),)", 'object_name': 'XModuleUserStateSummary'}, + 'created': ('django.db.models.fields.DateTimeField', [], {'auto_now_add': 'True', 'db_index': 'True', 'blank': 'True'}), + 'usage_id': ('django.db.models.fields.CharField', [], {'max_length': '255', 'db_index': 'True'}), + 'field_name': ('django.db.models.fields.CharField', [], {'max_length': '64', 'db_index': 'True'}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'modified': ('django.db.models.fields.DateTimeField', [], {'auto_now': 'True', 'db_index': 'True', 'blank': 'True'}), + 'value': ('django.db.models.fields.TextField', [], {'default': "'null'"}) + } + } + + complete_apps = ['courseware'] \ No newline at end of file diff --git a/lms/djangoapps/courseware/model_data.py b/lms/djangoapps/courseware/model_data.py index 44be16e4418..f480d0ef445 100644 --- a/lms/djangoapps/courseware/model_data.py +++ b/lms/djangoapps/courseware/model_data.py @@ -3,12 +3,11 @@ Classes to provide the LMS runtime data storage to XBlocks """ import json -from collections import namedtuple, defaultdict +from collections import defaultdict from itertools import chain from .models import ( StudentModule, - XModuleContentField, - XModuleSettingsField, + XModuleUserStateSummaryField, XModuleStudentPrefsField, XModuleStudentInfoField ) @@ -16,8 +15,9 @@ import logging from django.db import DatabaseError -from xblock.runtime import KeyValueStore, InvalidScopeError -from xblock.core import KeyValueMultiSaveError, Scope +from xblock.runtime import KeyValueStore +from xblock.exceptions import KeyValueMultiSaveError, InvalidScopeError +from xblock.fields import Scope log = logging.getLogger(__name__) @@ -37,7 +37,7 @@ def chunks(items, chunk_size): return (items[i:i + chunk_size] for i in xrange(0, len(items), chunk_size)) -class ModelDataCache(object): +class FieldDataCache(object): """ A cache of django model objects needed to supply the data for a module and its decendants @@ -106,7 +106,7 @@ class ModelDataCache(object): descriptors = get_child_descriptors(descriptor, depth, descriptor_filter) - return ModelDataCache(descriptors, course_id, user, select_for_update) + return FieldDataCache(descriptors, course_id, user, select_for_update) def _query(self, model_class, **kwargs): """ @@ -137,9 +137,7 @@ class ModelDataCache(object): """ Queries the database for all of the fields in the specified scope """ - if scope in (Scope.children, Scope.parent): - return [] - elif scope == Scope.user_state: + if scope == Scope.user_state: return self._chunked_query( StudentModule, 'module_state_key__in', @@ -147,21 +145,11 @@ class ModelDataCache(object): course_id=self.course_id, student=self.user.pk, ) - elif scope == Scope.content: - return self._chunked_query( - XModuleContentField, - 'definition_id__in', - (descriptor.location.url() for descriptor in self.descriptors), - field_name__in=set(field.name for field in fields), - ) - elif scope == Scope.settings: + elif scope == Scope.user_state_summary: return self._chunked_query( - XModuleSettingsField, + XModuleUserStateSummaryField, 'usage_id__in', - ( - '%s-%s' % (self.course_id, descriptor.location.url()) - for descriptor in self.descriptors - ), + (descriptor.location.url() for descriptor in self.descriptors), field_name__in=set(field.name for field in fields), ) elif scope == Scope.preferences: @@ -179,7 +167,7 @@ class ModelDataCache(object): field_name__in=set(field.name for field in fields), ) else: - raise InvalidScopeError(scope) + return [] def _fields_to_cache(self): """ @@ -187,20 +175,18 @@ class ModelDataCache(object): """ scope_map = defaultdict(set) for descriptor in self.descriptors: - for field in (descriptor.module_class.fields + descriptor.module_class.lms.fields): + for field in descriptor.fields.values(): scope_map[field.scope].add(field) return scope_map def _cache_key_from_kvs_key(self, key): """ - Return the key used in the ModelDataCache for the specified KeyValueStore key + Return the key used in the FieldDataCache for the specified KeyValueStore key """ if key.scope == Scope.user_state: return (key.scope, key.block_scope_id.url()) - elif key.scope == Scope.content: + elif key.scope == Scope.user_state_summary: return (key.scope, key.block_scope_id.url(), key.field_name) - elif key.scope == Scope.settings: - return (key.scope, '%s-%s' % (self.course_id, key.block_scope_id.url()), key.field_name) elif key.scope == Scope.preferences: return (key.scope, key.block_scope_id, key.field_name) elif key.scope == Scope.user_info: @@ -208,14 +194,12 @@ class ModelDataCache(object): def _cache_key_from_field_object(self, scope, field_object): """ - Return the key used in the ModelDataCache for the specified scope and + Return the key used in the FieldDataCache for the specified scope and field """ if scope == Scope.user_state: return (scope, field_object.module_state_key) - elif scope == Scope.content: - return (scope, field_object.definition_id, field_object.field_name) - elif scope == Scope.settings: + elif scope == Scope.user_state_summary: return (scope, field_object.usage_id, field_object.field_name) elif scope == Scope.preferences: return (scope, field_object.module_type, field_object.field_name) @@ -224,9 +208,9 @@ class ModelDataCache(object): def find(self, key): ''' - Look for a model data object using an LmsKeyValueStore.Key object + Look for a model data object using an DjangoKeyValueStore.Key object - key: An `LmsKeyValueStore.Key` object selecting the object to find + key: An `DjangoKeyValueStore.Key` object selecting the object to find returns the found object, or None if the object doesn't exist ''' @@ -252,15 +236,10 @@ class ModelDataCache(object): 'module_type': key.block_scope_id.category, }, ) - elif key.scope == Scope.content: - field_object, _ = XModuleContentField.objects.get_or_create( + elif key.scope == Scope.user_state_summary: + field_object, _ = XModuleUserStateSummaryField.objects.get_or_create( field_name=key.field_name, - definition_id=key.block_scope_id.url() - ) - elif key.scope == Scope.settings: - field_object, _ = XModuleSettingsField.objects.get_or_create( - field_name=key.field_name, - usage_id='%s-%s' % (self.course_id, key.block_scope_id.url()), + usage_id=key.block_scope_id.url() ) elif key.scope == Scope.preferences: field_object, _ = XModuleStudentPrefsField.objects.get_or_create( @@ -279,19 +258,15 @@ class ModelDataCache(object): return field_object -class LmsKeyValueStore(KeyValueStore): +class DjangoKeyValueStore(KeyValueStore): """ - This KeyValueStore will read data from descriptor_model_data if it exists, - but will not overwrite any keys set in descriptor_model_data. Attempts to do so will - raise an InvalidWriteError. - - If the scope to write to is not one of the 5 named scopes: - Scope.content - Scope.settings + This KeyValueStore will read and write data in the following scopes to django models + Scope.user_state_summary Scope.user_state Scope.preferences Scope.user_info - then an InvalidScopeError will be raised. + + Access to any other scopes will raise an InvalidScopeError Data for Scope.user_state is stored as StudentModule objects via the django orm. @@ -302,29 +277,21 @@ class LmsKeyValueStore(KeyValueStore): """ _allowed_scopes = ( - Scope.content, - Scope.settings, + Scope.user_state_summary, Scope.user_state, Scope.preferences, Scope.user_info, - Scope.children, ) - def __init__(self, descriptor_model_data, model_data_cache): - self._descriptor_model_data = descriptor_model_data - self._model_data_cache = model_data_cache - - def get(self, key): - if key.field_name in self._descriptor_model_data: - return self._descriptor_model_data[key.field_name] - if key.scope == Scope.parent: - return None + def __init__(self, field_data_cache): + self._field_data_cache = field_data_cache + def get(self, key): if key.scope not in self._allowed_scopes: raise InvalidScopeError(key.scope) - field_object = self._model_data_cache.find(key) + field_object = self._field_data_cache.find(key) if field_object is None: raise KeyError(key.field_name) @@ -352,14 +319,11 @@ class LmsKeyValueStore(KeyValueStore): field_objects = dict() for field in kv_dict: # Check field for validity - if field.field_name in self._descriptor_model_data: - raise InvalidWriteError("Not allowed to overwrite descriptor model data", field.field_name) - if field.scope not in self._allowed_scopes: raise InvalidScopeError(field.scope) # If the field is valid and isn't already in the dictionary, add it. - field_object = self._model_data_cache.find_or_create(field) + field_object = self._field_data_cache.find_or_create(field) if field_object not in field_objects.keys(): field_objects[field_object] = [] # Update the list of associated fields @@ -387,13 +351,10 @@ class LmsKeyValueStore(KeyValueStore): raise KeyValueMultiSaveError(saved_fields) def delete(self, key): - if key.field_name in self._descriptor_model_data: - raise InvalidWriteError("Not allowed to deleted descriptor model data", key.field_name) - if key.scope not in self._allowed_scopes: raise InvalidScopeError(key.scope) - field_object = self._model_data_cache.find(key) + field_object = self._field_data_cache.find(key) if field_object is None: raise KeyError(key.field_name) @@ -406,16 +367,10 @@ class LmsKeyValueStore(KeyValueStore): field_object.delete() def has(self, key): - if key.field_name in self._descriptor_model_data: - return key.field_name in self._descriptor_model_data - - if key.scope == Scope.parent: - return True - if key.scope not in self._allowed_scopes: raise InvalidScopeError(key.scope) - field_object = self._model_data_cache.find(key) + field_object = self._field_data_cache.find(key) if field_object is None: return False @@ -423,6 +378,3 @@ class LmsKeyValueStore(KeyValueStore): return key.field_name in json.loads(field_object.state) else: return True - - -LmsUsage = namedtuple('LmsUsage', 'id, def_id') diff --git a/lms/djangoapps/courseware/models.py b/lms/djangoapps/courseware/models.py index 79f1534f41d..a1645439302 100644 --- a/lms/djangoapps/courseware/models.py +++ b/lms/djangoapps/courseware/models.py @@ -102,40 +102,9 @@ class StudentModuleHistory(models.Model): history_entry.save() -class XModuleContentField(models.Model): +class XModuleUserStateSummaryField(models.Model): """ - Stores data set in the Scope.content scope by an xmodule field - """ - - class Meta: - unique_together = (('definition_id', 'field_name'),) - - # The name of the field - field_name = models.CharField(max_length=64, db_index=True) - - # The definition id for the module - definition_id = models.CharField(max_length=255, db_index=True) - - # The value of the field. Defaults to None dumped as json - value = models.TextField(default='null') - - created = models.DateTimeField(auto_now_add=True, db_index=True) - modified = models.DateTimeField(auto_now=True, db_index=True) - - def __repr__(self): - return 'XModuleContentField<%r>' % ({ - 'field_name': self.field_name, - 'definition_id': self.definition_id, - 'value': self.value, - },) - - def __unicode__(self): - return unicode(repr(self)) - - -class XModuleSettingsField(models.Model): - """ - Stores data set in the Scope.settings scope by an xmodule field + Stores data set in the Scope.user_state_summary scope by an xmodule field """ class Meta: @@ -144,17 +113,17 @@ class XModuleSettingsField(models.Model): # The name of the field field_name = models.CharField(max_length=64, db_index=True) - # The usage id for the module + # The definition id for the module usage_id = models.CharField(max_length=255, db_index=True) - # The value of the field. Defaults to None, dumped as json + # The value of the field. Defaults to None dumped as json value = models.TextField(default='null') created = models.DateTimeField(auto_now_add=True, db_index=True) modified = models.DateTimeField(auto_now=True, db_index=True) def __repr__(self): - return 'XModuleSettingsField<%r>' % ({ + return 'XModuleUserStateSummaryField<%r>' % ({ 'field_name': self.field_name, 'usage_id': self.usage_id, 'value': self.value, diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py index 981ccde4419..53f9c57f38a 100644 --- a/lms/djangoapps/courseware/module_render.py +++ b/lms/djangoapps/courseware/module_render.py @@ -33,12 +33,12 @@ from student.models import unique_id_for_user from courseware.access import has_access from courseware.masquerade import setup_masquerade -from courseware.model_data import LmsKeyValueStore, LmsUsage, ModelDataCache +from courseware.model_data import FieldDataCache, DjangoKeyValueStore from xblock.runtime import KeyValueStore -from xblock.core import Scope -from courseware.models import StudentModule +from xblock.fields import Scope from util.sandboxing import can_execute_unsafe_code from util.json_request import JsonResponse +from lms.xblock.field_data import lms_field_data log = logging.getLogger(__name__) @@ -67,7 +67,7 @@ def make_track_function(request): return function -def toc_for_course(user, request, course, active_chapter, active_section, model_data_cache): +def toc_for_course(user, request, course, active_chapter, active_section, field_data_cache): ''' Create a table of contents from the module store @@ -88,16 +88,16 @@ def toc_for_course(user, request, course, active_chapter, active_section, model_ NOTE: assumes that if we got this far, user has access to course. Returns None if this is not the case. - model_data_cache must include data from the course module and 2 levels of its descendents + field_data_cache must include data from the course module and 2 levels of its descendents ''' - course_module = get_module_for_descriptor(user, request, course, model_data_cache, course.id) + course_module = get_module_for_descriptor(user, request, course, field_data_cache, course.id) if course_module is None: return None chapters = list() for chapter in course_module.get_display_items(): - if chapter.lms.hide_from_toc: + if chapter.hide_from_toc: continue sections = list() @@ -106,13 +106,13 @@ def toc_for_course(user, request, course, active_chapter, active_section, model_ active = (chapter.url_name == active_chapter and section.url_name == active_section) - if not section.lms.hide_from_toc: + if not section.hide_from_toc: sections.append({'display_name': section.display_name_with_default, 'url_name': section.url_name, - 'format': section.lms.format if section.lms.format is not None else '', - 'due': section.lms.due, + 'format': section.format if section.format is not None else '', + 'due': section.due, 'active': active, - 'graded': section.lms.graded, + 'graded': section.graded, }) chapters.append({'display_name': chapter.display_name_with_default, @@ -122,7 +122,7 @@ def toc_for_course(user, request, course, active_chapter, active_section, model_ return chapters -def get_module(user, request, location, model_data_cache, course_id, +def get_module(user, request, location, field_data_cache, course_id, position=None, not_found_ok=False, wrap_xmodule_display=True, grade_bucket_type=None, depth=0, static_asset_path=''): @@ -136,7 +136,7 @@ def get_module(user, request, location, model_data_cache, course_id, - request : current django HTTPrequest. Note: request.user isn't used for anything--all auth and such works based on user. - location : A Location-like object identifying the module to load - - model_data_cache : a ModelDataCache + - field_data_cache : a FieldDataCache - course_id : the course_id in the context of which to load module - position : extra information from URL for user-specified position within module @@ -154,7 +154,7 @@ def get_module(user, request, location, model_data_cache, course_id, try: location = Location(location) descriptor = modulestore().get_instance(course_id, location, depth=depth) - return get_module_for_descriptor(user, request, descriptor, model_data_cache, course_id, + return get_module_for_descriptor(user, request, descriptor, field_data_cache, course_id, position=position, wrap_xmodule_display=wrap_xmodule_display, grade_bucket_type=grade_bucket_type, @@ -184,7 +184,7 @@ def get_xqueue_callback_url_prefix(request): return settings.XQUEUE_INTERFACE.get('callback_url', prefix) -def get_module_for_descriptor(user, request, descriptor, model_data_cache, course_id, +def get_module_for_descriptor(user, request, descriptor, field_data_cache, course_id, position=None, wrap_xmodule_display=True, grade_bucket_type=None, static_asset_path=''): """ @@ -199,13 +199,13 @@ def get_module_for_descriptor(user, request, descriptor, model_data_cache, cours track_function = make_track_function(request) xqueue_callback_url_prefix = get_xqueue_callback_url_prefix(request) - return get_module_for_descriptor_internal(user, descriptor, model_data_cache, course_id, + return get_module_for_descriptor_internal(user, descriptor, field_data_cache, course_id, track_function, xqueue_callback_url_prefix, position, wrap_xmodule_display, grade_bucket_type, static_asset_path) -def get_module_for_descriptor_internal(user, descriptor, model_data_cache, course_id, +def get_module_for_descriptor_internal(user, descriptor, field_data_cache, course_id, track_function, xqueue_callback_url_prefix, position=None, wrap_xmodule_display=True, grade_bucket_type=None, static_asset_path=''): @@ -289,34 +289,29 @@ def get_module_for_descriptor_internal(user, descriptor, model_data_cache, cours """ # TODO: fix this so that make_xqueue_callback uses the descriptor passed into # inner_get_module, not the parent's callback. Add it as an argument.... - return get_module_for_descriptor_internal(user, descriptor, model_data_cache, course_id, + return get_module_for_descriptor_internal(user, descriptor, field_data_cache, course_id, track_function, make_xqueue_callback, position, wrap_xmodule_display, grade_bucket_type, static_asset_path) - def xblock_model_data(descriptor): - return DbModel( - LmsKeyValueStore(descriptor._model_data, model_data_cache), - descriptor.module_class, - user.id, - LmsUsage(descriptor.location, descriptor.location) - ) + def xblock_field_data(descriptor): + student_data = DbModel(DjangoKeyValueStore(field_data_cache)) + return lms_field_data(descriptor._field_data, student_data) def publish(event): """A function that allows XModules to publish events. This only supports grade changes right now.""" if event.get('event_name') != 'grade': return - usage = LmsUsage(descriptor.location, descriptor.location) # Construct the key for the module key = KeyValueStore.Key( scope=Scope.user_state, student_id=user.id, - block_scope_id=usage.id, + block_scope_id=descriptor.location, field_name='grade' ) - student_module = model_data_cache.find_or_create(key) + student_module = field_data_cache.find_or_create(key) # Update the grades student_module.grade = event.get('value') student_module.max_grade = event.get('max_value') @@ -359,7 +354,7 @@ def get_module_for_descriptor_internal(user, descriptor, model_data_cache, cours static_replace.replace_static_urls, data_directory=getattr(descriptor, 'data_dir', None), course_id=course_id, - static_asset_path=static_asset_path or descriptor.lms.static_asset_path, + static_asset_path=static_asset_path or descriptor.static_asset_path, ), replace_course_urls=partial( static_replace.replace_course_urls, @@ -371,7 +366,7 @@ def get_module_for_descriptor_internal(user, descriptor, model_data_cache, cours jump_to_id_base_url=reverse('jump_to_id', kwargs={'course_id': course_id, 'module_id': ''}) ), node_path=settings.NODE_PATH, - xblock_model_data=xblock_model_data, + xblock_field_data=xblock_field_data, publish=publish, anonymous_student_id=unique_id_for_user(user), course_id=course_id, @@ -379,6 +374,8 @@ def get_module_for_descriptor_internal(user, descriptor, model_data_cache, cours s3_interface=s3_interface, cache=cache, can_execute_unsafe_code=(lambda: can_execute_unsafe_code(course_id)), + # TODO: When we merge the descriptor and module systems, we can stop reaching into the mixologist (cpennington) + mixins=descriptor.system.mixologist._mixins, ) # pass position specified in URL to module through ModuleSystem @@ -419,7 +416,7 @@ def get_module_for_descriptor_internal(user, descriptor, model_data_cache, cours _get_html, getattr(descriptor, 'data_dir', None), course_id=course_id, - static_asset_path=static_asset_path or descriptor.lms.static_asset_path + static_asset_path=static_asset_path or descriptor.static_asset_path ) # Allow URLs of the form '/course/' refer to the root of multicourse directory @@ -453,14 +450,14 @@ def find_target_student_module(request, user_id, course_id, mod_id): Retrieve target StudentModule """ user = User.objects.get(id=user_id) - model_data_cache = ModelDataCache.cache_for_descriptor_descendents( + field_data_cache = FieldDataCache.cache_for_descriptor_descendents( course_id, user, modulestore().get_instance(course_id, mod_id), depth=0, select_for_update=True ) - instance = get_module(user, request, mod_id, model_data_cache, course_id, grade_bucket_type='xqueue') + instance = get_module(user, request, mod_id, field_data_cache, course_id, grade_bucket_type='xqueue') if instance is None: msg = "No module {0} for user {1}--access denied?".format(mod_id, user) log.debug(msg) @@ -554,13 +551,13 @@ def modx_dispatch(request, dispatch, location, course_id): ) raise Http404 - model_data_cache = ModelDataCache.cache_for_descriptor_descendents( + field_data_cache = FieldDataCache.cache_for_descriptor_descendents( course_id, request.user, descriptor ) - instance = get_module(request.user, request, location, model_data_cache, course_id, grade_bucket_type='ajax') + instance = get_module(request.user, request, location, field_data_cache, course_id, grade_bucket_type='ajax') if instance is None: # Either permissions just changed, or someone is trying to be clever # and load something they shouldn't have access to. diff --git a/lms/djangoapps/courseware/tabs.py b/lms/djangoapps/courseware/tabs.py index d7e1158e99d..ce49e5a2013 100644 --- a/lms/djangoapps/courseware/tabs.py +++ b/lms/djangoapps/courseware/tabs.py @@ -21,7 +21,7 @@ from .module_render import get_module from courseware.access import has_access from xmodule.modulestore import Location from xmodule.modulestore.django import modulestore -from courseware.model_data import ModelDataCache +from courseware.model_data import FieldDataCache from open_ended_grading import open_ended_notifications @@ -378,10 +378,10 @@ def get_static_tab_by_slug(course, tab_slug): def get_static_tab_contents(request, course, tab): loc = Location(course.location.tag, course.location.org, course.location.course, 'static_tab', tab['url_slug']) - model_data_cache = ModelDataCache.cache_for_descriptor_descendents(course.id, + field_data_cache = FieldDataCache.cache_for_descriptor_descendents(course.id, request.user, modulestore().get_instance(course.id, loc), depth=0) - tab_module = get_module(request.user, request, loc, model_data_cache, course.id, - static_asset_path=course.lms.static_asset_path) + tab_module = get_module(request.user, request, loc, field_data_cache, course.id, + static_asset_path=course.static_asset_path) logging.debug('course_module = {0}'.format(tab_module)) diff --git a/lms/djangoapps/courseware/tests/__init__.py b/lms/djangoapps/courseware/tests/__init__.py index 64845688fb9..88129cc8d1c 100644 --- a/lms/djangoapps/courseware/tests/__init__.py +++ b/lms/djangoapps/courseware/tests/__init__.py @@ -13,11 +13,14 @@ from django.test.client import Client from student.tests.factories import UserFactory, CourseEnrollmentFactory from courseware.tests.modulestore_config import TEST_DATA_MIXED_MODULESTORE +from xblock.field_data import DictFieldData +from xblock.fields import Scope from xmodule.tests import get_test_system from xmodule.modulestore import Location from xmodule.modulestore.django import modulestore from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase +from lms.xblock.field_data import lms_field_data @override_settings(MODULESTORE=TEST_DATA_MIXED_MODULESTORE) @@ -45,6 +48,12 @@ class BaseTestXmodule(ModuleStoreTestCase): DATA = '' MODEL_DATA = {'data': '<some_module></some_module>'} + def xblock_field_data(self, descriptor): + field_data = {} + field_data.update(self.MODEL_DATA) + student_data = DictFieldData(field_data) + return lms_field_data(descriptor._field_data, student_data) + def setUp(self): self.course = CourseFactory.create(data=self.COURSE_DATA) @@ -82,14 +91,9 @@ class BaseTestXmodule(ModuleStoreTestCase): # different code paths while maintaining the type returned by render_template self.runtime.render_template = lambda template, context: u'{!r}, {!r}'.format(template, sorted(context.items())) - model_data = {'location': self.item_descriptor.location} - model_data.update(self.MODEL_DATA) + self.runtime.xblock_field_data = self.xblock_field_data - self.item_module = self.item_descriptor.module_class( - self.runtime, - self.item_descriptor, - model_data - ) + self.item_module = self.item_descriptor.xmodule(self.runtime) self.item_url = Location(self.item_module.location).url() diff --git a/lms/djangoapps/courseware/tests/factories.py b/lms/djangoapps/courseware/tests/factories.py index 69f8f54eec6..9f22dd3e74c 100644 --- a/lms/djangoapps/courseware/tests/factories.py +++ b/lms/djangoapps/courseware/tests/factories.py @@ -8,7 +8,7 @@ from student.tests.factories import GroupFactory as StudentGroupFactory from student.tests.factories import UserProfileFactory as StudentUserProfileFactory from student.tests.factories import CourseEnrollmentAllowedFactory as StudentCourseEnrollmentAllowedFactory from student.tests.factories import RegistrationFactory as StudentRegistrationFactory -from courseware.models import StudentModule, XModuleContentField, XModuleSettingsField +from courseware.models import StudentModule, XModuleUserStateSummaryField from courseware.models import XModuleStudentInfoField, XModuleStudentPrefsField from xmodule.modulestore import Location @@ -53,20 +53,12 @@ class StudentModuleFactory(DjangoModelFactory): done = 'na' -class ContentFactory(DjangoModelFactory): - FACTORY_FOR = XModuleContentField +class UserStateSummaryFactory(DjangoModelFactory): + FACTORY_FOR = XModuleUserStateSummaryField field_name = 'existing_field' value = json.dumps('old_value') - definition_id = location('def_id').url() - - -class SettingsFactory(DjangoModelFactory): - FACTORY_FOR = XModuleSettingsField - - field_name = 'existing_field' - value = json.dumps('old_value') - usage_id = '%s-%s' % ('edX/test_course/test', location('def_id').url()) + usage_id = location('def_id').url() class StudentPrefsFactory(DjangoModelFactory): diff --git a/lms/djangoapps/courseware/tests/test_model_data.py b/lms/djangoapps/courseware/tests/test_model_data.py index d8682d3d5cb..bde57c15534 100644 --- a/lms/djangoapps/courseware/tests/test_model_data.py +++ b/lms/djangoapps/courseware/tests/test_model_data.py @@ -5,17 +5,17 @@ import json from mock import Mock, patch from functools import partial -from courseware.model_data import LmsKeyValueStore, InvalidWriteError -from courseware.model_data import InvalidScopeError, ModelDataCache -from courseware.models import StudentModule, XModuleContentField, XModuleSettingsField +from courseware.model_data import DjangoKeyValueStore +from courseware.model_data import InvalidScopeError, FieldDataCache +from courseware.models import StudentModule, XModuleUserStateSummaryField from courseware.models import XModuleStudentInfoField, XModuleStudentPrefsField from student.tests.factories import UserFactory from courseware.tests.factories import StudentModuleFactory as cmfStudentModuleFactory -from courseware.tests.factories import ContentFactory, SettingsFactory +from courseware.tests.factories import UserStateSummaryFactory from courseware.tests.factories import StudentPrefsFactory, StudentInfoFactory -from xblock.core import Scope, BlockScope +from xblock.fields import Scope, BlockScope from xmodule.modulestore import Location from django.test import TestCase from django.db import DatabaseError @@ -29,22 +29,22 @@ def mock_field(scope, name): return field -def mock_descriptor(fields=[], lms_fields=[]): +def mock_descriptor(fields=[]): descriptor = Mock() descriptor.location = location('def_id') - descriptor.module_class.fields = fields - descriptor.module_class.lms.fields = lms_fields + descriptor.module_class.fields.values.return_value = fields + descriptor.fields.values.return_value = fields descriptor.module_class.__name__ = 'MockProblemModule' return descriptor location = partial(Location, 'i4x', 'edX', 'test_course', 'problem') course_id = 'edX/test_course/test' -content_key = partial(LmsKeyValueStore.Key, Scope.content, None, location('def_id')) -settings_key = partial(LmsKeyValueStore.Key, Scope.settings, None, location('def_id')) -user_state_key = partial(LmsKeyValueStore.Key, Scope.user_state, 'user', location('def_id')) -prefs_key = partial(LmsKeyValueStore.Key, Scope.preferences, 'user', 'MockProblemModule') -user_info_key = partial(LmsKeyValueStore.Key, Scope.user_info, 'user', None) +user_state_summary_key = partial(DjangoKeyValueStore.Key, Scope.user_state_summary, None, location('def_id')) +settings_key = partial(DjangoKeyValueStore.Key, Scope.settings, None, location('def_id')) +user_state_key = partial(DjangoKeyValueStore.Key, Scope.user_state, 'user', location('def_id')) +prefs_key = partial(DjangoKeyValueStore.Key, Scope.preferences, 'user', 'MockProblemModule') +user_info_key = partial(DjangoKeyValueStore.Key, Scope.user_info, 'user', None) class StudentModuleFactory(cmfStudentModuleFactory): @@ -52,48 +52,17 @@ class StudentModuleFactory(cmfStudentModuleFactory): course_id = course_id -class TestDescriptorFallback(TestCase): - - def setUp(self): - self.desc_md = { - 'field_a': 'content', - 'field_b': 'settings', - } - self.kvs = LmsKeyValueStore(self.desc_md, None) - - def test_get_from_descriptor(self): - self.assertEquals('content', self.kvs.get(content_key('field_a'))) - self.assertEquals('settings', self.kvs.get(settings_key('field_b'))) - - def test_write_to_descriptor(self): - self.assertRaises(InvalidWriteError, self.kvs.set, content_key('field_a'), 'foo') - self.assertEquals('content', self.desc_md['field_a']) - self.assertRaises(InvalidWriteError, self.kvs.set, settings_key('field_b'), 'foo') - self.assertEquals('settings', self.desc_md['field_b']) - - self.assertRaises(InvalidWriteError, self.kvs.set_many, {content_key('field_a'): 'foo'}) - self.assertEquals('content', self.desc_md['field_a']) - - self.assertRaises(InvalidWriteError, self.kvs.delete, content_key('field_a')) - self.assertEquals('content', self.desc_md['field_a']) - self.assertRaises(InvalidWriteError, self.kvs.delete, settings_key('field_b')) - self.assertEquals('settings', self.desc_md['field_b']) - - - - class TestInvalidScopes(TestCase): def setUp(self): - self.desc_md = {} self.user = UserFactory.create(username='user') - self.mdc = ModelDataCache([mock_descriptor([mock_field(Scope.user_state, 'a_field')])], course_id, self.user) - self.kvs = LmsKeyValueStore(self.desc_md, self.mdc) + self.field_data_cache = FieldDataCache([mock_descriptor([mock_field(Scope.user_state, 'a_field')])], course_id, self.user) + self.kvs = DjangoKeyValueStore(self.field_data_cache) def test_invalid_scopes(self): for scope in (Scope(user=True, block=BlockScope.DEFINITION), Scope(user=False, block=BlockScope.TYPE), Scope(user=False, block=BlockScope.ALL)): - key = LmsKeyValueStore.Key(scope, None, None, 'field') + key = DjangoKeyValueStore.Key(scope, None, None, 'field') self.assertRaises(InvalidScopeError, self.kvs.get, key) self.assertRaises(InvalidScopeError, self.kvs.set, key, 'value') @@ -105,11 +74,10 @@ class TestInvalidScopes(TestCase): class TestStudentModuleStorage(TestCase): def setUp(self): - self.desc_md = {} student_module = StudentModuleFactory(state=json.dumps({'a_field': 'a_value', 'b_field': 'b_value'})) self.user = student_module.student - self.mdc = ModelDataCache([mock_descriptor([mock_field(Scope.user_state, 'a_field')])], course_id, self.user) - self.kvs = LmsKeyValueStore(self.desc_md, self.mdc) + self.field_data_cache = FieldDataCache([mock_descriptor([mock_field(Scope.user_state, 'a_field')])], course_id, self.user) + self.kvs = DjangoKeyValueStore(self.field_data_cache) def test_get_existing_field(self): "Test that getting an existing field in an existing StudentModule works" @@ -184,9 +152,8 @@ class TestStudentModuleStorage(TestCase): class TestMissingStudentModule(TestCase): def setUp(self): self.user = UserFactory.create(username='user') - self.desc_md = {} - self.mdc = ModelDataCache([mock_descriptor()], course_id, self.user) - self.kvs = LmsKeyValueStore(self.desc_md, self.mdc) + self.field_data_cache = FieldDataCache([mock_descriptor()], course_id, self.user) + self.kvs = DjangoKeyValueStore(self.field_data_cache) def test_get_field_from_missing_student_module(self): "Test that getting a field from a missing StudentModule raises a KeyError" @@ -194,12 +161,12 @@ class TestMissingStudentModule(TestCase): def test_set_field_in_missing_student_module(self): "Test that setting a field in a missing StudentModule creates the student module" - self.assertEquals(0, len(self.mdc.cache)) + self.assertEquals(0, len(self.field_data_cache.cache)) self.assertEquals(0, StudentModule.objects.all().count()) self.kvs.set(user_state_key('a_field'), 'a_value') - self.assertEquals(1, len(self.mdc.cache)) + self.assertEquals(1, len(self.field_data_cache.cache)) self.assertEquals(1, StudentModule.objects.all().count()) student_module = StudentModule.objects.all()[0] @@ -237,12 +204,12 @@ class StorageTestBase(object): self.user = field_storage.student else: self.user = UserFactory.create() - self.desc_md = {} self.mock_descriptor = mock_descriptor([ mock_field(self.scope, 'existing_field'), mock_field(self.scope, 'other_existing_field')]) - self.mdc = ModelDataCache([self.mock_descriptor], course_id, self.user) - self.kvs = LmsKeyValueStore(self.desc_md, self.mdc) + self.field_data_cache = FieldDataCache([self.mock_descriptor], course_id, self.user) + self.kvs = DjangoKeyValueStore(self.field_data_cache) + def test_set_and_get_existing_field(self): self.kvs.set(self.key_factory('existing_field'), 'test_value') @@ -318,18 +285,11 @@ class StorageTestBase(object): self.assertEquals(exception.saved_field_names[0], 'existing_field') -class TestSettingsStorage(StorageTestBase, TestCase): - factory = SettingsFactory - scope = Scope.settings - key_factory = settings_key - storage_class = XModuleSettingsField - - class TestContentStorage(StorageTestBase, TestCase): - factory = ContentFactory - scope = Scope.content - key_factory = content_key - storage_class = XModuleContentField + factory = UserStateSummaryFactory + scope = Scope.user_state_summary + key_factory = user_state_summary_key + storage_class = XModuleUserStateSummaryField class TestStudentPrefsStorage(StorageTestBase, TestCase): diff --git a/lms/djangoapps/courseware/tests/test_module_render.py b/lms/djangoapps/courseware/tests/test_module_render.py index 911136da773..26f27bd90ac 100644 --- a/lms/djangoapps/courseware/tests/test_module_render.py +++ b/lms/djangoapps/courseware/tests/test_module_render.py @@ -16,8 +16,8 @@ from xmodule.modulestore.tests.factories import ItemFactory, CourseFactory from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase import courseware.module_render as render from courseware.tests.tests import LoginEnrollmentTestCase -from courseware.model_data import ModelDataCache from courseware.tests.modulestore_config import TEST_DATA_MIXED_MODULESTORE +from courseware.model_data import FieldDataCache from courseware.courses import get_course_with_access, course_image_url, get_course_info_section @@ -62,14 +62,14 @@ class ModuleRenderTestCase(ModuleStoreTestCase, LoginEnrollmentTestCase): course = get_course_with_access(self.mock_user, self.course_id, 'load') - model_data_cache = ModelDataCache.cache_for_descriptor_descendents( + field_data_cache = FieldDataCache.cache_for_descriptor_descendents( self.course_id, self.mock_user, course, depth=2) module = render.get_module( self.mock_user, mock_request, ['i4x', 'edX', 'toy', 'html', 'toyjumpto'], - model_data_cache, + field_data_cache, self.course_id ) @@ -210,7 +210,7 @@ class TestTOC(TestCase): chapter_url = '%s/%s/%s' % ('/courses', self.course_name, chapter) factory = RequestFactory() request = factory.get(chapter_url) - model_data_cache = ModelDataCache.cache_for_descriptor_descendents( + field_data_cache = FieldDataCache.cache_for_descriptor_descendents( self.toy_course.id, self.portal_user, self.toy_course, depth=2) expected = ([{'active': True, 'sections': @@ -228,7 +228,7 @@ class TestTOC(TestCase): 'format': '', 'due': None, 'active': False}], 'url_name': 'secret:magic', 'display_name': 'secret:magic'}]) - actual = render.toc_for_course(self.portal_user, request, self.toy_course, chapter, None, model_data_cache) + actual = render.toc_for_course(self.portal_user, request, self.toy_course, chapter, None, field_data_cache) for toc_section in expected: self.assertIn(toc_section, actual) @@ -238,7 +238,7 @@ class TestTOC(TestCase): section = 'Welcome' factory = RequestFactory() request = factory.get(chapter_url) - model_data_cache = ModelDataCache.cache_for_descriptor_descendents( + field_data_cache = FieldDataCache.cache_for_descriptor_descendents( self.toy_course.id, self.portal_user, self.toy_course, depth=2) expected = ([{'active': True, 'sections': @@ -256,7 +256,7 @@ class TestTOC(TestCase): 'format': '', 'due': None, 'active': False}], 'url_name': 'secret:magic', 'display_name': 'secret:magic'}]) - actual = render.toc_for_course(self.portal_user, request, self.toy_course, chapter, section, model_data_cache) + actual = render.toc_for_course(self.portal_user, request, self.toy_course, chapter, section, field_data_cache) for toc_section in expected: self.assertIn(toc_section, actual) @@ -282,7 +282,7 @@ class TestHtmlModifiers(ModuleStoreTestCase): data=self.content_string + self.rewrite_link + self.rewrite_bad_link + self.course_link ) self.location = self.descriptor.location - self.model_data_cache = ModelDataCache.cache_for_descriptor_descendents( + self.field_data_cache = FieldDataCache.cache_for_descriptor_descendents( self.course.id, self.user, self.descriptor @@ -293,7 +293,7 @@ class TestHtmlModifiers(ModuleStoreTestCase): self.user, self.request, self.location, - self.model_data_cache, + self.field_data_cache, self.course.id, wrap_xmodule_display=True, ) @@ -306,7 +306,7 @@ class TestHtmlModifiers(ModuleStoreTestCase): self.user, self.request, self.location, - self.model_data_cache, + self.field_data_cache, self.course.id, wrap_xmodule_display=False, ) @@ -319,7 +319,7 @@ class TestHtmlModifiers(ModuleStoreTestCase): self.user, self.request, self.location, - self.model_data_cache, + self.field_data_cache, self.course.id, ) result_fragment = module.runtime.render(module, None, 'student_view') @@ -337,7 +337,7 @@ class TestHtmlModifiers(ModuleStoreTestCase): self.user, self.request, self.location, - self.model_data_cache, + self.field_data_cache, self.course.id, ) result_fragment = module.runtime.render(module, None, 'student_view') @@ -360,7 +360,7 @@ class TestHtmlModifiers(ModuleStoreTestCase): self.user, self.request, self.location, - self.model_data_cache, + self.field_data_cache, self.course.id, static_asset_path="toy_course_dir", ) @@ -371,13 +371,13 @@ class TestHtmlModifiers(ModuleStoreTestCase): url = course_image_url(self.course) self.assertTrue(url.startswith('/c4x/')) - self.course.lms.static_asset_path = "toy_course_dir" + self.course.static_asset_path = "toy_course_dir" url = course_image_url(self.course) self.assertTrue(url.startswith('/static/toy_course_dir/')) - self.course.lms.static_asset_path = "" + self.course.static_asset_path = "" def test_get_course_info_section(self): - self.course.lms.static_asset_path = "toy_course_dir" + self.course.static_asset_path = "toy_course_dir" get_course_info_section(self.request, self.course, "handouts") # NOTE: check handouts output...right now test course seems to have no such content # at least this makes sure get_course_info_section returns without exception @@ -387,7 +387,7 @@ class TestHtmlModifiers(ModuleStoreTestCase): self.user, self.request, self.location, - self.model_data_cache, + self.field_data_cache, self.course.id, ) result_fragment = module.runtime.render(module, None, 'student_view') @@ -405,7 +405,7 @@ class TestHtmlModifiers(ModuleStoreTestCase): self.user, self.request, self.location, - self.model_data_cache, + self.field_data_cache, self.course.id, ) result_fragment = module.runtime.render(module, None, 'student_view') diff --git a/lms/djangoapps/courseware/tests/test_submitting_problems.py b/lms/djangoapps/courseware/tests/test_submitting_problems.py index f8cfaefd754..8857425d905 100644 --- a/lms/djangoapps/courseware/tests/test_submitting_problems.py +++ b/lms/djangoapps/courseware/tests/test_submitting_problems.py @@ -11,7 +11,7 @@ from django.test.utils import override_settings # Need access to internal func to put users in the right group from courseware import grades -from courseware.model_data import ModelDataCache +from courseware.model_data import FieldDataCache from xmodule.modulestore.django import modulestore, editable_modulestore @@ -234,14 +234,14 @@ class TestCourseGrader(TestSubmittingProblems): make up the final grade. (For display) """ - model_data_cache = ModelDataCache.cache_for_descriptor_descendents( + field_data_cache = FieldDataCache.cache_for_descriptor_descendents( self.course.id, self.student_user, self.course) fake_request = self.factory.get(reverse('progress', kwargs={'course_id': self.course.id})) return grades.grade(self.student_user, fake_request, - self.course, model_data_cache) + self.course, field_data_cache) def get_progress_summary(self): """ @@ -255,7 +255,7 @@ class TestCourseGrader(TestSubmittingProblems): etc. """ - model_data_cache = ModelDataCache.cache_for_descriptor_descendents( + field_data_cache = FieldDataCache.cache_for_descriptor_descendents( self.course.id, self.student_user, self.course) fake_request = self.factory.get(reverse('progress', @@ -264,7 +264,7 @@ class TestCourseGrader(TestSubmittingProblems): progress_summary = grades.progress_summary(self.student_user, fake_request, self.course, - model_data_cache) + field_data_cache) return progress_summary def check_grade_percent(self, percent): diff --git a/lms/djangoapps/courseware/tests/test_video_mongo.py b/lms/djangoapps/courseware/tests/test_video_mongo.py index b393b33da85..ea4949e4c9b 100644 --- a/lms/djangoapps/courseware/tests/test_video_mongo.py +++ b/lms/djangoapps/courseware/tests/test_video_mongo.py @@ -12,12 +12,9 @@ class TestVideo(BaseTestXmodule): CATEGORY = "video" DATA = SOURCE_XML - MODEL_DATA = { - 'data': DATA - } def setUp(self): - # Since the VideoDescriptor changes `self._model_data`, + # Since the VideoDescriptor changes `self._field_data`, # we need to instantiate `self.item_module` through # `self.item_descriptor` rather than directly constructing it super(TestVideo, self).setUp() diff --git a/lms/djangoapps/courseware/tests/test_video_xml.py b/lms/djangoapps/courseware/tests/test_video_xml.py index d7901734685..ea17af723db 100644 --- a/lms/djangoapps/courseware/tests/test_video_xml.py +++ b/lms/djangoapps/courseware/tests/test_video_xml.py @@ -23,6 +23,8 @@ from django.conf import settings from xmodule.video_module import VideoDescriptor, _create_youtube_string from xmodule.modulestore import Location from xmodule.tests import get_test_system, LogicTest +from xblock.field_data import DictFieldData +from xblock.fields import ScopeIds SOURCE_XML = """ @@ -52,13 +54,13 @@ class VideoFactory(object): """Method return Video Xmodule instance.""" location = Location(["i4x", "edX", "video", "default", "SampleProblem1"]) - model_data = {'data': VideoFactory.sample_problem_xml_youtube, + field_data = {'data': VideoFactory.sample_problem_xml_youtube, 'location': location} system = get_test_system() system.render_template = lambda template, context: context - descriptor = VideoDescriptor(system, model_data) + descriptor = VideoDescriptor(system, DictFieldData(field_data), ScopeIds(None, None, None, None)) module = descriptor.xmodule(system) @@ -112,7 +114,7 @@ class VideoModuleLogicTest(LogicTest): descriptor_class = VideoDescriptor - raw_model_data = { + raw_field_data = { 'data': '<video />' } diff --git a/lms/djangoapps/courseware/tests/test_view_authentication.py b/lms/djangoapps/courseware/tests/test_view_authentication.py index 849e5fdc452..ed27ae8efae 100644 --- a/lms/djangoapps/courseware/tests/test_view_authentication.py +++ b/lms/djangoapps/courseware/tests/test_view_authentication.py @@ -359,7 +359,7 @@ class TestViewAuth(ModuleStoreTestCase, LoginEnrollmentTestCase): self.assertFalse(self.course.has_started()) # but should be accessible for beta testers - self.course.lms.days_early_for_beta = 2 + self.course.days_early_for_beta = 2 # student user shouldn't see it student_user = User.objects.get(email=student_email) diff --git a/lms/djangoapps/courseware/tests/tests.py b/lms/djangoapps/courseware/tests/tests.py index 31dfea8e5f3..2cb80b6eda4 100644 --- a/lms/djangoapps/courseware/tests/tests.py +++ b/lms/djangoapps/courseware/tests/tests.py @@ -136,7 +136,6 @@ class TestXmlCoursesLoad(ModuleStoreTestCase, PageLoaderTestCase): self.setup_user() def test_toy_course_loads(self): - # Load one of the XML based courses # Our test mapping rules allow the MixedModuleStore # to load this course from XML, not Mongo. diff --git a/lms/djangoapps/courseware/views.py b/lms/djangoapps/courseware/views.py index a53d549555b..b89fa8b825c 100644 --- a/lms/djangoapps/courseware/views.py +++ b/lms/djangoapps/courseware/views.py @@ -22,7 +22,7 @@ from courseware.courses import (get_courses, get_course_with_access, get_courses_by_university, sort_by_announcement) import courseware.tabs as tabs from courseware.masquerade import setup_masquerade -from courseware.model_data import ModelDataCache +from courseware.model_data import FieldDataCache from .module_render import toc_for_course, get_module_for_descriptor, get_module from courseware.models import StudentModule, StudentModuleHistory from course_modes.models import CourseMode @@ -78,7 +78,7 @@ def courses(request): return render_to_response("courseware/courses.html", {'courses': courses}) -def render_accordion(request, course, chapter, section, model_data_cache): +def render_accordion(request, course, chapter, section, field_data_cache): """ Draws navigation bar. Takes current position in accordion as parameter. @@ -93,7 +93,7 @@ def render_accordion(request, course, chapter, section, model_data_cache): # grab the table of contents user = User.objects.prefetch_related("groups").get(id=request.user.id) request.user = user # keep just one instance of User - toc = toc_for_course(user, request, course, chapter, section, model_data_cache) + toc = toc_for_course(user, request, course, chapter, section, field_data_cache) context = dict([('toc', toc), ('course_id', course.id), @@ -187,7 +187,7 @@ def check_for_active_timelimit_module(request, course_id, course): # get the corresponding section_descriptor for the given StudentModel entry: module_state_key = timelimit_student_module.module_state_key timelimit_descriptor = modulestore().get_instance(course_id, Location(module_state_key)) - timelimit_module_cache = ModelDataCache.cache_for_descriptor_descendents(course.id, request.user, + timelimit_module_cache = FieldDataCache.cache_for_descriptor_descendents(course.id, request.user, timelimit_descriptor, depth=None) timelimit_module = get_module_for_descriptor(request.user, request, timelimit_descriptor, timelimit_module_cache, course.id, position=None) @@ -208,7 +208,7 @@ def check_for_active_timelimit_module(request, course_id, course): return context -def update_timelimit_module(user, course_id, model_data_cache, timelimit_descriptor, timelimit_module): +def update_timelimit_module(user, course_id, field_data_cache, timelimit_descriptor, timelimit_module): """ Updates the state of the provided timing module, starting it if it hasn't begun. Returns dict with timer-related values to enable display of time remaining. @@ -308,10 +308,10 @@ def index(request, course_id, chapter=None, section=None, masq = setup_masquerade(request, staff_access) try: - model_data_cache = ModelDataCache.cache_for_descriptor_descendents( + field_data_cache = FieldDataCache.cache_for_descriptor_descendents( course.id, user, course, depth=2) - course_module = get_module_for_descriptor(user, request, course, model_data_cache, course.id) + course_module = get_module_for_descriptor(user, request, course, field_data_cache, course.id) if course_module is None: log.warning('If you see this, something went wrong: if we got this' ' far, should have gotten a course module for this user') @@ -322,7 +322,7 @@ def index(request, course_id, chapter=None, section=None, context = { 'csrf': csrf(request)['csrf_token'], - 'accordion': render_accordion(request, course, chapter, section, model_data_cache), + 'accordion': render_accordion(request, course, chapter, section, field_data_cache), 'COURSE_TITLE': course.display_name_with_default, 'course': course, 'init': '', @@ -373,11 +373,11 @@ def index(request, course_id, chapter=None, section=None, # Load all descendants of the section, because we're going to display its # html, which in general will need all of its children - section_model_data_cache = ModelDataCache.cache_for_descriptor_descendents( + section_field_data_cache = FieldDataCache.cache_for_descriptor_descendents( course_id, user, section_descriptor, depth=None) section_module = get_module(request.user, request, section_descriptor.location, - section_model_data_cache, course_id, position, depth=None) + section_field_data_cache, course_id, position, depth=None) if section_module is None: # User may be trying to be clever and access something @@ -694,12 +694,12 @@ def progress(request, course_id, student_id=None): # additional DB lookup (this kills the Progress page in particular). student = User.objects.prefetch_related("groups").get(id=student.id) - model_data_cache = ModelDataCache.cache_for_descriptor_descendents( + field_data_cache = FieldDataCache.cache_for_descriptor_descendents( course_id, student, course, depth=None) courseware_summary = grades.progress_summary(student, request, course, - model_data_cache) - grade_summary = grades.grade(student, request, course, model_data_cache) + field_data_cache) + grade_summary = grades.grade(student, request, course, field_data_cache) if courseware_summary is None: #This means the student didn't have access to the course (which the instructor requested) diff --git a/lms/djangoapps/django_comment_client/utils.py b/lms/djangoapps/django_comment_client/utils.py index 9309f919d39..0466fb526dd 100644 --- a/lms/djangoapps/django_comment_client/utils.py +++ b/lms/djangoapps/django_comment_client/utils.py @@ -165,8 +165,8 @@ def initialize_discussion_info(course): category = " / ".join([x.strip() for x in category.split("/")]) last_category = category.split("/")[-1] discussion_id_map[id] = {"location": module.location, "title": last_category + " / " + title} - #Handle case where module.lms.start is None - entry_start_date = module.lms.start if module.lms.start else datetime.max.replace(tzinfo=pytz.UTC) + #Handle case where module.start is None + entry_start_date = module.start if module.start else datetime.max.replace(tzinfo=pytz.UTC) unexpanded_category_map[category].append({"title": title, "id": id, "sort_key": sort_key, "start_date": entry_start_date}) category_map = {"entries": defaultdict(dict), "subcategories": defaultdict(dict)} diff --git a/lms/djangoapps/instructor/hint_manager.py b/lms/djangoapps/instructor/hint_manager.py index f97115f19b3..9f555b9c56f 100644 --- a/lms/djangoapps/instructor/hint_manager.py +++ b/lms/djangoapps/instructor/hint_manager.py @@ -16,7 +16,7 @@ from django_future.csrf import ensure_csrf_cookie from mitxmako.shortcuts import render_to_response, render_to_string from courseware.courses import get_course_with_access -from courseware.models import XModuleContentField +from courseware.models import XModuleUserStateSummaryField import courseware.module_render as module_render import courseware.model_data as model_data from xmodule.modulestore import Location @@ -87,11 +87,11 @@ def get_hints(request, course_id, field): field_label = 'Approved Hints' other_field_label = 'Hints Awaiting Moderation' # The course_id is of the form school/number/classname. - # We want to use the course_id to find all matching definition_id's. + # We want to use the course_id to find all matching usage_id's. # To do this, just take the school/number part - leave off the classname. chopped_id = '/'.join(course_id.split('/')[:-1]) chopped_id = re.escape(chopped_id) - all_hints = XModuleContentField.objects.filter(field_name=field, definition_id__regex=chopped_id) + all_hints = XModuleUserStateSummaryField.objects.filter(field_name=field, usage_id__regex=chopped_id) # big_out_dict[problem id] = [[answer, {pk: [hint, votes]}], sorted by answer] # big_out_dict maps a problem id to a list of [answer, hints] pairs, sorted in order of answer. big_out_dict = {} @@ -100,11 +100,11 @@ def get_hints(request, course_id, field): id_to_name = {} for hints_by_problem in all_hints: - loc = Location(hints_by_problem.definition_id) + loc = Location(hints_by_problem.usage_id) name = location_to_problem_name(course_id, loc) if name is None: continue - id_to_name[hints_by_problem.definition_id] = name + id_to_name[hints_by_problem.usage_id] = name def answer_sorter(thing): """ @@ -120,7 +120,7 @@ def get_hints(request, course_id, field): # Answer list contains [answer, dict_of_hints] pairs. answer_list = sorted(json.loads(hints_by_problem.value).items(), key=answer_sorter) - big_out_dict[hints_by_problem.definition_id] = answer_list + big_out_dict[hints_by_problem.usage_id] = answer_list render_dict = {'field': field, 'other_field': other_field, @@ -165,7 +165,7 @@ def delete_hints(request, course_id, field): problem_id, answer, pk = request.POST.getlist(key) # Can be optimized - sort the delete list by problem_id, and load each problem # from the database only once. - this_problem = XModuleContentField.objects.get(field_name=field, definition_id=problem_id) + this_problem = XModuleUserStateSummaryField.objects.get(field_name=field, usage_id=problem_id) problem_dict = json.loads(this_problem.value) del problem_dict[answer][pk] this_problem.value = json.dumps(problem_dict) @@ -190,7 +190,7 @@ def change_votes(request, course_id, field): if key == 'op' or key == 'field': continue problem_id, answer, pk, new_votes = request.POST.getlist(key) - this_problem = XModuleContentField.objects.get(field_name=field, definition_id=problem_id) + this_problem = XModuleUserStateSummaryField.objects.get(field_name=field, usage_id=problem_id) problem_dict = json.loads(this_problem.value) # problem_dict[answer][pk] points to a [hint_text, #votes] pair. problem_dict[answer][pk][1] = int(new_votes) @@ -216,16 +216,16 @@ def add_hint(request, course_id, field): # is annoying. loc = Location(problem_id) descriptors = modulestore().get_items(loc, course_id=course_id) - m_d_c = model_data.ModelDataCache(descriptors, course_id, request.user) - hinter_module = module_render.get_module(request.user, request, loc, m_d_c, course_id) + field_data_cache = model_data.FieldDataCache(descriptors, course_id, request.user) + hinter_module = module_render.get_module(request.user, request, loc, field_data_cache, course_id) if not hinter_module.validate_answer(answer): # Invalid answer. Don't add it to the database, or else the # hinter will crash when we encounter it. return 'Error - the answer you specified is not properly formatted: ' + str(answer) - this_problem = XModuleContentField.objects.get(field_name=field, definition_id=problem_id) + this_problem = XModuleUserStateSummaryField.objects.get(field_name=field, usage_id=problem_id) - hint_pk_entry = XModuleContentField.objects.get(field_name='hint_pk', definition_id=problem_id) + hint_pk_entry = XModuleUserStateSummaryField.objects.get(field_name='hint_pk', usage_id=problem_id) this_pk = int(hint_pk_entry.value) hint_pk_entry.value = this_pk + 1 hint_pk_entry.save() @@ -254,14 +254,14 @@ def approve(request, course_id, field): problem_id, answer, pk = request.POST.getlist(key) # Can be optimized - sort the delete list by problem_id, and load each problem # from the database only once. - problem_in_mod = XModuleContentField.objects.get(field_name=field, definition_id=problem_id) + problem_in_mod = XModuleUserStateSummaryField.objects.get(field_name=field, usage_id=problem_id) problem_dict = json.loads(problem_in_mod.value) hint_to_move = problem_dict[answer][pk] del problem_dict[answer][pk] problem_in_mod.value = json.dumps(problem_dict) problem_in_mod.save() - problem_in_hints = XModuleContentField.objects.get(field_name='hints', definition_id=problem_id) + problem_in_hints = XModuleUserStateSummaryField.objects.get(field_name='hints', usage_id=problem_id) problem_dict = json.loads(problem_in_hints.value) if answer not in problem_dict: problem_dict[answer] = {} diff --git a/lms/djangoapps/instructor/tests/test_hint_manager.py b/lms/djangoapps/instructor/tests/test_hint_manager.py index fae2e48bb48..09788a23b2c 100644 --- a/lms/djangoapps/instructor/tests/test_hint_manager.py +++ b/lms/djangoapps/instructor/tests/test_hint_manager.py @@ -4,8 +4,8 @@ from django.test.client import Client, RequestFactory from django.test.utils import override_settings from mock import patch, MagicMock -from courseware.models import XModuleContentField -from courseware.tests.factories import ContentFactory +from courseware.models import XModuleUserStateSummaryField +from courseware.tests.factories import UserStateSummaryFactory from courseware.tests.modulestore_config import TEST_DATA_MIXED_MODULESTORE import instructor.hint_manager as view from student.tests.factories import UserFactory @@ -28,18 +28,18 @@ class HintManagerTest(ModuleStoreTestCase): self.c.login(username='robot', password='test') self.problem_id = 'i4x://Me/19.002/crowdsource_hinter/crowdsource_hinter_001' self.course_id = 'Me/19.002/test_course' - ContentFactory.create(field_name='hints', - definition_id=self.problem_id, + UserStateSummaryFactory.create(field_name='hints', + usage_id=self.problem_id, value=json.dumps({'1.0': {'1': ['Hint 1', 2], '3': ['Hint 3', 12]}, '2.0': {'4': ['Hint 4', 3]} })) - ContentFactory.create(field_name='mod_queue', - definition_id=self.problem_id, + UserStateSummaryFactory.create(field_name='mod_queue', + usage_id=self.problem_id, value=json.dumps({'2.0': {'2': ['Hint 2', 1]}})) - ContentFactory.create(field_name='hint_pk', - definition_id=self.problem_id, + UserStateSummaryFactory.create(field_name='hint_pk', + usage_id=self.problem_id, value=5) # Mock out location_to_problem_name, which ordinarily accesses the modulestore. # (I can't figure out how to get fake structures into the modulestore.) @@ -117,7 +117,7 @@ class HintManagerTest(ModuleStoreTestCase): 'op': 'delete hints', 1: [self.problem_id, '1.0', '1']}) view.delete_hints(post, self.course_id, 'hints') - problem_hints = XModuleContentField.objects.get(field_name='hints', definition_id=self.problem_id).value + problem_hints = XModuleUserStateSummaryField.objects.get(field_name='hints', usage_id=self.problem_id).value self.assertTrue('1' not in json.loads(problem_hints)['1.0']) def test_changevotes(self): @@ -129,7 +129,7 @@ class HintManagerTest(ModuleStoreTestCase): 'op': 'change votes', 1: [self.problem_id, '1.0', '1', 5]}) view.change_votes(post, self.course_id, 'hints') - problem_hints = XModuleContentField.objects.get(field_name='hints', definition_id=self.problem_id).value + problem_hints = XModuleUserStateSummaryField.objects.get(field_name='hints', usage_id=self.problem_id).value # hints[answer][hint_pk (string)] = [hint text, vote count] print json.loads(problem_hints)['1.0']['1'] self.assertTrue(json.loads(problem_hints)['1.0']['1'][1] == 5) @@ -151,9 +151,9 @@ class HintManagerTest(ModuleStoreTestCase): 'hint': 'This is a new hint.'}) post.user = 'fake user' with patch('courseware.module_render.get_module', MagicMock(return_value=hinter)): - with patch('courseware.model_data.ModelDataCache', MagicMock(return_value=None)): + with patch('courseware.model_data.FieldDataCache', MagicMock(return_value=None)): view.add_hint(post, self.course_id, 'mod_queue') - problem_hints = XModuleContentField.objects.get(field_name='mod_queue', definition_id=self.problem_id).value + problem_hints = XModuleUserStateSummaryField.objects.get(field_name='mod_queue', usage_id=self.problem_id).value self.assertTrue('3.14' in json.loads(problem_hints)) def test_addbadhint(self): @@ -172,9 +172,9 @@ class HintManagerTest(ModuleStoreTestCase): 'hint': 'This is a new hint.'}) post.user = 'fake user' with patch('courseware.module_render.get_module', MagicMock(return_value=hinter)): - with patch('courseware.model_data.ModelDataCache', MagicMock(return_value=None)): + with patch('courseware.model_data.FieldDataCache', MagicMock(return_value=None)): view.add_hint(post, self.course_id, 'mod_queue') - problem_hints = XModuleContentField.objects.get(field_name='mod_queue', definition_id=self.problem_id).value + problem_hints = XModuleUserStateSummaryField.objects.get(field_name='mod_queue', usage_id=self.problem_id).value self.assertTrue('fish' not in json.loads(problem_hints)) def test_approve(self): @@ -187,8 +187,8 @@ class HintManagerTest(ModuleStoreTestCase): 'op': 'approve', 1: [self.problem_id, '2.0', '2']}) view.approve(post, self.course_id, 'mod_queue') - problem_hints = XModuleContentField.objects.get(field_name='mod_queue', definition_id=self.problem_id).value + problem_hints = XModuleUserStateSummaryField.objects.get(field_name='mod_queue', usage_id=self.problem_id).value self.assertTrue('2.0' not in json.loads(problem_hints) or len(json.loads(problem_hints)['2.0']) == 0) - problem_hints = XModuleContentField.objects.get(field_name='hints', definition_id=self.problem_id).value + problem_hints = XModuleUserStateSummaryField.objects.get(field_name='hints', usage_id=self.problem_id).value self.assertTrue(json.loads(problem_hints)['2.0']['2'] == ['Hint 2', 1]) self.assertTrue(len(json.loads(problem_hints)['2.0']) == 2) diff --git a/lms/djangoapps/instructor/views/legacy.py b/lms/djangoapps/instructor/views/legacy.py index bcf2910582d..5c9aa2b4cf7 100644 --- a/lms/djangoapps/instructor/views/legacy.py +++ b/lms/djangoapps/instructor/views/legacy.py @@ -52,6 +52,8 @@ from psychometrics import psychoanalyze from student.models import CourseEnrollment, CourseEnrollmentAllowed import track.views from mitxmako.shortcuts import render_to_string +from xblock.field_data import DictFieldData +from xblock.fields import ScopeIds from bulk_email.models import CourseEmail @@ -109,17 +111,11 @@ def instructor_dashboard(request, course_id): data += [['Date', timezone.now().isoformat()]] data += compute_course_stats(course).items() if request.user.is_staff: - for field in course.fields: + for field in course.fields.values(): if getattr(field.scope, 'user', False): continue data.append([field.name, json.dumps(field.read_json(course))]) - for namespace in course.namespaces: - for field in getattr(course, namespace).fields: - if getattr(field.scope, 'user', False): - continue - - data.append(["{}.{}".format(namespace, field.name), json.dumps(field.read_json(course))]) datatable['data'] = data return datatable @@ -799,7 +795,7 @@ def instructor_dashboard(request, course_id): email_editor = None # HTML editor for email if idash_mode == 'Email' and is_studio_course: - html_module = HtmlDescriptor(course.system, {'data': html_message}) + html_module = HtmlDescriptor(course.system, DictFieldData({'data': html_message}), ScopeIds(None, None, None, None)) email_editor = wrap_xmodule(html_module.get_html, html_module, 'xmodule_edit.html')() studio_url = None @@ -1479,7 +1475,7 @@ def dump_grading_context(course): msg += "--> Section %s:\n" % (gs) for sec in gsvals: s = sec['section_descriptor'] - grade_format = getattr(s.lms, 'grade_format', None) + grade_format = getattr(s, 'grade_format', None) aname = '' if grade_format in graders: g = graders[grade_format] diff --git a/lms/djangoapps/instructor_task/tasks_helper.py b/lms/djangoapps/instructor_task/tasks_helper.py index 6ee72ec7b92..4f2d37a212c 100644 --- a/lms/djangoapps/instructor_task/tasks_helper.py +++ b/lms/djangoapps/instructor_task/tasks_helper.py @@ -23,7 +23,7 @@ from xmodule.modulestore.django import modulestore from track.views import task_track from courseware.models import StudentModule -from courseware.model_data import ModelDataCache +from courseware.model_data import FieldDataCache from courseware.module_render import get_module_for_descriptor_internal from instructor_task.models import InstructorTask, PROGRESS @@ -251,7 +251,7 @@ def _get_module_instance_for_task(course_id, student, module_descriptor, xmodule the need for a Request object when instantiating an xmodule instance. """ # reconstitute the problem's corresponding XModule: - model_data_cache = ModelDataCache.cache_for_descriptor_descendents(course_id, student, module_descriptor) + field_data_cache = FieldDataCache.cache_for_descriptor_descendents(course_id, student, module_descriptor) # get request-related tracking information from args passthrough, and supplement with task-specific # information: @@ -271,7 +271,7 @@ def _get_module_instance_for_task(course_id, student, module_descriptor, xmodule xqueue_callback_url_prefix = xmodule_instance_args.get('xqueue_callback_url_prefix', '') \ if xmodule_instance_args is not None else '' - return get_module_for_descriptor_internal(student, module_descriptor, model_data_cache, course_id, + return get_module_for_descriptor_internal(student, module_descriptor, field_data_cache, course_id, make_track_function(), xqueue_callback_url_prefix, grade_bucket_type=grade_bucket_type) diff --git a/lms/djangoapps/open_ended_grading/open_ended_notifications.py b/lms/djangoapps/open_ended_grading/open_ended_notifications.py index 6d61516f8e7..34b8e7036b1 100644 --- a/lms/djangoapps/open_ended_grading/open_ended_notifications.py +++ b/lms/djangoapps/open_ended_grading/open_ended_notifications.py @@ -68,7 +68,7 @@ def peer_grading_notifications(course, user): get_module = None, render_template=render_to_string, replace_urls=None, - xblock_model_data= {} + xblock_field_data= {} ) peer_gs = peer_grading_service.PeerGradingService(settings.OPEN_ENDED_GRADING_INTERFACE, system) pending_grading = False @@ -129,7 +129,7 @@ def combined_notifications(course, user): get_module = None, render_template=render_to_string, replace_urls=None, - xblock_model_data= {} + xblock_field_data= {} ) #Initialize controller query service using our mock system controller_qs = ControllerQueryService(settings.OPEN_ENDED_GRADING_INTERFACE, system) diff --git a/lms/djangoapps/open_ended_grading/staff_grading_service.py b/lms/djangoapps/open_ended_grading/staff_grading_service.py index a6a2847bc1a..4e93eb8581a 100644 --- a/lms/djangoapps/open_ended_grading/staff_grading_service.py +++ b/lms/djangoapps/open_ended_grading/staff_grading_service.py @@ -73,7 +73,7 @@ class StaffGradingService(GradingService): get_module = None, render_template=render_to_string, replace_urls=None, - xblock_model_data= {} + xblock_field_data= {} ) super(StaffGradingService, self).__init__(config) self.url = config['url'] + config['staff_grading'] diff --git a/lms/djangoapps/open_ended_grading/tests.py b/lms/djangoapps/open_ended_grading/tests.py index 7ae5994dc1e..f3f0748c84a 100644 --- a/lms/djangoapps/open_ended_grading/tests.py +++ b/lms/djangoapps/open_ended_grading/tests.py @@ -15,8 +15,8 @@ from mitxmako.shortcuts import render_to_string from xmodule.open_ended_grading_classes import peer_grading_service, controller_query_service from xmodule import peer_grading_module from xmodule.modulestore.django import modulestore -import xmodule.modulestore.django from xmodule.x_module import ModuleSystem +from xblock.fields import ScopeIds from open_ended_grading import staff_grading_service, views from courseware.access import _course_staff_group_name @@ -28,6 +28,7 @@ from django.test.utils import override_settings from xmodule.tests import test_util_open_ended from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase +from xblock.field_data import DictFieldData from courseware.tests import factories from courseware.tests.modulestore_config import TEST_DATA_MIXED_MODULESTORE @@ -158,7 +159,7 @@ class TestPeerGradingService(ModuleStoreTestCase, LoginEnrollmentTestCase): self.course_id = "edX/toy/2012_Fall" self.toy = modulestore().get_course(self.course_id) location = "i4x://edX/toy/peergrading/init" - model_data = {'data': "<peergrading/>", 'location': location, 'category':'peergrading'} + field_data = DictFieldData({'data': "<peergrading/>", 'location': location, 'category':'peergrading'}) self.mock_service = peer_grading_service.MockPeerGradingService() self.system = ModuleSystem( ajax_url=location, @@ -166,13 +167,12 @@ class TestPeerGradingService(ModuleStoreTestCase, LoginEnrollmentTestCase): get_module=None, render_template=render_to_string, replace_urls=None, - xblock_model_data={}, + xblock_field_data=lambda d: d._field_data, s3_interface=test_util_open_ended.S3_INTERFACE, open_ended_grading_interface=test_util_open_ended.OPEN_ENDED_GRADING_INTERFACE ) - self.descriptor = peer_grading_module.PeerGradingDescriptor(self.system, model_data) - model_data = {'location': location} - self.peer_module = peer_grading_module.PeerGradingModule(self.system, self.descriptor, model_data) + self.descriptor = peer_grading_module.PeerGradingDescriptor(self.system, field_data, ScopeIds(None, None, None, None)) + self.peer_module = self.descriptor.xmodule(self.system) self.peer_module.peer_gs = self.mock_service self.logout() diff --git a/lms/djangoapps/open_ended_grading/views.py b/lms/djangoapps/open_ended_grading/views.py index 11d47650699..4c299db38dc 100644 --- a/lms/djangoapps/open_ended_grading/views.py +++ b/lms/djangoapps/open_ended_grading/views.py @@ -33,7 +33,7 @@ system = ModuleSystem( get_module=None, render_template=render_to_string, replace_urls=None, - xblock_model_data={} + xblock_field_data={} ) controller_qs = ControllerQueryService(settings.OPEN_ENDED_GRADING_INTERFACE, system) diff --git a/lms/envs/common.py b/lms/envs/common.py index 54d950bd945..3dcc7f6325f 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -30,6 +30,9 @@ from path import path from .discussionsettings import * +from lms.xblock.mixin import LmsBlockMixin +from xmodule.modulestore.inheritance import InheritanceMixin + ################################### FEATURES ################################### # The display name of the platform to be used in templates/emails/etc. PLATFORM_NAME = "edX" @@ -320,6 +323,12 @@ MODULESTORE = { } CONTENTSTORE = None +############# XBlock Configuration ########## + +# This should be moved into an XBlock Runtime/Application object +# once the responsibility of XBlock creation is moved out of modulestore - cpennington +XBLOCK_MIXINS = (LmsBlockMixin, InheritanceMixin) + #################### Python sandbox ############################################ CODE_JAIL = { diff --git a/lms/startup.py b/lms/startup.py index 901a559edbe..7b51151ca78 100644 --- a/lms/startup.py +++ b/lms/startup.py @@ -1,6 +1,8 @@ """ Module for code that should run during LMS startup """ +import logging + from django.conf import settings # Force settings to run so that the python path is modified @@ -8,6 +10,7 @@ settings.INSTALLED_APPS # pylint: disable=W0104 from django_startup import autostartup +log = logging.getLogger(__name__) def run(): """ diff --git a/lms/templates/staff_problem_info.html b/lms/templates/staff_problem_info.html index 6d1517c447e..97730bca413 100644 --- a/lms/templates/staff_problem_info.html +++ b/lms/templates/staff_problem_info.html @@ -59,12 +59,6 @@ location = ${location | h} <tr><td>${name}</td><td><pre style="display:inline-block; margin: 0;">${field | h}</pre></td></tr> %endfor </table> -<table> - <tr><th>${_('{platform_name} Fields').format(platform_name=settings.PLATFORM_NAME)}</th></tr> - %for name, field in lms_fields: - <tr><td>${name}</td><td><pre style="display:inline-block; margin: 0;">${field | h}</pre></td></tr> - %endfor -</table> <table> <tr><th>${_('XML attributes')}</th></tr> %for name, field in xml_attributes.items(): diff --git a/lms/xblock/__init__.py b/lms/xblock/__init__.py new file mode 100644 index 00000000000..e69de29bb2d diff --git a/lms/xblock/field_data.py b/lms/xblock/field_data.py new file mode 100644 index 00000000000..6073a868630 --- /dev/null +++ b/lms/xblock/field_data.py @@ -0,0 +1,26 @@ +""" +:class:`~xblock.field_data.FieldData` subclasses used by the LMS +""" + +from xblock.field_data import ReadOnlyFieldData, SplitFieldData +from xblock.fields import Scope + + +def lms_field_data(authored_data, student_data): + """ + Returns a new :class:`~xblock.field_data.FieldData` that + reads all UserScope.ONE and UserScope.ALL fields from `student_data` + and all UserScope.NONE fields from `authored_data`. It also prevents + writing to `authored_data`. + """ + authored_data = ReadOnlyFieldData(authored_data) + return SplitFieldData({ + Scope.content: authored_data, + Scope.settings: authored_data, + Scope.parent: authored_data, + Scope.children: authored_data, + Scope.user_state_summary: student_data, + Scope.user_state: student_data, + Scope.user_info: student_data, + Scope.preferences: student_data, + }) diff --git a/lms/xblock/mixin.py b/lms/xblock/mixin.py new file mode 100644 index 00000000000..edf84fbe6b4 --- /dev/null +++ b/lms/xblock/mixin.py @@ -0,0 +1,22 @@ +""" +Namespace that defines fields common to all blocks used in the LMS +""" +from xblock.fields import Boolean, Scope, String, XBlockMixin + + +class LmsBlockMixin(XBlockMixin): + """ + Mixin that defines fields common to all blocks used in the LMS + """ + hide_from_toc = Boolean( + help="Whether to display this module in the table of contents", + default=False, + scope=Scope.settings + ) + format = String( + help="What format this module is in (used for deciding which " + "grader to apply, and what to show in the TOC)", + scope=Scope.settings, + ) + source_file = String(help="source file name (eg for latex)", scope=Scope.settings) + ispublic = Boolean(help="Whether this course is open to the public, or only to admins", scope=Scope.settings) diff --git a/lms/xmodule_namespace.py b/lms/xmodule_namespace.py deleted file mode 100644 index ad3f6349772..00000000000 --- a/lms/xmodule_namespace.py +++ /dev/null @@ -1,59 +0,0 @@ -""" -Namespace that defines fields common to all blocks used in the LMS -""" -from xblock.core import Namespace, Boolean, Scope, String, Float -from xmodule.fields import Date, Timedelta -from datetime import datetime -from pytz import UTC - - -class LmsNamespace(Namespace): - """ - Namespace that defines fields common to all blocks used in the LMS - """ - hide_from_toc = Boolean( - help="Whether to display this module in the table of contents", - default=False, - scope=Scope.settings - ) - graded = Boolean( - help="Whether this module contributes to the final course grade", - default=False, - scope=Scope.settings - ) - format = String( - help="What format this module is in (used for deciding which " - "grader to apply, and what to show in the TOC)", - scope=Scope.settings, - ) - - start = Date( - help="Start time when this module is visible", - default=datetime.fromtimestamp(0, UTC), - scope=Scope.settings - ) - due = Date(help="Date that this problem is due by", scope=Scope.settings) - source_file = String(help="source file name (eg for latex)", scope=Scope.settings) - giturl = String(help="url root for course data git repository", scope=Scope.settings) - xqa_key = String(help="DO NOT USE", scope=Scope.settings) - ispublic = Boolean(help="Whether this course is open to the public, or only to admins", scope=Scope.settings) - graceperiod = Timedelta( - help="Amount of time after the due date that submissions will be accepted", - scope=Scope.settings - ) - showanswer = String( - help="When to show the problem answer to the student", - scope=Scope.settings, - default="finished" - ) - rerandomize = String( - help="When to rerandomize the problem", - default="never", - scope=Scope.settings - ) - days_early_for_beta = Float( - help="Number of days early to show content to beta users", - default=None, - scope=Scope.settings - ) - static_asset_path = String(help="Path to use for static assets - overrides Studio c4x://", scope=Scope.settings, default='') diff --git a/requirements/edx/github.txt b/requirements/edx/github.txt index 4a4cde7eccc..9fab941c468 100644 --- a/requirements/edx/github.txt +++ b/requirements/edx/github.txt @@ -8,7 +8,7 @@ -e git+https://github.com/eventbrite/zendesk.git@d53fe0e81b623f084e91776bcf6369f8b7b63879#egg=zendesk # Our libraries: --e git+https://github.com/edx/XBlock.git@446668fddc75b78512eef4e9425cbc9a3327606f#egg=XBlock +-e git+https://github.com/edx/XBlock.git@aa0d60627#egg=XBlock -e git+https://github.com/edx/codejail.git@0a1b468#egg=codejail -e git+https://github.com/edx/diff-cover.git@v0.2.2#egg=diff_cover -e git+https://github.com/edx/js-test-tool.git@v0.0.7#egg=js_test_tool diff --git a/requirements/edx/local.txt b/requirements/edx/local.txt index 04a1f7f2c62..7941f9fa5e0 100644 --- a/requirements/edx/local.txt +++ b/requirements/edx/local.txt @@ -5,4 +5,3 @@ -e common/lib/sandbox-packages -e common/lib/symmath -e common/lib/xmodule --e . diff --git a/scripts/runone.py b/scripts/runone.py index 8baf6790b80..19b5f7195b6 100755 --- a/scripts/runone.py +++ b/scripts/runone.py @@ -53,7 +53,8 @@ def main(argv): # Run as a django test suite from django.core import management - django_args = ["./manage.py", system, "--settings", "test", "test"] + os.environ['DJANGO_SETTINGS_MODULE'] = system + '.envs.test' + django_args = ["./manage.py", "test"] if args.nocapture: django_args.append("-s") if args.pdb: diff --git a/setup.cfg b/setup.cfg index da9525a3008..abc86c43fd4 100644 --- a/setup.cfg +++ b/setup.cfg @@ -7,5 +7,6 @@ with-id=1 exclude-dir=lms/envs cms/envs -# Uncomment the following line to open pdb when a test fails +# Uncomment the following lines to open pdb when a test fails +#nocapture=1 #pdb=1 diff --git a/setup.py b/setup.py deleted file mode 100644 index 48572de6def..00000000000 --- a/setup.py +++ /dev/null @@ -1,19 +0,0 @@ -from setuptools import setup, find_packages - -setup( - name="edX Apps", - version="0.1", - install_requires=['distribute'], - requires=[ - 'xmodule', - ], - py_modules=['lms.xmodule_namespace', 'cms.xmodule_namespace'], - # See http://guide.python-distribute.org/creation.html#entry-points - # for a description of entry_points - entry_points={ - 'xblock.namespace': [ - 'lms = lms.xmodule_namespace:LmsNamespace', - 'cms = cms.xmodule_namespace:CmsNamespace', - ], - } -) \ No newline at end of file -- GitLab