Commit b7765902 by Don Mitchell

Rename BlockUsageLocator's block_id field from usage_id

usage_id implies self-sufficiency. The block_id is relative to its structure.
parent 3cf75198
...@@ -64,7 +64,7 @@ class TemplateTests(unittest.TestCase): ...@@ -64,7 +64,7 @@ class TemplateTests(unittest.TestCase):
self.assertIsInstance(test_chapter, SequenceDescriptor) self.assertIsInstance(test_chapter, SequenceDescriptor)
# refetch parent which should now point to child # refetch parent which should now point to child
test_course = modulestore('split').get_course(test_chapter.location) test_course = modulestore('split').get_course(test_chapter.location)
self.assertIn(test_chapter.location.usage_id, test_course.children) self.assertIn(test_chapter.location.block_id, test_course.children)
def test_temporary_xblocks(self): def test_temporary_xblocks(self):
""" """
...@@ -166,7 +166,7 @@ class TemplateTests(unittest.TestCase): ...@@ -166,7 +166,7 @@ class TemplateTests(unittest.TestCase):
second_problem = persistent_factories.ItemFactory.create( second_problem = persistent_factories.ItemFactory.create(
display_name='problem 2', display_name='problem 2',
parent_location=BlockUsageLocator(updated_loc, usage_id=sub.location.usage_id), parent_location=BlockUsageLocator(updated_loc, block_id=sub.location.block_id),
user_id='testbot', category='problem', user_id='testbot', category='problem',
data="<problem></problem>" data="<problem></problem>"
) )
......
...@@ -51,7 +51,7 @@ def assets_handler(request, tag=None, course_id=None, branch=None, version_guid= ...@@ -51,7 +51,7 @@ def assets_handler(request, tag=None, course_id=None, branch=None, version_guid=
DELETE DELETE
json: delete an asset json: delete an asset
""" """
location = BlockUsageLocator(course_id=course_id, branch=branch, version_guid=version_guid, usage_id=block) location = BlockUsageLocator(course_id=course_id, branch=branch, version_guid=version_guid, block_id=block)
if not has_access(request.user, location): if not has_access(request.user, location):
raise PermissionDenied() raise PermissionDenied()
......
...@@ -36,7 +36,7 @@ def checklists_handler(request, tag=None, course_id=None, branch=None, version_g ...@@ -36,7 +36,7 @@ def checklists_handler(request, tag=None, course_id=None, branch=None, version_g
POST or PUT POST or PUT
json: updates the checked state for items within a particular checklist. checklist_index is required. json: updates the checked state for items within a particular checklist. checklist_index is required.
""" """
location = BlockUsageLocator(course_id=course_id, branch=branch, version_guid=version_guid, usage_id=block) location = BlockUsageLocator(course_id=course_id, branch=branch, version_guid=version_guid, block_id=block)
if not has_access(request.user, location): if not has_access(request.user, location):
raise PermissionDenied() raise PermissionDenied()
......
...@@ -62,7 +62,7 @@ def subsection_handler(request, tag=None, course_id=None, branch=None, version_g ...@@ -62,7 +62,7 @@ def subsection_handler(request, tag=None, course_id=None, branch=None, version_g
json: not currently supported json: not currently supported
""" """
if 'text/html' in request.META.get('HTTP_ACCEPT', 'text/html'): if 'text/html' in request.META.get('HTTP_ACCEPT', 'text/html'):
locator = BlockUsageLocator(course_id=course_id, branch=branch, version_guid=version_guid, usage_id=block) locator = BlockUsageLocator(course_id=course_id, branch=branch, version_guid=version_guid, block_id=block)
try: try:
old_location, course, item, lms_link = _get_item_in_course(request, locator) old_location, course, item, lms_link = _get_item_in_course(request, locator)
except ItemNotFoundError: except ItemNotFoundError:
...@@ -151,7 +151,7 @@ def unit_handler(request, tag=None, course_id=None, branch=None, version_guid=No ...@@ -151,7 +151,7 @@ def unit_handler(request, tag=None, course_id=None, branch=None, version_guid=No
json: not currently supported json: not currently supported
""" """
if 'text/html' in request.META.get('HTTP_ACCEPT', 'text/html'): if 'text/html' in request.META.get('HTTP_ACCEPT', 'text/html'):
locator = BlockUsageLocator(course_id=course_id, branch=branch, version_guid=version_guid, usage_id=block) locator = BlockUsageLocator(course_id=course_id, branch=branch, version_guid=version_guid, block_id=block)
try: try:
old_location, course, item, lms_link = _get_item_in_course(request, locator) old_location, course, item, lms_link = _get_item_in_course(request, locator)
except ItemNotFoundError: except ItemNotFoundError:
......
...@@ -11,7 +11,7 @@ from django.utils.translation import ugettext as _ ...@@ -11,7 +11,7 @@ from django.utils.translation import ugettext as _
from django.contrib.auth.decorators import login_required from django.contrib.auth.decorators import login_required
from django_future.csrf import ensure_csrf_cookie from django_future.csrf import ensure_csrf_cookie
from django.conf import settings from django.conf import settings
from django.views.decorators.http import require_http_methods, require_POST from django.views.decorators.http import require_http_methods
from django.core.exceptions import PermissionDenied from django.core.exceptions import PermissionDenied
from django.core.urlresolvers import reverse from django.core.urlresolvers import reverse
from django.http import HttpResponseBadRequest, HttpResponseNotFound from django.http import HttpResponseBadRequest, HttpResponseNotFound
...@@ -60,12 +60,12 @@ __all__ = ['course_info_handler', 'course_handler', 'course_info_update_handler' ...@@ -60,12 +60,12 @@ __all__ = ['course_info_handler', 'course_handler', 'course_info_update_handler'
'textbooks_list_handler', 'textbooks_detail_handler'] 'textbooks_list_handler', 'textbooks_detail_handler']
def _get_locator_and_course(course_id, branch, version_guid, usage_id, user, depth=0): def _get_locator_and_course(course_id, branch, version_guid, block_id, user, depth=0):
""" """
Internal method used to calculate and return the locator and course module Internal method used to calculate and return the locator and course module
for the view functions in this file. for the view functions in this file.
""" """
locator = BlockUsageLocator(course_id=course_id, branch=branch, version_guid=version_guid, usage_id=usage_id) locator = BlockUsageLocator(course_id=course_id, branch=branch, version_guid=version_guid, block_id=block_id)
if not has_access(user, locator): if not has_access(user, locator):
raise PermissionDenied() raise PermissionDenied()
course_location = loc_mapper().translate_locator_to_location(locator) course_location = loc_mapper().translate_locator_to_location(locator)
...@@ -104,7 +104,7 @@ def course_handler(request, tag=None, course_id=None, branch=None, version_guid= ...@@ -104,7 +104,7 @@ def course_handler(request, tag=None, course_id=None, branch=None, version_guid=
return create_new_course(request) return create_new_course(request)
elif not has_access( elif not has_access(
request.user, request.user,
BlockUsageLocator(course_id=course_id, branch=branch, version_guid=version_guid, usage_id=block) BlockUsageLocator(course_id=course_id, branch=branch, version_guid=version_guid, block_id=block)
): ):
raise PermissionDenied() raise PermissionDenied()
elif request.method == 'PUT': elif request.method == 'PUT':
...@@ -366,7 +366,7 @@ def course_info_update_handler(request, tag=None, course_id=None, branch=None, v ...@@ -366,7 +366,7 @@ def course_info_update_handler(request, tag=None, course_id=None, branch=None, v
""" """
if 'application/json' not in request.META.get('HTTP_ACCEPT', 'application/json'): if 'application/json' not in request.META.get('HTTP_ACCEPT', 'application/json'):
return HttpResponseBadRequest("Only supports json requests") return HttpResponseBadRequest("Only supports json requests")
updates_locator = BlockUsageLocator(course_id=course_id, branch=branch, version_guid=version_guid, usage_id=block) updates_locator = BlockUsageLocator(course_id=course_id, branch=branch, version_guid=version_guid, block_id=block)
updates_location = loc_mapper().translate_locator_to_location(updates_locator) updates_location = loc_mapper().translate_locator_to_location(updates_locator)
if provided_id == '': if provided_id == '':
provided_id = None provided_id = None
......
...@@ -60,7 +60,7 @@ def import_handler(request, tag=None, course_id=None, branch=None, version_guid= ...@@ -60,7 +60,7 @@ def import_handler(request, tag=None, course_id=None, branch=None, version_guid=
POST or PUT POST or PUT
json: import a course via the .tar.gz file specified in request.FILES json: import a course via the .tar.gz file specified in request.FILES
""" """
location = BlockUsageLocator(course_id=course_id, branch=branch, version_guid=version_guid, usage_id=block) location = BlockUsageLocator(course_id=course_id, branch=branch, version_guid=version_guid, block_id=block)
if not has_access(request.user, location): if not has_access(request.user, location):
raise PermissionDenied() raise PermissionDenied()
...@@ -271,7 +271,7 @@ def import_status_handler(request, tag=None, course_id=None, branch=None, versio ...@@ -271,7 +271,7 @@ def import_status_handler(request, tag=None, course_id=None, branch=None, versio
3 : Importing to mongo 3 : Importing to mongo
""" """
location = BlockUsageLocator(course_id=course_id, branch=branch, version_guid=version_guid, usage_id=block) location = BlockUsageLocator(course_id=course_id, branch=branch, version_guid=version_guid, block_id=block)
if not has_access(request.user, location): if not has_access(request.user, location):
raise PermissionDenied() raise PermissionDenied()
...@@ -302,7 +302,7 @@ def export_handler(request, tag=None, course_id=None, branch=None, version_guid= ...@@ -302,7 +302,7 @@ def export_handler(request, tag=None, course_id=None, branch=None, version_guid=
If the tar.gz file has been requested but the export operation fails, an HTML page will be returned If the tar.gz file has been requested but the export operation fails, an HTML page will be returned
which describes the error. which describes the error.
""" """
location = BlockUsageLocator(course_id=course_id, branch=branch, version_guid=version_guid, usage_id=block) location = BlockUsageLocator(course_id=course_id, branch=branch, version_guid=version_guid, block_id=block)
if not has_access(request.user, location): if not has_access(request.user, location):
raise PermissionDenied() raise PermissionDenied()
......
...@@ -79,7 +79,7 @@ def xblock_handler(request, tag=None, course_id=None, branch=None, version_guid= ...@@ -79,7 +79,7 @@ def xblock_handler(request, tag=None, course_id=None, branch=None, version_guid=
The locator (and old-style id) for the created xblock (minus children) is returned. The locator (and old-style id) for the created xblock (minus children) is returned.
""" """
if course_id is not None: if course_id is not None:
locator = BlockUsageLocator(course_id=course_id, branch=branch, version_guid=version_guid, usage_id=block) locator = BlockUsageLocator(course_id=course_id, branch=branch, version_guid=version_guid, block_id=block)
if not has_access(request.user, locator): if not has_access(request.user, locator):
raise PermissionDenied() raise PermissionDenied()
old_location = loc_mapper().translate_locator_to_location(locator) old_location = loc_mapper().translate_locator_to_location(locator)
...@@ -330,7 +330,7 @@ def orphan_handler(request, tag=None, course_id=None, branch=None, version_guid= ...@@ -330,7 +330,7 @@ def orphan_handler(request, tag=None, course_id=None, branch=None, version_guid=
:param request: :param request:
:param course_id: Locator syntax course_id :param course_id: Locator syntax course_id
""" """
location = BlockUsageLocator(course_id=course_id, branch=branch, version_guid=version_guid, usage_id=block) location = BlockUsageLocator(course_id=course_id, branch=branch, version_guid=version_guid, block_id=block)
# DHM: when split becomes back-end, move or conditionalize this conversion # DHM: when split becomes back-end, move or conditionalize this conversion
old_location = loc_mapper().translate_locator_to_location(location) old_location = loc_mapper().translate_locator_to_location(location)
if request.method == 'GET': if request.method == 'GET':
......
...@@ -62,7 +62,7 @@ def tabs_handler(request, tag=None, course_id=None, branch=None, version_guid=No ...@@ -62,7 +62,7 @@ def tabs_handler(request, tag=None, course_id=None, branch=None, version_guid=No
Creating a tab, deleting a tab, or changing its contents is not supported through this method. Creating a tab, deleting a tab, or changing its contents is not supported through this method.
Instead use the general xblock URL (see item.xblock_handler). Instead use the general xblock URL (see item.xblock_handler).
""" """
locator = BlockUsageLocator(course_id=course_id, branch=branch, version_guid=version_guid, usage_id=block) locator = BlockUsageLocator(course_id=course_id, branch=branch, version_guid=version_guid, block_id=block)
if not has_access(request.user, locator): if not has_access(request.user, locator):
raise PermissionDenied() raise PermissionDenied()
......
...@@ -52,7 +52,7 @@ def course_team_handler(request, tag=None, course_id=None, branch=None, version_ ...@@ -52,7 +52,7 @@ def course_team_handler(request, tag=None, course_id=None, branch=None, version_
DELETE: DELETE:
json: remove a particular course team member from the course team (email is required). json: remove a particular course team member from the course team (email is required).
""" """
location = BlockUsageLocator(course_id=course_id, branch=branch, version_guid=version_guid, usage_id=block) location = BlockUsageLocator(course_id=course_id, branch=branch, version_guid=version_guid, block_id=block)
if not has_access(request.user, location): if not has_access(request.user, location):
raise PermissionDenied() raise PermissionDenied()
......
...@@ -6,7 +6,7 @@ import re ...@@ -6,7 +6,7 @@ import re
import pymongo import pymongo
import bson.son import bson.son
from xmodule.modulestore.exceptions import InvalidLocationError, ItemNotFoundError, DuplicateItemError from xmodule.modulestore.exceptions import InvalidLocationError, ItemNotFoundError
from xmodule.modulestore.locator import BlockUsageLocator from xmodule.modulestore.locator import BlockUsageLocator
from xmodule.modulestore import Location from xmodule.modulestore import Location
import urllib import urllib
...@@ -157,27 +157,27 @@ class LocMapperStore(object): ...@@ -157,27 +157,27 @@ class LocMapperStore(object):
entry = item entry = item
break break
usage_id = entry['block_map'].get(self._encode_for_mongo(location.name)) block_id = entry['block_map'].get(self._encode_for_mongo(location.name))
if usage_id is None: if block_id is None:
if add_entry_if_missing: if add_entry_if_missing:
usage_id = self._add_to_block_map(location, location_id, entry['block_map']) block_id = self._add_to_block_map(location, location_id, entry['block_map'])
else: else:
raise ItemNotFoundError(location) raise ItemNotFoundError(location)
elif isinstance(usage_id, dict): elif isinstance(block_id, dict):
# name is not unique, look through for the right category # name is not unique, look through for the right category
if location.category in usage_id: if location.category in block_id:
usage_id = usage_id[location.category] block_id = block_id[location.category]
elif add_entry_if_missing: elif add_entry_if_missing:
usage_id = self._add_to_block_map(location, location_id, entry['block_map']) block_id = self._add_to_block_map(location, location_id, entry['block_map'])
else: else:
raise ItemNotFoundError() raise ItemNotFoundError()
else: else:
raise InvalidLocationError() raise InvalidLocationError()
published_usage = BlockUsageLocator( published_usage = BlockUsageLocator(
course_id=entry['course_id'], branch=entry['prod_branch'], usage_id=usage_id) course_id=entry['course_id'], branch=entry['prod_branch'], block_id=block_id)
draft_usage = BlockUsageLocator( draft_usage = BlockUsageLocator(
course_id=entry['course_id'], branch=entry['draft_branch'], usage_id=usage_id) course_id=entry['course_id'], branch=entry['draft_branch'], block_id=block_id)
if published: if published:
result = published_usage result = published_usage
else: else:
...@@ -192,7 +192,7 @@ class LocMapperStore(object): ...@@ -192,7 +192,7 @@ class LocMapperStore(object):
Returns an old style Location for the given Locator if there's an appropriate entry in the Returns an old style Location for the given Locator if there's an appropriate entry in the
mapping collection. Note, it requires that the course was previously mapped (a side effect of mapping collection. Note, it requires that the course was previously mapped (a side effect of
translate_location or explicitly via create_map_entry) and translate_location or explicitly via create_map_entry) and
the block's usage_id was previously stored in the the block's block_id was previously stored in the
map (a side effect of translate_location or via add|update_block_location). map (a side effect of translate_location or via add|update_block_location).
If get_course, then rather than finding the map for this locator, it finds the 'course' root If get_course, then rather than finding the map for this locator, it finds the 'course' root
...@@ -201,7 +201,7 @@ class LocMapperStore(object): ...@@ -201,7 +201,7 @@ class LocMapperStore(object):
If there are no matches, it returns None. If there are no matches, it returns None.
If there's more than one location to locator mapping to the same course_id, it looks for the first If there's more than one location to locator mapping to the same course_id, it looks for the first
one with a mapping for the block usage_id and picks that arbitrary course location. one with a mapping for the block block_id and picks that arbitrary course location.
:param locator: a BlockUsageLocator :param locator: a BlockUsageLocator
""" """
...@@ -215,14 +215,14 @@ class LocMapperStore(object): ...@@ -215,14 +215,14 @@ class LocMapperStore(object):
# This does not require that the course exist in any modulestore # This does not require that the course exist in any modulestore
# only that it has a mapping entry. # only that it has a mapping entry.
maps = self.location_map.find({'course_id': locator.course_id}) maps = self.location_map.find({'course_id': locator.course_id})
# look for one which maps to this block usage_id # look for one which maps to this block block_id
if maps.count() == 0: if maps.count() == 0:
return None return None
result = None result = None
for candidate in maps: for candidate in maps:
old_course_id = self._generate_location_course_id(candidate['_id']) old_course_id = self._generate_location_course_id(candidate['_id'])
for old_name, cat_to_usage in candidate['block_map'].iteritems(): for old_name, cat_to_usage in candidate['block_map'].iteritems():
for category, usage_id in cat_to_usage.iteritems(): for category, block_id in cat_to_usage.iteritems():
# cache all entries and then figure out if we have the one we want # cache all entries and then figure out if we have the one we want
# Always return revision=None because the # Always return revision=None because the
# old draft module store wraps locations as draft before # old draft module store wraps locations as draft before
...@@ -235,16 +235,16 @@ class LocMapperStore(object): ...@@ -235,16 +235,16 @@ class LocMapperStore(object):
self._decode_from_mongo(old_name), self._decode_from_mongo(old_name),
None) None)
published_locator = BlockUsageLocator( published_locator = BlockUsageLocator(
candidate['course_id'], branch=candidate['prod_branch'], usage_id=usage_id candidate['course_id'], branch=candidate['prod_branch'], block_id=block_id
) )
draft_locator = BlockUsageLocator( draft_locator = BlockUsageLocator(
candidate['course_id'], branch=candidate['draft_branch'], usage_id=usage_id candidate['course_id'], branch=candidate['draft_branch'], block_id=block_id
) )
self._cache_location_map_entry(old_course_id, location, published_locator, draft_locator) self._cache_location_map_entry(old_course_id, location, published_locator, draft_locator)
if get_course and category == 'course': if get_course and category == 'course':
result = location result = location
elif not get_course and usage_id == locator.usage_id: elif not get_course and block_id == locator.block_id:
result = location result = location
if result is not None: if result is not None:
return result return result
...@@ -257,13 +257,13 @@ class LocMapperStore(object): ...@@ -257,13 +257,13 @@ class LocMapperStore(object):
# The downside is that if there's more than one course mapped to from the same org/course root # The downside is that if there's more than one course mapped to from the same org/course root
# the block ids will likely be out of sync and collide from an id perspective. HOWEVER, # the block ids will likely be out of sync and collide from an id perspective. HOWEVER,
# if there are few == org/course roots or their content is unrelated, this will work well. # if there are few == org/course roots or their content is unrelated, this will work well.
usage_id = self._verify_uniqueness(location.category + location.name[:3], block_map) block_id = self._verify_uniqueness(location.category + location.name[:3], block_map)
else: else:
usage_id = location.name block_id = location.name
encoded_location_name = self._encode_for_mongo(location.name) encoded_location_name = self._encode_for_mongo(location.name)
block_map.setdefault(encoded_location_name, {})[location.category] = usage_id block_map.setdefault(encoded_location_name, {})[location.category] = block_id
self.location_map.update(location_id, {'$set': {'block_map': block_map}}) self.location_map.update(location_id, {'$set': {'block_map': block_map}})
return usage_id return block_id
def _interpret_location_course_id(self, course_id, location): def _interpret_location_course_id(self, course_id, location):
""" """
......
...@@ -257,7 +257,7 @@ class CourseLocator(Locator): ...@@ -257,7 +257,7 @@ class CourseLocator(Locator):
Returns a copy of itself (downcasting) as a CourseLocator. Returns a copy of itself (downcasting) as a CourseLocator.
The copy has the same CourseLocator fields as the original. The copy has the same CourseLocator fields as the original.
The copy does not include subclass information, such as The copy does not include subclass information, such as
a usage_id (a property of BlockUsageLocator). a block_id (a property of BlockUsageLocator).
""" """
return CourseLocator(course_id=self.course_id, return CourseLocator(course_id=self.course_id,
version_guid=self.version_guid, version_guid=self.version_guid,
...@@ -390,23 +390,23 @@ class BlockUsageLocator(CourseLocator): ...@@ -390,23 +390,23 @@ class BlockUsageLocator(CourseLocator):
""" """
# Default value # Default value
usage_id = None block_id = None
def __init__(self, url=None, version_guid=None, course_id=None, def __init__(self, url=None, version_guid=None, course_id=None,
branch=None, usage_id=None): branch=None, block_id=None):
""" """
Construct a BlockUsageLocator Construct a BlockUsageLocator
Caller may provide url, version_guid, or course_id, and optionally provide branch. Caller may provide url, version_guid, or course_id, and optionally provide branch.
The usage_id may be specified, either explictly or as part of The block_id may be specified, either explictly or as part of
the url or course_id. If omitted, the locator is created but it the url or course_id. If omitted, the locator is created but it
has not yet been initialized. has not yet been initialized.
Resulting BlockUsageLocator will have a usage_id property. Resulting BlockUsageLocator will have a block_id property.
It will have either a version_guid property or a course_id (with optional branch) property, or both. It will have either a version_guid property or a course_id (with optional branch) property, or both.
version_guid must be an instance of bson.objectid.ObjectId or None version_guid must be an instance of bson.objectid.ObjectId or None
url, course_id, branch, and usage_id must be strings or None url, course_id, branch, and block_id must be strings or None
""" """
self._validate_args(url, version_guid, course_id) self._validate_args(url, version_guid, course_id)
...@@ -414,8 +414,8 @@ class BlockUsageLocator(CourseLocator): ...@@ -414,8 +414,8 @@ class BlockUsageLocator(CourseLocator):
self.init_block_ref_from_str(url) self.init_block_ref_from_str(url)
if course_id: if course_id:
self.init_block_ref_from_course_id(course_id) self.init_block_ref_from_course_id(course_id)
if usage_id: if block_id:
self.init_block_ref(usage_id) self.init_block_ref(block_id)
super(BlockUsageLocator, self).__init__( super(BlockUsageLocator, self).__init__(
url=url, url=url,
version_guid=version_guid, version_guid=version_guid,
...@@ -425,9 +425,9 @@ class BlockUsageLocator(CourseLocator): ...@@ -425,9 +425,9 @@ class BlockUsageLocator(CourseLocator):
def is_initialized(self): def is_initialized(self):
""" """
Returns True if usage_id has been initialized, else returns False Returns True if block_id has been initialized, else returns False
""" """
return self.usage_id is not None return self.block_id is not None
def version_agnostic(self): def version_agnostic(self):
""" """
...@@ -442,18 +442,18 @@ class BlockUsageLocator(CourseLocator): ...@@ -442,18 +442,18 @@ class BlockUsageLocator(CourseLocator):
if self.version_guid: if self.version_guid:
return BlockUsageLocator(version_guid=self.version_guid, return BlockUsageLocator(version_guid=self.version_guid,
branch=self.branch, branch=self.branch,
usage_id=self.usage_id) block_id=self.block_id)
else: else:
return BlockUsageLocator(course_id=self.course_id, return BlockUsageLocator(course_id=self.course_id,
branch=self.branch, branch=self.branch,
usage_id=self.usage_id) block_id=self.block_id)
def set_usage_id(self, new): def set_usage_id(self, new):
""" """
Initialize usage_id to new value. Initialize block_id to new value.
If usage_id has already been initialized to a different value, raise an exception. If block_id has already been initialized to a different value, raise an exception.
""" """
self.set_property('usage_id', new) self.set_property('block_id', new)
def init_block_ref(self, block_ref): def init_block_ref(self, block_ref):
if isinstance(block_ref, LocalId): if isinstance(block_ref, LocalId):
...@@ -468,8 +468,8 @@ class BlockUsageLocator(CourseLocator): ...@@ -468,8 +468,8 @@ class BlockUsageLocator(CourseLocator):
""" """
Create a block locator from the given string which may be a url or just the repr (no tag) Create a block locator from the given string which may be a url or just the repr (no tag)
""" """
if hasattr(value, 'usage_id'): if hasattr(value, 'block_id'):
self.init_block_ref(value.usage_id) self.init_block_ref(value.block_id)
return return
if not isinstance(value, basestring): if not isinstance(value, basestring):
return None return None
...@@ -493,7 +493,7 @@ class BlockUsageLocator(CourseLocator): ...@@ -493,7 +493,7 @@ class BlockUsageLocator(CourseLocator):
Return a string representing this location. Return a string representing this location.
""" """
rep = super(BlockUsageLocator, self).__unicode__() rep = super(BlockUsageLocator, self).__unicode__()
return rep + '/' + BLOCK_PREFIX + unicode(self.usage_id) return rep + '/' + BLOCK_PREFIX + unicode(self.block_id)
class DefinitionLocator(Locator): class DefinitionLocator(Locator):
......
...@@ -50,7 +50,7 @@ class SplitMigrator(object): ...@@ -50,7 +50,7 @@ class SplitMigrator(object):
course_location.org, original_course.display_name, course_location.org, original_course.display_name,
user_id, id_root=new_course_id, user_id, id_root=new_course_id,
fields=self._get_json_fields_translate_children(original_course, old_course_id, True), fields=self._get_json_fields_translate_children(original_course, old_course_id, True),
root_usage_id=new_course_root_locator.usage_id, root_block_id=new_course_root_locator.block_id,
master_branch=new_course_root_locator.branch master_branch=new_course_root_locator.branch
) )
...@@ -74,13 +74,13 @@ class SplitMigrator(object): ...@@ -74,13 +74,13 @@ class SplitMigrator(object):
# don't copy the course again. No drafts should get here but check # don't copy the course again. No drafts should get here but check
if module.location != old_course_loc and not getattr(module, 'is_draft', False): if module.location != old_course_loc and not getattr(module, 'is_draft', False):
# create split_xblock using split.create_item # create split_xblock using split.create_item
# where usage_id is computed by translate_location_to_locator # where block_id is computed by translate_location_to_locator
new_locator = self.loc_mapper.translate_location( new_locator = self.loc_mapper.translate_location(
old_course_id, module.location, True, add_entry_if_missing=True old_course_id, module.location, True, add_entry_if_missing=True
) )
_new_module = self.split_modulestore.create_item( _new_module = self.split_modulestore.create_item(
course_version_locator, module.category, user_id, course_version_locator, module.category, user_id,
usage_id=new_locator.usage_id, block_id=new_locator.block_id,
fields=self._get_json_fields_translate_children(module, old_course_id, True), fields=self._get_json_fields_translate_children(module, old_course_id, True),
continue_version=True continue_version=True
) )
...@@ -130,18 +130,18 @@ class SplitMigrator(object): ...@@ -130,18 +130,18 @@ class SplitMigrator(object):
# create a new course version just in case the current head is also the prod head # create a new course version just in case the current head is also the prod head
_new_module = self.split_modulestore.create_item( _new_module = self.split_modulestore.create_item(
new_draft_course_loc, module.category, user_id, new_draft_course_loc, module.category, user_id,
usage_id=new_locator.usage_id, block_id=new_locator.block_id,
fields=self._get_json_fields_translate_children(module, old_course_id, True) fields=self._get_json_fields_translate_children(module, old_course_id, True)
) )
awaiting_adoption[module.location] = new_locator.usage_id awaiting_adoption[module.location] = new_locator.block_id
for draft_location, new_usage_id in awaiting_adoption.iteritems(): for draft_location, new_block_id in awaiting_adoption.iteritems():
for parent_loc in self.draft_modulestore.get_parent_locations(draft_location, old_course_id): for parent_loc in self.draft_modulestore.get_parent_locations(draft_location, old_course_id):
old_parent = self.draft_modulestore.get_item(parent_loc) old_parent = self.draft_modulestore.get_item(parent_loc)
new_parent = self.split_modulestore.get_item( new_parent = self.split_modulestore.get_item(
self.loc_mapper.translate_location(old_course_id, old_parent.location, False) self.loc_mapper.translate_location(old_course_id, old_parent.location, False)
) )
# this only occurs if the parent was also awaiting adoption # this only occurs if the parent was also awaiting adoption
if new_usage_id in new_parent.children: if new_block_id in new_parent.children:
break break
# find index for module: new_parent may be missing quite a few of old_parent's children # find index for module: new_parent may be missing quite a few of old_parent's children
new_parent_cursor = 0 new_parent_cursor = 0
...@@ -152,15 +152,15 @@ class SplitMigrator(object): ...@@ -152,15 +152,15 @@ class SplitMigrator(object):
sibling_loc = self.loc_mapper.translate_location(old_course_id, Location(old_child_loc), False) sibling_loc = self.loc_mapper.translate_location(old_course_id, Location(old_child_loc), False)
# sibling may move cursor # sibling may move cursor
for idx in range(new_parent_cursor, len(new_parent.children)): for idx in range(new_parent_cursor, len(new_parent.children)):
if new_parent.children[idx] == sibling_loc.usage_id: if new_parent.children[idx] == sibling_loc.block_id:
new_parent_cursor = idx + 1 new_parent_cursor = idx + 1
break break
new_parent.children.insert(new_parent_cursor, new_usage_id) new_parent.children.insert(new_parent_cursor, new_block_id)
new_parent = self.split_modulestore.update_item(new_parent, user_id) new_parent = self.split_modulestore.update_item(new_parent, user_id)
def _get_json_fields_translate_children(self, xblock, old_course_id, published): def _get_json_fields_translate_children(self, xblock, old_course_id, published):
""" """
Return the json repr for explicitly set fields but convert all children to their usage_id's Return the json repr for explicitly set fields but convert all children to their block_id's
""" """
fields = self.get_json_fields_explicitly_set(xblock) fields = self.get_json_fields_explicitly_set(xblock)
# this will too generously copy the children even for ones that don't exist in the published b/c the old mongo # this will too generously copy the children even for ones that don't exist in the published b/c the old mongo
...@@ -169,7 +169,7 @@ class SplitMigrator(object): ...@@ -169,7 +169,7 @@ class SplitMigrator(object):
fields['children'] = [ fields['children'] = [
self.loc_mapper.translate_location( self.loc_mapper.translate_location(
old_course_id, Location(child), published, add_entry_if_missing=True old_course_id, Location(child), published, add_entry_if_missing=True
).usage_id ).block_id
for child in fields['children']] for child in fields['children']]
return fields return fields
......
...@@ -47,26 +47,26 @@ class CachingDescriptorSystem(MakoDescriptorSystem): ...@@ -47,26 +47,26 @@ class CachingDescriptorSystem(MakoDescriptorSystem):
self.default_class = default_class self.default_class = default_class
self.local_modules = {} self.local_modules = {}
def _load_item(self, usage_id, course_entry_override=None): def _load_item(self, block_id, course_entry_override=None):
if isinstance(usage_id, BlockUsageLocator) and isinstance(usage_id.usage_id, LocalId): if isinstance(block_id, BlockUsageLocator) and isinstance(block_id.block_id, LocalId):
try: try:
return self.local_modules[usage_id] return self.local_modules[block_id]
except KeyError: except KeyError:
raise ItemNotFoundError raise ItemNotFoundError
json_data = self.module_data.get(usage_id) json_data = self.module_data.get(block_id)
if json_data is None: if json_data is None:
# deeper than initial descendant fetch or doesn't exist # deeper than initial descendant fetch or doesn't exist
self.modulestore.cache_items(self, [usage_id], lazy=self.lazy) self.modulestore.cache_items(self, [block_id], lazy=self.lazy)
json_data = self.module_data.get(usage_id) json_data = self.module_data.get(block_id)
if json_data is None: if json_data is None:
raise ItemNotFoundError(usage_id) raise ItemNotFoundError(block_id)
class_ = XModuleDescriptor.load_class( class_ = XModuleDescriptor.load_class(
json_data.get('category'), json_data.get('category'),
self.default_class self.default_class
) )
return self.xblock_from_json(class_, usage_id, json_data, course_entry_override) return self.xblock_from_json(class_, block_id, json_data, course_entry_override)
# xblock's runtime does not always pass enough contextual information to figure out # xblock's runtime does not always pass enough contextual information to figure out
# which named container (course x branch) or which parent is requesting an item. Because split allows # which named container (course x branch) or which parent is requesting an item. Because split allows
...@@ -79,7 +79,7 @@ class CachingDescriptorSystem(MakoDescriptorSystem): ...@@ -79,7 +79,7 @@ class CachingDescriptorSystem(MakoDescriptorSystem):
# low; thus, the course_entry is most likely correct. If the thread is looking at > 1 named container # low; thus, the course_entry is most likely correct. If the thread is looking at > 1 named container
# pointing to the same structure, the access is likely to be chunky enough that the last known container # pointing to the same structure, the access is likely to be chunky enough that the last known container
# is the intended one when not given a course_entry_override; thus, the caching of the last branch/course_id. # is the intended one when not given a course_entry_override; thus, the caching of the last branch/course_id.
def xblock_from_json(self, class_, usage_id, json_data, course_entry_override=None): def xblock_from_json(self, class_, block_id, json_data, course_entry_override=None):
if course_entry_override is None: if course_entry_override is None:
course_entry_override = self.course_entry course_entry_override = self.course_entry
else: else:
...@@ -91,12 +91,12 @@ class CachingDescriptorSystem(MakoDescriptorSystem): ...@@ -91,12 +91,12 @@ class CachingDescriptorSystem(MakoDescriptorSystem):
definition_id = self.modulestore.definition_locator(definition) definition_id = self.modulestore.definition_locator(definition)
# If no usage id is provided, generate an in-memory id # If no usage id is provided, generate an in-memory id
if usage_id is None: if block_id is None:
usage_id = LocalId() block_id = LocalId()
block_locator = BlockUsageLocator( block_locator = BlockUsageLocator(
version_guid=course_entry_override['structure']['_id'], version_guid=course_entry_override['structure']['_id'],
usage_id=usage_id, block_id=block_id,
course_id=course_entry_override.get('course_id'), course_id=course_entry_override.get('course_id'),
branch=course_entry_override.get('branch') branch=course_entry_override.get('branch')
) )
...@@ -121,7 +121,7 @@ class CachingDescriptorSystem(MakoDescriptorSystem): ...@@ -121,7 +121,7 @@ class CachingDescriptorSystem(MakoDescriptorSystem):
self, self,
BlockUsageLocator( BlockUsageLocator(
version_guid=course_entry_override['structure']['_id'], version_guid=course_entry_override['structure']['_id'],
usage_id=usage_id block_id=block_id
), ),
error_msg=exc_info_to_str(sys.exc_info()) error_msg=exc_info_to_str(sys.exc_info())
) )
...@@ -136,7 +136,7 @@ class CachingDescriptorSystem(MakoDescriptorSystem): ...@@ -136,7 +136,7 @@ class CachingDescriptorSystem(MakoDescriptorSystem):
module.save() module.save()
# If this is an in-memory block, store it in this system # If this is an in-memory block, store it in this system
if isinstance(block_locator.usage_id, LocalId): if isinstance(block_locator.block_id, LocalId):
self.local_modules[block_locator] = module self.local_modules[block_locator] = module
return module return module
...@@ -76,9 +76,9 @@ class TestLocationMapper(unittest.TestCase): ...@@ -76,9 +76,9 @@ class TestLocationMapper(unittest.TestCase):
self.assertEqual(entry['block_map'], block_map) self.assertEqual(entry['block_map'], block_map)
def translate_n_check(self, location, old_style_course_id, new_style_course_id, usage_id, branch, add_entry=False): def translate_n_check(self, location, old_style_course_id, new_style_course_id, block_id, branch, add_entry=False):
""" """
Request translation, check course_id, usage_id, and branch Request translation, check course_id, block_id, and branch
""" """
prob_locator = loc_mapper().translate_location( prob_locator = loc_mapper().translate_location(
old_style_course_id, old_style_course_id,
...@@ -87,7 +87,7 @@ class TestLocationMapper(unittest.TestCase): ...@@ -87,7 +87,7 @@ class TestLocationMapper(unittest.TestCase):
add_entry_if_missing=add_entry add_entry_if_missing=add_entry
) )
self.assertEqual(prob_locator.course_id, new_style_course_id) self.assertEqual(prob_locator.course_id, new_style_course_id)
self.assertEqual(prob_locator.usage_id, usage_id) self.assertEqual(prob_locator.block_id, block_id)
self.assertEqual(prob_locator.branch, branch) self.assertEqual(prob_locator.branch, branch)
def test_translate_location_read_only(self): def test_translate_location_read_only(self):
...@@ -243,7 +243,7 @@ class TestLocationMapper(unittest.TestCase): ...@@ -243,7 +243,7 @@ class TestLocationMapper(unittest.TestCase):
new_style_course_id = '{}.geek_dept.{}.baz_run'.format(org, course) new_style_course_id = '{}.geek_dept.{}.baz_run'.format(org, course)
prob_locator = BlockUsageLocator( prob_locator = BlockUsageLocator(
course_id=new_style_course_id, course_id=new_style_course_id,
usage_id='problem2', block_id='problem2',
branch='published' branch='published'
) )
prob_location = loc_mapper().translate_locator_to_location(prob_locator) prob_location = loc_mapper().translate_locator_to_location(prob_locator)
...@@ -267,21 +267,21 @@ class TestLocationMapper(unittest.TestCase): ...@@ -267,21 +267,21 @@ class TestLocationMapper(unittest.TestCase):
self.assertEqual(prob_location, Location('i4x', org, course, 'course', 'baz_run', None)) self.assertEqual(prob_location, Location('i4x', org, course, 'course', 'baz_run', None))
# explicit branch # explicit branch
prob_locator = BlockUsageLocator( prob_locator = BlockUsageLocator(
course_id=prob_locator.course_id, branch='draft', usage_id=prob_locator.usage_id course_id=prob_locator.course_id, branch='draft', block_id=prob_locator.block_id
) )
prob_location = loc_mapper().translate_locator_to_location(prob_locator) prob_location = loc_mapper().translate_locator_to_location(prob_locator)
# Even though the problem was set as draft, we always return revision=None to work # Even though the problem was set as draft, we always return revision=None to work
# with old mongo/draft modulestores. # with old mongo/draft modulestores.
self.assertEqual(prob_location, Location('i4x', org, course, 'problem', 'abc123', None)) self.assertEqual(prob_location, Location('i4x', org, course, 'problem', 'abc123', None))
prob_locator = BlockUsageLocator( prob_locator = BlockUsageLocator(
course_id=new_style_course_id, usage_id='problem2', branch='production' course_id=new_style_course_id, block_id='problem2', branch='production'
) )
prob_location = loc_mapper().translate_locator_to_location(prob_locator) prob_location = loc_mapper().translate_locator_to_location(prob_locator)
self.assertEqual(prob_location, Location('i4x', org, course, 'problem', 'abc123', None)) self.assertEqual(prob_location, Location('i4x', org, course, 'problem', 'abc123', None))
# same for chapter except chapter cannot be draft in old system # same for chapter except chapter cannot be draft in old system
chap_locator = BlockUsageLocator( chap_locator = BlockUsageLocator(
course_id=new_style_course_id, course_id=new_style_course_id,
usage_id='chapter48f', block_id='chapter48f',
branch='production' branch='production'
) )
chap_location = loc_mapper().translate_locator_to_location(chap_locator) chap_location = loc_mapper().translate_locator_to_location(chap_locator)
...@@ -291,7 +291,7 @@ class TestLocationMapper(unittest.TestCase): ...@@ -291,7 +291,7 @@ class TestLocationMapper(unittest.TestCase):
chap_location = loc_mapper().translate_locator_to_location(chap_locator) chap_location = loc_mapper().translate_locator_to_location(chap_locator)
self.assertEqual(chap_location, Location('i4x', org, course, 'chapter', '48f23a10395384929234')) self.assertEqual(chap_location, Location('i4x', org, course, 'chapter', '48f23a10395384929234'))
chap_locator = BlockUsageLocator( chap_locator = BlockUsageLocator(
course_id=new_style_course_id, usage_id='chapter48f', branch='production' course_id=new_style_course_id, block_id='chapter48f', branch='production'
) )
chap_location = loc_mapper().translate_locator_to_location(chap_locator) chap_location = loc_mapper().translate_locator_to_location(chap_locator)
self.assertEqual(chap_location, Location('i4x', org, course, 'chapter', '48f23a10395384929234')) self.assertEqual(chap_location, Location('i4x', org, course, 'chapter', '48f23a10395384929234'))
...@@ -300,7 +300,7 @@ class TestLocationMapper(unittest.TestCase): ...@@ -300,7 +300,7 @@ class TestLocationMapper(unittest.TestCase):
prob_locator2 = BlockUsageLocator( prob_locator2 = BlockUsageLocator(
course_id=new_style_course_id, course_id=new_style_course_id,
branch='draft', branch='draft',
usage_id='problem3' block_id='problem3'
) )
prob_location = loc_mapper().translate_locator_to_location(prob_locator2) prob_location = loc_mapper().translate_locator_to_location(prob_locator2)
self.assertIsNone(prob_location, 'Found non-existent problem') self.assertIsNone(prob_location, 'Found non-existent problem')
...@@ -325,7 +325,7 @@ class TestLocationMapper(unittest.TestCase): ...@@ -325,7 +325,7 @@ class TestLocationMapper(unittest.TestCase):
prob_locator = BlockUsageLocator( prob_locator = BlockUsageLocator(
course_id=new_style_course_id, course_id=new_style_course_id,
branch='production', branch='production',
usage_id='problem3' block_id='problem3'
) )
prob_location = loc_mapper().translate_locator_to_location(prob_locator) prob_location = loc_mapper().translate_locator_to_location(prob_locator)
self.assertEqual(prob_location, Location('i4x', org, course, 'problem', 'abc123')) self.assertEqual(prob_location, Location('i4x', org, course, 'problem', 'abc123'))
......
...@@ -255,9 +255,9 @@ class LocatorTest(TestCase): ...@@ -255,9 +255,9 @@ class LocatorTest(TestCase):
""" """
course_id = 'mit.eecs-1' course_id = 'mit.eecs-1'
branch = 'foo' branch = 'foo'
usage_id = 'problem:with-colon~2' block_id = 'problem:with-colon~2'
testobj = BlockUsageLocator(course_id=course_id, branch=branch, usage_id=usage_id) testobj = BlockUsageLocator(course_id=course_id, branch=branch, block_id=block_id)
self.check_block_locn_fields(testobj, 'Cannot handle colon', course_id=course_id, branch=branch, block=usage_id) self.check_block_locn_fields(testobj, 'Cannot handle colon', course_id=course_id, branch=branch, block=block_id)
def test_repr(self): def test_repr(self):
testurn = 'mit.eecs.6002x/' + BRANCH_PREFIX + 'published/' + BLOCK_PREFIX + 'HW3' testurn = 'mit.eecs.6002x/' + BRANCH_PREFIX + 'published/' + BLOCK_PREFIX + 'HW3'
...@@ -275,7 +275,7 @@ class LocatorTest(TestCase): ...@@ -275,7 +275,7 @@ class LocatorTest(TestCase):
self.assertEqual(location, Locator.to_locator_or_location(list(location_tuple))) self.assertEqual(location, Locator.to_locator_or_location(list(location_tuple)))
self.assertEqual(location, Locator.to_locator_or_location(location.dict())) self.assertEqual(location, Locator.to_locator_or_location(location.dict()))
locator = BlockUsageLocator(course_id='foo.bar', branch='alpha', usage_id='deep') locator = BlockUsageLocator(course_id='foo.bar', branch='alpha', block_id='deep')
self.assertEqual(locator, Locator.to_locator_or_location(locator)) self.assertEqual(locator, Locator.to_locator_or_location(locator))
self.assertEqual(locator.as_course_locator(), Locator.to_locator_or_location(locator.as_course_locator())) self.assertEqual(locator.as_course_locator(), Locator.to_locator_or_location(locator.as_course_locator()))
self.assertEqual(location, Locator.to_locator_or_location(location.url())) self.assertEqual(location, Locator.to_locator_or_location(location.url()))
...@@ -347,4 +347,4 @@ class LocatorTest(TestCase): ...@@ -347,4 +347,4 @@ class LocatorTest(TestCase):
""" """
self.check_course_locn_fields(testobj, msg, version_guid, course_id, self.check_course_locn_fields(testobj, msg, version_guid, course_id,
branch) branch)
self.assertEqual(testobj.usage_id, block) self.assertEqual(testobj.block_id, block)
...@@ -87,14 +87,14 @@ class TestOrphan(unittest.TestCase): ...@@ -87,14 +87,14 @@ class TestOrphan(unittest.TestCase):
course_or_parent_locator = BlockUsageLocator( course_or_parent_locator = BlockUsageLocator(
course_id=self.split_course_id, course_id=self.split_course_id,
branch='draft', branch='draft',
usage_id=parent_name block_id=parent_name
) )
else: else:
course_or_parent_locator = CourseLocator( course_or_parent_locator = CourseLocator(
course_id='test_org.test_course.runid', course_id='test_org.test_course.runid',
branch='draft', branch='draft',
) )
self.split_mongo.create_item(course_or_parent_locator, category, self.userid, usage_id=name, fields=fields) self.split_mongo.create_item(course_or_parent_locator, category, self.userid, block_id=name, fields=fields)
def _create_course(self): def _create_course(self):
""" """
...@@ -114,7 +114,7 @@ class TestOrphan(unittest.TestCase): ...@@ -114,7 +114,7 @@ class TestOrphan(unittest.TestCase):
fields.update(data) fields.update(data)
# split requires the course to be created separately from creating items # split requires the course to be created separately from creating items
self.split_mongo.create_course( self.split_mongo.create_course(
'test_org', 'my course', self.userid, self.split_course_id, fields=fields, root_usage_id='runid' 'test_org', 'my course', self.userid, self.split_course_id, fields=fields, root_block_id='runid'
) )
self.course_location = Location('i4x', 'test_org', 'test_course', 'course', 'runid') self.course_location = Location('i4x', 'test_org', 'test_course', 'course', 'runid')
self.old_mongo.create_and_save_xmodule(self.course_location, data, metadata) self.old_mongo.create_and_save_xmodule(self.course_location, data, metadata)
...@@ -150,9 +150,9 @@ class TestOrphan(unittest.TestCase): ...@@ -150,9 +150,9 @@ class TestOrphan(unittest.TestCase):
""" """
orphans = self.split_mongo.get_orphans(self.split_course_id, ['static_tab', 'about', 'course_info'], 'draft') orphans = self.split_mongo.get_orphans(self.split_course_id, ['static_tab', 'about', 'course_info'], 'draft')
self.assertEqual(len(orphans), 3, "Wrong # {}".format(orphans)) self.assertEqual(len(orphans), 3, "Wrong # {}".format(orphans))
location = BlockUsageLocator(course_id=self.split_course_id, branch='draft', usage_id='OrphanChapter') location = BlockUsageLocator(course_id=self.split_course_id, branch='draft', block_id='OrphanChapter')
self.assertIn(location, orphans) self.assertIn(location, orphans)
location = BlockUsageLocator(course_id=self.split_course_id, branch='draft', usage_id='OrphanVert') location = BlockUsageLocator(course_id=self.split_course_id, branch='draft', block_id='OrphanVert')
self.assertIn(location, orphans) self.assertIn(location, orphans)
location = BlockUsageLocator(course_id=self.split_course_id, branch='draft', usage_id='OrphanHtml') location = BlockUsageLocator(course_id=self.split_course_id, branch='draft', block_id='OrphanHtml')
self.assertIn(location, orphans) self.assertIn(location, orphans)
...@@ -168,7 +168,7 @@ class XModuleMixin(XBlockMixin): ...@@ -168,7 +168,7 @@ class XModuleMixin(XBlockMixin):
if isinstance(self.location, Location): if isinstance(self.location, Location):
return self.location.name return self.location.name
elif isinstance(self.location, BlockUsageLocator): elif isinstance(self.location, BlockUsageLocator):
return self.location.usage_id return self.location.block_id
else: else:
raise InsufficientSpecificationError() raise InsufficientSpecificationError()
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment