diff --git a/cms/djangoapps/contentstore/management/commands/import.py b/cms/djangoapps/contentstore/management/commands/import.py
index a7f95ea3c049aafcdedc86de3f3f843f68ecb14a..e24111dbb77e6cb7b6964cb86e0c660a8e03bf2a 100644
--- a/cms/djangoapps/contentstore/management/commands/import.py
+++ b/cms/djangoapps/contentstore/management/commands/import.py
@@ -25,8 +25,8 @@ class Command(BaseCommand):
module_store = XMLModuleStore(org, course, data_dir, 'xmodule.raw_module.RawDescriptor')
for module in module_store.modules.itervalues():
- keystore().create_item(module.url)
+ keystore().create_item(module.location)
if 'data' in module.definition:
- keystore().update_item(module.url, module.definition['data'])
+ keystore().update_item(module.location, module.definition['data'])
if 'children' in module.definition:
- keystore().update_children(module.url, module.definition['children'])
+ keystore().update_children(module.location, module.definition['children'])
diff --git a/cms/templates/widgets/navigation.html b/cms/templates/widgets/navigation.html
index 38b1cd9d94ad539b00ff56623275be94ea416030..bed0d1b4f804394591aaa5b487ca9fe40b5c0cb9 100644
--- a/cms/templates/widgets/navigation.html
+++ b/cms/templates/widgets/navigation.html
@@ -38,7 +38,7 @@
% for week in weeks:
<li>
<header>
- <h1><a href="#" class="module-edit" id="${week.url}">${week.name}</a></h1>
+ <h1><a href="#" class="module-edit" id="${week.location.url()}">${week.name}</a></h1>
<ul>
% if week.goals:
% for goal in week.goals:
@@ -52,8 +52,8 @@
<ul>
% for module in week.get_children():
- <li class="${module.type}">
- <a href="#" class="module-edit" id="${module.url}">${module.name}</a>
+ <li class="${module.category}">
+ <a href="#" class="module-edit" id="${module.location.url()}">${module.name}</a>
<a href="#" class="draggable">handle</a>
</li>
% endfor
diff --git a/cms/templates/widgets/sequence-edit.html b/cms/templates/widgets/sequence-edit.html
index abeec9209d97ec41666299632c95791c128bc3cb..319e137638557e21652a30c5ff948de3857389e6 100644
--- a/cms/templates/widgets/sequence-edit.html
+++ b/cms/templates/widgets/sequence-edit.html
@@ -37,7 +37,7 @@
<ol>
% for child in module.get_children():
<li>
- <a href="#" class="module-edit" id="${child.url}">${child.name}</a>
+ <a href="#" class="module-edit" id="${child.location.url()}">${child.name}</a>
<a href="#" class="draggable">handle</a>
</li>
%endfor
diff --git a/common/lib/keystore/__init__.py b/common/lib/keystore/__init__.py
index b204e487e22b36e2d85f2a181c900699d56a538c..dab758fef1227705cae279cdd3cb7031a503f286 100644
--- a/common/lib/keystore/__init__.py
+++ b/common/lib/keystore/__init__.py
@@ -4,6 +4,7 @@ that are stored in a database an accessible using their Location as an identifie
"""
import re
+from collections import namedtuple
from .exceptions import InvalidLocationError
URL_RE = re.compile("""
@@ -17,8 +18,8 @@ URL_RE = re.compile("""
INVALID_CHARS = re.compile(r"[^\w.-]")
-
-class Location(object):
+_LocationBase = namedtuple('LocationBase', 'tag org course category name revision')
+class Location(_LocationBase):
'''
Encodes a location.
@@ -28,6 +29,7 @@ class Location(object):
However, they can also be represented a dictionaries (specifying each component),
tuples or list (specified in order), or as strings of the url
'''
+ __slots__ = ()
@classmethod
def clean(cls, value):
@@ -36,7 +38,7 @@ class Location(object):
"""
return re.sub('_+', '_', INVALID_CHARS.sub('_', value))
- def __init__(self, location):
+ def __new__(_cls, loc_or_tag, org=None, course=None, category=None, name=None, revision=None):
"""
Create a new location that is a clone of the specifed one.
@@ -60,47 +62,50 @@ class Location(object):
Components may be set to None, which may be interpreted by some contexts to mean
wildcard selection
"""
- self.update(location)
- def update(self, location):
- """
- Update this instance with data from another Location object.
+ 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:
+ location = (loc_or_tag, org, course, category, name, revision)
- location: can take the same forms as specified by `__init__`
- """
- self.tag = self.org = self.course = self.category = self.name = self.revision = None
+ def check_dict(dict_):
+ check_list(dict_.values())
+
+ def check_list(list_):
+ for val in list_:
+ if val is not None and INVALID_CHARS.search(val) is not None:
+ raise InvalidLocationError(location)
if isinstance(location, basestring):
match = URL_RE.match(location)
if match is None:
raise InvalidLocationError(location)
else:
- self.update(match.groupdict())
- elif isinstance(location, list):
+ groups = match.groupdict()
+ check_dict(groups)
+ return _LocationBase.__new__(_cls, **groups)
+ elif isinstance(location, (list, tuple)):
if len(location) not in (5, 6):
raise InvalidLocationError(location)
- (self.tag, self.org, self.course, self.category, self.name) = location[0:5]
- self.revision = location[5] if len(location) == 6 else None
+ if len(location) == 5:
+ args = tuple(location) + (None, )
+ else:
+ args = tuple(location)
+
+ check_list(args)
+ return _LocationBase.__new__(_cls, *args)
elif isinstance(location, dict):
- try:
- self.tag = location['tag']
- self.org = location['org']
- self.course = location['course']
- self.category = location['category']
- self.name = location['name']
- except KeyError:
- raise InvalidLocationError(location)
- self.revision = location.get('revision')
+ kwargs = dict(location)
+ kwargs.setdefault('revision', None)
+
+ check_dict(kwargs)
+ return _LocationBase.__new__(_cls, **kwargs)
elif isinstance(location, Location):
- self.update(location.list())
+ return _LocationBase.__new__(_cls, location)
else:
raise InvalidLocationError(location)
- for val in self.list():
- if val is not None and INVALID_CHARS.search(val) is not None:
- raise InvalidLocationError(location)
-
def url(self):
"""
Return a string containing the URL for this location
@@ -114,38 +119,19 @@ class Location(object):
"""
Return a string with a version of the location that is safe for use in html id attributes
"""
- return "-".join(str(v) for v in self.list() if v is not None)
-
- def list(self):
- """
- Return a list representing this location
- """
- return [self.tag, self.org, self.course, self.category, self.name, self.revision]
+ return "-".join(str(v) for v in self if v is not None)
def dict(self):
- """
- Return a dictionary representing this location
- """
- return {'tag': self.tag,
- 'org': self.org,
- 'course': self.course,
- 'category': self.category,
- 'name': self.name,
- 'revision': self.revision}
+ return self.__dict__
+
+ def list(self):
+ return list(self)
def __str__(self):
return self.url()
def __repr__(self):
- return 'Location(%r)' % str(self)
-
- def __hash__(self):
- return self.url()
-
- def __eq__(self, other):
- return (isinstance(other, Location) and
- str(self) == str(other))
-
+ return "Location%r" % tuple(self)
class ModuleStore(object):
"""
diff --git a/common/lib/keystore/tests/test_location.py b/common/lib/keystore/tests/test_location.py
index f10f03c0b01a5e2ec5776ebf62f31f6fb3ae4fe9..01d36d946b6f023d504f323c35a6500fd698df76 100644
--- a/common/lib/keystore/tests/test_location.py
+++ b/common/lib/keystore/tests/test_location.py
@@ -1,4 +1,4 @@
-from nose.tools import assert_equals, assert_raises
+from nose.tools import assert_equals, assert_raises, assert_not_equals
from keystore import Location
from keystore.exceptions import InvalidLocationError
@@ -11,7 +11,6 @@ def check_string_roundtrip(url):
def test_string_roundtrip():
check_string_roundtrip("tag://org/course/category/name")
check_string_roundtrip("tag://org/course/category/name/revision")
- check_string_roundtrip("tag://org/course/category/name with spaces/revision")
def test_dict():
@@ -50,3 +49,15 @@ def test_invalid_locations():
assert_raises(InvalidLocationError, Location, ["foo", "bar"])
assert_raises(InvalidLocationError, Location, ["foo", "bar", "baz", "blat", "foo/bar"])
assert_raises(InvalidLocationError, Location, None)
+ assert_raises(InvalidLocationError, Location, "tag://org/course/category/name with spaces/revision")
+
+def test_equality():
+ assert_equals(
+ Location('tag', 'org', 'course', 'category', 'name'),
+ Location('tag', 'org', 'course', 'category', 'name')
+ )
+
+ assert_not_equals(
+ Location('tag', 'org', 'course', 'category', 'name1'),
+ Location('tag', 'org', 'course', 'category', 'name')
+ )
diff --git a/common/lib/keystore/xml.py b/common/lib/keystore/xml.py
index c3327b8ff39eb7a59cf513e82b008e107e2f1786..e7adb56ad6e98a69639c26810dfd1fb6a566930b 100644
--- a/common/lib/keystore/xml.py
+++ b/common/lib/keystore/xml.py
@@ -47,7 +47,7 @@ class XMLModuleStore(ModuleStore):
xml_data.set('slug', '{tag}_{count}'.format(tag=xml_data.tag, count=self.unnamed_modules))
module = XModuleDescriptor.load_from_xml(etree.tostring(xml_data), self, org, course, modulestore.default_class)
- modulestore.modules[module.url] = module
+ modulestore.modules[module.location] = module
return module
XMLParsingSystem.__init__(self, modulestore.get_item, OSFS(data_dir), process_xml)
@@ -68,7 +68,7 @@ class XMLModuleStore(ModuleStore):
"""
location = Location(location)
try:
- return self.modules[location.url()]
+ return self.modules[location]
except KeyError:
raise ItemNotFoundError(location)
diff --git a/common/lib/xmodule/abtest_module.py b/common/lib/xmodule/abtest_module.py
index 3bd268184a356a827330a1b22285177768d8a507..beaeb4ad1cd56827c725042e1078655bfe11254a 100644
--- a/common/lib/xmodule/abtest_module.py
+++ b/common/lib/xmodule/abtest_module.py
@@ -102,7 +102,7 @@ class ABTestDescriptor(RawDescriptor, XmlDescriptor):
)
child_content_urls = [
- system.process_xml(etree.tostring(child)).url
+ system.process_xml(etree.tostring(child)).location.url()
for child in group
]
diff --git a/common/lib/xmodule/seq_module.py b/common/lib/xmodule/seq_module.py
index b11d540143b4fe2c79251a071fee5bb7126efb18..6d493b96ad7756307558b9e11144f7acfdea716a 100644
--- a/common/lib/xmodule/seq_module.py
+++ b/common/lib/xmodule/seq_module.py
@@ -105,6 +105,6 @@ class SequenceDescriptor(MakoModuleDescriptor, XmlDescriptor):
@classmethod
def definition_from_xml(cls, xml_object, system):
return {'children': [
- system.process_xml(etree.tostring(child_module)).url
+ system.process_xml(etree.tostring(child_module)).location.url()
for child_module in xml_object
]}
diff --git a/common/lib/xmodule/x_module.py b/common/lib/xmodule/x_module.py
index 0b8fbb54d47947b56a6f1b348d3d8167f6cdec2e..191cda6b06d85530785872ac6d61287cc0eb6651 100644
--- a/common/lib/xmodule/x_module.py
+++ b/common/lib/xmodule/x_module.py
@@ -239,7 +239,7 @@ class XModuleDescriptor(Plugin):
self.definition = definition if definition is not None else {}
self.name = Location(kwargs.get('location')).name
self.category = Location(kwargs.get('location')).category
- self.url = Location(kwargs.get('location')).url()
+ self.location = Location(kwargs.get('location'))
self.metadata = kwargs.get('metadata', {})
self.shared_state_key = kwargs.get('shared_state_key')
@@ -364,7 +364,7 @@ class XModuleDescriptor(Plugin):
return partial(
self.module_class,
system,
- self.url,
+ self.location,
self.definition,
metadata=self.metadata
)
diff --git a/lms/djangoapps/courseware/models.py b/lms/djangoapps/courseware/models.py
index 262d177248a5f94d3a529e8f04e96503428db2c1..f0bd8dc17e0df7a8f90be5bd78697013a50a050f 100644
--- a/lms/djangoapps/courseware/models.py
+++ b/lms/djangoapps/courseware/models.py
@@ -85,6 +85,7 @@ class StudentModuleCache(object):
student=user,
module_state_key__in=id_chunk)
)
+
else:
self.cache = []
@@ -93,7 +94,7 @@ class StudentModuleCache(object):
Get a list of the state_keys needed for StudentModules
required for this chunk of module xml
'''
- keys = [descriptor.url]
+ keys = [descriptor.location.url()]
shared_state_key = getattr(descriptor, 'shared_state_key', None)
if shared_state_key is not None:
diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py
index 2d47a552484542f944f5f3c513066fcacc37552f..5119cc291025efe658fccba39918428a4aad2bb7 100644
--- a/lms/djangoapps/courseware/module_render.py
+++ b/lms/djangoapps/courseware/module_render.py
@@ -207,7 +207,7 @@ def get_module(user, request, location, student_module_cache, position=None):
'''
descriptor = keystore().get_item(location)
- instance_module = student_module_cache.lookup(descriptor.category, descriptor.url)
+ instance_module = student_module_cache.lookup(descriptor.category, descriptor.location.url())
shared_state_key = getattr(descriptor, 'shared_state_key', None)
if shared_state_key is not None:
shared_module = student_module_cache.lookup(descriptor.category, shared_state_key)
@@ -218,7 +218,7 @@ def get_module(user, request, location, student_module_cache, position=None):
shared_state = shared_module.state if shared_module is not None else None
# Setup system context for module instance
- ajax_url = settings.MITX_ROOT_URL + '/modx/' + descriptor.url + '/'
+ ajax_url = settings.MITX_ROOT_URL + '/modx/' + descriptor.location.url() + '/'
def _get_module(location):
(module, _, _, _) = get_module(user, request, location, student_module_cache, position)
diff --git a/lms/djangoapps/courseware/views.py b/lms/djangoapps/courseware/views.py
index e1e1c16632b68adc5c4a5f6b48adb1a21f6d9282..48e9bcc7957a852b4bcba199af79c488a2f5cb8a 100644
--- a/lms/djangoapps/courseware/views.py
+++ b/lms/djangoapps/courseware/views.py
@@ -209,7 +209,7 @@ def index(request, course=None, chapter=None, section=None,
course_location = multicourse_settings.get_course_location(course)
section = get_section(course_location, chapter, section)
student_module_cache = StudentModuleCache(request.user, section)
- module, _, _, _ = get_module(request.user, request, section.url, student_module_cache)
+ module, _, _, _ = get_module(request.user, request, section.location, student_module_cache)
context['content'] = module.get_html()
result = render_to_response('courseware.html', context)