diff --git a/cms/djangoapps/contentstore/views/assets.py b/cms/djangoapps/contentstore/views/assets.py index 4067e06e48..a6b3ef2cd2 100644 --- a/cms/djangoapps/contentstore/views/assets.py +++ b/cms/djangoapps/contentstore/views/assets.py @@ -201,10 +201,11 @@ def _upload_asset(request, course_key): 'File {filename} exceeds maximum size of ' '{size_mb} MB. Please follow the instructions here ' 'to upload a file elsewhere and link to it instead: ' - '{faq_url}').format( - filename=filename, - size_mb=settings.MAX_ASSET_UPLOAD_FILE_SIZE_IN_MB, - faq_url=settings.MAX_ASSET_UPLOAD_FILE_SIZE_URL, + '{faq_url}' + ).format( + filename=filename, + size_mb=settings.MAX_ASSET_UPLOAD_FILE_SIZE_IN_MB, + faq_url=settings.MAX_ASSET_UPLOAD_FILE_SIZE_URL, ) }, status=413) diff --git a/cms/djangoapps/contentstore/views/item.py b/cms/djangoapps/contentstore/views/item.py index 28c9a5e480..ffbcd68d18 100644 --- a/cms/djangoapps/contentstore/views/item.py +++ b/cms/djangoapps/contentstore/views/item.py @@ -45,7 +45,7 @@ from contentstore.views.helpers import is_unit, xblock_studio_url, xblock_primar from contentstore.views.preview import get_preview_fragment from edxmako.shortcuts import render_to_string from models.settings.course_grading import CourseGradingModel -from cms.lib.xblock.runtime import handler_url, local_resource_url +from cms.lib.xblock.runtime import handler_url, local_resource_url, get_asides from opaque_keys.edx.keys import UsageKey, CourseKey __all__ = ['orphan_handler', 'xblock_handler', 'xblock_view_handler', 'xblock_outline_handler'] @@ -64,6 +64,7 @@ ALWAYS = lambda x: True # TODO: Remove this code when Runtimes are no longer created by modulestores xmodule.x_module.descriptor_global_handler_url = handler_url xmodule.x_module.descriptor_global_local_resource_url = local_resource_url +xmodule.x_module.descriptor_global_get_asides = get_asides def hash_resource(resource): diff --git a/cms/djangoapps/contentstore/views/preview.py b/cms/djangoapps/contentstore/views/preview.py index 0d1dfec64f..a67b6019b6 100644 --- a/cms/djangoapps/contentstore/views/preview.py +++ b/cms/djangoapps/contentstore/views/preview.py @@ -95,6 +95,10 @@ class PreviewModuleSystem(ModuleSystem): # pylint: disable=abstract-method def local_resource_url(self, block, uri): return local_resource_url(block, uri) + def get_asides(self, block): + # TODO: Implement this to enable XBlockAsides on previews in Studio + return [] + class StudioUserService(object): """ @@ -110,7 +114,7 @@ class StudioUserService(object): return self._request.user.id -def _preview_module_system(request, descriptor): +def _preview_module_system(request, descriptor, field_data): """ Returns a ModuleSystem for the specified descriptor that is specialized for rendering module previews. @@ -163,6 +167,7 @@ def _preview_module_system(request, descriptor): descriptor_runtime=descriptor.runtime, services={ "i18n": ModuleI18nService(), + "field-data": field_data, }, ) @@ -181,7 +186,7 @@ def _load_preview_module(request, descriptor): else: field_data = LmsFieldData(descriptor._field_data, student_data) # pylint: disable=protected-access descriptor.bind_for_student( - _preview_module_system(request, descriptor), + _preview_module_system(request, descriptor, field_data), field_data ) return descriptor diff --git a/cms/lib/xblock/runtime.py b/cms/lib/xblock/runtime.py index b460f175c2..bd2d533ad1 100644 --- a/cms/lib/xblock/runtime.py +++ b/cms/lib/xblock/runtime.py @@ -33,3 +33,14 @@ def local_resource_url(block, uri): 'block_type': block.scope_ids.block_type, 'uri': uri, }) + + +def get_asides(block): # pylint: disable=unused-argument + """ + Return all of the asides which might be decorating this `block`. + + Arguments: + block (:class:`.XBlock`): The block to render retrieve asides for. + """ + # TODO: Implement this method to make XBlockAsides for editing views in Studio + return [] diff --git a/cms/static/coffee/spec/main.coffee b/cms/static/coffee/spec/main.coffee index 7ab87a4461..b9b3546dbf 100644 --- a/cms/static/coffee/spec/main.coffee +++ b/cms/static/coffee/spec/main.coffee @@ -32,6 +32,7 @@ requirejs.config({ "jquery.tinymce": "xmodule_js/common_static/js/vendor/tinymce/js/tinymce/jquery.tinymce", "xmodule": "xmodule_js/src/xmodule", "xblock/cms.runtime.v1": "coffee/src/xblock/cms.runtime.v1", + "xblock/core": "xmodule_js/common_static/js/xblock/core", "xblock": "xmodule_js/common_static/coffee/src/xblock", "utility": "xmodule_js/common_static/js/src/utility", "accessibility": "xmodule_js/common_static/js/src/accessibility_tools", diff --git a/cms/static/coffee/spec/main_squire.coffee b/cms/static/coffee/spec/main_squire.coffee index 66669b374f..24d855a75d 100644 --- a/cms/static/coffee/spec/main_squire.coffee +++ b/cms/static/coffee/spec/main_squire.coffee @@ -30,6 +30,7 @@ requirejs.config({ "jquery.tinymce": "xmodule_js/common_static/js/vendor/tinymce/js/tinymce/jquery.tinymce", "xmodule": "xmodule_js/src/xmodule", "xblock/cms.runtime.v1": "coffee/src/xblock/cms.runtime.v1", + "xblock/core": "xmodule_js/common_static/js/xblock/core", "xblock": "xmodule_js/common_static/coffee/src/xblock", "utility": "xmodule_js/common_static/js/src/utility", "sinon": "xmodule_js/common_static/js/vendor/sinon-1.7.1", diff --git a/cms/static/js_test.yml b/cms/static/js_test.yml index 2f039d5c02..52176290dd 100644 --- a/cms/static/js_test.yml +++ b/cms/static/js_test.yml @@ -60,6 +60,7 @@ lib_paths: - xmodule_js/common_static/js/vendor/URI.min.js - xmodule_js/common_static/js/vendor/jquery.smooth-scroll.min.js - xmodule_js/common_static/coffee/src/jquery.immediateDescendents.js + - xmodule_js/common_static/js/xblock/ - xmodule_js/common_static/coffee/src/xblock/ - xmodule_js/common_static/js/vendor/URI.min.js - xmodule_js/common_static/js/vendor/jQuery-File-Upload/js/jquery.iframe-transport.js diff --git a/cms/static/js_test_squire.yml b/cms/static/js_test_squire.yml index dbafe0859c..2dcb1e75ef 100644 --- a/cms/static/js_test_squire.yml +++ b/cms/static/js_test_squire.yml @@ -55,6 +55,7 @@ lib_paths: - xmodule_js/src/xmodule.js - xmodule_js/common_static/coffee/src/jquery.immediateDescendents.js - xmodule_js/common_static/js/test/i18n.js + - xmodule_js/common_static/js/xblock/ - xmodule_js/common_static/coffee/src/xblock/ - xmodule_js/common_static/js/vendor/URI.min.js - xmodule_js/common_static/js/vendor/jQuery-File-Upload/js/jquery.iframe-transport.js diff --git a/cms/static/require-config.js b/cms/static/require-config.js index 790312ed25..0c5646d037 100644 --- a/cms/static/require-config.js +++ b/cms/static/require-config.js @@ -35,6 +35,7 @@ require.config({ "tinymce": "js/vendor/tinymce/js/tinymce/tinymce.full.min", "jquery.tinymce": "js/vendor/tinymce/js/tinymce/jquery.tinymce.min", "xmodule": "/xmodule/xmodule", + "xblock/core": "js/xblock/core", "xblock": "coffee/src/xblock", "utility": "js/src/utility", "accessibility": "js/src/accessibility_tools", diff --git a/common/djangoapps/xmodule_django/models.py b/common/djangoapps/xmodule_django/models.py index b6449884e2..92257bc26d 100644 --- a/common/djangoapps/xmodule_django/models.py +++ b/common/djangoapps/xmodule_django/models.py @@ -1,9 +1,13 @@ +""" +Useful django models for implementing XBlock infrastructure in django. +""" + import warnings from django.db import models from django.core.exceptions import ValidationError from opaque_keys.edx.locations import SlashSeparatedCourseKey, Location -from opaque_keys.edx.keys import CourseKey, UsageKey +from opaque_keys.edx.keys import CourseKey, UsageKey, BlockTypeKey from opaque_keys.edx.locator import Locator from south.modelsinspector import add_introspection_rules @@ -91,7 +95,6 @@ class OpaqueKeyField(models.CharField): super(OpaqueKeyField, self).__init__(*args, **kwargs) - def to_python(self, value): if value is self.Empty or value is None: return None @@ -140,22 +143,38 @@ class OpaqueKeyField(models.CharField): class CourseKeyField(OpaqueKeyField): + """ + A django Field that stores a CourseKey object as a string. + """ description = "A CourseKey object, saved to the DB in the form of a string" KEY_CLASS = CourseKey class UsageKeyField(OpaqueKeyField): + """ + A django Field that stores a UsageKey object as a string. + """ description = "A Location object, saved to the DB in the form of a string" KEY_CLASS = UsageKey class LocationKeyField(UsageKeyField): + """ + A django Field that stores a UsageKey object as a string. + """ def __init__(self, *args, **kwargs): warnings.warn("LocationKeyField is deprecated. Please use UsageKeyField instead.", stacklevel=2) super(LocationKeyField, self).__init__(*args, **kwargs) +class BlockTypeKeyField(OpaqueKeyField): + """ + A django Field that stores a BlockTypeKey object as a string. + """ + description = "A BlockTypeKey object, saved to the DB in the form of a string." + KEY_CLASS = BlockTypeKey -add_introspection_rules([], ["^xmodule_django\.models\.CourseKeyField"]) -add_introspection_rules([], ["^xmodule_django\.models\.LocationKeyField"]) -add_introspection_rules([], ["^xmodule_django\.models\.UsageKeyField"]) + +add_introspection_rules([], [r"^xmodule_django\.models\.CourseKeyField"]) +add_introspection_rules([], [r"^xmodule_django\.models\.LocationKeyField"]) +add_introspection_rules([], [r"^xmodule_django\.models\.UsageKeyField"]) diff --git a/common/djangoapps/xmodule_modifiers.py b/common/djangoapps/xmodule_modifiers.py index 2349f1e3be..4d3bb974cc 100644 --- a/common/djangoapps/xmodule_modifiers.py +++ b/common/djangoapps/xmodule_modifiers.py @@ -72,7 +72,11 @@ def wrap_xblock(runtime_class, block, view, frag, context, usage_id_serializer, data = {} data.update(extra_data) - css_classes = ['xblock', 'xblock-{}'.format(markupsafe.escape(view))] + + css_classes = [ + 'xblock', + 'xblock-{}'.format(markupsafe.escape(view)) + ] if isinstance(block, (XModule, XModuleDescriptor)): if view in PREVIEW_VIEWS: @@ -90,9 +94,10 @@ def wrap_xblock(runtime_class, block, view, frag, context, usage_id_serializer, data['init'] = frag.js_init_fn data['runtime-class'] = runtime_class data['runtime-version'] = frag.js_init_version - data['block-type'] = block.scope_ids.block_type - data['usage-id'] = usage_id_serializer(block.scope_ids.usage_id) - data['request-token'] = request_token + + data['block-type'] = block.scope_ids.block_type + data['usage-id'] = usage_id_serializer(block.scope_ids.usage_id) + data['request-token'] = request_token if block.name: data['name'] = block.name diff --git a/common/lib/xmodule/xmodule/fields.py b/common/lib/xmodule/xmodule/fields.py index 93618be63c..a3c6aa19d7 100644 --- a/common/lib/xmodule/xmodule/fields.py +++ b/common/lib/xmodule/xmodule/fields.py @@ -33,7 +33,7 @@ class Date(JSONField): result = dateutil.parser.parse(field, default=self.PREVENT_DEFAULT_DAY_MON_SEED1) result_other = dateutil.parser.parse(field, default=self.PREVENT_DEFAULT_DAY_MON_SEED2) if result != result_other: - log.warning("Field {0} is missing month or day".format(self._name, field)) + log.warning("Field {0} is missing month or day".format(self.name)) return None if result.tzinfo is None: result = result.replace(tzinfo=UTC) @@ -59,7 +59,7 @@ class Date(JSONField): return field else: msg = "Field {0} has bad value '{1}'".format( - self._name, field) + self.name, field) raise TypeError(msg) def to_json(self, value): @@ -199,7 +199,7 @@ class RelativeTime(JSONField): if isinstance(value, basestring): return self.isotime_to_timedelta(value) - msg = "RelativeTime Field {0} has bad value '{1!r}'".format(self._name, value) + msg = "RelativeTime Field {0} has bad value '{1!r}'".format(self.name, value) raise TypeError(msg) def to_json(self, value): diff --git a/common/lib/xmodule/xmodule/modulestore/mongo/base.py b/common/lib/xmodule/xmodule/modulestore/mongo/base.py index 9f8899e399..5d432525c1 100644 --- a/common/lib/xmodule/xmodule/modulestore/mongo/base.py +++ b/common/lib/xmodule/xmodule/modulestore/mongo/base.py @@ -50,6 +50,7 @@ from xmodule.modulestore.draft_and_published import ModuleStoreDraftAndPublished from xmodule.modulestore.edit_info import EditInfoRuntimeMixin from xmodule.modulestore.exceptions import ItemNotFoundError, DuplicateCourseError, ReferentialIntegrityError from xmodule.modulestore.inheritance import InheritanceMixin, inherit_metadata, InheritanceKeyValueStore +from xmodule.modulestore.xml import CourseLocationManager log = logging.getLogger(__name__) @@ -173,6 +174,9 @@ class CachingDescriptorSystem(MakoDescriptorSystem, EditInfoRuntimeMixin): render_template: a function for rendering templates, as per MakoDescriptorSystem """ + id_manager = CourseLocationManager(course_key) + kwargs.setdefault('id_reader', id_manager) + kwargs.setdefault('id_generator', id_manager) super(CachingDescriptorSystem, self).__init__( field_data=None, load_item=self.load_item, 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 7881be1c2f..205bf1eaf5 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 @@ -1,6 +1,7 @@ import sys import logging from contracts import contract, new_contract +from fs.osfs import OSFS from lazy import lazy from xblock.runtime import KvsFieldData from xblock.fields import ScopeIds @@ -8,13 +9,13 @@ from opaque_keys.edx.locator import BlockUsageLocator, LocalId, CourseLocator, L from xmodule.mako_module import MakoDescriptorSystem from xmodule.error_module import ErrorDescriptor from xmodule.errortracker import exc_info_to_str -from ..exceptions import ItemNotFoundError -from .split_mongo_kvs import SplitMongoKVS -from fs.osfs import OSFS -from .definition_lazy_loader import DefinitionLazyLoader from xmodule.modulestore.edit_info import EditInfoRuntimeMixin +from xmodule.modulestore.exceptions import ItemNotFoundError from xmodule.modulestore.inheritance import inheriting_field_data, InheritanceMixin from xmodule.modulestore.split_mongo import BlockKey, CourseEnvelope +from xmodule.modulestore.split_mongo.id_manager import SplitMongoIdManager +from xmodule.modulestore.split_mongo.definition_lazy_loader import DefinitionLazyLoader +from xmodule.modulestore.split_mongo.split_mongo_kvs import SplitMongoKVS log = logging.getLogger(__name__) @@ -54,6 +55,10 @@ class CachingDescriptorSystem(MakoDescriptorSystem, EditInfoRuntimeMixin): root = modulestore.fs_root / course_entry.structure['_id'] root.makedirs_p() # create directory if it doesn't exist + id_manager = SplitMongoIdManager(self) + kwargs.setdefault('id_reader', id_manager) + kwargs.setdefault('id_generator', id_manager) + super(CachingDescriptorSystem, self).__init__( field_data=None, load_item=self._load_item, diff --git a/common/lib/xmodule/xmodule/modulestore/split_mongo/id_manager.py b/common/lib/xmodule/xmodule/modulestore/split_mongo/id_manager.py new file mode 100644 index 0000000000..4ff486372d --- /dev/null +++ b/common/lib/xmodule/xmodule/modulestore/split_mongo/id_manager.py @@ -0,0 +1,32 @@ +""" +An implementation of IdReader and IdGenerator that manages ids for the SplitMongo storage +mechanism. +""" + +from opaque_keys.edx.locator import LocalId, DefinitionLocator +from xmodule.x_module import OpaqueKeyReader, AsideKeyGenerator +from xmodule.modulestore.split_mongo import BlockKey + + +# TODO: Migrate split_mongo to use this class for all key mapping/creation. +class SplitMongoIdManager(OpaqueKeyReader, AsideKeyGenerator): # pylint: disable=abstract-method + """ + An IdManager that knows how to retrieve the DefinitionLocator, given + a usage_id and a :class:`.CachingDescriptorSystem`. + """ + def __init__(self, caching_descriptor_system): + self._cds = caching_descriptor_system + + def get_definition_id(self, usage_id): + if isinstance(usage_id.block_id, LocalId): + # a LocalId indicates that this block hasn't been persisted yet, and is instead stored + # in-memory in the local_modules dictionary. + return self._cds.local_modules[usage_id].scope_ids.def_id + else: + block_key = BlockKey.from_usage_key(usage_id) + module_data = self._cds.get_module_data(block_key, usage_id.course_key) + + if 'definition' in module_data: + return DefinitionLocator(usage_id.block_type, module_data['definition']) + else: + raise ValueError("All non-local blocks should have a definition specified") diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_libraries.py b/common/lib/xmodule/xmodule/modulestore/tests/test_libraries.py index c380da1e8b..1447b02384 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_libraries.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_libraries.py @@ -203,5 +203,6 @@ class TestLibraries(MixedSplitTestCase): message = u"Hello world" hello_render = lambda _, context: Fragment(message) with patch('xmodule.html_module.HtmlDescriptor.author_view', hello_render, create=True): - result = library.render(AUTHOR_VIEW, context) + with patch('xmodule.x_module.descriptor_global_get_asides', lambda block: []): + result = library.render(AUTHOR_VIEW, context) self.assertIn(message, result.content) diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py b/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py index a6f1dd5aa7..aa1f80443b 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py @@ -420,7 +420,7 @@ class TestMongoModuleStore(TestMongoModuleStoreBase): assert_equals(len(course_locations), 1) assert_in(SlashSeparatedCourseKey('edX', 'simple', '2012_Fall'), course_locations) - @Plugin.register_temp_plugin(ReferenceTestXBlock, 'ref_test') + @XBlock.register_temp_plugin(ReferenceTestXBlock, 'ref_test') def test_reference_converters(self): """ Test that references types get deserialized correctly diff --git a/common/lib/xmodule/xmodule/modulestore/xml.py b/common/lib/xmodule/xmodule/modulestore/xml.py index e6e08a416b..e6a0ca15c3 100644 --- a/common/lib/xmodule/xmodule/modulestore/xml.py +++ b/common/lib/xmodule/xmodule/modulestore/xml.py @@ -18,7 +18,7 @@ from contextlib import contextmanager from xmodule.error_module import ErrorDescriptor from xmodule.errortracker import make_error_tracker, exc_info_to_str from xmodule.mako_module import MakoDescriptorSystem -from xmodule.x_module import XMLParsingSystem, policy_key +from xmodule.x_module import XMLParsingSystem, policy_key, OpaqueKeyReader, AsideKeyGenerator from xmodule.modulestore.xml_exporter import DEFAULT_CONTENT_FIELDS from xmodule.modulestore import ModuleStoreEnum, ModuleStoreReadBase from xmodule.tabs import CourseTabList @@ -27,7 +27,7 @@ from opaque_keys.edx.locations import SlashSeparatedCourseKey, Location from opaque_keys.edx.locator import CourseLocator from xblock.field_data import DictFieldData -from xblock.runtime import DictKeyValueStore, IdGenerator +from xblock.runtime import DictKeyValueStore from .exceptions import ItemNotFoundError @@ -64,7 +64,6 @@ class ImportSystem(XMLParsingSystem, MakoDescriptorSystem): """ self.unnamed = defaultdict(int) # category -> num of new url_names for that category self.used_names = defaultdict(set) # category -> set of used url_names - id_generator = CourseLocationGenerator(course_id) # cdodge: adding the course_id as passed in for later reference rather than having to recomine the org/course/url_name self.course_id = course_id @@ -175,7 +174,7 @@ class ImportSystem(XMLParsingSystem, MakoDescriptorSystem): descriptor = create_block_from_xml( etree.tostring(xml_data, encoding='unicode'), self, - id_generator, + id_manager, ) except Exception as err: # pylint: disable=broad-except if not self.load_error_modules: @@ -201,7 +200,7 @@ class ImportSystem(XMLParsingSystem, MakoDescriptorSystem): descriptor = ErrorDescriptor.from_xml( xml, self, - id_generator, + id_manager, err_msg ) @@ -229,12 +228,16 @@ class ImportSystem(XMLParsingSystem, MakoDescriptorSystem): resources_fs = OSFS(xmlstore.data_dir / course_dir) + id_manager = CourseLocationManager(course_id) + super(ImportSystem, self).__init__( load_item=load_item, resources_fs=resources_fs, render_template=render_template, error_tracker=error_tracker, process_xml=process_xml, + id_generator=id_manager, + id_reader=id_manager, **kwargs ) @@ -245,12 +248,13 @@ class ImportSystem(XMLParsingSystem, MakoDescriptorSystem): block.children.append(child_block.scope_ids.usage_id) -class CourseLocationGenerator(IdGenerator): +class CourseLocationManager(OpaqueKeyReader, AsideKeyGenerator): """ IdGenerator for Location-based definition ids and usage ids based within a course """ def __init__(self, course_id): + super(CourseLocationManager, self).__init__() self.course_id = course_id self.autogen_ids = itertools.count(0) @@ -263,6 +267,17 @@ class CourseLocationGenerator(IdGenerator): slug = 'autogen_{}_{}'.format(block_type, self.autogen_ids.next()) return self.course_id.make_usage_key(block_type, slug) + def get_definition_id(self, usage_id): + """Retrieve the definition that a usage is derived from. + + Args: + usage_id: The id of the usage to query + + Returns: + The `definition_id` the usage is derived from + """ + return usage_id + def _make_usage_key(course_key, value): """ diff --git a/common/lib/xmodule/xmodule/tests/__init__.py b/common/lib/xmodule/xmodule/tests/__init__.py index 44d9fc0123..eac771de45 100644 --- a/common/lib/xmodule/xmodule/tests/__init__.py +++ b/common/lib/xmodule/xmodule/tests/__init__.py @@ -28,6 +28,7 @@ from xmodule.mako_module import MakoDescriptorSystem from xmodule.error_module import ErrorDescriptor from xmodule.modulestore import ModuleStoreEnum from xmodule.modulestore.mongo.draft import DraftModuleStore +from xmodule.modulestore.xml import CourseLocationManager from xmodule.modulestore.draft_and_published import DIRECT_ONLY_CATEGORIES, ModuleStoreDraftAndPublished @@ -51,9 +52,16 @@ class TestModuleSystem(ModuleSystem): # pylint: disable=abstract-method """ ModuleSystem for testing """ + def __init__(self, **kwargs): + id_manager = CourseLocationManager(kwargs['course_id']) + kwargs.setdefault('id_reader', id_manager) + kwargs.setdefault('id_generator', id_manager) + kwargs.setdefault('services', {}).setdefault('field-data', DictFieldData({})) + super(TestModuleSystem, self).__init__(**kwargs) + def handler_url(self, block, handler, suffix='', query='', thirdparty=False): return '{usage_id}/{handler}{suffix}?{query}'.format( - usage_id=block.scope_ids.usage_id.to_deprecated_string(), + usage_id=unicode(block.scope_ids.usage_id), handler=handler, suffix=suffix, query=query, @@ -61,10 +69,14 @@ class TestModuleSystem(ModuleSystem): # pylint: disable=abstract-method def local_resource_url(self, block, uri): return 'resource/{usage_id}/{uri}'.format( - usage_id=block.scope_ids.usage_id.to_deprecated_string(), + usage_id=unicode(block.scope_ids.usage_id), uri=uri, ) + # Disable XBlockAsides in most tests + def get_asides(self, block): + return [] + def get_test_system(course_id=SlashSeparatedCourseKey('org', 'course', 'run')): """ @@ -113,13 +125,16 @@ def get_test_descriptor_system(): """ Construct a test DescriptorSystem instance. """ + field_data = DictFieldData({}) + return MakoDescriptorSystem( load_item=Mock(), resources_fs=Mock(), error_tracker=Mock(), render_template=mock_render_template, mixins=(InheritanceMixin, XModuleMixin), - field_data=DictFieldData({}), + field_data=field_data, + services={'field-data': field_data}, ) @@ -149,13 +164,8 @@ class LogicTest(unittest.TestCase): raw_field_data = {} def setUp(self): - class EmptyClass: - """Empty object.""" - url_name = '' - category = 'test' - self.system = get_test_system() - self.descriptor = EmptyClass() + self.descriptor = Mock(name="descriptor", url_name='', category='test') self.xmodule_class = self.descriptor_class.module_class usage_key = self.system.course_id.make_usage_key(self.descriptor.category, 'test_loc') diff --git a/common/lib/xmodule/xmodule/tests/test_conditional.py b/common/lib/xmodule/xmodule/tests/test_conditional.py index bc20075704..373388a7ac 100644 --- a/common/lib/xmodule/xmodule/tests/test_conditional.py +++ b/common/lib/xmodule/xmodule/tests/test_conditional.py @@ -8,7 +8,7 @@ from xblock.field_data import DictFieldData from xblock.fields import ScopeIds from xmodule.error_module import NonStaffErrorDescriptor from opaque_keys.edx.locations import SlashSeparatedCourseKey, Location -from xmodule.modulestore.xml import ImportSystem, XMLModuleStore, CourseLocationGenerator +from xmodule.modulestore.xml import ImportSystem, XMLModuleStore, CourseLocationManager from xmodule.conditional_module import ConditionalDescriptor from xmodule.tests import DATA_DIR, get_test_system, get_test_descriptor_system from xmodule.x_module import STUDENT_VIEW @@ -60,7 +60,7 @@ class ConditionalFactory(object): source_descriptor = NonStaffErrorDescriptor.from_xml( 'some random xml data', system, - id_generator=CourseLocationGenerator(SlashSeparatedCourseKey('edX', 'conditional_test', 'test_run')), + id_generator=CourseLocationManager(source_location.course_key), error_msg='random error message' ) else: diff --git a/common/lib/xmodule/xmodule/tests/test_error_module.py b/common/lib/xmodule/xmodule/tests/test_error_module.py index 85c28de7ca..ddb9207576 100644 --- a/common/lib/xmodule/xmodule/tests/test_error_module.py +++ b/common/lib/xmodule/xmodule/tests/test_error_module.py @@ -4,7 +4,7 @@ Tests for ErrorModule and NonStaffErrorModule import unittest from xmodule.tests import get_test_system from xmodule.error_module import ErrorDescriptor, ErrorModule, NonStaffErrorDescriptor -from xmodule.modulestore.xml import CourseLocationGenerator +from xmodule.modulestore.xml import CourseLocationManager from opaque_keys.edx.locations import SlashSeparatedCourseKey, Location from xmodule.x_module import XModuleDescriptor, XModule, STUDENT_VIEW from mock import MagicMock, Mock, patch @@ -34,7 +34,7 @@ class TestErrorModule(unittest.TestCase, SetupTestErrorModules): descriptor = ErrorDescriptor.from_xml( self.valid_xml, self.system, - CourseLocationGenerator(self.course_id), + CourseLocationManager(self.course_id), self.error_msg ) self.assertIsInstance(descriptor, ErrorDescriptor) @@ -69,7 +69,7 @@ class TestNonStaffErrorModule(unittest.TestCase, SetupTestErrorModules): descriptor = NonStaffErrorDescriptor.from_xml( self.valid_xml, self.system, - CourseLocationGenerator(self.course_id) + CourseLocationManager(self.course_id) ) self.assertIsInstance(descriptor, NonStaffErrorDescriptor) @@ -77,7 +77,7 @@ class TestNonStaffErrorModule(unittest.TestCase, SetupTestErrorModules): descriptor = NonStaffErrorDescriptor.from_xml( self.valid_xml, self.system, - CourseLocationGenerator(self.course_id) + CourseLocationManager(self.course_id) ) descriptor.xmodule_runtime = self.system context_repr = self.system.render(descriptor, STUDENT_VIEW).content diff --git a/common/lib/xmodule/xmodule/tests/xml/__init__.py b/common/lib/xmodule/xmodule/tests/xml/__init__.py index 599fa49840..534d3d3d4c 100644 --- a/common/lib/xmodule/xmodule/tests/xml/__init__.py +++ b/common/lib/xmodule/xmodule/tests/xml/__init__.py @@ -7,7 +7,7 @@ from unittest import TestCase from xmodule.x_module import XMLParsingSystem, policy_key from xmodule.mako_module import MakoDescriptorSystem -from xmodule.modulestore.xml import create_block_from_xml, CourseLocationGenerator +from xmodule.modulestore.xml import create_block_from_xml, CourseLocationManager from opaque_keys.edx.locations import SlashSeparatedCourseKey, Location from xblock.runtime import KvsFieldData, DictKeyValueStore @@ -43,7 +43,7 @@ class InMemorySystem(XMLParsingSystem, MakoDescriptorSystem): # pylint: disable descriptor = create_block_from_xml( xml, self, - CourseLocationGenerator(self.course_id), + CourseLocationManager(self.course_id), ) self._descriptors[descriptor.location.to_deprecated_string()] = descriptor return descriptor diff --git a/common/lib/xmodule/xmodule/x_module.py b/common/lib/xmodule/xmodule/x_module.py index 5332296e15..5b96721153 100644 --- a/common/lib/xmodule/xmodule/x_module.py +++ b/common/lib/xmodule/xmodule/x_module.py @@ -18,12 +18,13 @@ from webob.multidict import MultiDict from xblock.core import XBlock from xblock.fields import Scope, Integer, Float, List, XBlockMixin, String, Dict from xblock.fragment import Fragment -from xblock.runtime import Runtime, IdReader +from xblock.runtime import Runtime, IdReader, IdGenerator from xmodule.fields import RelativeTime from xmodule.errortracker import exc_info_to_str from xmodule.modulestore.exceptions import ItemNotFoundError from opaque_keys.edx.keys import UsageKey +from opaque_keys.edx.asides import AsideUsageKeyV1, AsideDefinitionKeyV1 from xmodule.exceptions import UndefinedContext import dogstats_wrapper as dog_stats_api @@ -65,7 +66,7 @@ class OpaqueKeyReader(IdReader): Returns: The `definition_id` the usage is derived from """ - return usage_id.definition_key + raise NotImplementedError("Specific Modulestores must implement get_definition_id") def get_block_type(self, def_id): """Retrieve the block_type of a particular definition @@ -78,6 +79,91 @@ class OpaqueKeyReader(IdReader): """ return def_id.block_type + def get_usage_id_from_aside(self, aside_id): + """ + Retrieve the XBlock `usage_id` associated with this aside usage id. + + Args: + aside_id: The usage id of the XBlockAside. + + Returns: + The `usage_id` of the usage the aside is commenting on. + """ + return aside_id.usage_key + + def get_definition_id_from_aside(self, aside_id): + """ + Retrieve the XBlock `definition_id` associated with this aside definition id. + + Args: + aside_id: The usage id of the XBlockAside. + + Returns: + The `definition_id` of the usage the aside is commenting on. + """ + return aside_id.definition_key + + def get_aside_type_from_usage(self, aside_id): + """ + Retrieve the XBlockAside `aside_type` associated with this aside + usage id. + + Args: + aside_id: The usage id of the XBlockAside. + + Returns: + The `aside_type` of the aside. + """ + return aside_id.aside_type + + def get_aside_type_from_definition(self, aside_id): + """ + Retrieve the XBlockAside `aside_type` associated with this aside + definition id. + + Args: + aside_id: The definition id of the XBlockAside. + + Returns: + The `aside_type` of the aside. + """ + return aside_id.aside_type + + +class AsideKeyGenerator(IdGenerator): # pylint: disable=abstract-method + """ + An :class:`.IdGenerator` that only provides facilities for constructing new XBlockAsides. + """ + def create_aside(self, definition_id, usage_id, aside_type): + """ + Make a new aside definition and usage ids, indicating an :class:`.XBlockAside` of type `aside_type` + commenting on an :class:`.XBlock` usage `usage_id` + + Returns: + (aside_definition_id, aside_usage_id) + """ + def_key = AsideDefinitionKeyV1(definition_id, aside_type) + usage_key = AsideUsageKeyV1(usage_id, aside_type) + return (def_key, usage_key) + + def create_usage(self, def_id): + """Make a usage, storing its definition id. + + Returns the newly-created usage id. + """ + raise NotImplementedError("Specific Modulestores must provide implementations of create_usage") + + def create_definition(self, block_type, slug=None): + """Make a definition, storing its block type. + + If `slug` is provided, it is a suggestion that the definition id + incorporate the slug somehow. + + Returns the newly-created definition id. + + """ + raise NotImplementedError("Specific Modulestores must provide implementations of create_definition") + def dummy_track(_event_type, _event): pass @@ -160,6 +246,8 @@ class XModuleMixin(XBlockMixin): Adding this Mixin to an :class:`XBlock` allows it to cooperate with old-style :class:`XModules` """ + entry_point = "xmodule.v1" + # Attributes for inspection of the descriptor # This indicates whether the xmodule is a problem-type. @@ -526,6 +614,7 @@ class XModule(XModuleMixin, HTMLSnippet, XBlock): # pylint: disable=abstract-me field_data: A dictionary-like object that maps field names to values for those fields. """ + # Set the descriptor first so that we can proxy to it self.descriptor = descriptor super(XModule, self).__init__(*args, **kwargs) @@ -715,7 +804,6 @@ class XModuleDescriptor(XModuleMixin, HTMLSnippet, ResourceTemplates, XBlock): create a problem, and can generate XModules (which do know about student state). """ - entry_point = "xmodule.v1" module_class = XModule # VS[compat]. Backwards compatibility code that can go away after @@ -997,7 +1085,7 @@ class XModuleDescriptor(XModuleMixin, HTMLSnippet, ResourceTemplates, XBlock): class ConfigurableFragmentWrapper(object): # pylint: disable=abstract-method """ - Runtime mixin that allows for composition of many `wrap_child` wrappers + Runtime mixin that allows for composition of many `wrap_xblock` wrappers """ def __init__(self, wrappers=None, **kwargs): """ @@ -1013,7 +1101,7 @@ class ConfigurableFragmentWrapper(object): # pylint: disable=abstract-method else: self.wrappers = [] - def wrap_child(self, block, view, frag, context): + def wrap_xblock(self, block, view, frag, context): """ See :func:`Runtime.wrap_child` """ @@ -1043,6 +1131,16 @@ def descriptor_global_local_resource_url(block, uri): # pylint: disable=invalid raise NotImplementedError("Applications must monkey-patch this function before using local_resource_url for studio_view") +# This function exists to give applications (LMS/CMS) a place to monkey-patch until +# we can refactor modulestore to split out the FieldData half of its interface from +# the Runtime part of its interface. This function matches the Runtime.get_asides interface +def descriptor_global_get_asides(block): # pylint: disable=unused-argument + """ + See :meth:`xblock.runtime.Runtime.get_asides`. + """ + raise NotImplementedError("Applications must monkey-patch this function before using get_asides from a DescriptorSystem.") + + class MetricsMixin(object): """ Mixin for adding metric logging for render and handle methods in the DescriptorSystem and ModuleSystem. @@ -1137,7 +1235,9 @@ class DescriptorSystem(MetricsMixin, ConfigurableFragmentWrapper, Runtime): # p local_resource_url: an implementation of :meth:`xblock.runtime.Runtime.local_resource_url` """ - super(DescriptorSystem, self).__init__(id_reader=OpaqueKeyReader(), **kwargs) + kwargs.setdefault('id_reader', OpaqueKeyReader()) + kwargs.setdefault('id_generator', AsideKeyGenerator()) + super(DescriptorSystem, self).__init__(**kwargs) # This is used by XModules to write out separate files during xml export self.export_fs = None @@ -1215,6 +1315,19 @@ class DescriptorSystem(MetricsMixin, ConfigurableFragmentWrapper, Runtime): # p # global function that the application can override. return descriptor_global_local_resource_url(block, uri) + def get_asides(self, block): + """ + See :meth:`xblock.runtime.Runtime:get_asides` for documentation. + """ + if getattr(block, 'xmodule_runtime', None) is not None: + return block.xmodule_runtime.get_asides(block) + else: + # Currently, Modulestore is responsible for instantiating DescriptorSystems + # This means that LMS/CMS don't have a way to define a subclass of DescriptorSystem + # that implements the correct get_asides. So, for now, instead, we will reference a + # global function that the application can override. + return descriptor_global_get_asides(block) + def resource_url(self, resource): """ See :meth:`xblock.runtime.Runtime:resource_url` for documentation. @@ -1335,7 +1448,9 @@ class ModuleSystem(MetricsMixin, ConfigurableFragmentWrapper, Runtime): # pylin # Usage_store is unused, and field_data is often supplanted with an # explicit field_data during construct_xblock. - super(ModuleSystem, self).__init__(id_reader=OpaqueKeyReader(), field_data=field_data, **kwargs) + kwargs.setdefault('id_reader', getattr(descriptor_runtime, 'id_reader', OpaqueKeyReader())) + kwargs.setdefault('id_generator', getattr(descriptor_runtime, 'id_generator', AsideKeyGenerator())) + super(ModuleSystem, self).__init__(field_data=field_data, **kwargs) self.STATIC_URL = static_url self.xqueue = xqueue @@ -1373,6 +1488,9 @@ class ModuleSystem(MetricsMixin, ConfigurableFragmentWrapper, Runtime): # pylin self.descriptor_runtime = descriptor_runtime self.rebind_noauth_module_to_user = rebind_noauth_module_to_user + if user: + self.user_id = user.id + def get(self, attr): """ provide uniform access to attributes (like etree).""" return self.__dict__.get(attr) diff --git a/common/static/coffee/spec/xblock/core_spec.coffee b/common/static/coffee/spec/xblock/core_spec.coffee index 7df6fbf263..7e7c1e081a 100644 --- a/common/static/coffee/spec/xblock/core_spec.coffee +++ b/common/static/coffee/spec/xblock/core_spec.coffee @@ -35,7 +35,7 @@ describe "XBlock", -> window.initFnZ = jasmine.createSpy() @fakeChildren = ['list', 'of', 'children'] - spyOn(XBlock, 'initializeBlocks').andReturn(@fakeChildren) + spyOn(XBlock, 'initializeXBlocks').andReturn(@fakeChildren) @vANode = $('#vA')[0] @vZNode = $('#vZ')[0] @@ -50,8 +50,8 @@ describe "XBlock", -> expect(TestRuntime.vZ).toHaveBeenCalledWith() it "loads the right init function", -> - expect(window.initFnA).toHaveBeenCalledWith(@runtimeA, @vANode) - expect(window.initFnZ).toHaveBeenCalledWith(@runtimeZ, @vZNode) + expect(window.initFnA).toHaveBeenCalledWith(@runtimeA, @vANode, {}) + expect(window.initFnZ).toHaveBeenCalledWith(@runtimeZ, @vZNode, {}) it "loads when missing versions", -> expect(@missingVersionBlock.element).toBe($('#missing-version')) @@ -74,8 +74,8 @@ describe "XBlock", -> expect(@missingInitBlock.element).toBe($('#missing-init')[0]) it "passes through the request token", -> - expect(XBlock.initializeBlocks).toHaveBeenCalledWith($(@vANode), 'req-token-a') - expect(XBlock.initializeBlocks).toHaveBeenCalledWith($(@vZNode), 'req-token-z') + expect(XBlock.initializeXBlocks).toHaveBeenCalledWith($(@vANode), 'req-token-a') + expect(XBlock.initializeXBlocks).toHaveBeenCalledWith($(@vZNode), 'req-token-z') describe "initializeBlocks", -> diff --git a/common/static/coffee/src/xblock/core.coffee b/common/static/coffee/src/xblock/core.coffee deleted file mode 100644 index bafcff97c9..0000000000 --- a/common/static/coffee/src/xblock/core.coffee +++ /dev/null @@ -1,57 +0,0 @@ -@XBlock = - Runtime: {} - - ### - Initialize the javascript for a single xblock element, and for all of it's - xblock children that match requestToken. If requestToken is omitted, use the - data-request-token attribute from element, or use the request-tokens specified on - the children themselves. - ### - initializeBlock: (element, requestToken) -> - $element = $(element) - requestToken = requestToken or $element.data('request-token') - children = @initializeBlocks($element, requestToken) - runtime = $element.data("runtime-class") - version = $element.data("runtime-version") - initFnName = $element.data("init") - $element.prop('xblock_children', children) - if runtime? and version? and initFnName? - runtime = new window[runtime]["v#{version}"] - initFn = window[initFnName] - if initFn.length > 2 - initargs = $(".xblock_json_init_args", element) - if initargs.length == 0 - console.log("Warning: XBlock expects data parameters") - data = JSON.parse(initargs.text()) - block = initFn(runtime, element, data) ? {} - else - block = initFn(runtime, element) ? {} - block.runtime = runtime - else - elementTag = $('