diff --git a/cms/djangoapps/contentstore/tests/test_contentstore.py b/cms/djangoapps/contentstore/tests/test_contentstore.py
index 39bb0e3147..c23d9bb44b 100644
--- a/cms/djangoapps/contentstore/tests/test_contentstore.py
+++ b/cms/djangoapps/contentstore/tests/test_contentstore.py
@@ -1501,7 +1501,7 @@ class ContentStoreTest(ModuleStoreTestCase):
"""Test new course creation - error path for bad organization name"""
self.course_data['org'] = 'University of California, Berkeley'
self.assert_course_creation_failed(
- "Unable to create course 'Robot Super Course'.\n\nInvalid characters in 'University of California, Berkeley'.")
+ "Unable to create course 'Robot Super Course'.\n\nInvalid characters in u'University of California, Berkeley'.")
def test_create_course_with_course_creation_disabled_staff(self):
"""Test new course creation -- course creation disabled, but staff access."""
diff --git a/cms/djangoapps/contentstore/views/preview.py b/cms/djangoapps/contentstore/views/preview.py
index 69677f1ec3..3a8cba68f2 100644
--- a/cms/djangoapps/contentstore/views/preview.py
+++ b/cms/djangoapps/contentstore/views/preview.py
@@ -12,7 +12,7 @@ from xmodule.error_module import ErrorDescriptor
from xmodule.exceptions import NotFoundError, ProcessingError
from xmodule.modulestore.django import modulestore
from xmodule.x_module import ModuleSystem
-from xblock.runtime import DbModel
+from xblock.runtime import KvsFieldData
from xblock.django.request import webob_to_django_response, django_to_webob_request
from xblock.exceptions import NoSuchHandlerError
@@ -142,7 +142,7 @@ def _load_preview_module(request, descriptor):
request: The active django request
descriptor: An XModuleDescriptor
"""
- student_data = DbModel(SessionKeyValueStore(request))
+ student_data = KvsFieldData(SessionKeyValueStore(request))
descriptor.bind_for_student(
_preview_module_system(request, descriptor),
LmsFieldData(descriptor._field_data, student_data), # pylint: disable=protected-access
diff --git a/common/lib/xmodule/xmodule/abtest_module.py b/common/lib/xmodule/xmodule/abtest_module.py
index 06d4e0b2d2..8fe9c67554 100644
--- a/common/lib/xmodule/xmodule/abtest_module.py
+++ b/common/lib/xmodule/xmodule/abtest_module.py
@@ -111,7 +111,8 @@ class ABTestDescriptor(ABTestFields, RawDescriptor, XmlDescriptor):
child_content_urls = []
for child in group:
try:
- child_content_urls.append(system.process_xml(etree.tostring(child)).location.url())
+ child_block = system.process_xml(etree.tostring(child))
+ child_content_urls.append(child_block.scope_ids.usage_id)
except:
log.exception("Unable to load child when parsing ABTest. Continuing...")
continue
diff --git a/common/lib/xmodule/xmodule/backcompat_module.py b/common/lib/xmodule/xmodule/backcompat_module.py
index 9e7b132e9e..67ab204cf2 100644
--- a/common/lib/xmodule/xmodule/backcompat_module.py
+++ b/common/lib/xmodule/xmodule/backcompat_module.py
@@ -17,7 +17,7 @@ def process_includes(fn):
are supposed to include
"""
@wraps(fn)
- def from_xml(cls, xml_data, system, org=None, course=None):
+ def from_xml(cls, xml_data, system, id_generator):
xml_object = etree.fromstring(xml_data)
next_include = xml_object.find('include')
while next_include is not None:
@@ -55,14 +55,14 @@ def process_includes(fn):
parent.remove(next_include)
next_include = xml_object.find('include')
- return fn(cls, etree.tostring(xml_object), system, org, course)
+ return fn(cls, etree.tostring(xml_object), system, id_generator)
return from_xml
class SemanticSectionDescriptor(XModuleDescriptor):
@classmethod
@process_includes
- def from_xml(cls, xml_data, system, org=None, course=None):
+ def from_xml(cls, xml_data, system, id_generator):
"""
Removes sections with single child elements in favor of just embedding
the child element
@@ -83,7 +83,7 @@ class SemanticSectionDescriptor(XModuleDescriptor):
class TranslateCustomTagDescriptor(XModuleDescriptor):
@classmethod
- def from_xml(cls, xml_data, system, org=None, course=None):
+ def from_xml(cls, xml_data, system, id_generator):
"""
Transforms the xml_data from <$custom_tag attr="" attr=""/> to
diff --git a/common/lib/xmodule/xmodule/conditional_module.py b/common/lib/xmodule/xmodule/conditional_module.py
index f9b47da7ea..9a2332d556 100644
--- a/common/lib/xmodule/xmodule/conditional_module.py
+++ b/common/lib/xmodule/xmodule/conditional_module.py
@@ -20,7 +20,7 @@ log = logging.getLogger('edx.' + __name__)
class ConditionalFields(object):
has_children = True
- show_tag_list = List(help="Poll answers", scope=Scope.content)
+ show_tag_list = List(help="List of urls of children that are references to external modules", scope=Scope.content)
class ConditionalModule(ConditionalFields, XModule):
@@ -196,6 +196,7 @@ class ConditionalDescriptor(ConditionalFields, SequenceDescriptor):
locations = [location.strip() for location in sources.split(';')]
for location in locations:
if Location.is_valid(location): # Check valid location url.
+ location = Location(location)
try:
if return_descriptor:
descriptor = system.load_item(location)
@@ -221,15 +222,13 @@ class ConditionalDescriptor(ConditionalFields, SequenceDescriptor):
show_tag_list = []
for child in xml_object:
if child.tag == 'show':
- location = ConditionalDescriptor.parse_sources(
- child, system)
+ location = ConditionalDescriptor.parse_sources(child, system)
children.extend(location)
- show_tag_list.extend(location)
+ show_tag_list.extend(location.url())
else:
try:
descriptor = system.process_xml(etree.tostring(child))
- module_url = descriptor.location.url()
- children.append(module_url)
+ children.append(descriptor.scope_ids.usage_id)
except:
msg = "Unable to load child when parsing Conditional."
log.exception(msg)
diff --git a/common/lib/xmodule/xmodule/course_module.py b/common/lib/xmodule/xmodule/course_module.py
index e8a27a4583..3e8682ee9a 100644
--- a/common/lib/xmodule/xmodule/course_module.py
+++ b/common/lib/xmodule/xmodule/course_module.py
@@ -497,8 +497,8 @@ class CourseDescriptor(CourseFields, SequenceDescriptor):
return policy_str
@classmethod
- def from_xml(cls, xml_data, system, org=None, course=None):
- instance = super(CourseDescriptor, cls).from_xml(xml_data, system, org, course)
+ def from_xml(cls, xml_data, system, id_generator):
+ instance = super(CourseDescriptor, cls).from_xml(xml_data, system, id_generator)
# bleh, have to parse the XML here to just pull out the url_name attribute
# I don't think it's stored anywhere in the instance.
diff --git a/common/lib/xmodule/xmodule/crowdsource_hinter.py b/common/lib/xmodule/xmodule/crowdsource_hinter.py
index 6f12aa1dc9..d2c6224112 100644
--- a/common/lib/xmodule/xmodule/crowdsource_hinter.py
+++ b/common/lib/xmodule/xmodule/crowdsource_hinter.py
@@ -388,7 +388,8 @@ class CrowdsourceHinterDescriptor(CrowdsourceHinterFields, RawDescriptor):
children = []
for child in xml_object:
try:
- children.append(system.process_xml(etree.tostring(child, encoding='unicode')).location.url())
+ child_block = system.process_xml(etree.tostring(child, encoding='unicode'))
+ children.append(child_block.scope_ids.usage_id)
except Exception as e:
log.exception("Unable to load child when parsing CrowdsourceHinter. Continuing...")
if system.error_tracker is not None:
diff --git a/common/lib/xmodule/xmodule/error_module.py b/common/lib/xmodule/xmodule/error_module.py
index c6c798a114..4f7ad25eb0 100644
--- a/common/lib/xmodule/xmodule/error_module.py
+++ b/common/lib/xmodule/xmodule/error_module.py
@@ -81,12 +81,10 @@ class ErrorDescriptor(ErrorFields, XModuleDescriptor):
@classmethod
def _construct(cls, system, contents, error_msg, location):
+ location = Location(location)
- if isinstance(location, dict) and 'course' in location:
- location = Location(location)
- if isinstance(location, Location) and location.name is None:
+ if location.category == 'error':
location = location.replace(
- category='error',
# Pick a unique url_name -- the sha1 hash of the contents.
# NOTE: We could try to pull out the url_name of the errored descriptor,
# but url_names aren't guaranteed to be unique between descriptor types,
@@ -136,7 +134,7 @@ class ErrorDescriptor(ErrorFields, XModuleDescriptor):
)
@classmethod
- def from_xml(cls, xml_data, system, org=None, course=None,
+ def from_xml(cls, xml_data, system, id_generator,
error_msg='Error not available'):
'''Create an instance of this descriptor from the supplied data.
@@ -162,7 +160,7 @@ class ErrorDescriptor(ErrorFields, XModuleDescriptor):
# Save the error to display later--overrides other problems
error_msg = exc_info_to_str(sys.exc_info())
- return cls._construct(system, xml_data, error_msg, location=Location('i4x', org, course, None, None))
+ return cls._construct(system, xml_data, error_msg, location=id_generator.create_definition('error'))
def export_to_xml(self, resource_fs):
'''
diff --git a/common/lib/xmodule/xmodule/modulestore/__init__.py b/common/lib/xmodule/xmodule/modulestore/__init__.py
index e3386608dc..0a1cf9ee10 100644
--- a/common/lib/xmodule/xmodule/modulestore/__init__.py
+++ b/common/lib/xmodule/xmodule/modulestore/__init__.py
@@ -40,6 +40,21 @@ INVALID_HTML_CHARS = re.compile(r"[^\w-]")
_LocationBase = namedtuple('LocationBase', 'tag org course category name revision')
+def _check_location_part(val, regexp):
+ """
+ Check that `regexp` doesn't match inside `val`. If it does, raise an exception
+
+ Args:
+ val (string): The value to check
+ regexp (re.RegexObject): The regular expression specifying invalid characters
+
+ Raises:
+ InvalidLocationError: Raised if any invalid character is found in `val`
+ """
+ if val is not None and regexp.search(val) is not None:
+ raise InvalidLocationError("Invalid characters in {!r}.".format(val))
+
+
class Location(_LocationBase):
'''
Encodes a location.
@@ -145,7 +160,6 @@ class Location(_LocationBase):
Components may be set to None, which may be interpreted in some contexts
to mean wildcard selection.
"""
-
if (org is None and course is None and category is None and name is None and revision is None):
location = loc_or_tag
else:
@@ -161,23 +175,18 @@ class Location(_LocationBase):
check_list(list_)
def check_list(list_):
- def check(val, regexp):
- if val is not None and regexp.search(val) is not None:
- log.debug('invalid characters val="%s", list_="%s"' % (val, list_))
- raise InvalidLocationError("Invalid characters in '%s'." % (val))
-
list_ = list(list_)
for val in list_[:4] + [list_[5]]:
- check(val, INVALID_CHARS)
+ _check_location_part(val, INVALID_CHARS)
# names allow colons
- check(list_[4], INVALID_CHARS_NAME)
+ _check_location_part(list_[4], INVALID_CHARS_NAME)
if isinstance(location, Location):
return location
elif isinstance(location, basestring):
match = URL_RE.match(location)
if match is None:
- log.debug('location is instance of %s but no URL match' % basestring)
+ log.debug("location %r doesn't match URL", location)
raise InvalidLocationError(location)
groups = match.groupdict()
check_dict(groups)
@@ -249,6 +258,18 @@ class Location(_LocationBase):
return "/".join([self.org, self.course, self.name])
+ def _replace(self, **kwargs):
+ """
+ Return a new :class:`Location` with values replaced
+ by the values specified in `**kwargs`
+ """
+ for name, value in kwargs.iteritems():
+ if name == 'name':
+ _check_location_part(value, INVALID_CHARS_NAME)
+ else:
+ _check_location_part(value, INVALID_CHARS)
+ return super(Location, self)._replace(**kwargs)
+
def replace(self, **kwargs):
'''
Expose a public method for replacing location elements
diff --git a/common/lib/xmodule/xmodule/modulestore/inheritance.py b/common/lib/xmodule/xmodule/modulestore/inheritance.py
index 5839dd6d1c..8f824f38c6 100644
--- a/common/lib/xmodule/xmodule/modulestore/inheritance.py
+++ b/common/lib/xmodule/xmodule/modulestore/inheritance.py
@@ -1,26 +1,33 @@
+"""
+Support for inheritance of fields down an XBlock hierarchy.
+"""
+
from datetime import datetime
from pytz import UTC
from xblock.fields import Scope, Boolean, String, Float, XBlockMixin, Dict
+from xblock.runtime import KeyValueStore, KvsFieldData
+
from xmodule.fields import Date, Timedelta
-from xblock.runtime import KeyValueStore
class InheritanceMixin(XBlockMixin):
- """Field definitions for inheritable fields"""
+ """Field definitions for inheritable fields."""
graded = Boolean(
help="Whether this module contributes to the final course grade",
+ scope=Scope.settings,
default=False,
- scope=Scope.settings
)
-
start = Date(
help="Start time when this module is visible",
default=datetime(2030, 1, 1, tzinfo=UTC),
scope=Scope.settings
)
- due = Date(help="Date that this problem is due by", scope=Scope.settings)
+ due = Date(
+ help="Date that this problem is due by",
+ scope=Scope.settings,
+ )
extended_due = Date(
help="Date that this problem is due by for a particular student. This "
"can be set by an instructor, and will override the global due "
@@ -29,31 +36,38 @@ class InheritanceMixin(XBlockMixin):
default=None,
scope=Scope.user_state,
)
- giturl = String(help="url root for course data git repository", 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
+ scope=Scope.settings,
)
showanswer = String(
help="When to show the problem answer to the student",
scope=Scope.settings,
- default="finished"
+ default="finished",
)
rerandomize = String(
help="When to rerandomize the problem",
+ scope=Scope.settings,
default="never",
- scope=Scope.settings
)
days_early_for_beta = Float(
help="Number of days early to show content to beta users",
+ scope=Scope.settings,
default=None,
- scope=Scope.settings
)
- static_asset_path = String(help="Path to use for static assets - overrides Studio c4x://", scope=Scope.settings, default='')
+ static_asset_path = String(
+ help="Path to use for static assets - overrides Studio c4x://",
+ scope=Scope.settings,
+ default='',
+ )
text_customization = Dict(
help="String customization substitutions for particular locations",
- scope=Scope.settings
+ scope=Scope.settings,
)
use_latex_compiler = Boolean(
help="Enable LaTeX templates?",
@@ -104,6 +118,37 @@ def own_metadata(module):
return module.get_explicitly_set_fields_by_scope(Scope.settings)
+class InheritingFieldData(KvsFieldData):
+ """A `FieldData` implementation that can inherit value from parents to children."""
+
+ def __init__(self, inheritable_names, **kwargs):
+ """
+ `inheritable_names` is a list of names that can be inherited from
+ parents.
+
+ """
+ super(InheritingFieldData, self).__init__(**kwargs)
+ self.inheritable_names = set(inheritable_names)
+
+ def default(self, block, name):
+ """
+ The default for an inheritable name is found on a parent.
+ """
+ if name in self.inheritable_names and block.parent is not None:
+ parent = block.get_parent()
+ if parent:
+ return getattr(parent, name)
+ super(InheritingFieldData, self).default(block, name)
+
+
+def inheriting_field_data(kvs):
+ """Create an InheritanceFieldData that inherits the names in InheritanceMixin."""
+ return InheritingFieldData(
+ inheritable_names=InheritanceMixin.fields.keys(),
+ kvs=kvs,
+ )
+
+
class InheritanceKeyValueStore(KeyValueStore):
"""
Common superclass for kvs's which know about inheritance of settings. Offers simple
diff --git a/common/lib/xmodule/xmodule/modulestore/locator.py b/common/lib/xmodule/xmodule/modulestore/locator.py
index e29c779400..bde6cee838 100644
--- a/common/lib/xmodule/xmodule/modulestore/locator.py
+++ b/common/lib/xmodule/xmodule/modulestore/locator.py
@@ -26,7 +26,8 @@ class LocalId(object):
Should be hashable and distinguishable, but nothing else
"""
- pass
+ def __str__(self):
+ return "localid_{}".format(id(self))
class Locator(object):
diff --git a/common/lib/xmodule/xmodule/modulestore/mongo/base.py b/common/lib/xmodule/xmodule/modulestore/mongo/base.py
index c655800609..1961247a6f 100644
--- a/common/lib/xmodule/xmodule/modulestore/mongo/base.py
+++ b/common/lib/xmodule/xmodule/modulestore/mongo/base.py
@@ -26,13 +26,14 @@ from importlib import import_module
from xmodule.errortracker import null_error_tracker, exc_info_to_str
from xmodule.mako_module import MakoDescriptorSystem
from xmodule.error_module import ErrorDescriptor
-from xblock.runtime import DbModel
+from xblock.runtime import KvsFieldData
from xblock.exceptions import InvalidScopeError
from xblock.fields import Scope, ScopeIds
from xmodule.modulestore import ModuleStoreWriteBase, Location, MONGO_MODULESTORE_TYPE
from xmodule.modulestore.exceptions import ItemNotFoundError
from xmodule.modulestore.inheritance import own_metadata, InheritanceMixin, inherit_metadata, InheritanceKeyValueStore
+from xmodule.modulestore.xml import LocationReader
log = logging.getLogger(__name__)
@@ -146,7 +147,12 @@ class CachingDescriptorSystem(MakoDescriptorSystem):
render_template: a function for rendering templates, as per
MakoDescriptorSystem
"""
- super(CachingDescriptorSystem, self).__init__(load_item=self.load_item, **kwargs)
+ super(CachingDescriptorSystem, self).__init__(
+ id_reader=LocationReader(),
+ field_data=None,
+ load_item=self.load_item,
+ **kwargs
+ )
self.modulestore = modulestore
self.module_data = module_data
@@ -187,7 +193,7 @@ class CachingDescriptorSystem(MakoDescriptorSystem):
metadata,
)
- field_data = DbModel(kvs)
+ field_data = KvsFieldData(kvs)
scope_ids = ScopeIds(None, category, location, location)
module = self.construct_xblock_from_class(class_, scope_ids, field_data)
if self.cached_metadata is not None:
@@ -480,7 +486,8 @@ class MongoModuleStore(ModuleStoreWriteBase):
"""
Load an XModuleDescriptor from item, using the children stored in data_cache
"""
- data_dir = getattr(item, 'data_dir', item['location']['course'])
+ location = Location(item['location'])
+ data_dir = getattr(item, 'data_dir', location.course)
root = self.fs_root / data_dir
if not root.isdir():
@@ -490,7 +497,7 @@ class MongoModuleStore(ModuleStoreWriteBase):
cached_metadata = {}
if apply_cached_metadata:
- cached_metadata = self.get_cached_metadata_inheritance_tree(Location(item['location']))
+ cached_metadata = self.get_cached_metadata_inheritance_tree(location)
# TODO (cdodge): When the 'split module store' work has been completed, we should remove
# the 'metadata_inheritance_tree' parameter
@@ -505,7 +512,7 @@ class MongoModuleStore(ModuleStoreWriteBase):
mixins=self.xblock_mixins,
select=self.xblock_select,
)
- return system.load_item(item['location'])
+ return system.load_item(location)
def _load_items(self, items, depth=0):
"""
@@ -779,6 +786,8 @@ class MongoModuleStore(ModuleStoreWriteBase):
location: Something that can be passed to Location
children: A list of child item identifiers
"""
+ # Normalize the children to urls
+ children = [Location(child).url() for child in children]
self._update_single_item(location, {'definition.children': children})
# recompute (and update) the metadata inheritance tree which is cached
@@ -888,5 +897,5 @@ class MongoModuleStore(ModuleStoreWriteBase):
metadata,
)
- field_data = DbModel(kvs)
+ field_data = KvsFieldData(kvs)
return field_data
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 b755ba691f..f11ed49151 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
@@ -4,7 +4,7 @@ from xmodule.mako_module import MakoDescriptorSystem
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 xblock.runtime import KvsFieldData, IdReader
from ..exceptions import ItemNotFoundError
from .split_mongo_kvs import SplitMongoKVS
from xblock.fields import ScopeIds
@@ -12,6 +12,23 @@ from xblock.fields import ScopeIds
log = logging.getLogger(__name__)
+class SplitMongoIdReader(IdReader):
+ """
+ An :class:`~xblock.runtime.IdReader` associated with a particular
+ :class:`.CachingDescriptorSystem`.
+ """
+ def __init__(self, system):
+ self.system = system
+
+ def get_definition_id(self, usage_id):
+ usage = self.system.load_item(usage_id)
+ return usage.definition_locator
+
+ def get_block_type(self, def_id):
+ definition = self.system.modulestore.db_connection.get_definition(def_id)
+ return definition['category']
+
+
class CachingDescriptorSystem(MakoDescriptorSystem):
"""
A system that has a cache of a course version's json that it will use to load modules
@@ -33,7 +50,12 @@ class CachingDescriptorSystem(MakoDescriptorSystem):
module_data: a dict mapping Location -> json that was cached from the
underlying modulestore
"""
- super(CachingDescriptorSystem, self).__init__(load_item=self._load_item, **kwargs)
+ super(CachingDescriptorSystem, self).__init__(
+ id_reader=SplitMongoIdReader(self),
+ field_data=None,
+ load_item=self._load_item,
+ **kwargs
+ )
self.modulestore = modulestore
self.course_entry = course_entry
self.lazy = lazy
@@ -102,7 +124,7 @@ class CachingDescriptorSystem(MakoDescriptorSystem):
json_data.get('fields', {}),
json_data.get('_inherited_settings'),
)
- field_data = DbModel(kvs)
+ field_data = KvsFieldData(kvs)
try:
module = self.construct_xblock_from_class(
diff --git a/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py b/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py
index 1a92f961ec..04f15727a4 100644
--- a/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py
+++ b/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py
@@ -485,7 +485,8 @@ class SplitMongoModuleStore(ModuleStoreWriteBase):
}
"""
course = self._lookup_course(course_locator)['structure']
- return {'original_version': course['original_version'],
+ return {
+ 'original_version': course['original_version'],
'previous_version': course['previous_version'],
'edited_by': course['edited_by'],
'edited_on': course['edited_on']
@@ -1328,8 +1329,7 @@ class SplitMongoModuleStore(ModuleStoreWriteBase):
if depth is None or depth > 0:
depth = depth - 1 if depth is not None else None
for child in block_map[block_id]['fields'].get('children', []):
- descendent_map = self.descendants(block_map, child, depth,
- descendent_map)
+ descendent_map = self.descendants(block_map, child, depth, descendent_map)
return descendent_map
diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_location.py b/common/lib/xmodule/xmodule/modulestore/tests/test_location.py
index c190559c73..5f4983e1bf 100644
--- a/common/lib/xmodule/xmodule/modulestore/tests/test_location.py
+++ b/common/lib/xmodule/xmodule/modulestore/tests/test_location.py
@@ -1,170 +1,188 @@
-from nose.tools import assert_equals, assert_raises, assert_not_equals # pylint: disable=E0611
+import ddt
+
+from unittest import TestCase
from xmodule.modulestore import Location
from xmodule.modulestore.exceptions import InvalidLocationError
-
-def check_string_roundtrip(url):
- assert_equals(url, Location(url).url())
- assert_equals(url, str(Location(url)))
+# Pairs for testing the clean* functions.
+# The first item in the tuple is the input string.
+# The second item in the tuple is what the result of
+# replacement should be.
+GENERAL_PAIRS = [
+ ('', ''),
+ (' ', '_'),
+ ('abc,', 'abc_'),
+ ('ab fg!@//\\aj', 'ab_fg_aj'),
+ (u"ab\xA9", "ab_"), # no unicode allowed for now
+]
-def test_string_roundtrip():
- check_string_roundtrip("tag://org/course/category/name")
- check_string_roundtrip("tag://org/course/category/name@revision")
-
-
-input_dict = {
- 'tag': 'tag',
- 'course': 'course',
- 'category': 'category',
- 'name': 'name',
- 'org': 'org'
-}
-
-
-also_valid_dict = {
- 'tag': 'tag',
- 'course': 'course',
- 'category': 'category',
- 'name': 'name:more_name',
- 'org': 'org'
-}
-
-
-input_list = ['tag', 'org', 'course', 'category', 'name']
-
-input_str = "tag://org/course/category/name"
-input_str_rev = "tag://org/course/category/name@revision"
-
-valid = (input_list, input_dict, input_str, input_str_rev, also_valid_dict)
-
-invalid_dict = {
- 'tag': 'tag',
- 'course': 'course',
- 'category': 'category',
- 'name': 'name@more_name',
- 'org': 'org'
-}
-
-invalid_dict2 = {
- 'tag': 'tag',
- 'course': 'course',
- 'category': 'category',
- 'name': 'name ', # extra space
- 'org': 'org'
-}
-
-invalid = ("foo", ["foo"], ["foo", "bar"],
- ["foo", "bar", "baz", "blat:blat", "foo:bar"], # ':' ok in name, not in category
- "tag://org/course/category/name with spaces@revision",
- "tag://org/course/category/name/with/slashes@revision",
- invalid_dict,
- invalid_dict2)
-
-
-def test_is_valid():
- for v in valid:
- assert_equals(Location.is_valid(v), True)
-
- for v in invalid:
- assert_equals(Location.is_valid(v), False)
-
-
-def test_dict():
- assert_equals("tag://org/course/category/name", Location(input_dict).url())
- assert_equals(dict(revision=None, **input_dict), Location(input_dict).dict())
-
- input_dict['revision'] = 'revision'
- assert_equals("tag://org/course/category/name@revision", Location(input_dict).url())
- assert_equals(input_dict, Location(input_dict).dict())
-
-
-def test_list():
- assert_equals("tag://org/course/category/name", Location(input_list).url())
- assert_equals(input_list + [None], Location(input_list).list())
-
- input_list.append('revision')
- assert_equals("tag://org/course/category/name@revision", Location(input_list).url())
- assert_equals(input_list, Location(input_list).list())
-
-
-def test_location():
- input_list = ['tag', 'org', 'course', 'category', 'name']
- assert_equals("tag://org/course/category/name", Location(Location(input_list)).url())
-
-
-def test_none():
- assert_equals([None] * 6, Location(None).list())
-
-
-def test_invalid_locations():
- assert_raises(InvalidLocationError, Location, "foo")
- assert_raises(InvalidLocationError, Location, ["foo", "bar"])
- assert_raises(InvalidLocationError, Location, ["foo", "bar", "baz", "blat/blat", "foo"])
- assert_raises(InvalidLocationError, Location, ["foo", "bar", "baz", "blat", "foo/bar"])
- assert_raises(InvalidLocationError, Location, "tag://org/course/category/name with spaces@revision")
- assert_raises(InvalidLocationError, Location, "tag://org/course/category/name/revision")
-
-
-def test_equality():
- assert_equals(
- Location('tag', 'org', 'course', 'category', 'name'),
- Location('tag', 'org', 'course', 'category', 'name')
+@ddt.ddt
+class TestLocations(TestCase):
+ @ddt.data(
+ "tag://org/course/category/name",
+ "tag://org/course/category/name@revision"
)
+ def test_string_roundtrip(self, url):
+ self.assertEquals(url, Location(url).url())
+ self.assertEquals(url, str(Location(url)))
- assert_not_equals(
- Location('tag', 'org', 'course', 'category', 'name1'),
- Location('tag', 'org', 'course', 'category', 'name')
+ @ddt.data(
+ {
+ 'tag': 'tag',
+ 'course': 'course',
+ 'category': 'category',
+ 'name': 'name',
+ 'org': 'org'
+ },
+ {
+ 'tag': 'tag',
+ 'course': 'course',
+ 'category': 'category',
+ 'name': 'name:more_name',
+ 'org': 'org'
+ },
+ ['tag', 'org', 'course', 'category', 'name'],
+ "tag://org/course/category/name",
+ "tag://org/course/category/name@revision",
+ u"tag://org/course/category/name",
+ u"tag://org/course/category/name@revision",
)
+ def test_is_valid(self, loc):
+ self.assertTrue(Location.is_valid(loc))
-# All the cleaning functions should do the same thing with these
-general_pairs = [('', ''),
- (' ', '_'),
- ('abc,', 'abc_'),
- ('ab fg!@//\\aj', 'ab_fg_aj'),
- (u"ab\xA9", "ab_"), # no unicode allowed for now
- ]
+ @ddt.data(
+ {
+ 'tag': 'tag',
+ 'course': 'course',
+ 'category': 'category',
+ 'name': 'name@more_name',
+ 'org': 'org'
+ },
+ {
+ 'tag': 'tag',
+ 'course': 'course',
+ 'category': 'category',
+ 'name': 'name ', # extra space
+ 'org': 'org'
+ },
+ "foo",
+ ["foo"],
+ ["foo", "bar"],
+ ["foo", "bar", "baz", "blat:blat", "foo:bar"], # ':' ok in name, not in category
+ "tag://org/course/category/name with spaces@revision",
+ "tag://org/course/category/name/with/slashes@revision",
+ u"tag://org/course/category/name\xae", # No non-ascii characters for now
+ u"tag://org/course/category\xae/name", # No non-ascii characters for now
+ )
+ def test_is_invalid(self, loc):
+ self.assertFalse(Location.is_valid(loc))
+ def test_dict(self):
+ input_dict = {
+ 'tag': 'tag',
+ 'course': 'course',
+ 'category': 'category',
+ 'name': 'name',
+ 'org': 'org'
+ }
-def test_clean():
- pairs = general_pairs + [
+ self.assertEquals("tag://org/course/category/name", Location(input_dict).url())
+ self.assertEquals(dict(revision=None, **input_dict), Location(input_dict).dict())
+
+ input_dict['revision'] = 'revision'
+ self.assertEquals("tag://org/course/category/name@revision", Location(input_dict).url())
+ self.assertEquals(input_dict, Location(input_dict).dict())
+
+ def test_list(self):
+ input_list = ['tag', 'org', 'course', 'category', 'name']
+ self.assertEquals("tag://org/course/category/name", Location(input_list).url())
+ self.assertEquals(input_list + [None], Location(input_list).list())
+
+ input_list.append('revision')
+ self.assertEquals("tag://org/course/category/name@revision", Location(input_list).url())
+ self.assertEquals(input_list, Location(input_list).list())
+
+ def test_location(self):
+ input_list = ['tag', 'org', 'course', 'category', 'name']
+ self.assertEquals("tag://org/course/category/name", Location(Location(input_list)).url())
+
+ def test_none(self):
+ self.assertEquals([None] * 6, Location(None).list())
+
+ @ddt.data(
+ "foo",
+ ["foo", "bar"],
+ ["foo", "bar", "baz", "blat/blat", "foo"],
+ ["foo", "bar", "baz", "blat", "foo/bar"],
+ "tag://org/course/category/name with spaces@revision",
+ "tag://org/course/category/name/revision",
+ )
+ def test_invalid_locations(self, loc):
+ with self.assertRaises(InvalidLocationError):
+ Location(loc)
+
+ def test_equality(self):
+ self.assertEquals(
+ Location('tag', 'org', 'course', 'category', 'name'),
+ Location('tag', 'org', 'course', 'category', 'name')
+ )
+
+ self.assertNotEquals(
+ Location('tag', 'org', 'course', 'category', 'name1'),
+ Location('tag', 'org', 'course', 'category', 'name')
+ )
+
+ @ddt.data(
('a:b', 'a_b'), # no colons in non-name components
('a-b', 'a-b'), # dashes ok
('a.b', 'a.b'), # dot ok
- ]
- for input, output in pairs:
- assert_equals(Location.clean(input), output)
+ *GENERAL_PAIRS
+ )
+ def test_clean(self, pair):
+ self.assertEquals(Location.clean(pair[0]), pair[1])
-
-def test_clean_for_url_name():
- pairs = general_pairs + [
+ @ddt.data(
('a:b', 'a:b'), # colons ok in names
('a-b', 'a-b'), # dashes ok in names
('a.b', 'a.b'), # dot ok in names
- ]
- for input, output in pairs:
- assert_equals(Location.clean_for_url_name(input), output)
+ *GENERAL_PAIRS
+ )
+ def test_clean_for_url_name(self, pair):
+ self.assertEquals(Location.clean_for_url_name(pair[0]), pair[1])
-
-def test_clean_for_html():
- pairs = general_pairs + [
+ @ddt.data(
("a:b", "a_b"), # no colons for html use
("a-b", "a-b"), # dashes ok (though need to be replaced in various use locations. ugh.)
('a.b', 'a_b'), # no dots.
- ]
- for input, output in pairs:
- assert_equals(Location.clean_for_html(input), output)
+ *GENERAL_PAIRS
+ )
+ def test_clean_for_html(self, pair):
+ self.assertEquals(Location.clean_for_html(pair[0]), pair[1])
+ def test_html_id(self):
+ loc = Location("tag://org/course/cat/name:more_name@rev")
+ self.assertEquals(loc.html_id(), "tag-org-course-cat-name_more_name-rev")
-def test_html_id():
- loc = Location("tag://org/course/cat/name:more_name@rev")
- assert_equals(loc.html_id(), "tag-org-course-cat-name_more_name-rev")
+ def test_course_id(self):
+ loc = Location('i4x', 'mitX', '103', 'course', 'test2')
+ self.assertEquals('mitX/103/test2', loc.course_id)
+ loc = Location('i4x', 'mitX', '103', '_not_a_course', 'test2')
+ with self.assertRaises(InvalidLocationError):
+ loc.course_id # pylint: disable=pointless-statement
-def test_course_id():
- loc = Location('i4x', 'mitX', '103', 'course', 'test2')
- assert_equals('mitX/103/test2', loc.course_id)
+ def test_replacement(self):
+ self.assertEquals(
+ Location('t://o/c/c/n@r')._replace(name='new_name'),
+ Location('t://o/c/c/new_name@r'),
+ )
- loc = Location('i4x', 'mitX', '103', '_not_a_course', 'test2')
- with assert_raises(InvalidLocationError):
- loc.course_id
+ with self.assertRaises(InvalidLocationError):
+ Location('t://o/c/c/n@r')._replace(name=u'name\xae')
+
+ @ddt.data('org', 'course', 'category', 'name', 'revision')
+ def test_immutable(self, attr):
+ loc = Location('t://o/c/c/n@r')
+ with self.assertRaises(AttributeError):
+ setattr(loc, attr, attr)
diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py b/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py
index e1c08f8efd..f492c321d2 100644
--- a/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py
+++ b/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py
@@ -285,13 +285,6 @@ class TestMongoKeyValueStore(object):
self.metadata = {'meta': 'meta_val'}
self.kvs = MongoKeyValueStore(self.data, self.children, self.metadata)
- def _check_read(self, key, expected_value):
- """
- Asserts the get and has methods.
- """
- assert_equals(expected_value, self.kvs.get(key))
- assert self.kvs.has(key)
-
def test_read(self):
assert_equals(self.data['foo'], self.kvs.get(KeyValueStore.Key(Scope.content, None, None, 'foo')))
assert_equals(self.children, self.kvs.get(KeyValueStore.Key(Scope.children, None, None, 'children')))
diff --git a/common/lib/xmodule/xmodule/modulestore/xml.py b/common/lib/xmodule/xmodule/modulestore/xml.py
index 02df55569a..13079bf9b3 100644
--- a/common/lib/xmodule/xmodule/modulestore/xml.py
+++ b/common/lib/xmodule/xmodule/modulestore/xml.py
@@ -1,4 +1,5 @@
import hashlib
+import itertools
import json
import logging
import os
@@ -17,17 +18,18 @@ from xmodule.error_module import ErrorDescriptor
from xmodule.errortracker import make_error_tracker, exc_info_to_str
from xmodule.course_module import CourseDescriptor
from xmodule.mako_module import MakoDescriptorSystem
-from xmodule.x_module import XMLParsingSystem, prefer_xmodules
+from xmodule.x_module import XMLParsingSystem, prefer_xmodules, policy_key
from xmodule.html_module import HtmlDescriptor
from xblock.core import XBlock
from xblock.fields import ScopeIds
from xblock.field_data import DictFieldData
+from xblock.runtime import DictKeyValueStore, IdReader, IdGenerator
from . import ModuleStoreReadBase, Location, XML_MODULESTORE_TYPE
from .exceptions import ItemNotFoundError
-from .inheritance import compute_inherited_metadata
+from .inheritance import compute_inherited_metadata, inheriting_field_data
edx_xml_parser = etree.XMLParser(dtd_validation=False, load_dtd=False,
remove_comments=True, remove_blank_text=True)
@@ -49,7 +51,7 @@ def clean_out_mako_templating(xml_string):
class ImportSystem(XMLParsingSystem, MakoDescriptorSystem):
def __init__(self, xmlstore, course_id, course_dir,
error_tracker, parent_tracker,
- load_error_modules=True, **kwargs):
+ load_error_modules=True, id_reader=None, **kwargs):
"""
A class that handles loading from xml. Does some munging to ensure that
all elements have unique slugs.
@@ -59,6 +61,10 @@ 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
self.org, self.course, self.url_name = course_id.split('/')
+ if id_reader is None:
+ id_reader = LocationReader()
+ id_generator = CourseLocationGenerator(self.org, self.course)
+
# 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
self.load_error_modules = load_error_modules
@@ -165,8 +171,10 @@ class ImportSystem(XMLParsingSystem, MakoDescriptorSystem):
make_name_unique(xml_data)
descriptor = create_block_from_xml(
- etree.tostring(xml_data, encoding='unicode'), self, self.org,
- self.course, xmlstore.default_class)
+ etree.tostring(xml_data, encoding='unicode'),
+ self,
+ id_generator,
+ )
except Exception as err:
if not self.load_error_modules:
raise
@@ -174,30 +182,34 @@ class ImportSystem(XMLParsingSystem, MakoDescriptorSystem):
# Didn't load properly. Fall back on loading as an error
# descriptor. This should never error due to formatting.
- msg = "Error loading from xml. " + unicode(err)[:200]
- log.warning(msg)
- # Normally, we don't want lots of exception traces in our logs from common
- # content problems. But if you're debugging the xml loading code itself,
- # uncomment the next line.
- # log.exception(msg)
+ msg = "Error loading from xml. %s"
+ log.warning(
+ msg,
+ unicode(err)[:200],
+ # Normally, we don't want lots of exception traces in our logs from common
+ # content problems. But if you're debugging the xml loading code itself,
+ # uncomment the next line.
+ # exc_info=True
+ )
+
+ msg = msg % (unicode(err)[:200])
self.error_tracker(msg)
err_msg = msg + "\n" + exc_info_to_str(sys.exc_info())
descriptor = ErrorDescriptor.from_xml(
xml,
self,
- self.org,
- self.course,
+ id_generator,
err_msg
)
descriptor.data_dir = course_dir
- xmlstore.modules[course_id][descriptor.location] = descriptor
+ xmlstore.modules[course_id][descriptor.scope_ids.usage_id] = descriptor
- if hasattr(descriptor, 'children'):
+ if descriptor.has_children:
for child in descriptor.get_children():
- parent_tracker.add_parent(child.location, descriptor.location)
+ parent_tracker.add_parent(child.scope_ids.usage_id, descriptor.scope_ids.usage_id)
# After setting up the descriptor, save any changes that we have
# made to attributes on the descriptor to the underlying KeyValueStore.
@@ -205,50 +217,94 @@ class ImportSystem(XMLParsingSystem, MakoDescriptorSystem):
return descriptor
render_template = lambda template, context: u''
+
# TODO (vshnayder): we are somewhat architecturally confused in the loading code:
# load_item should actually be get_instance, because it expects the course-specific
# policy to be loaded. For now, just add the course_id here...
- load_item = lambda location: xmlstore.get_instance(course_id, location)
+ def load_item(location):
+ return xmlstore.get_instance(course_id, Location(location))
+
resources_fs = OSFS(xmlstore.data_dir / course_dir)
+
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_reader=id_reader,
**kwargs
)
+ # id_generator is ignored, because each ImportSystem is already local to
+ # a course, and has it's own id_generator already in place
+ def add_node_as_child(self, block, node, id_generator):
+ child_block = self.process_xml(etree.tostring(node))
+ block.children.append(child_block.scope_ids.usage_id)
-def create_block_from_xml(xml_data, system, org=None, course=None, default_class=None):
+
+class LocationReader(IdReader):
+ """
+ IdReader for definition and usage ids that are Locations
+ """
+ def get_definition_id(self, usage_id):
+ return usage_id
+
+ def get_block_type(self, def_id):
+ location = def_id
+ return location.category
+
+
+class CourseLocationGenerator(IdGenerator):
+ """
+ IdGenerator for Location-based definition ids and usage ids
+ based within a course
+ """
+ def __init__(self, org, course):
+ self.org = org
+ self.course = course
+ self.autogen_ids = itertools.count(0)
+
+ def create_usage(self, def_id):
+ return Location(def_id)
+
+ def create_definition(self, block_type, slug=None):
+ assert block_type is not None
+ if slug is None:
+ slug = 'autogen_{}_{}'.format(block_type, self.autogen_ids.next())
+ location = Location('i4x', self.org, self.course, block_type, slug)
+ return location
+
+
+def create_block_from_xml(xml_data, system, id_generator):
"""
Create an XBlock instance from XML data.
- `xml_data' is a string containing valid xml.
+ Args:
+ xml_data (string): A string containing valid xml.
+ system (XMLParsingSystem): The :class:`.XMLParsingSystem` used to connect the block
+ to the outside world.
+ id_generator (IdGenerator): An :class:`~xblock.runtime.IdGenerator` that
+ will be used to construct the usage_id and definition_id for the block.
- `system` is an XMLParsingSystem.
-
- `org` and `course` are optional strings that will be used in the generated
- block's url identifiers.
-
- `default_class` is the class to instantiate of the XML indicates a class
- that can't be loaded.
-
- Returns the fully instantiated XBlock.
+ Returns:
+ XBlock: The fully instantiated :class:`~xblock.core.XBlock`.
"""
node = etree.fromstring(xml_data)
- raw_class = XBlock.load_class(node.tag, default_class, select=prefer_xmodules)
+ raw_class = system.load_block_type(node.tag)
xblock_class = system.mixologist.mix(raw_class)
# leave next line commented out - useful for low-level debugging
# log.debug('[create_block_from_xml] tag=%s, class=%s' % (node.tag, xblock_class))
- url_name = node.get('url_name', node.get('slug'))
- location = Location('i4x', org, course, node.tag, url_name)
+ block_type = node.tag
+ url_name = node.get('url_name')
+ def_id = id_generator.create_definition(block_type, url_name)
+ usage_id = id_generator.create_usage(def_id)
- scope_ids = ScopeIds(None, location.category, location, location)
- xblock = xblock_class.parse_xml(node, system, scope_ids)
+ scope_ids = ScopeIds(None, block_type, def_id, usage_id)
+ xblock = xblock_class.parse_xml(node, system, scope_ids, id_generator)
return xblock
@@ -265,10 +321,8 @@ class ParentTracker(object):
"""
Add a parent of child location to the set of parents. Duplicate calls have no effect.
- child and parent must be something that can be passed to Location.
+ child and parent must be :class:`.Location` instances.
"""
- child = Location(child)
- parent = Location(parent)
s = self._parents.setdefault(child, set())
s.add(parent)
@@ -276,7 +330,6 @@ class ParentTracker(object):
"""
returns True iff child has some parents.
"""
- child = Location(child)
return child in self._parents
def make_known(self, location):
@@ -288,7 +341,6 @@ class ParentTracker(object):
"""
Return a list of the parents of this child. If not is_known(child), will throw a KeyError
"""
- child = Location(child)
return list(self._parents[child])
@@ -326,6 +378,9 @@ class XMLModuleStore(ModuleStoreReadBase):
self.parent_trackers = defaultdict(ParentTracker)
+ # All field data will be stored in an inheriting field data.
+ self.field_data = inheriting_field_data(kvs=DictKeyValueStore())
+
# If we are specifically asked for missing courses, that should
# be an error. If we are asked for "all" courses, find the ones
# that have a course.xml. We sort the dirs in alpha order so we always
@@ -357,8 +412,8 @@ class XMLModuleStore(ModuleStoreReadBase):
if course_descriptor is not None and not isinstance(course_descriptor, ErrorDescriptor):
self.courses[course_dir] = course_descriptor
- self._location_errors[course_descriptor.location] = errorlog
- self.parent_trackers[course_descriptor.id].make_known(course_descriptor.location)
+ self._location_errors[course_descriptor.scope_ids.usage_id] = errorlog
+ self.parent_trackers[course_descriptor.id].make_known(course_descriptor.scope_ids.usage_id)
else:
# Didn't load course. Instead, save the errors elsewhere.
self.errored_courses[course_dir] = errorlog
@@ -368,7 +423,8 @@ class XMLModuleStore(ModuleStoreReadBase):
String representation - for debugging
'''
return '' % (
- self.data_dir, len(self.courses), len(self.modules))
+ self.data_dir, len(self.courses), len(self.modules)
+ )
def load_policy(self, policy_path, tracker):
"""
@@ -451,6 +507,10 @@ class XMLModuleStore(ModuleStoreReadBase):
"(or 'name') set. Set url_name.")
course_id = CourseDescriptor.make_id(org, course, url_name)
+
+ def get_policy(usage_id):
+ return policy.get(policy_key(usage_id), {})
+
system = ImportSystem(
xmlstore=self,
course_id=course_id,
@@ -458,9 +518,11 @@ class XMLModuleStore(ModuleStoreReadBase):
error_tracker=tracker,
parent_tracker=self.parent_trackers[course_id],
load_error_modules=self.load_error_modules,
- policy=policy,
+ get_policy=get_policy,
mixins=self.xblock_mixins,
+ default_class=self.default_class,
select=self.xblock_select,
+ field_data=self.field_data,
)
course_descriptor = system.process_xml(etree.tostring(course_data, encoding='unicode'))
@@ -508,7 +570,7 @@ class XMLModuleStore(ModuleStoreReadBase):
html = f.read().decode('utf-8')
# 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)
+ loc = course_descriptor.scope_ids.usage_id._replace(category=category, name=slug)
module = system.construct_xblock_from_class(
HtmlDescriptor,
# We're loading a descriptor, so student_id is meaningless
@@ -526,7 +588,7 @@ class XMLModuleStore(ModuleStoreReadBase):
module.display_name = tab['name']
module.data_dir = course_dir
module.save()
- self.modules[course_descriptor.id][module.location] = module
+ self.modules[course_descriptor.id][module.scope_ids.usage_id] = module
except Exception, e:
logging.exception("Failed to load %s. Skipping... \
Exception: %s", filepath, unicode(e))
diff --git a/common/lib/xmodule/xmodule/modulestore/xml_importer.py b/common/lib/xmodule/xmodule/modulestore/xml_importer.py
index bcb75ca282..47dedecc6b 100644
--- a/common/lib/xmodule/xmodule/modulestore/xml_importer.py
+++ b/common/lib/xmodule/xmodule/modulestore/xml_importer.py
@@ -310,9 +310,7 @@ def import_module(
source_course_location, dest_course_location, allow_not_found=False,
do_import_static=True):
- logging.debug('processing import of module {url}...'.format(
- url=module.location.url()
- ))
+ logging.debug('processing import of module {}...'.format(module.location.url()))
content = {}
for field in module.fields.values():
@@ -393,10 +391,10 @@ def import_course_draft(
xmlstore=xml_module_store,
course_id=target_location_namespace.course_id,
course_dir=draft_course_dir,
- policy={},
error_tracker=errorlog.tracker,
parent_tracker=ParentTracker(),
load_error_modules=False,
+ field_data=None,
)
# now walk the /vertical directory where each file in there
diff --git a/common/lib/xmodule/xmodule/peer_grading_module.py b/common/lib/xmodule/xmodule/peer_grading_module.py
index 95835a0ba4..273db14a7d 100644
--- a/common/lib/xmodule/xmodule/peer_grading_module.py
+++ b/common/lib/xmodule/xmodule/peer_grading_module.py
@@ -1,20 +1,21 @@
import json
import logging
-from lxml import etree
-
from datetime import datetime
+from lxml import etree
from pkg_resources import resource_string
-from .capa_module import ComplexEncoder
-from .x_module import XModule, module_attr
-from .raw_module import RawDescriptor
-from .modulestore.exceptions import ItemNotFoundError, NoPathToItem
-from .timeinfo import TimeInfo
-from .util.duedate import get_extended_due_date
-from xblock.fields import Dict, String, Scope, Boolean, Float
-from xmodule.fields import Date, Timedelta
+from xblock.fields import Dict, String, Scope, Boolean, Float, Reference
+from xmodule.capa_module import ComplexEncoder
+from xmodule.fields import Date, Timedelta
+from xmodule.modulestore import Location
+from xmodule.modulestore.exceptions import ItemNotFoundError, NoPathToItem
+from xmodule.raw_module import RawDescriptor
+from xmodule.timeinfo import TimeInfo
+from xmodule.util.duedate import get_extended_due_date
+from xmodule.x_module import XModule, module_attr
from xmodule.open_ended_grading_classes.peer_grading_service import PeerGradingService, GradingServiceError, MockPeerGradingService
+
from open_ended_grading_classes import combined_open_ended_rubric
from django.utils.timezone import UTC
@@ -32,7 +33,7 @@ class PeerGradingFields(object):
default=False,
scope=Scope.settings
)
- link_to_location = String(
+ link_to_location = Reference(
display_name="Link to Problem Location",
help='The location of the problem being graded. Only used when "Show Single Problem" is True.',
default="",
@@ -560,7 +561,7 @@ class PeerGradingModule(PeerGradingFields, XModule):
good_problem_list = []
for problem in problem_list:
- problem_location = problem['location']
+ problem_location = Location(problem['location'])
try:
descriptor = self._find_corresponding_module_for_location(problem_location)
except (NoPathToItem, ItemNotFoundError):
@@ -608,10 +609,10 @@ class PeerGradingModule(PeerGradingFields, XModule):
log.error(
"Peer grading problem in peer_grading_module called with no get parameters, but use_for_single_location is False.")
return {'html': "", 'success': False}
- problem_location = self.link_to_location
+ problem_location = Location(self.link_to_location)
elif data.get('location') is not None:
- problem_location = data.get('location')
+ problem_location = Location(data.get('location'))
module = self._find_corresponding_module_for_location(problem_location)
@@ -660,8 +661,8 @@ class PeerGradingDescriptor(PeerGradingFields, RawDescriptor):
metadata_translations = {
'is_graded': 'graded',
'attempts': 'max_attempts',
- 'due_data' : 'due'
- }
+ 'due_data': 'due'
+ }
@property
def non_editable_metadata_fields(self):
diff --git a/common/lib/xmodule/xmodule/seq_module.py b/common/lib/xmodule/xmodule/seq_module.py
index 26820c7658..4387d6286b 100644
--- a/common/lib/xmodule/xmodule/seq_module.py
+++ b/common/lib/xmodule/xmodule/seq_module.py
@@ -138,7 +138,8 @@ class SequenceDescriptor(SequenceFields, MakoModuleDescriptor, XmlDescriptor):
children = []
for child in xml_object:
try:
- children.append(system.process_xml(etree.tostring(child, encoding='unicode')).location.url())
+ child_block = system.process_xml(etree.tostring(child, encoding='unicode'))
+ children.append(child_block.scope_ids.usage_id)
except Exception as e:
log.exception("Unable to load child when parsing Sequence. Continuing...")
if system.error_tracker is not None:
diff --git a/common/lib/xmodule/xmodule/tests/__init__.py b/common/lib/xmodule/xmodule/tests/__init__.py
index bfb5bd7f93..543d0eb095 100644
--- a/common/lib/xmodule/xmodule/tests/__init__.py
+++ b/common/lib/xmodule/xmodule/tests/__init__.py
@@ -16,10 +16,12 @@ from mock import Mock
from path import path
from xblock.field_data import DictFieldData
+
from xmodule.x_module import ModuleSystem, XModuleDescriptor, XModuleMixin
from xmodule.modulestore.inheritance import InheritanceMixin
from xmodule.mako_module import MakoDescriptorSystem
from xmodule.error_module import ErrorDescriptor
+from xmodule.modulestore.xml import LocationReader
# Location of common test DATA directory
@@ -88,6 +90,8 @@ def get_test_descriptor_system():
error_tracker=Mock(),
render_template=mock_render_template,
mixins=(InheritanceMixin, XModuleMixin),
+ field_data=DictFieldData({}),
+ id_reader=LocationReader(),
)
diff --git a/common/lib/xmodule/xmodule/tests/test_conditional.py b/common/lib/xmodule/xmodule/tests/test_conditional.py
index c1b6ccbae7..74aa83073f 100644
--- a/common/lib/xmodule/xmodule/tests/test_conditional.py
+++ b/common/lib/xmodule/xmodule/tests/test_conditional.py
@@ -9,7 +9,7 @@ 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
+from xmodule.modulestore.xml import ImportSystem, XMLModuleStore, CourseLocationGenerator
from xmodule.conditional_module import ConditionalDescriptor
from xmodule.tests import DATA_DIR, get_test_system, get_test_descriptor_system
@@ -32,7 +32,6 @@ class DummySystem(ImportSystem):
error_tracker=Mock(),
parent_tracker=Mock(),
load_error_modules=load_error_modules,
- policy={},
)
def render_template(self, template, context):
@@ -61,8 +60,7 @@ class ConditionalFactory(object):
source_descriptor = NonStaffErrorDescriptor.from_xml(
'some random xml data',
system,
- org=source_location.org,
- course=source_location.course,
+ id_generator=CourseLocationGenerator(source_location.org, source_location.course),
error_msg='random error message'
)
else:
diff --git a/common/lib/xmodule/xmodule/tests/test_course_module.py b/common/lib/xmodule/xmodule/tests/test_course_module.py
index 59cf686f9d..74a10fe91f 100644
--- a/common/lib/xmodule/xmodule/tests/test_course_module.py
+++ b/common/lib/xmodule/xmodule/tests/test_course_module.py
@@ -5,8 +5,10 @@ from fs.memoryfs import MemoryFS
from mock import Mock, patch
-from xmodule.modulestore.xml import ImportSystem, XMLModuleStore
+from xblock.runtime import KvsFieldData, DictKeyValueStore
+
import xmodule.course_module
+from xmodule.modulestore.xml import ImportSystem, XMLModuleStore, LocationReader
from django.utils.timezone import UTC
@@ -32,7 +34,6 @@ class DummySystem(ImportSystem):
load_error_modules=load_error_modules)
course_id = "/".join([ORG, COURSE, 'test_run'])
course_dir = "test_dir"
- policy = {}
error_tracker = Mock()
parent_tracker = Mock()
@@ -40,10 +41,11 @@ class DummySystem(ImportSystem):
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,
+ field_data=KvsFieldData(DictKeyValueStore()),
+ id_reader=LocationReader(),
)
diff --git a/common/lib/xmodule/xmodule/tests/test_error_module.py b/common/lib/xmodule/xmodule/tests/test_error_module.py
index 39e64e3185..be88575eb1 100644
--- a/common/lib/xmodule/xmodule/tests/test_error_module.py
+++ b/common/lib/xmodule/xmodule/tests/test_error_module.py
@@ -5,9 +5,10 @@ import unittest
from xmodule.tests import get_test_system
from xmodule.error_module import ErrorDescriptor, ErrorModule, NonStaffErrorDescriptor
from xmodule.modulestore import Location
+from xmodule.modulestore.xml import CourseLocationGenerator
from xmodule.x_module import XModuleDescriptor, XModule
from mock import MagicMock, Mock, patch
-from xblock.runtime import Runtime, UsageStore
+from xblock.runtime import Runtime, IdReader
from xblock.field_data import FieldData
from xblock.fields import ScopeIds
from xblock.test.tools import unabc
@@ -32,7 +33,11 @@ class TestErrorModule(unittest.TestCase, SetupTestErrorModules):
def test_error_module_xml_rendering(self):
descriptor = ErrorDescriptor.from_xml(
- self.valid_xml, self.system, self.org, self.course, self.error_msg)
+ self.valid_xml,
+ self.system,
+ CourseLocationGenerator(self.org, self.course),
+ self.error_msg
+ )
self.assertIsInstance(descriptor, ErrorDescriptor)
descriptor.xmodule_runtime = self.system
context_repr = self.system.render(descriptor, 'student_view').content
@@ -63,12 +68,18 @@ class TestNonStaffErrorModule(unittest.TestCase, SetupTestErrorModules):
def test_non_staff_error_module_create(self):
descriptor = NonStaffErrorDescriptor.from_xml(
- self.valid_xml, self.system, self.org, self.course)
+ self.valid_xml,
+ self.system,
+ CourseLocationGenerator(self.org, self.course)
+ )
self.assertIsInstance(descriptor, NonStaffErrorDescriptor)
def test_from_xml_render(self):
descriptor = NonStaffErrorDescriptor.from_xml(
- self.valid_xml, self.system, self.org, self.course)
+ self.valid_xml,
+ self.system,
+ CourseLocationGenerator(self.org, self.course)
+ )
descriptor.xmodule_runtime = self.system
context_repr = self.system.render(descriptor, 'student_view').content
self.assertNotIn(self.error_msg, context_repr)
@@ -117,11 +128,11 @@ class TestErrorModuleConstruction(unittest.TestCase):
def setUp(self):
field_data = Mock(spec=FieldData)
self.descriptor = BrokenDescriptor(
- TestRuntime(Mock(spec=UsageStore), field_data),
+ TestRuntime(Mock(spec=IdReader), field_data),
field_data,
ScopeIds(None, None, None, 'i4x://org/course/broken/name')
)
- self.descriptor.xmodule_runtime = TestRuntime(Mock(spec=UsageStore), field_data)
+ self.descriptor.xmodule_runtime = TestRuntime(Mock(spec=IdReader), field_data)
self.descriptor.xmodule_runtime.error_descriptor_class = ErrorDescriptor
self.descriptor.xmodule_runtime.xmodule_instance = None
diff --git a/common/lib/xmodule/xmodule/tests/test_import.py b/common/lib/xmodule/xmodule/tests/test_import.py
index 3a192318a6..994603e2e1 100644
--- a/common/lib/xmodule/xmodule/tests/test_import.py
+++ b/common/lib/xmodule/xmodule/tests/test_import.py
@@ -1,6 +1,7 @@
# -*- coding: utf-8 -*-
import datetime
+import ddt
import unittest
from fs.memoryfs import MemoryFS
@@ -11,13 +12,17 @@ from django.utils.timezone import UTC
from xmodule.xml_module import is_pointer_tag
from xmodule.modulestore import Location
-from xmodule.modulestore.xml import ImportSystem, XMLModuleStore
+from xmodule.modulestore.xml import ImportSystem, XMLModuleStore, LocationReader
from xmodule.modulestore.inheritance import compute_inherited_metadata
from xmodule.x_module import XModuleMixin, only_xmodules
from xmodule.fields import Date
from xmodule.tests import DATA_DIR
from xmodule.modulestore.inheritance import InheritanceMixin
+from xblock.core import XBlock
+from xblock.fields import Scope, String, Integer
+from xblock.runtime import KvsFieldData, DictKeyValueStore
+
ORG = 'test_org'
COURSE = 'test_course'
@@ -31,7 +36,6 @@ class DummySystem(ImportSystem):
xmlstore = XMLModuleStore("data_dir", course_dirs=[], load_error_modules=load_error_modules)
course_id = "/".join([ORG, COURSE, 'test_run'])
course_dir = "test_dir"
- policy = {}
error_tracker = Mock()
parent_tracker = Mock()
@@ -39,11 +43,12 @@ class DummySystem(ImportSystem):
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, XModuleMixin)
+ mixins=(InheritanceMixin, XModuleMixin),
+ field_data=KvsFieldData(DictKeyValueStore()),
+ id_reader=LocationReader(),
)
def render_template(self, _template, _context):
@@ -72,6 +77,40 @@ class BaseCourseTestCase(unittest.TestCase):
return courses[0]
+class GenericXBlock(XBlock):
+ has_children = True
+ field1 = String(default="something", scope=Scope.user_state)
+ field2 = Integer(scope=Scope.user_state)
+
+
+@ddt.ddt
+class PureXBlockImportTest(BaseCourseTestCase):
+
+ def assert_xblocks_are_good(self, block):
+ """Assert a number of conditions that must be true for `block` to be good."""
+ scope_ids = block.scope_ids
+ self.assertIsNotNone(scope_ids.usage_id)
+ self.assertIsNotNone(scope_ids.def_id)
+
+ for child_id in block.children:
+ child = block.runtime.get_block(child_id)
+ self.assert_xblocks_are_good(child)
+
+ @XBlock.register_temp_plugin(GenericXBlock)
+ @ddt.data(
+ "",
+ "",
+ "",
+ )
+ @patch('xmodule.x_module.XModuleMixin.location')
+ def test_parsing_pure_xblock(self, xml, mock_location):
+ system = self.get_system(load_error_modules=False)
+ descriptor = system.process_xml(xml)
+ self.assertIsInstance(descriptor, GenericXBlock)
+ self.assert_xblocks_are_good(descriptor)
+ self.assertFalse(mock_location.called)
+
+
class ImportTestCase(BaseCourseTestCase):
date = Date()
@@ -119,13 +158,10 @@ class ImportTestCase(BaseCourseTestCase):
tag_xml = descriptor.export_to_xml(resource_fs)
re_import_descriptor = system.process_xml(tag_xml)
- self.assertEqual(re_import_descriptor.__class__.__name__,
- 'ErrorDescriptorWithMixins')
+ self.assertEqual(re_import_descriptor.__class__.__name__, 'ErrorDescriptorWithMixins')
- self.assertEqual(descriptor.contents,
- re_import_descriptor.contents)
- self.assertEqual(descriptor.error_msg,
- re_import_descriptor.error_msg)
+ self.assertEqual(descriptor.contents, re_import_descriptor.contents)
+ self.assertEqual(descriptor.error_msg, re_import_descriptor.error_msg)
def test_fixed_xml_tag(self):
"""Make sure a tag that's been fixed exports as the original tag type"""
@@ -410,7 +446,7 @@ class ImportTestCase(BaseCourseTestCase):
self.assertTrue(any(expect in msg or expect in err
for msg, err in errors))
chapters = course.get_children()
- self.assertEqual(len(chapters), 3)
+ self.assertEqual(len(chapters), 4)
def test_url_name_mangling(self):
"""
diff --git a/common/lib/xmodule/xmodule/tests/test_peer_grading.py b/common/lib/xmodule/xmodule/tests/test_peer_grading.py
index 66929c19d6..ceac29035c 100644
--- a/common/lib/xmodule/xmodule/tests/test_peer_grading.py
+++ b/common/lib/xmodule/xmodule/tests/test_peer_grading.py
@@ -414,6 +414,6 @@ class PeerGradingModuleTrackChangesTest(unittest.TestCase, DummyModulestore):
@return:
"""
self.peer_grading._find_corresponding_module_for_location = self.mock_track_changes_problem
- response = self.peer_grading.peer_grading_problem({'location': 'mocked'})
+ response = self.peer_grading.peer_grading_problem({'location': 'i4x://mock_org/mock_course/mock_cat/mock_name'})
self.assertTrue(response['success'])
self.assertIn("'track_changes': True", response['html'])
diff --git a/common/lib/xmodule/xmodule/tests/test_video.py b/common/lib/xmodule/xmodule/tests/test_video.py
index 5e2f915ba4..78f5205c44 100644
--- a/common/lib/xmodule/xmodule/tests/test_video.py
+++ b/common/lib/xmodule/xmodule/tests/test_video.py
@@ -114,9 +114,10 @@ class VideoDescriptorTest(unittest.TestCase):
def setUp(self):
system = get_test_descriptor_system()
+ location = Location('i4x://org/course/video/name')
self.descriptor = system.construct_xblock_from_class(
VideoDescriptor,
- scope_ids=ScopeIds(None, None, None, None),
+ scope_ids=ScopeIds(None, None, location, location),
field_data=DictFieldData({}),
)
@@ -226,7 +227,7 @@ class VideoDescriptorImportTestCase(unittest.TestCase):
'''
- output = VideoDescriptor.from_xml(xml_data, module_system)
+ output = VideoDescriptor.from_xml(xml_data, module_system, Mock())
self.assert_attributes_equal(output, {
'youtube_id_0_75': 'izygArpw-Qo',
'youtube_id_1_0': 'p2Q6BrNhdh8',
@@ -255,7 +256,7 @@ class VideoDescriptorImportTestCase(unittest.TestCase):
'''
- output = VideoDescriptor.from_xml(xml_data, module_system)
+ output = VideoDescriptor.from_xml(xml_data, module_system, Mock())
self.assert_attributes_equal(output, {
'youtube_id_0_75': '',
'youtube_id_1_0': 'p2Q6BrNhdh8',
@@ -276,7 +277,7 @@ class VideoDescriptorImportTestCase(unittest.TestCase):
"""
module_system = DummySystem(load_error_modules=True)
xml_data = ''
- output = VideoDescriptor.from_xml(xml_data, module_system)
+ output = VideoDescriptor.from_xml(xml_data, module_system, Mock())
self.assert_attributes_equal(output, {
'youtube_id_0_75': '',
'youtube_id_1_0': 'OEoXaMPEzfM',
@@ -310,7 +311,7 @@ class VideoDescriptorImportTestCase(unittest.TestCase):
youtube_id_1_0=""OEoXaMPEzf10""
/>
'''
- output = VideoDescriptor.from_xml(xml_data, module_system)
+ output = VideoDescriptor.from_xml(xml_data, module_system, Mock())
self.assert_attributes_equal(output, {
'youtube_id_0_75': 'OEoXaMPEzf65',
'youtube_id_1_0': 'OEoXaMPEzf10',
@@ -332,7 +333,7 @@ class VideoDescriptorImportTestCase(unittest.TestCase):
youtube="1.0:"p2Q6BrNhdh8",1.25:"1EeWXzPdhSA"">
'''
- output = VideoDescriptor.from_xml(xml_data, module_system)
+ output = VideoDescriptor.from_xml(xml_data, module_system, Mock())
self.assert_attributes_equal(output, {
'youtube_id_0_75': '',
'youtube_id_1_0': 'p2Q6BrNhdh8',
@@ -362,7 +363,7 @@ class VideoDescriptorImportTestCase(unittest.TestCase):
"""
- output = VideoDescriptor.from_xml(xml_data, module_system)
+ output = VideoDescriptor.from_xml(xml_data, module_system, Mock())
self.assert_attributes_equal(output, {
'youtube_id_0_75': 'izygArpw-Qo',
'youtube_id_1_0': 'p2Q6BrNhdh8',
@@ -391,7 +392,7 @@ class VideoDescriptorImportTestCase(unittest.TestCase):
"""
- video = VideoDescriptor.from_xml(xml_data, module_system)
+ video = VideoDescriptor.from_xml(xml_data, module_system, Mock())
self.assert_attributes_equal(video, {
'youtube_id_0_75': 'izygArpw-Qo',
'youtube_id_1_0': 'p2Q6BrNhdh8',
@@ -420,7 +421,7 @@ class VideoDescriptorImportTestCase(unittest.TestCase):
"""
- video = VideoDescriptor.from_xml(xml_data, module_system)
+ video = VideoDescriptor.from_xml(xml_data, module_system, Mock())
self.assert_attributes_equal(video, {
'youtube_id_0_75': 'izygArpw-Qo',
'youtube_id_1_0': 'p2Q6BrNhdh8',
diff --git a/common/lib/xmodule/xmodule/tests/test_xblock_wrappers.py b/common/lib/xmodule/xmodule/tests/test_xblock_wrappers.py
index c8ac5fcdd3..a5247b97de 100644
--- a/common/lib/xmodule/xmodule/tests/test_xblock_wrappers.py
+++ b/common/lib/xmodule/xmodule/tests/test_xblock_wrappers.py
@@ -13,6 +13,8 @@ from mock import Mock
from xblock.field_data import DictFieldData
from xblock.fields import ScopeIds
+from xmodule.modulestore import Location
+
from xmodule.x_module import ModuleSystem, XModule, XModuleDescriptor
from xmodule.mako_module import MakoDescriptorSystem
from xmodule.annotatable_module import AnnotatableDescriptor
@@ -75,7 +77,7 @@ class TestXBlockWrapper(object):
return get_test_system()
def leaf_descriptor(self, descriptor_cls):
- location = 'i4x://org/course/category/name'
+ location = Location('i4x://org/course/category/name')
runtime = get_test_descriptor_system()
return runtime.construct_xblock_from_class(
descriptor_cls,
@@ -100,7 +102,7 @@ class TestXBlockWrapper(object):
def container_descriptor(self, descriptor_cls, depth):
"""Return an instance of `descriptor_cls` with `depth` levels of children"""
- location = 'i4x://org/course/category/name'
+ location = Location('i4x://org/course/category/name')
runtime = get_test_descriptor_system()
if depth == 0:
diff --git a/common/lib/xmodule/xmodule/tests/test_xml_module.py b/common/lib/xmodule/xmodule/tests/test_xml_module.py
index b649397df4..62ebf4dce0 100644
--- a/common/lib/xmodule/xmodule/tests/test_xml_module.py
+++ b/common/lib/xmodule/xmodule/tests/test_xml_module.py
@@ -8,10 +8,10 @@ from nose.tools import assert_equals, assert_not_equals, assert_true, assert_fal
from xblock.field_data import DictFieldData
from xblock.fields import Scope, String, Dict, Boolean, Integer, Float, Any, List
-from xblock.runtime import DbModel
+from xblock.runtime import KvsFieldData, DictKeyValueStore
from xmodule.fields import Date, Timedelta, RelativeTime
-from xmodule.modulestore.inheritance import InheritanceKeyValueStore, InheritanceMixin
+from xmodule.modulestore.inheritance import InheritanceKeyValueStore, InheritanceMixin, InheritingFieldData
from xmodule.xml_module import XmlDescriptor, serialize_field, deserialize_field
from xmodule.course_module import CourseDescriptor
from xmodule.seq_module import SequenceDescriptor
@@ -58,6 +58,87 @@ class TestFields(object):
# Used for testing Lists
list_field = List(scope=Scope.settings, default=[])
+
+class InheritingFieldDataTest(unittest.TestCase):
+ """Tests of InheritingFieldData."""
+
+ class TestableInheritingXBlock(XmlDescriptor):
+ """An XBlock we can use in these tests."""
+ inherited = String(scope=Scope.settings, default="the default")
+ not_inherited = String(scope=Scope.settings, default="nothing")
+
+ def setUp(self):
+ self.system = get_test_descriptor_system()
+ self.all_blocks = {}
+ self.system.get_block = self.all_blocks.get
+ self.field_data = InheritingFieldData(
+ inheritable_names=['inherited'],
+ kvs=DictKeyValueStore({}),
+ )
+
+ def get_a_block(self, usage_id=None):
+ """Construct an XBlock for testing with."""
+ scope_ids = Mock()
+ if usage_id is None:
+ usage_id = "_auto%d" % len(self.all_blocks)
+ scope_ids.usage_id = usage_id
+ block = self.system.construct_xblock_from_class(
+ self.TestableInheritingXBlock,
+ field_data=self.field_data,
+ scope_ids=scope_ids,
+ )
+ self.all_blocks[usage_id] = block
+ return block
+
+ def test_default_value(self):
+ # Blocks with nothing set with return the fields' defaults.
+ block = self.get_a_block()
+ self.assertEqual(block.inherited, "the default")
+ self.assertEqual(block.not_inherited, "nothing")
+
+ def test_set_value(self):
+ # If you set a value, that's what you get back.
+ block = self.get_a_block()
+ block.inherited = "Changed!"
+ block.not_inherited = "New Value!"
+ self.assertEqual(block.inherited, "Changed!")
+ self.assertEqual(block.not_inherited, "New Value!")
+
+ def test_inherited(self):
+ # A child with get a value inherited from the parent.
+ parent = self.get_a_block(usage_id="parent")
+ parent.inherited = "Changed!"
+ self.assertEqual(parent.inherited, "Changed!")
+
+ child = self.get_a_block(usage_id="child")
+ child.parent = "parent"
+ self.assertEqual(child.inherited, "Changed!")
+
+ def test_inherited_across_generations(self):
+ # A child with get a value inherited from a great-grandparent.
+ parent = self.get_a_block(usage_id="parent")
+ parent.inherited = "Changed!"
+ self.assertEqual(parent.inherited, "Changed!")
+ parent_id = "parent"
+ for child_num in range(10):
+ usage_id = "child_{}".format(child_num)
+ child = self.get_a_block(usage_id=usage_id)
+ child.parent = "parent"
+ self.assertEqual(child.inherited, "Changed!")
+ parent_id = usage_id
+
+ def test_not_inherited(self):
+ # Fields not in the inherited_names list won't be inherited.
+ parent = self.get_a_block(usage_id="parent")
+ parent.not_inherited = "Changed!"
+ self.assertEqual(parent.not_inherited, "Changed!")
+
+ child = self.get_a_block(usage_id="child")
+ child.parent = "parent"
+ self.assertEqual(child.not_inherited, "nothing")
+
+
+
class EditableMetadataFieldsTest(unittest.TestCase):
def test_display_name_field(self):
editable_fields = self.get_xml_editable_fields(DictFieldData({}))
@@ -100,7 +181,7 @@ class EditableMetadataFieldsTest(unittest.TestCase):
def test_inherited_field(self):
kvs = InheritanceKeyValueStore(initial_values={}, inherited_settings={'showanswer': 'inherited'})
- model_data = DbModel(kvs)
+ model_data = KvsFieldData(kvs)
descriptor = self.get_descriptor(model_data)
editable_fields = descriptor.editable_metadata_fields
self.assert_field_values(
@@ -113,7 +194,7 @@ class EditableMetadataFieldsTest(unittest.TestCase):
initial_values={'showanswer': 'explicit'},
inherited_settings={'showanswer': 'inheritable value'}
)
- model_data = DbModel(kvs)
+ model_data = KvsFieldData(kvs)
descriptor = self.get_descriptor(model_data)
editable_fields = descriptor.editable_metadata_fields
self.assert_field_values(
diff --git a/common/lib/xmodule/xmodule/tests/xml/__init__.py b/common/lib/xmodule/xmodule/tests/xml/__init__.py
index 624a61d13f..9131459d08 100644
--- a/common/lib/xmodule/xmodule/tests/xml/__init__.py
+++ b/common/lib/xmodule/xmodule/tests/xml/__init__.py
@@ -4,9 +4,12 @@ Xml parsing tests for XModules
import pprint
from mock import Mock
-from xmodule.x_module import XMLParsingSystem
+from xmodule.x_module import XMLParsingSystem, policy_key
from xmodule.mako_module import MakoDescriptorSystem
-from xmodule.modulestore.xml import create_block_from_xml
+from xmodule.modulestore.xml import create_block_from_xml, LocationReader, CourseLocationGenerator
+from xmodule.modulestore import Location
+
+from xblock.runtime import KvsFieldData, DictKeyValueStore
class InMemorySystem(XMLParsingSystem, MakoDescriptorSystem): # pylint: disable=abstract-method
@@ -18,26 +21,37 @@ class InMemorySystem(XMLParsingSystem, MakoDescriptorSystem): # pylint: disable
self.course = xml_import_data.course
self.default_class = xml_import_data.default_class
self._descriptors = {}
+
+ def get_policy(usage_id):
+ """Return the policy data for the specified usage"""
+ return xml_import_data.policy.get(policy_key(usage_id), {})
+
super(InMemorySystem, self).__init__(
- policy=xml_import_data.policy,
+ get_policy=get_policy,
process_xml=self.process_xml,
load_item=self.load_item,
error_tracker=Mock(),
resources_fs=xml_import_data.filesystem,
mixins=xml_import_data.xblock_mixins,
select=xml_import_data.xblock_select,
- render_template=lambda template, context: pprint.pformat((template, context))
+ render_template=lambda template, context: pprint.pformat((template, context)),
+ field_data=KvsFieldData(DictKeyValueStore()),
+ id_reader=LocationReader(),
)
def process_xml(self, xml): # pylint: disable=method-hidden
"""Parse `xml` as an XBlock, and add it to `self._descriptors`"""
- descriptor = create_block_from_xml(xml, self, self.org, self.course, self.default_class)
+ descriptor = create_block_from_xml(
+ xml,
+ self,
+ CourseLocationGenerator(self.org, self.course),
+ )
self._descriptors[descriptor.location.url()] = descriptor
return descriptor
def load_item(self, location): # pylint: disable=method-hidden
"""Return the descriptor loaded for `location`"""
- return self._descriptors[location]
+ return self._descriptors[Location(location).url()]
class XModuleXmlImportTest(object):
diff --git a/common/lib/xmodule/xmodule/video_module.py b/common/lib/xmodule/xmodule/video_module.py
index 02c458b3a5..364a8ee417 100644
--- a/common/lib/xmodule/xmodule/video_module.py
+++ b/common/lib/xmodule/xmodule/video_module.py
@@ -16,7 +16,6 @@ import logging
from lxml import etree
from pkg_resources import resource_string
import datetime
-import time
import copy
from django.http import Http404
@@ -31,7 +30,8 @@ from xblock.fields import Scope, String, Boolean, List, Integer, ScopeIds
from xmodule.fields import RelativeTime
from xmodule.modulestore.inheritance import InheritanceKeyValueStore
-from xblock.runtime import DbModel
+from xblock.runtime import KvsFieldData
+
log = logging.getLogger(__name__)
@@ -214,7 +214,7 @@ class VideoDescriptor(VideoFields, TabsEditingDescriptor, EmptyDataRawDescriptor
del self.data
@classmethod
- def from_xml(cls, xml_data, system, org=None, course=None):
+ def from_xml(cls, xml_data, system, id_generator):
"""
Creates an instance of this descriptor from the supplied xml_data.
This may be overridden by subclasses
@@ -227,22 +227,21 @@ class VideoDescriptor(VideoFields, TabsEditingDescriptor, EmptyDataRawDescriptor
"""
xml_object = etree.fromstring(xml_data)
url_name = xml_object.get('url_name', xml_object.get('slug'))
- location = Location(
- 'i4x', org, course, 'video', url_name
- )
+ block_type = 'video'
+ definition_id = id_generator.create_definition(block_type, url_name)
+ usage_id = id_generator.create_usage(definition_id)
if is_pointer_tag(xml_object):
filepath = cls._format_filepath(xml_object.tag, name_to_pathname(url_name))
- xml_data = etree.tostring(cls.load_file(filepath, system.resources_fs, location))
+ xml_data = etree.tostring(cls.load_file(filepath, system.resources_fs, usage_id))
field_data = cls._parse_video_xml(xml_data)
- field_data['location'] = location
kvs = InheritanceKeyValueStore(initial_values=field_data)
- field_data = DbModel(kvs)
+ field_data = KvsFieldData(kvs)
video = system.construct_xblock_from_class(
cls,
# 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),
+ ScopeIds(None, block_type, definition_id, usage_id),
field_data,
)
return video
diff --git a/common/lib/xmodule/xmodule/x_module.py b/common/lib/xmodule/xmodule/x_module.py
index ffc6862115..e7bc5c47fb 100644
--- a/common/lib/xmodule/xmodule/x_module.py
+++ b/common/lib/xmodule/xmodule/x_module.py
@@ -10,18 +10,19 @@ from pkg_resources import resource_listdir, resource_string, resource_isdir
from webob import Response
from webob.multidict import MultiDict
-from xmodule.modulestore import Location
-from xmodule.modulestore.exceptions import ItemNotFoundError, InsufficientSpecificationError, InvalidLocationError
-
from xblock.core import XBlock
from xblock.fields import Scope, Integer, Float, List, XBlockMixin, String
from xblock.fragment import Fragment
from xblock.plugin import default_select
-from xblock.runtime import Runtime
+from xblock.runtime import Runtime, MemoryIdManager
from xmodule.fields import RelativeTime
+
from xmodule.errortracker import exc_info_to_str
+from xmodule.modulestore import Location
+from xmodule.modulestore.exceptions import ItemNotFoundError, InsufficientSpecificationError, InvalidLocationError
from xmodule.modulestore.locator import BlockUsageLocator
+
log = logging.getLogger(__name__)
@@ -650,28 +651,30 @@ class XModuleDescriptor(XModuleMixin, HTMLSnippet, ResourceTemplates, XBlock):
# ================================= XML PARSING ============================
@classmethod
- def parse_xml(cls, node, runtime, keys):
+ def parse_xml(cls, node, runtime, keys, id_generator):
"""
Interpret the parsed XML in `node`, creating an XModuleDescriptor.
"""
xml = etree.tostring(node)
# TODO: change from_xml to not take org and course, it can use self.system.
- block = cls.from_xml(xml, runtime, runtime.org, runtime.course)
+ block = cls.from_xml(xml, runtime, id_generator)
return block
@classmethod
- def from_xml(cls, xml_data, system, org=None, course=None):
+ def from_xml(cls, xml_data, system, id_generator):
"""
Creates an instance of this descriptor from the supplied xml_data.
This may be overridden by subclasses.
- xml_data: A string of xml that will be translated into data and children
- for this module
+ Args:
+ xml_data (str): A string of xml that will be translated into data and children
+ for this module
- system is an XMLParsingSystem
+ system (:class:`.XMLParsingSystem):
+
+ id_generator (:class:`xblock.runtime.IdGenerator`): Used to generate the
+ usage_ids and definition_ids when loading this xml
- org and course are optional strings that will be used in the generated
- module's url identifiers
"""
raise NotImplementedError('Modules must implement from_xml to be parsable from xml')
@@ -872,7 +875,9 @@ class DescriptorSystem(ConfigurableFragmentWrapper, Runtime): # pylint: disable
Base class for :class:`Runtime`s to be used with :class:`XModuleDescriptor`s
"""
- def __init__(self, load_item, resources_fs, error_tracker, **kwargs):
+ def __init__(
+ self, load_item, resources_fs, error_tracker, get_policy=None, **kwargs
+ ):
"""
load_item: Takes a Location and returns an XModuleDescriptor
@@ -907,15 +912,20 @@ class DescriptorSystem(ConfigurableFragmentWrapper, Runtime): # pylint: disable
NOTE: To avoid duplication, do not call the tracker on errors
that you're about to re-raise---let the caller track them.
- """
- # Right now, usage_store is unused, and field_data is always supplanted
- # with an explicit field_data during construct_xblock, so None's suffice.
- super(DescriptorSystem, self).__init__(usage_store=None, field_data=None, **kwargs)
+ get_policy: a function that takes a usage id and returns a dict of
+ policy to apply.
+
+ """
+ super(DescriptorSystem, self).__init__(**kwargs)
self.load_item = load_item
self.resources_fs = resources_fs
self.error_tracker = error_tracker
+ if get_policy:
+ self.get_policy = get_policy
+ else:
+ self.get_policy = lambda u: {}
def get_block(self, usage_id):
"""See documentation for `xblock.runtime:Runtime.get_block`"""
@@ -978,17 +988,14 @@ class DescriptorSystem(ConfigurableFragmentWrapper, Runtime): # pylint: disable
class XMLParsingSystem(DescriptorSystem):
- def __init__(self, process_xml, policy, **kwargs):
+ def __init__(self, process_xml, **kwargs):
"""
- policy: a policy dictionary for overriding xml metadata
-
process_xml: Takes an xml string, and returns a XModuleDescriptor
created from that xml
"""
super(XMLParsingSystem, self).__init__(**kwargs)
self.process_xml = process_xml
- self.policy = policy
class ModuleSystem(ConfigurableFragmentWrapper, Runtime): # pylint: disable=abstract-method
@@ -1010,7 +1017,9 @@ class ModuleSystem(ConfigurableFragmentWrapper, Runtime): # pylint: disable=abs
anonymous_student_id='', course_id=None,
open_ended_grading_interface=None, s3_interface=None,
cache=None, can_execute_unsafe_code=None, replace_course_urls=None,
- replace_jump_to_id_urls=None, error_descriptor_class=None, get_real_user=None, **kwargs):
+ replace_jump_to_id_urls=None, error_descriptor_class=None, get_real_user=None,
+ field_data=None,
+ **kwargs):
"""
Create a closure around the system environment.
@@ -1062,11 +1071,12 @@ class ModuleSystem(ConfigurableFragmentWrapper, Runtime): # pylint: disable=abs
get_real_user - function that takes `anonymous_student_id` and returns real user_id,
associated with `anonymous_student_id`.
+ field_data - the `FieldData` to use for backing XBlock storage.
"""
- # Right now, usage_store is unused, and field_data is always supplanted
- # with an explicit field_data during construct_xblock, so None's suffice.
- super(ModuleSystem, self).__init__(usage_store=None, field_data=None, **kwargs)
+ # Usage_store is unused, and field_data is often supplanted with an
+ # explicit field_data during construct_xblock.
+ super(ModuleSystem, self).__init__(id_reader=None, field_data=field_data, **kwargs)
self.STATIC_URL = static_url
self.xqueue = xqueue
diff --git a/common/lib/xmodule/xmodule/xml_module.py b/common/lib/xmodule/xmodule/xml_module.py
index 47682b8ea2..c659b7787b 100644
--- a/common/lib/xmodule/xmodule/xml_module.py
+++ b/common/lib/xmodule/xmodule/xml_module.py
@@ -6,11 +6,11 @@ import sys
from lxml import etree
from xblock.fields import Dict, Scope, ScopeIds
-from xmodule.x_module import (XModuleDescriptor, policy_key)
+from xmodule.x_module import XModuleDescriptor, policy_key
from xmodule.modulestore import Location
from xmodule.modulestore.inheritance import own_metadata, InheritanceKeyValueStore
from xmodule.modulestore.xml_exporter import EdxJSONEncoder
-from xblock.runtime import DbModel
+from xblock.runtime import KvsFieldData
log = logging.getLogger(__name__)
@@ -153,8 +153,7 @@ class XmlDescriptor(XModuleDescriptor):
xml_object: An etree Element
"""
- raise NotImplementedError(
- "%s does not implement definition_from_xml" % cls.__name__)
+ raise NotImplementedError("%s does not implement definition_from_xml" % cls.__name__)
@classmethod
def clean_metadata_from_xml(cls, xml_object):
@@ -177,7 +176,7 @@ class XmlDescriptor(XModuleDescriptor):
return etree.parse(file_object, parser=edx_xml_parser).getroot()
@classmethod
- def load_file(cls, filepath, fs, location):
+ def load_file(cls, filepath, fs, def_id):
'''
Open the specified file in fs, and call cls.file_to_xml on it,
returning the lxml object.
@@ -190,11 +189,11 @@ class XmlDescriptor(XModuleDescriptor):
except Exception as err:
# Add info about where we are, but keep the traceback
msg = 'Unable to load file contents at path %s for item %s: %s ' % (
- filepath, location.url(), str(err))
+ filepath, def_id, err)
raise Exception, msg, sys.exc_info()[2]
@classmethod
- def load_definition(cls, xml_object, system, location):
+ def load_definition(cls, xml_object, system, def_id):
'''Load a descriptor definition from the specified xml_object.
Subclasses should not need to override this except in special
cases (e.g. html module)'''
@@ -220,7 +219,7 @@ class XmlDescriptor(XModuleDescriptor):
filepath = candidate
break
- definition_xml = cls.load_file(filepath, system.resources_fs, location)
+ definition_xml = cls.load_file(filepath, system.resources_fs, def_id)
definition_metadata = get_metadata_from_xml(definition_xml)
cls.clean_metadata_from_xml(definition_xml)
@@ -269,7 +268,7 @@ class XmlDescriptor(XModuleDescriptor):
metadata[attr] = value
@classmethod
- def from_xml(cls, xml_data, system, org=None, course=None):
+ def from_xml(cls, xml_data, system, id_generator):
"""
Creates an instance of this descriptor from the supplied xml_data.
This may be overridden by subclasses
@@ -277,26 +276,25 @@ class XmlDescriptor(XModuleDescriptor):
xml_data: A string of xml that will be translated into data and children for
this module
system: A DescriptorSystem for interacting with external resources
- org and course are optional strings that will be used in the generated modules
- url identifiers
"""
xml_object = etree.fromstring(xml_data)
# VS[compat] -- just have the url_name lookup, once translation is done
url_name = xml_object.get('url_name', xml_object.get('slug'))
- location = Location('i4x', org, course, xml_object.tag, url_name)
+ def_id = id_generator.create_definition(xml_object.tag, url_name)
+ usage_id = id_generator.create_usage(def_id)
# VS[compat] -- detect new-style each-in-a-file mode
if is_pointer_tag(xml_object):
# new style:
# read the actual definition file--named using url_name.replace(':','/')
filepath = cls._format_filepath(xml_object.tag, name_to_pathname(url_name))
- definition_xml = cls.load_file(filepath, system.resources_fs, location)
+ definition_xml = cls.load_file(filepath, system.resources_fs, def_id)
else:
definition_xml = xml_object
filepath = None
- definition, children = cls.load_definition(definition_xml, system, location) # note this removes metadata
+ definition, children = cls.load_definition(definition_xml, system, def_id) # note this removes metadata
# VS[compat] -- make Ike's github preview links work in both old and
# new file layouts
@@ -313,13 +311,11 @@ class XmlDescriptor(XModuleDescriptor):
try:
metadata.update(json.loads(dmdata))
except Exception as err:
- log.debug('Error %s in loading metadata %s' % (err, dmdata))
+ log.debug('Error in loading metadata %r', dmdata, exc_info=True)
metadata['definition_metadata_err'] = str(err)
# Set/override any metadata specified by policy
- k = policy_key(location)
- if k in system.policy:
- cls.apply_policy(metadata, system.policy[k])
+ cls.apply_policy(metadata, system.get_policy(usage_id))
field_data = {}
field_data.update(metadata)
@@ -327,17 +323,13 @@ class XmlDescriptor(XModuleDescriptor):
field_data['children'] = children
field_data['xml_attributes']['filename'] = definition.get('filename', ['', None]) # for git link
- field_data['location'] = location
- field_data['category'] = xml_object.tag
kvs = InheritanceKeyValueStore(initial_values=field_data)
- field_data = DbModel(kvs)
+ field_data = KvsFieldData(kvs)
return system.construct_xblock_from_class(
cls,
# 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),
+ ScopeIds(None, xml_object.tag, def_id, usage_id),
field_data,
)
diff --git a/common/test/data/simple/course.xml b/common/test/data/simple/course.xml
index 529528ca0a..5cd6d54d29 100644
--- a/common/test/data/simple/course.xml
+++ b/common/test/data/simple/course.xml
@@ -6,7 +6,7 @@
-
-
- Foobar
-
-
+
+
+ Foobar
+
+
-
diff --git a/lms/djangoapps/courseware/management/commands/dump_course_structure.py b/lms/djangoapps/courseware/management/commands/dump_course_structure.py
index 1a0f3b4378..6814b3cfe4 100644
--- a/lms/djangoapps/courseware/management/commands/dump_course_structure.py
+++ b/lms/djangoapps/courseware/management/commands/dump_course_structure.py
@@ -95,7 +95,7 @@ def dump_module(module, destination=None, inherited=False, defaults=False):
destination[module.location.url()] = {
'category': module.location.category,
- 'children': module.children if hasattr(module, 'children') else [],
+ 'children': [str(child) for child in getattr(module, 'children', [])],
'metadata': filtered_metadata,
}
@@ -110,7 +110,7 @@ def dump_module(module, destination=None, inherited=False, defaults=False):
return False
elif field.scope != Scope.settings:
return False
- elif defaults == True:
+ elif defaults:
return True
else:
return field.values != field.default
diff --git a/lms/djangoapps/courseware/model_data.py b/lms/djangoapps/courseware/model_data.py
index 150792ae98..328c65ea5c 100644
--- a/lms/djangoapps/courseware/model_data.py
+++ b/lms/djangoapps/courseware/model_data.py
@@ -317,7 +317,7 @@ class DjangoKeyValueStore(KeyValueStore):
Provide a bulk save mechanism.
`kv_dict`: A dictionary of dirty fields that maps
- xblock.DbModel._key : value
+ xblock.KvsFieldData._key : value
"""
saved_fields = []
diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py
index 39371a3448..e717743ba0 100644
--- a/lms/djangoapps/courseware/module_render.py
+++ b/lms/djangoapps/courseware/module_render.py
@@ -29,7 +29,7 @@ from util.json_request import JsonResponse
from util.sandboxing import can_execute_unsafe_code
from xblock.core import XBlock
from xblock.fields import Scope
-from xblock.runtime import DbModel, KeyValueStore
+from xblock.runtime import KvsFieldData, KeyValueStore
from xblock.exceptions import NoSuchHandlerError
from xblock.django.request import django_to_webob_request, webob_to_django_response
from xmodule.error_module import ErrorDescriptor, NonStaffErrorDescriptor
@@ -222,7 +222,7 @@ def get_module_for_descriptor_internal(user, descriptor, field_data_cache, cours
if not has_access(user, descriptor, 'load', course_id):
return None
- student_data = DbModel(DjangoKeyValueStore(field_data_cache))
+ student_data = KvsFieldData(DjangoKeyValueStore(field_data_cache))
descriptor._field_data = LmsFieldData(descriptor._field_data, student_data)
diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt
index 16cd5ecd64..7dd8b7f1fd 100644
--- a/requirements/edx/base.txt
+++ b/requirements/edx/base.txt
@@ -93,7 +93,6 @@ transifex-client==0.9.1
# Used for testing
coverage==3.7
-ddt==0.4.0
factory_boy==2.2.1
mock==1.0.1
nosexcover==1.0.7
diff --git a/requirements/edx/github.txt b/requirements/edx/github.txt
index 19ed7b283c..f53b600013 100644
--- a/requirements/edx/github.txt
+++ b/requirements/edx/github.txt
@@ -15,10 +15,13 @@
-e git+https://github.com/eventbrite/zendesk.git@d53fe0e81b623f084e91776bcf6369f8b7b63879#egg=zendesk
# Our libraries:
--e git+https://github.com/edx/XBlock.git@cd77808aadd3ea1c2027ca8c0aa5624d8ccccc52#egg=XBlock
+-e git+https://github.com/edx/XBlock.git@a1a3e76b269d15b7bbd11976d8aef63e1db6c4c2#egg=XBlock
-e git+https://github.com/edx/codejail.git@e3d98f9455#egg=codejail
-e git+https://github.com/edx/diff-cover.git@v0.2.6#egg=diff_cover
-e git+https://github.com/edx/js-test-tool.git@v0.1.5#egg=js_test_tool
-e git+https://github.com/edx/django-waffle.git@823a102e48#egg=django-waffle
-e git+https://github.com/edx/event-tracking.git@f0211d702d#egg=event-tracking
-e git+https://github.com/edx/bok-choy.git@bc6f1adbe439618162079f1004b2b3db3b6f8916#egg=bok_choy
+
+# Move back to upstream release once https://github.com/txels/ddt/pull/13 is merged
+-e git+https://github.com/edx/ddt.git@9e8010b8777aa40b848fdb76de6e60081616325a#egg=ddt
\ No newline at end of file