From 8201b1412ebae84bade3a6b3c932916a0e787a58 Mon Sep 17 00:00:00 2001 From: Calen Pennington 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 c0e1ff7207e4..b43a32f635d5 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 696b60fbe595..f76b563d6a5a 100644 --- a/cms/djangoapps/contentstore/tests/test_contentstore.py +++ b/cms/djangoapps/contentstore/tests/test_contentstore.py @@ -219,7 +219,7 @@ def test_draft_metadata(self): '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 @@ def test_draft_metadata(self): # 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 @@ def test_draft_metadata(self): # 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 @@ def test_draft_metadata(self): 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 @@ def test_draft_metadata(self): 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 @@ def test_draft_metadata(self): 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 @@ def test_clone_course(self): # 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 @@ def test_export_course(self, mock_get): 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 @@ def test_export_course(self, mock_get): 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 @@ def test_export_course(self, mock_get): 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 @@ def test_metadata_inheritance(self): # 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 @@ def test_metadata_inheritance(self): 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 @@ def test_metadata_inheritance(self): 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 @@ def test_default_metadata_inheritance(self): 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 @@ def test_default_metadata_inheritance(self): 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 @@ def test_metadata_not_persistence(self): '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 286477228b37..b27d1494e9c0 100644 --- a/cms/djangoapps/contentstore/tests/test_course_settings.py +++ b/cms/djangoapps/contentstore/tests/test_course_settings.py @@ -343,8 +343,8 @@ def test_update_section_grader_type(self): 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 @@ def test_update_section_grader_type(self): 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 @@ def test_update_section_grader_type(self): 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 e543b7b51714..3a8e0b0222d1 100644 --- a/cms/djangoapps/contentstore/tests/test_crud.py +++ b/cms/djangoapps/contentstore/tests/test_crud.py @@ -220,18 +220,14 @@ def load_from_json(json_data, system, default_class=None, parent_xblock=None): 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 f0f65c9b0768..275f9b6b1f9d 100644 --- a/cms/djangoapps/contentstore/tests/test_import_nostatic.py +++ b/cms/djangoapps/contentstore/tests/test_import_nostatic.py @@ -97,9 +97,9 @@ def test_static_import(self): 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 e5ff992cb8ea..dcd94838c8b7 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 @@ def test_create_nicely(self): # 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 @@ def test_date_fields(self): 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 @@ def test_date_fields(self): 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 @@ def test_date_fields(self): 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 a5fec7c033be..0a09bd8a0427 100644 --- a/cms/djangoapps/contentstore/views/component.py +++ b/cms/djangoapps/contentstore/views/component.py @@ -2,14 +2,14 @@ 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.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 bbd95dba8431..84a199d71dae 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 7a3a224d8686..7fc669888e50 100644 --- a/cms/djangoapps/contentstore/views/preview.py +++ b/cms/djangoapps/contentstore/views/preview.py @@ -11,12 +11,12 @@ 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_model_data(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 87a92a9e2e41..ddee2c1dbc5b 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 0746fc7a902e..578961fad6cd 100644 --- a/cms/djangoapps/models/settings/course_grading.py +++ b/cms/djangoapps/models/settings/course_grading.py @@ -125,7 +125,7 @@ def update_grader_from_json(course_location, grader): # 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 @@ def update_cutoffs_from_json(course_location, cutoffs): # 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 @@ def update_grace_period_from_json(course_location, graceperiodjson): 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 @@ def delete_grader(course_location, index): # 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 @@ def delete_grace_period(course_location): 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 @@ def get_section_grader_type(location): 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 @@ def update_section_grader_type(location, jsondict): 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 adbbf06c646f..f186fbf11648 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 @@ def fetch(cls, course_location): 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 @@ def update_from_json(cls, course_location, jsondict, filter_tabs=True): 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 @@ def delete_key(cls, course_location, payload): 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 d0650e9ed24d..ecb85a05e025 100644 --- a/cms/envs/common.py +++ b/cms/envs/common.py @@ -28,6 +28,10 @@ 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 @@ '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 eb1098a7074c..111cb9f96aed 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 @@ 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 5b03643f3b90..b1be834709e4 100644 --- a/cms/templates/edit_subsection.html +++ b/cms/templates/edit_subsection.html @@ -37,21 +37,21 @@

${_("Subsection Settings")}

- % 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:

${_("The date above differs from the release date of {name}, which is unset.").format(name=parent_item.display_name_with_default)} % else: -

${_("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))}. +

${_("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

% endif @@ -60,7 +60,7 @@

${_("Subsection Settings")}

-
+
@@ -69,13 +69,13 @@

${_("Subsection Settings")}

${_("Remove due date")} diff --git a/cms/templates/overview.html b/cms/templates/overview.html index 2c42df187ae6..a8cdfff55419 100644 --- a/cms/templates/overview.html +++ b/cms/templates/overview.html @@ -157,19 +157,19 @@

${_("Page Actions")}

<% - 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: ${_("This section has not been released.")} ${_("Schedule")} %else: ${_("Will Release:")} - ${date_utils.get_default_time_display(section.lms.start)} + ${date_utils.get_default_time_display(section.start)} ${_("Edit")} %endif @@ -199,7 +199,7 @@

-
+
diff --git a/cms/templates/widgets/sequence-edit.html b/cms/templates/widgets/sequence-edit.html index 7956fbfbafe5..c729f28950ff 100644 --- a/cms/templates/widgets/sequence-edit.html +++ b/cms/templates/widgets/sequence-edit.html @@ -35,7 +35,7 @@

Filter:

    • % for child in module.get_children(): -
    1. +
    2. - + ${unit.display_name_with_default} % if actions: diff --git a/cms/xmodule_namespace.py b/cms/xmodule_namespace.py index eef4b41f3784..c028ab37106c 100644 --- a/cms/xmodule_namespace.py +++ b/cms/xmodule_namespace.py @@ -4,12 +4,12 @@ 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 @@ def to_json(self, value): 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 37dcd5313a94..a4d6752276b3 100644 --- a/common/djangoapps/external_auth/views.py +++ b/common/djangoapps/external_auth/views.py @@ -44,7 +44,7 @@ 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 makeErrorURL(error_url, error_code): 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 4a61a106d446..17e9c07f6a68 100644 --- a/common/djangoapps/tests.py +++ b/common/djangoapps/tests.py @@ -32,7 +32,7 @@ def _test_add_histogram(self): 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 16fe12371bb7..14f80c0a70e4 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 _get_html(): # 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 _get_html(): 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 = "Yes!" if (now > mstart) else "Not yet" - 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 53f080eb3a86..c0a53d048f4f 100644 --- a/common/lib/xmodule/xmodule/abtest_module.py +++ b/common/lib/xmodule/xmodule/abtest_module.py @@ -6,7 +6,7 @@ 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 f80e3e488e6e..ca85065577e9 100644 --- a/common/lib/xmodule/xmodule/annotatable_module.py +++ b/common/lib/xmodule/xmodule/annotatable_module.py @@ -5,7 +5,7 @@ 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 dbd535a471be..8de38022b73c 100644 --- a/common/lib/xmodule/xmodule/capa_module.py +++ b/common/lib/xmodule/xmodule/capa_module.py @@ -18,7 +18,7 @@ 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 5354915b860a..c6dc882bb6cc 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 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 5bdc8e77976c..82416d48d1b4 100644 --- a/common/lib/xmodule/xmodule/conditional_module.py +++ b/common/lib/xmodule/xmodule/conditional_module.py @@ -10,7 +10,7 @@ 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 c583aec3d126..aca804d5e2e2 100644 --- a/common/lib/xmodule/xmodule/course_module.py +++ b/common/lib/xmodule/xmodule/course_module.py @@ -13,7 +13,7 @@ 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 @@ def table_of_contents(self): 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 @@ def grading_context(self): 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 @@ def yield_descriptor_descendents(module_descriptor): 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 7e538efa24fe..fcc74d5cb9a3 100644 --- a/common/lib/xmodule/xmodule/crowdsource_hinter.py +++ b/common/lib/xmodule/xmodule/crowdsource_hinter.py @@ -15,7 +15,7 @@ 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 2c6cc9f4ddb3..8051cb23a37a 100644 --- a/common/lib/xmodule/xmodule/discussion_module.py +++ b/common/lib/xmodule/xmodule/discussion_module.py @@ -3,7 +3,7 @@ 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 eed8fb310f5e..35e3e4c6d104 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 027067f4c091..2013ee2003db 100644 --- a/common/lib/xmodule/xmodule/error_module.py +++ b/common/lib/xmodule/xmodule/error_module.py @@ -12,7 +12,8 @@ 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 @@ def _construct(cls, system, contents, error_msg, location): ) # 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 b7094203c41e..a0b53859ce71 100644 --- a/common/lib/xmodule/xmodule/fields.py +++ b/common/lib/xmodule/xmodule/fields.py @@ -2,7 +2,7 @@ import logging import re -from xblock.core import ModelType +from xblock.fields import Field import datetime import dateutil.parser @@ -11,7 +11,7 @@ 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 @@ def to_json(self, value): 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\d+?) day(?:s?))?(\s)?((?P\d+?) hour(?:s?))?(\s)?((?P\d+?) minute(?:s)?)?(\s)?((?P\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 cadf6cef0bcd..e1714ff96b61 100644 --- a/common/lib/xmodule/xmodule/foldit_module.py +++ b/common/lib/xmodule/xmodule/foldit_module.py @@ -6,7 +6,7 @@ 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 d6516d92ef9f..ef69130f84be 100644 --- a/common/lib/xmodule/xmodule/gst_module.py +++ b/common/lib/xmodule/xmodule/gst_module.py @@ -14,7 +14,7 @@ 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 7a68c42ac930..5b3ca5661ab6 100644 --- a/common/lib/xmodule/xmodule/html_module.py +++ b/common/lib/xmodule/xmodule/html_module.py @@ -7,7 +7,7 @@ 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 526fc1a0eb27..3d37bd615edd 100644 --- a/common/lib/xmodule/xmodule/mako_module.py +++ b/common/lib/xmodule/xmodule/mako_module.py @@ -2,10 +2,8 @@ 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 f2b70ad3659b..e7bcc171f76b 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 @@ def __init__(self, metadata_inheritance_cache_subsystem=None, request_cache=None 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 9ff82e113746..93416cae17ee 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 aeec53cc294d..a9de875128c5 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 bdc9e61fce75..916a970ad0d6 100644 --- a/common/lib/xmodule/xmodule/modulestore/locator.py +++ b/common/lib/xmodule/xmodule/modulestore/locator.py @@ -6,7 +6,6 @@ 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 @@ 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 @@ def set_usage_id(self, new): 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 @@ def __unicode__(self): """ 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 8827f3336047..6a1c3b37c9f4 100644 --- a/common/lib/xmodule/xmodule/modulestore/mixed.py +++ b/common/lib/xmodule/xmodule/modulestore/mixed.py @@ -29,9 +29,9 @@ def __init__(self, mappings, stores, **kwargs): 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 bdeda76d3210..c20ca43e84cc 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 b1fecec120d3..105167648487 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.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 @@ def get(self, key): 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 @@ def set(self, key, value): 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 @@ def delete(self, key): 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 @@ def has(self, key): 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 @@ def has(self, key): 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 @@ def __init__(self, modulestore, module_data, default_class, resources_fs, 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 @@ def load_item(self, location): 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 @@ def load_item(self, location): ) 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 @@ def load_item(self, location): 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 @@ def compute_metadata_inheritance_tree(self, location): # 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 @@ def _load_item(self, item, data_cache, apply_cached_metadata=True): # 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 @@ def create_xmodule(self, location, definition_data=None, metadata=None, system=N :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 @@ def create_xmodule(self, location, definition_data=None, metadata=None, system=N 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 @@ def create_xmodule(self, location, definition_data=None, metadata=None, system=N 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 @@ def create_and_save_xmodule(self, location, definition_data=None, metadata=None, :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 @@ def get_modulestore_type(self, course_id): """ 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 @@ def _create_new_model_data(self, category, location, definition_data, metadata): 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 d289e037399a..610f7b1b8a9e 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 @@ def publish(self, location, published_by_id): """ 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 73dcabfa6956..020ebdbbfe57 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 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 @@ def __init__(self, modulestore, course_entry, module_data, lazy, 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 @@ def xblock_from_json(self, class_, usage_id, json_data, course_entry_override=No 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 @@ def xblock_from_json(self, class_, usage_id, json_data, course_entry_override=No 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 @@ def xblock_from_json(self, class_, usage_id, json_data, course_entry_override=No 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 c5f4c2404c12..d9cf2245cd9c 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 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 @@ #============================================================================== + + class SplitMongoModuleStore(ModuleStoreBase): """ A Mongodb backed ModuleStore supporting versions, inheritance, @@ -53,7 +56,7 @@ def __init__(self, host, db, collection, fs_root, render_template, mongo_options=None, **kwargs): - ModuleStoreBase.__init__(self) + super(SplitMongoModuleStore, self).__init__(**kwargs) if mongo_options is None: mongo_options = {} @@ -93,6 +96,11 @@ def __init__(self, host, db, collection, fs_root, render_template, 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 @@ def _load_items(self, course_entry, usage_ids, depth=0, lazy=True): 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 @@ def _persist_subdag(self, xblock, user_id, structure_blocks): 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 @@ def _persist_subdag(self, xblock, user_id, structure_blocks): 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 @@ def inherit_settings(self, block_map, block, inheriting_settings=None): # 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 @@ def _partition_fields_by_scope(self, category, fields): """ 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 e4a4b9202750..361ffff85d0c 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 @@ def __init__(self, definition, fields, _inherited_settings, location, category): # 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 @@ def get(self, key): # 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 @@ def set(self, key, value): 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 @@ def delete(self, key): 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 @@ def has(self, key): """ # 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 725d4aebb74e..a025eb7c8275 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 7913434086c3..4ad801aef84b 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/factories.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/factories.py @@ -6,8 +6,6 @@ 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 @@ def _create(cls, target_class, **kwargs): 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 25ad2586688f..6f3276bf446a 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 @@ 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 @@ def setUp(self): 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 @@ def _check_read(self, key, expected_value): 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 @@ def _check_write(self, key, value): 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 @@ def _check_delete_key_error(self, key): 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 d90c3020b445..b299f56711e2 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 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 @@ def test_get_course(self): 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 @@ def test_derived_course(self): 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 6527a5e34abc..dd7888179b3c 100644 --- a/common/lib/xmodule/xmodule/modulestore/xml.py +++ b/common/lib/xmodule/xmodule/modulestore/xml.py @@ -20,8 +20,11 @@ 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 @@ def fallback_name(orig_name=None): # 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 @@ def load_course(self, course_dir, tracker): 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 @@ def _load_extra_content(self, system, course_descriptor, category, path, course_ # 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 beb255fb147c..cd34d294a78e 100644 --- a/common/lib/xmodule/xmodule/modulestore/xml_importer.py +++ b/common/lib/xmodule/xmodule/modulestore/xml_importer.py @@ -3,7 +3,7 @@ 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 ae4a8b87de43..3a4bbdb9f657 100644 --- a/common/lib/xmodule/xmodule/peer_grading_module.py +++ b/common/lib/xmodule/xmodule/peer_grading_module.py @@ -10,7 +10,7 @@ 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 @@ def __init__(self, *args, **kwargs): 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 @@ def _find_corresponding_module_for_location(location): 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 5cf9c647aa17..000000000000 --- 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 8e7407752a0f..86852cd698e3 100644 --- a/common/lib/xmodule/xmodule/poll_module.py +++ b/common/lib/xmodule/xmodule/poll_module.py @@ -19,7 +19,7 @@ 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 04b9a6e21545..38535d38bb2c 100644 --- a/common/lib/xmodule/xmodule/randomize_module.py +++ b/common/lib/xmodule/xmodule/randomize_module.py @@ -6,7 +6,7 @@ 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 4c6ddb5b5124..42c20344b684 100644 --- a/common/lib/xmodule/xmodule/raw_module.py +++ b/common/lib/xmodule/xmodule/raw_module.py @@ -3,7 +3,7 @@ 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 580475e1ae7d..ebd49d75cf74 100644 --- a/common/lib/xmodule/xmodule/seq_module.py +++ b/common/lib/xmodule/xmodule/seq_module.py @@ -8,7 +8,7 @@ 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 @@ def __init__(self, *args, **kwargs): 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 df197edb870f..ad5fa9c3cba4 100644 --- a/common/lib/xmodule/xmodule/tests/__init__.py +++ b/common/lib/xmodule/xmodule/tests/__init__.py @@ -18,6 +18,7 @@ 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 @@ def getlist(self, key): 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 EmptyClass: 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 6993841ab276..2b454ff45b6e 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 @@ 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): ''' 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('test') diff --git a/common/lib/xmodule/xmodule/tests/test_capa_module.py b/common/lib/xmodule/xmodule/tests/test_capa_module.py index 92d30fac8ca7..c47cfc2ca85e 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 @@ 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 @@ def create(graceperiod=None, """ 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="
      Test Template HTML
      ") - 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 4eadca110c95..1b3fd36f458b 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 @@ 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 95472a62ede9..040af7a23038 100644 --- a/common/lib/xmodule/xmodule/tests/test_conditional.py +++ b/common/lib/xmodule/xmodule/tests/test_conditional.py @@ -6,6 +6,8 @@ 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 @@ def create(system, source_is_error_module=False): # construct conditional module: cond_location = Location(["i4x", "edX", "conditional_test", "conditional", "SampleConditional"]) - model_data = {'data': '', 'location': cond_location} - cond_module = ConditionalModule(system, cond_descriptor, model_data) + field_data = DictFieldData({'data': '', '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 48690a8b2578..bf83fbd17d85 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 @@ def __init__(self, load_error_modules): 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 8347b71076ed..0369fffe8e63 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 @@ 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 @@ def create(hints=None, """ 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 @@ def create(hints=None, } 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 @@ def fake_get_module(descriptor): 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 @@ def next_num(): @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 838a4f9adaaf..515e367ee0b5 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 @@ 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 @@ def setUp(self): 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 f91bf7cb372c..186754e16cec 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 @@ def test_error_module_from_descriptor(self): 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 @@ def test_error_module_from_descriptor(self): 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 b0c1617580ee..1ee059f9feb5 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 4fe02423787e..2f2e9114c6f3 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 @@ 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 @@ def test_substitution_without_magic_string(self):

      Hi USER_ID!11!

      ''' - 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 b6758dc9179e..6ab5e29ad10c 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.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 @@ def __init__(self, load_error_modules): 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 @@ def get_course(self, name): """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 @@ def test_fallback(self): 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 @@ def test_reimport(self): 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 @@ def test_metadata_import_export(self): 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 @@ def test_metadata_no_inheritance(self): 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 @@ def test_metadata_override_default(self): 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 @@ def test_metadata_inherit(self): 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 @@ def test_policy_loading(self): # 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 bbd0c3bb12af..ea9fba794861 100644 --- a/common/lib/xmodule/xmodule/tests/test_poll.py +++ b/common/lib/xmodule/xmodule/tests/test_poll.py @@ -7,7 +7,7 @@ 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 e7af7a0c0981..053768a00763 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 a6a7d86510b6..1001ae818094 100644 --- a/common/lib/xmodule/xmodule/tests/test_video.py +++ b/common/lib/xmodule/xmodule/tests/test_video.py @@ -14,12 +14,16 @@ """ 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': '