From 7bc697a1f9ed64f7ab00d93c32ce3981b4dae955 Mon Sep 17 00:00:00 2001 From: Don Mitchell Date: Thu, 17 Oct 2013 12:49:38 -0400 Subject: [PATCH] Parse urls in one regex w/ more consistency --- .../xmodule/xmodule/modulestore/locator.py | 81 ++++++++++++++----- .../xmodule/xmodule/modulestore/parsers.py | 67 ++++++--------- .../modulestore/tests/test_location_mapper.py | 34 +++++--- .../modulestore/tests/test_locators.py | 54 +++++++------ 4 files changed, 132 insertions(+), 104 deletions(-) diff --git a/common/lib/xmodule/xmodule/modulestore/locator.py b/common/lib/xmodule/xmodule/modulestore/locator.py index dffd182764..9194b9a400 100644 --- a/common/lib/xmodule/xmodule/modulestore/locator.py +++ b/common/lib/xmodule/xmodule/modulestore/locator.py @@ -13,7 +13,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 +from .parsers import BRANCH_PREFIX, BLOCK_PREFIX, VERSION_PREFIX import re from xmodule.modulestore import Location @@ -190,8 +190,8 @@ class CourseLocator(Locator): self.init_from_version_guid(version_guid) if course_id or branch: self.init_from_course_id(course_id, branch) - assert self.version_guid or self.course_id, \ - "Either version_guid or course_id should be set." + if self.version_guid is None and self.course_id is None: + raise ValueError("Either version_guid or course_id should be set: {}".format(url)) def __unicode__(self): """ @@ -200,10 +200,10 @@ class CourseLocator(Locator): if self.course_id: result = self.course_id if self.branch: - result += BRANCH_PREFIX + self.branch + result += '/' + BRANCH_PREFIX + self.branch return result elif self.version_guid: - return URL_VERSION_PREFIX + str(self.version_guid) + return VERSION_PREFIX + str(self.version_guid) else: # raise InsufficientSpecificationError("missing course_id or version_guid") return '' @@ -263,22 +263,46 @@ class CourseLocator(Locator): version_guid=self.version_guid, branch=self.branch) + def url_reverse(self, prefix, postfix): + """ + Do what reverse is supposed to do but seems unable to do. Generate a url using prefix unicode(self) postfix + :param prefix: the beginning of the url (should begin and end with / if non-empty) + :param postfix: the part to append to the url (should begin w/ / if non-empty) + """ + if prefix: + if not prefix.endswith('/'): + prefix += '/' + if not prefix.startswith('/'): + prefix = '/' + prefix + else: + prefix = '/' + if postfix and not postfix.startswith('/'): + postfix = '/' + postfix + return prefix + unicode(self) + postfix + + def reverse_kwargs(self): + """ + Get the kwargs list to supply to reverse (basically, a dict of the set properties) + """ + return {key: value for key, value in self.__dict__.iteritems() if value is not None} + def init_from_url(self, url): """ url must be a string beginning with 'edx://' and containing either a valid version_guid or course_id (with optional branch), or both. """ if isinstance(url, Locator): - url = url.url() - if not isinstance(url, basestring): + parse = url.__dict__ + elif not isinstance(url, basestring): raise TypeError('%s is not an instance of basestring' % url) - parse = parse_url(url, tag_optional=True) - if not parse: - raise ValueError('Could not parse "%s" as a url' % url) + else: + parse = parse_url(url, tag_optional=True) + if not parse: + raise ValueError('Could not parse "%s" as a url' % url) self._set_value( parse, 'version_guid', lambda (new_guid): self.set_version_guid(self.as_object_id(new_guid)) ) - self._set_value(parse, 'id', lambda (new_id): self.set_course_id(new_id)) + self._set_value(parse, 'course_id', lambda (new_id): self.set_course_id(new_id)) self._set_value(parse, 'branch', lambda (new_branch): self.set_branch(new_branch)) def init_from_version_guid(self, version_guid): @@ -313,9 +337,9 @@ class CourseLocator(Locator): raise ValueError("%s does not have a valid course_id" % course_id) parse = parse_course_id(course_id) - if not parse: + if not parse or parse['course_id'] is None: raise ValueError('Could not parse "%s" as a course_id' % course_id) - self.set_course_id(parse['id']) + self.set_course_id(parse['course_id']) rev = parse['branch'] if rev: self.set_branch(rev) @@ -396,11 +420,12 @@ class BlockUsageLocator(CourseLocator): self.init_block_ref_from_course_id(course_id) if usage_id: self.init_block_ref(usage_id) - CourseLocator.__init__(self, - url=url, - version_guid=version_guid, - course_id=course_id, - branch=branch) + super(BlockUsageLocator, self).__init__( + url=url, + version_guid=version_guid, + course_id=course_id, + branch=branch + ) def is_initialized(self): """ @@ -427,6 +452,16 @@ class BlockUsageLocator(CourseLocator): branch=self.branch, usage_id=self.usage_id) + def reverse_kwargs(self): + """ + Get the kwargs list to supply to reverse (basically, a dict of the set properties) + """ + result = super(BlockUsageLocator, self).reverse_kwargs() + if self.usage_id: + del result['usage_id'] + result['block'] = self.usage_id + return result + def set_usage_id(self, new): """ Initialize usage_id to new value. @@ -459,10 +494,12 @@ class BlockUsageLocator(CourseLocator): def init_block_ref_from_course_id(self, course_id): if isinstance(course_id, CourseLocator): + # FIXME the parsed course_id should never contain a block ref course_id = course_id.course_id assert course_id, "%s does not have a valid course_id" parse = parse_course_id(course_id) - assert parse, 'Could not parse "%s" as a course_id' % course_id + if parse is None: + raise ValueError('Could not parse "%s" as a course_id' % course_id) self._set_value(parse, 'block', lambda(new_block): self.set_usage_id(new_block)) def __unicode__(self): @@ -470,7 +507,7 @@ class BlockUsageLocator(CourseLocator): Return a string representing this location. """ rep = super(BlockUsageLocator, self).__unicode__() - return rep + BLOCK_PREFIX + unicode(self.usage_id) + return rep + '/' + BLOCK_PREFIX + unicode(self.usage_id) class DefinitionLocator(Locator): @@ -478,7 +515,7 @@ class DefinitionLocator(Locator): Container for how to locate a description (the course-independent content). """ - URL_RE = re.compile(r'^defx://' + URL_VERSION_PREFIX + '([^/]+)$', re.IGNORECASE) + URL_RE = re.compile(r'^defx://' + VERSION_PREFIX + '([^/]+)$', re.IGNORECASE) def __init__(self, definition_id): if isinstance(definition_id, basestring): regex_match = self.URL_RE.match(definition_id) @@ -491,7 +528,7 @@ class DefinitionLocator(Locator): Return a string representing this location. unicode(self) returns something like this: "version/519665f6223ebd6980884f2b" ''' - return URL_VERSION_PREFIX + str(self.definition_id) + return VERSION_PREFIX + str(self.definition_id) def url(self): """ diff --git a/common/lib/xmodule/xmodule/modulestore/parsers.py b/common/lib/xmodule/xmodule/modulestore/parsers.py index d0d7ffbe8c..64d3c25a51 100644 --- a/common/lib/xmodule/xmodule/modulestore/parsers.py +++ b/common/lib/xmodule/xmodule/modulestore/parsers.py @@ -1,17 +1,28 @@ import re # Prefix for the branch portion of a locator URL -BRANCH_PREFIX = "/branch/" +BRANCH_PREFIX = r"branch/" # Prefix for the block portion of a locator URL -BLOCK_PREFIX = "/block/" +BLOCK_PREFIX = r"block/" # Prefix for the version portion of a locator URL, when it is preceded by a course ID -VERSION_PREFIX = "/version/" -# Prefix for version when it begins the URL (no course ID). -URL_VERSION_PREFIX = 'version/' +VERSION_PREFIX = r"version/" -URL_RE = re.compile(r'^(edx://)?(.+)$', re.IGNORECASE) ALLOWED_ID_CHARS = r'[a-zA-Z0-9_\-~.]' +URL_RE_SOURCE = r""" + (?Pedx://)? + ((?P{ALLOWED_ID_CHARS}+)/?)? + ({BRANCH_PREFIX}(?P{ALLOWED_ID_CHARS}+)/?)? + ({VERSION_PREFIX}(?P[A-F0-9]+)/?)? + ({BLOCK_PREFIX}(?P{ALLOWED_ID_CHARS}+))? + """.format( + ALLOWED_ID_CHARS=ALLOWED_ID_CHARS, BRANCH_PREFIX=BRANCH_PREFIX, + VERSION_PREFIX=VERSION_PREFIX, BLOCK_PREFIX=BLOCK_PREFIX + ) + +URL_RE = re.compile('^' + URL_RE_SOURCE + '$', re.IGNORECASE | re.VERBOSE) + + def parse_url(string, tag_optional=False): """ A url usually begins with 'edx://' (case-insensitive match), @@ -21,9 +32,9 @@ def parse_url(string, tag_optional=False): Examples: 'edx://version/0123FFFF' 'edx://mit.eecs.6002x' - 'edx://mit.eecs.6002x;published' - 'edx://mit.eecs.6002x;published/block/HW3' - 'edx://mit.eecs.6002x;published/version/000eee12345/block/HW3' + 'edx://mit.eecs.6002x/branch/published' + 'edx://mit.eecs.6002x/branch/published/block/HW3' + 'edx://mit.eecs.6002x/branch/published/version/000eee12345/block/HW3' This returns None if string cannot be parsed. @@ -37,12 +48,10 @@ def parse_url(string, tag_optional=False): match = URL_RE.match(string) if not match: return None - if match.group(1) is None and not tag_optional: + matched_dict = match.groupdict() + if matched_dict['tag'] is None and not tag_optional: return None - path = match.group(2) - if path.startswith(URL_VERSION_PREFIX): - return parse_guid(path[len(URL_VERSION_PREFIX):]) - return parse_course_id(path) + return matched_dict BLOCK_RE = re.compile(r'^' + ALLOWED_ID_CHARS + r'+$', re.IGNORECASE) @@ -60,34 +69,6 @@ def parse_block_ref(string): return None -GUID_RE = re.compile( - r'^(?P[A-F0-9]+)(' + BLOCK_PREFIX + '(?P' + ALLOWED_ID_CHARS + r'+))?$', - re.IGNORECASE -) - - -def parse_guid(string): - """ - A version_guid is a string of hex digits (0-F). - - If string is a version_guid, returns a dict with key 'version_guid' and the value, - otherwise returns None. - """ - m = GUID_RE.match(string) - if m is not None: - return m.groupdict() - else: - return None - - -COURSE_ID_RE = re.compile( - r'^(?P' + ALLOWED_ID_CHARS + r'+)(' + - BRANCH_PREFIX + r'(?P' + ALLOWED_ID_CHARS + r'+))?(' + - VERSION_PREFIX + r'(?P[A-F0-9]+))?(' + - BLOCK_PREFIX + r'(?P' + ALLOWED_ID_CHARS + r'+))?$', re.IGNORECASE -) - - def parse_course_id(string): r""" @@ -122,7 +103,7 @@ def parse_course_id(string): Block is optional: if missing returned_dict['block'] is None. Else returns None. """ - match = COURSE_ID_RE.match(string) + match = URL_RE.match(string) if not match: return None return match.groupdict() diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_location_mapper.py b/common/lib/xmodule/xmodule/modulestore/tests/test_location_mapper.py index c0044cb5de..6c35887b94 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_location_mapper.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_location_mapper.py @@ -247,7 +247,8 @@ class TestLocationMapper(unittest.TestCase): new_style_course_id = '{}.geek_dept.{}.baz_run'.format(org, course) prob_locator = BlockUsageLocator( course_id=new_style_course_id, - usage_id='problem2' + usage_id='problem2', + branch='published' ) prob_location = loc_mapper().translate_locator_to_location(prob_locator) self.assertIsNone(prob_location, 'found entry in empty map table') @@ -265,7 +266,9 @@ class TestLocationMapper(unittest.TestCase): # default branch self.assertEqual(prob_location, Location('i4x', org, course, 'problem', 'abc123', None)) # explicit branch - prob_locator = BlockUsageLocator(prob_locator, branch='draft') + prob_locator = BlockUsageLocator( + course_id=prob_locator.course_id, branch='draft', usage_id=prob_locator.usage_id + ) prob_location = loc_mapper().translate_locator_to_location(prob_locator) self.assertEqual(prob_location, Location('i4x', org, course, 'problem', 'abc123', 'draft')) prob_locator = BlockUsageLocator( @@ -276,12 +279,13 @@ class TestLocationMapper(unittest.TestCase): # same for chapter except chapter cannot be draft in old system chap_locator = BlockUsageLocator( course_id=new_style_course_id, - usage_id='chapter48f' + usage_id='chapter48f', + branch='production' ) chap_location = loc_mapper().translate_locator_to_location(chap_locator) self.assertEqual(chap_location, Location('i4x', org, course, 'chapter', '48f23a10395384929234')) # explicit branch - chap_locator = BlockUsageLocator(chap_locator, branch='draft') + chap_locator.branch = 'draft' chap_location = loc_mapper().translate_locator_to_location(chap_locator) self.assertEqual(chap_location, Location('i4x', org, course, 'chapter', '48f23a10395384929234')) chap_locator = BlockUsageLocator( @@ -382,7 +386,7 @@ class TestLocationMapper(unittest.TestCase): self.assertEqual(new_usage_id, new_usage_id2) # it should be in the distractor now new_location = loc_mapper().translate_locator_to_location( - BlockUsageLocator(course_id=new_style_course_id, usage_id=new_usage_id2) + BlockUsageLocator(course_id=new_style_course_id, usage_id=new_usage_id2, branch='published') ) self.assertEqual(new_location, location) # add one close to the existing chapter (cause name collision) @@ -391,11 +395,15 @@ class TestLocationMapper(unittest.TestCase): self.assertRegexpMatches(new_usage_id, r'^chapter48f\d') # retrievable from both courses new_location = loc_mapper().translate_locator_to_location( - BlockUsageLocator(course_id=new_style_course_id, usage_id=new_usage_id) + BlockUsageLocator(course_id=new_style_course_id, usage_id=new_usage_id, branch='published') ) self.assertEqual(new_location, location) new_location = loc_mapper().translate_locator_to_location( - BlockUsageLocator(course_id='{}.{}.{}'.format(org, course, 'baz_run'), usage_id=new_usage_id) + BlockUsageLocator( + course_id='{}.{}.{}'.format(org, course, 'baz_run'), + usage_id=new_usage_id, + branch='published' + ) ) self.assertEqual(new_location, location) @@ -443,11 +451,11 @@ class TestLocationMapper(unittest.TestCase): # change in all courses to same value loc_mapper().update_block_location_translator(location, 'problem1') trans_loc = loc_mapper().translate_locator_to_location( - BlockUsageLocator(course_id=new_style_course_id, usage_id='problem1') + BlockUsageLocator(course_id=new_style_course_id, usage_id='problem1', branch='published') ) self.assertEqual(trans_loc, location) trans_loc = loc_mapper().translate_locator_to_location( - BlockUsageLocator(course_id=new_style_course_id2, usage_id='problem1') + BlockUsageLocator(course_id=new_style_course_id2, usage_id='problem1', branch='published') ) self.assertEqual(trans_loc, location) # try to change to overwrite used usage_id @@ -457,12 +465,12 @@ class TestLocationMapper(unittest.TestCase): # just change the one course loc_mapper().update_block_location_translator(location, 'chapter2', '{}/{}/{}'.format(org, course, 'baz_run')) trans_loc = loc_mapper().translate_locator_to_location( - BlockUsageLocator(course_id=new_style_course_id, usage_id='chapter2') + BlockUsageLocator(course_id=new_style_course_id, usage_id='chapter2', branch='published') ) self.assertEqual(trans_loc.name, '48f23a10395384929234') # but this still points to the old trans_loc = loc_mapper().translate_locator_to_location( - BlockUsageLocator(course_id=new_style_course_id2, usage_id='chapter2') + BlockUsageLocator(course_id=new_style_course_id2, usage_id='chapter2', branch='published') ) self.assertEqual(trans_loc.name, '1') @@ -496,10 +504,10 @@ class TestLocationMapper(unittest.TestCase): # delete from all courses loc_mapper().delete_block_location_translator(location) self.assertIsNone(loc_mapper().translate_locator_to_location( - BlockUsageLocator(course_id=new_style_course_id, usage_id='problem1') + BlockUsageLocator(course_id=new_style_course_id, usage_id='problem1', branch='published') )) self.assertIsNone(loc_mapper().translate_locator_to_location( - BlockUsageLocator(course_id=new_style_course_id2, usage_id='problem2') + BlockUsageLocator(course_id=new_style_course_id2, usage_id='problem2', branch='published') )) # delete from one course location = location.replace(name='abc123') diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_locators.py b/common/lib/xmodule/xmodule/modulestore/tests/test_locators.py index 2db00f279d..f60af2b210 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_locators.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_locators.py @@ -5,7 +5,7 @@ from unittest import TestCase from bson.objectid import ObjectId from xmodule.modulestore.locator import Locator, CourseLocator, BlockUsageLocator, DefinitionLocator -from xmodule.modulestore.parsers import BRANCH_PREFIX, BLOCK_PREFIX, VERSION_PREFIX, URL_VERSION_PREFIX +from xmodule.modulestore.parsers import BRANCH_PREFIX, BLOCK_PREFIX, VERSION_PREFIX from xmodule.modulestore.exceptions import InsufficientSpecificationError, OverSpecificationError from xmodule.modulestore import Location import random @@ -36,12 +36,12 @@ class LocatorTest(TestCase): self.assertRaises( OverSpecificationError, CourseLocator, - url='edx://mit.eecs.6002x' + BRANCH_PREFIX + 'published', + url='edx://mit.eecs.6002x/' + BRANCH_PREFIX + 'published', branch='draft') self.assertRaises( OverSpecificationError, CourseLocator, - course_id='mit.eecs.6002x' + BRANCH_PREFIX + 'published', + course_id='mit.eecs.6002x/' + BRANCH_PREFIX + 'published', branch='draft') def test_course_constructor_underspecified(self): @@ -59,8 +59,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), URL_VERSION_PREFIX + test_id_1_loc) - self.assertEqual(testobj_1.url(), 'edx://' + URL_VERSION_PREFIX + test_id_1_loc) + self.assertEqual(str(testobj_1), VERSION_PREFIX + test_id_1_loc) + self.assertEqual(testobj_1.url(), 'edx://' + VERSION_PREFIX + test_id_1_loc) # Test using a given string test_id_2_loc = '519665f6223ebd6980884f2b' @@ -68,8 +68,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), URL_VERSION_PREFIX + test_id_2_loc) - self.assertEqual(testobj_2.url(), 'edx://' + URL_VERSION_PREFIX + test_id_2_loc) + self.assertEqual(str(testobj_2), VERSION_PREFIX + test_id_2_loc) + self.assertEqual(testobj_2.url(), 'edx://' + VERSION_PREFIX + test_id_2_loc) def test_course_constructor_bad_course_id(self): """ @@ -77,19 +77,19 @@ class LocatorTest(TestCase): """ for bad_id in (' mit.eecs', 'mit.eecs ', - URL_VERSION_PREFIX + 'mit.eecs', - BLOCK_PREFIX + 'block/mit.eecs', + VERSION_PREFIX + 'mit.eecs', + BLOCK_PREFIX + 'black/mit.eecs', 'mit.ee cs', 'mit.ee,cs', 'mit.ee/cs', 'mit.ee&cs', 'mit.ee()cs', 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 ', + '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(ValueError, CourseLocator, course_id=bad_id) self.assertRaises(ValueError, CourseLocator, url='edx://' + bad_id) @@ -107,7 +107,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' + BRANCH_PREFIX + 'published' + testurn = 'mit.eecs.6002x/' + BRANCH_PREFIX + 'published' expected_urn = 'mit.eecs.6002x' expected_rev = 'published' testobj = CourseLocator(course_id=testurn, url='edx://' + testurn) @@ -119,7 +119,7 @@ class LocatorTest(TestCase): # 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") + testobj = CourseLocator(url="edx://{}{}/{}hw3".format(VERSION_PREFIX, test_id_loc, BLOCK_PREFIX)) self.check_course_locn_fields( testobj, 'test_block constructor', @@ -128,14 +128,14 @@ class LocatorTest(TestCase): def test_course_constructor_url_course_id_and_version_guid(self): test_id_loc = '519665f6223ebd6980884f2b' - testobj = CourseLocator(url='edx://mit.eecs-honors.6002x' + VERSION_PREFIX + test_id_loc) + testobj = CourseLocator(url='edx://mit.eecs-honors.6002x/' + VERSION_PREFIX + test_id_loc) self.check_course_locn_fields(testobj, 'error parsing url with both course ID and version GUID', course_id='mit.eecs-honors.6002x', version_guid=ObjectId(test_id_loc)) def test_course_constructor_url_course_id_branch_and_version_guid(self): test_id_loc = '519665f6223ebd6980884f2b' - testobj = CourseLocator(url='edx://mit.eecs.~6002x' + BRANCH_PREFIX + 'draft-1' + VERSION_PREFIX + test_id_loc) + testobj = CourseLocator(url='edx://mit.eecs.~6002x/' + BRANCH_PREFIX + 'draft-1/' + VERSION_PREFIX + test_id_loc) self.check_course_locn_fields(testobj, 'error parsing url with both course ID branch, and version GUID', course_id='mit.eecs.~6002x', branch='draft-1', @@ -150,7 +150,7 @@ class LocatorTest(TestCase): self.assertEqual(testobj.url(), 'edx://' + testurn) def test_course_constructor_course_id_with_branch(self): - testurn = 'mit.eecs.6002x' + BRANCH_PREFIX + 'published' + testurn = 'mit.eecs.6002x/' + BRANCH_PREFIX + 'published' expected_id = 'mit.eecs.6002x' expected_branch = 'published' testobj = CourseLocator(course_id=testurn) @@ -166,7 +166,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' + BRANCH_PREFIX + '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, @@ -181,7 +181,7 @@ class LocatorTest(TestCase): """ The same branch appears in the course_id and the branch field. """ - test_id = 'mit.eecs.6002x' + BRANCH_PREFIX + 'published' + test_id = 'mit.eecs.6002x/' + BRANCH_PREFIX + 'published' test_branch = 'published' expected_id = 'mit.eecs.6002x' expected_urn = test_id @@ -196,7 +196,7 @@ class LocatorTest(TestCase): self.assertEqual(testobj.url(), 'edx://' + expected_urn) def test_block_constructor(self): - testurn = 'mit.eecs.6002x' + BRANCH_PREFIX + 'published' + BLOCK_PREFIX + 'HW3' + testurn = 'mit.eecs.6002x/' + BRANCH_PREFIX + 'published/' + BLOCK_PREFIX + 'HW3' expected_id = 'mit.eecs.6002x' expected_branch = 'published' expected_block_ref = 'HW3' @@ -217,7 +217,7 @@ class LocatorTest(TestCase): def test_block_constructor_url_version_prefix(self): test_id_loc = '519665f6223ebd6980884f2b' testobj = BlockUsageLocator( - url='edx://mit.eecs.6002x' + VERSION_PREFIX + test_id_loc + BLOCK_PREFIX + 'lab2' + url='edx://mit.eecs.6002x/{}{}/{}lab2'.format(VERSION_PREFIX, test_id_loc, BLOCK_PREFIX) ) self.check_block_locn_fields( testobj, 'error parsing URL with version and block', @@ -237,7 +237,9 @@ class LocatorTest(TestCase): def test_block_constructor_url_kitchen_sink(self): test_id_loc = '519665f6223ebd6980884f2b' testobj = BlockUsageLocator( - url='edx://mit.eecs.6002x' + BRANCH_PREFIX + 'draft' + VERSION_PREFIX + test_id_loc + BLOCK_PREFIX + 'lab2' + url='edx://mit.eecs.6002x/{}draft/{}{}/{}lab2'.format( + BRANCH_PREFIX, VERSION_PREFIX, test_id_loc, BLOCK_PREFIX + ) ) self.check_block_locn_fields( testobj, 'error parsing URL with branch, version, and block', @@ -248,7 +250,7 @@ class LocatorTest(TestCase): ) def test_repr(self): - testurn = 'mit.eecs.6002x' + BRANCH_PREFIX + 'published' + BLOCK_PREFIX + 'HW3' + 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)) @@ -286,7 +288,7 @@ class LocatorTest(TestCase): def test_description_locator_url(self): object_id = '{:024x}'.format(random.randrange(16 ** 24)) definition_locator = DefinitionLocator(object_id) - self.assertEqual('defx://' + URL_VERSION_PREFIX + object_id, definition_locator.url()) + self.assertEqual('defx://' + VERSION_PREFIX + object_id, definition_locator.url()) self.assertEqual(definition_locator, DefinitionLocator(definition_locator.url())) def test_description_locator_version(self):