diff --git a/common/lib/xmodule/xmodule/modulestore/locator.py b/common/lib/xmodule/xmodule/modulestore/locator.py index 591ef3115f..3e20f3e1b4 100644 --- a/common/lib/xmodule/xmodule/modulestore/locator.py +++ b/common/lib/xmodule/xmodule/modulestore/locator.py @@ -1,8 +1,7 @@ """ -Created on Mar 13, 2013 +Identifier for course resources. +""" -@author: dmitchell -""" from __future__ import absolute_import import logging import inspect @@ -15,6 +14,7 @@ from bson.errors import InvalidId from xmodule.modulestore.exceptions import InsufficientSpecificationError, OverSpecificationError from .parsers import parse_url, parse_course_id, parse_block_ref +from .parsers import BRANCH_PREFIX, BLOCK_PREFIX, URL_VERSION_PREFIX log = logging.getLogger(__name__) @@ -37,9 +37,6 @@ class Locator(object): """ raise InsufficientSpecificationError() - def quoted_url(self): - return quote(self.url(), '@;#') - def __eq__(self, other): return self.__dict__ == other.__dict__ @@ -90,11 +87,11 @@ class CourseLocator(Locator): Examples of valid CourseLocator specifications: CourseLocator(version_guid=ObjectId('519665f6223ebd6980884f2b')) CourseLocator(course_id='mit.eecs.6002x') - CourseLocator(course_id='mit.eecs.6002x;published') + CourseLocator(course_id='mit.eecs.6002x/branch/published') CourseLocator(course_id='mit.eecs.6002x', branch='published') - CourseLocator(url='edx://@519665f6223ebd6980884f2b') + CourseLocator(url='edx://version/519665f6223ebd6980884f2b') CourseLocator(url='edx://mit.eecs.6002x') - CourseLocator(url='edx://mit.eecs.6002x;published') + CourseLocator(url='edx://mit.eecs.6002x/branch/published') Should have at lease a specific course_id (id for the course as if it were a project w/ versions) with optional 'branch', @@ -115,10 +112,10 @@ class CourseLocator(Locator): if self.course_id: result = self.course_id if self.branch: - result += ';' + self.branch + result += BRANCH_PREFIX + self.branch return result elif self.version_guid: - return '@' + str(self.version_guid) + return URL_VERSION_PREFIX + str(self.version_guid) else: # raise InsufficientSpecificationError("missing course_id or version_guid") return '' @@ -224,7 +221,7 @@ class CourseLocator(Locator): """ url must be a string beginning with 'edx://' and containing either a valid version_guid or course_id (with optional branch) - If a block ('#HW3') is present, it is ignored. + If a block ('/block/HW3') is present, it is ignored. """ if isinstance(url, Locator): url = url.url() @@ -253,14 +250,14 @@ class CourseLocator(Locator): def init_from_course_id(self, course_id, explicit_branch=None): """ - Course_id is a string like 'mit.eecs.6002x' or 'mit.eecs.6002x;published'. + Course_id is a string like 'mit.eecs.6002x' or 'mit.eecs.6002x/branch/published'. Revision (optional) is a string like 'published'. It may be provided explicitly (explicit_branch) or embedded into course_id. - If branch is part of course_id ("...;published"), parse it out separately. + If branch is part of course_id (".../branch/published"), parse it out separately. If branch is provided both ways, that's ok as long as they are the same value. - If a block ('#HW3') is a part of course_id, it is ignored. + If a block ('/block/HW3') is a part of course_id, it is ignored. """ @@ -411,9 +408,9 @@ class BlockUsageLocator(CourseLocator): rep = CourseLocator.__unicode__(self) if self.usage_id is None: # usage_id has not been initialized - return rep + '#NONE' + return rep + BLOCK_PREFIX + 'NONE' else: - return rep + '#' + self.usage_id + return rep + BLOCK_PREFIX + self.usage_id class DescriptionLocator(Locator): @@ -427,14 +424,14 @@ class DescriptionLocator(Locator): def __unicode__(self): ''' Return a string representing this location. - unicode(self) returns something like this: "@519665f6223ebd6980884f2b" + unicode(self) returns something like this: "version/519665f6223ebd6980884f2b" ''' - return '@' + str(self.definition_guid) + return URL_VERSION_PREFIX + str(self.definition_id) def url(self): """ Return a string containing the URL for this location. - url(self) returns something like this: 'edx://@519665f6223ebd6980884f2b' + url(self) returns something like this: 'edx://version/519665f6223ebd6980884f2b' """ return 'edx://' + unicode(self) @@ -442,7 +439,7 @@ class DescriptionLocator(Locator): """ Returns the ObjectId referencing this specific location. """ - return self.definition_guid + return self.definition_id class VersionTree(object): diff --git a/common/lib/xmodule/xmodule/modulestore/parsers.py b/common/lib/xmodule/xmodule/modulestore/parsers.py index 8e5b685cec..efdf1c9e18 100644 --- a/common/lib/xmodule/xmodule/modulestore/parsers.py +++ b/common/lib/xmodule/xmodule/modulestore/parsers.py @@ -1,5 +1,12 @@ import re +# Prefix for the branch portion of a locator URL +BRANCH_PREFIX = "/branch/" +# Prefix for the block portion of a locator URL +BLOCK_PREFIX = "/block/" +# Prefix for when a course URL begins with a version ID +URL_VERSION_PREFIX = 'version/' + URL_RE = re.compile(r'^edx://(.+)$', re.IGNORECASE) @@ -9,10 +16,10 @@ def parse_url(string): followed by either a version_guid or a course_id. Examples: - 'edx://@0123FFFF' + 'edx://version/0123FFFF' 'edx://edu.mit.eecs.6002x' - 'edx://edu.mit.eecs.6002x;published' - 'edx://edu.mit.eecs.6002x;published#HW3' + 'edx://edu.mit.eecs.6002x/branch/published' + 'edx://edu.mit.eecs.6002x/branch/published/block/HW3' This returns None if string cannot be parsed. @@ -27,8 +34,8 @@ def parse_url(string): if not match: return None path = match.group(1) - if path[0] == '@': - return parse_guid(path[1:]) + if path.startswith(URL_VERSION_PREFIX): + return parse_guid(path[len(URL_VERSION_PREFIX):]) return parse_course_id(path) @@ -52,8 +59,7 @@ def parse_block_ref(string): return None -GUID_RE = re.compile(r'^(?P[A-F0-9]+)(#(?P\w+))?$', re.IGNORECASE) - +GUID_RE = re.compile(r'^(?P[A-F0-9]+)(' + BLOCK_PREFIX + '(?P\w+))?$', re.IGNORECASE) def parse_guid(string): """ @@ -69,27 +75,27 @@ def parse_guid(string): return None -COURSE_ID_RE = re.compile(r'^(?P(\w+)(\.\w+\w*)*)(;(?P\w+))?(#(?P\w+))?$', re.IGNORECASE) +COURSE_ID_RE = re.compile(r'^(?P(\w+)(\.\w+\w*)*)('+ BRANCH_PREFIX + '(?P\w+))?(' + BLOCK_PREFIX + '(?P\w+))?$', re.IGNORECASE) def parse_course_id(string): r""" A course_id has a main id component. - There may also be an optional branch (;published or ;draft). - There may also be an optional block (#HW3 or #Quiz2). + There may also be an optional branch (/branch/published or /branch/draft). + There may also be an optional block (/block/HW3 or /block/Quiz2). Examples of valid course_ids: 'edu.mit.eecs.6002x' - 'edu.mit.eecs.6002x;published' - 'edu.mit.eecs.6002x#HW3' - 'edu.mit.eecs.6002x;published#HW3' + 'edu.mit.eecs.6002x/branch/published' + 'edu.mit.eecs.6002x/block/HW3' + 'edu.mit.eecs.6002x/branch/published/block/HW3' Syntax: - course_id = main_id [; branch] [# block] + course_id = main_id [/branch/ branch] [/block/ block] main_id = name [. name]* diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_locators.py b/common/lib/xmodule/xmodule/modulestore/tests/test_locators.py index bb41131234..0f39a4c66f 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_locators.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_locators.py @@ -1,12 +1,11 @@ -''' -Created on Mar 14, 2013 - -@author: dmitchell -''' +""" +Tests for xmodule.modulestore.locator. +""" from unittest import TestCase from bson.objectid import ObjectId -from xmodule.modulestore.locator import Locator, CourseLocator, BlockUsageLocator +from xmodule.modulestore.locator import Locator, CourseLocator, BlockUsageLocator, DescriptionLocator +from xmodule.modulestore.parsers import BRANCH_PREFIX, BLOCK_PREFIX, URL_VERSION_PREFIX from xmodule.modulestore.exceptions import InsufficientSpecificationError, OverSpecificationError @@ -32,12 +31,12 @@ class LocatorTest(TestCase): self.assertRaises( OverSpecificationError, CourseLocator, - url='edx://mit.eecs.6002x;published', + url='edx://mit.eecs.6002x' + BRANCH_PREFIX + 'published', branch='draft') self.assertRaises( OverSpecificationError, CourseLocator, - course_id='mit.eecs.6002x;published', + course_id='mit.eecs.6002x' + BRANCH_PREFIX + 'published', branch='draft') def test_course_constructor_underspecified(self): @@ -55,8 +54,8 @@ class LocatorTest(TestCase): testobj_1 = CourseLocator(version_guid=test_id_1) self.check_course_locn_fields(testobj_1, 'version_guid', version_guid=test_id_1) self.assertEqual(str(testobj_1.version_guid), test_id_1_loc) - self.assertEqual(str(testobj_1), '@' + test_id_1_loc) - self.assertEqual(testobj_1.url(), 'edx://@' + test_id_1_loc) + self.assertEqual(str(testobj_1), URL_VERSION_PREFIX + test_id_1_loc) + self.assertEqual(testobj_1.url(), 'edx://' + URL_VERSION_PREFIX + test_id_1_loc) # Test using a given string test_id_2_loc = '519665f6223ebd6980884f2b' @@ -64,8 +63,8 @@ class LocatorTest(TestCase): testobj_2 = CourseLocator(version_guid=test_id_2) self.check_course_locn_fields(testobj_2, 'version_guid', version_guid=test_id_2) self.assertEqual(str(testobj_2.version_guid), test_id_2_loc) - self.assertEqual(str(testobj_2), '@' + test_id_2_loc) - self.assertEqual(testobj_2.url(), 'edx://@' + test_id_2_loc) + self.assertEqual(str(testobj_2), URL_VERSION_PREFIX + test_id_2_loc) + self.assertEqual(testobj_2.url(), 'edx://'+ URL_VERSION_PREFIX + test_id_2_loc) def test_course_constructor_bad_course_id(self): """ @@ -74,20 +73,20 @@ class LocatorTest(TestCase): for bad_id in ('mit.', ' mit.eecs', 'mit.eecs ', - '@mit.eecs', - '#mit.eecs', + URL_VERSION_PREFIX + 'mit.eecs', + BLOCK_PREFIX + 'block/mit.eecs', 'mit.ee cs', 'mit.ee,cs', 'mit.ee/cs', 'mit.ee$cs', 'mit.ee&cs', 'mit.ee()cs', - ';this', - 'mit.eecs;', - 'mit.eecs;this;that', - 'mit.eecs;this;', - 'mit.eecs;this ', - 'mit.eecs;th%is ', + BRANCH_PREFIX + 'this', + 'mit.eecs' + BRANCH_PREFIX, + 'mit.eecs' + BRANCH_PREFIX + 'this' + BRANCH_PREFIX +'that', + 'mit.eecs' + BRANCH_PREFIX + 'this' + BRANCH_PREFIX , + 'mit.eecs' + BRANCH_PREFIX + 'this ', + 'mit.eecs' + BRANCH_PREFIX + 'th%is ', ): self.assertRaises(AssertionError, CourseLocator, course_id=bad_id) self.assertRaises(AssertionError, CourseLocator, url='edx://' + bad_id) @@ -106,7 +105,7 @@ class LocatorTest(TestCase): self.check_course_locn_fields(testobj, 'course_id', course_id=testurn) def test_course_constructor_redundant_002(self): - testurn = 'mit.eecs.6002x;published' + testurn = 'mit.eecs.6002x' + BRANCH_PREFIX + 'published' expected_urn = 'mit.eecs.6002x' expected_rev = 'published' testobj = CourseLocator(course_id=testurn, url='edx://' + testurn) @@ -114,6 +113,17 @@ class LocatorTest(TestCase): course_id=expected_urn, branch=expected_rev) + def test_course_constructor_url(self): + # Test parsing a url when it starts with a version ID and there is also a block ID. + # This hits the parsers parse_guid method. + test_id_loc = '519665f6223ebd6980884f2b' + testobj = CourseLocator(url="edx://" + URL_VERSION_PREFIX + test_id_loc + BLOCK_PREFIX + "hw3") + self.check_course_locn_fields( + testobj, + 'test_block constructor', + version_guid=ObjectId(test_id_loc) + ) + def test_course_constructor_course_id_no_branch(self): testurn = 'mit.eecs.6002x' testobj = CourseLocator(course_id=testurn) @@ -123,7 +133,7 @@ class LocatorTest(TestCase): self.assertEqual(testobj.url(), 'edx://' + testurn) def test_course_constructor_course_id_with_branch(self): - testurn = 'mit.eecs.6002x;published' + testurn = 'mit.eecs.6002x' + BRANCH_PREFIX + 'published' expected_id = 'mit.eecs.6002x' expected_branch = 'published' testobj = CourseLocator(course_id=testurn) @@ -139,7 +149,7 @@ class LocatorTest(TestCase): def test_course_constructor_course_id_separate_branch(self): test_id = 'mit.eecs.6002x' test_branch = 'published' - expected_urn = 'mit.eecs.6002x;published' + expected_urn = 'mit.eecs.6002x' + BRANCH_PREFIX + 'published' testobj = CourseLocator(course_id=test_id, branch=test_branch) self.check_course_locn_fields(testobj, 'course_id with separate branch', course_id=test_id, @@ -154,10 +164,10 @@ class LocatorTest(TestCase): """ The same branch appears in the course_id and the branch field. """ - test_id = 'mit.eecs.6002x;published' + test_id = 'mit.eecs.6002x' + BRANCH_PREFIX + 'published' test_branch = 'published' expected_id = 'mit.eecs.6002x' - expected_urn = 'mit.eecs.6002x;published' + expected_urn = test_id testobj = CourseLocator(course_id=test_id, branch=test_branch) self.check_course_locn_fields(testobj, 'course_id with repeated branch', course_id=expected_id, @@ -169,7 +179,7 @@ class LocatorTest(TestCase): self.assertEqual(testobj.url(), 'edx://' + expected_urn) def test_block_constructor(self): - testurn = 'mit.eecs.6002x;published#HW3' + testurn = 'mit.eecs.6002x' + BRANCH_PREFIX + 'published' + BLOCK_PREFIX + 'HW3' expected_id = 'mit.eecs.6002x' expected_branch = 'published' expected_block_ref = 'HW3' @@ -181,6 +191,18 @@ class LocatorTest(TestCase): self.assertEqual(str(testobj), testurn) self.assertEqual(testobj.url(), 'edx://' + testurn) + def test_repr(self): + testurn = 'mit.eecs.6002x' + BRANCH_PREFIX + 'published' + BLOCK_PREFIX + 'HW3' + testobj = BlockUsageLocator(course_id=testurn) + self.assertEqual('BlockUsageLocator("mit.eecs.6002x/branch/published/block/HW3")', repr(testobj)) + + def test_description_locator_url(self): + definition_locator=DescriptionLocator("chapter12345_2") + self.assertEqual('edx://' + URL_VERSION_PREFIX + 'chapter12345_2', definition_locator.url()) + + def test_description_locator_version(self): + definition_locator=DescriptionLocator("chapter12345_2") + self.assertEqual("chapter12345_2", definition_locator.version()) # ------------------------------------------------------------------ # Utilities