Make course ids and usage ids opaque to LMS and Studio [partial commit]

This commit updates common/lib/xmodule.

These keys are now objects with a limited interface, and the particular
internal representation is managed by the data storage layer (the
modulestore).

For the LMS, there should be no outward-facing changes to the system.
The keys are, for now, a change to internal representation only. For
Studio, the new serialized form of the keys is used in urls, to allow
for further migration in the future.

Co-Author: Andy Armstrong <andya@edx.org>
Co-Author: Christina Roberts <christina@edx.org>
Co-Author: David Baumgold <db@edx.org>
Co-Author: Diana Huang <dkh@edx.org>
Co-Author: Don Mitchell <dmitchell@edx.org>
Co-Author: Julia Hansbrough <julia@edx.org>
Co-Author: Nimisha Asthagiri <nasthagiri@edx.org>
Co-Author: Sarina Canelake <sarina@edx.org>

[LMS-2370]

common/lib/xmodule/setup.py

@@ -67,5 +67,17 @@ setup(
         'console_scripts': [
             'xmodule_assets = xmodule.static_content:main',
         ],
+        'course_key': [
+            'slashes = xmodule.modulestore.locations:SlashSeparatedCourseKey',
+            'course-locator = xmodule.modulestore.locator:CourseLocator',
+        ],
+        'usage_key': [
+            'location = xmodule.modulestore.locations:Location',
+            'edx = xmodule.modulestore.locator:BlockUsageLocator',
+        ],
+        'asset_key': [
+            'asset-location = xmodule.modulestore.locations:AssetLocation',
+            'edx = xmodule.modulestore.locator:BlockUsageLocator',
+        ],
     },
 )

common/lib/xmodule/xmodule/abtest_module.py

@@ -67,7 +67,7 @@ class ABTestModule(ABTestFields, XModule):
 
     def get_child_descriptors(self):
         active_locations = set(self.group_content[self.group])
-        return [desc for desc in self.descriptor.get_children() if desc.location.url() in active_locations]
+        return [desc for desc in self.descriptor.get_children() if desc.location.to_deprecated_string() in active_locations]
 
     def displayable_items(self):
         # Most modules return "self" as the displayable_item. We never display ourself

common/lib/xmodule/xmodule/capa_base.py

@@ -207,7 +207,7 @@ class CapaMixin(CapaFields):
         # Need the problem location in openendedresponse to send out.  Adding
         # it to the system here seems like the least clunky way to get it
         # there.
-        self.runtime.set('location', self.location.url())
+        self.runtime.set('location', self.location.to_deprecated_string())
 
         try:
             # TODO (vshnayder): move as much as possible of this work and error
@@ -225,7 +225,7 @@ class CapaMixin(CapaFields):
 
         except Exception as err:  # pylint: disable=broad-except
             msg = u'cannot create LoncapaProblem {loc}: {err}'.format(
-                loc=self.location.url(), err=err)
+                loc=self.location.to_deprecated_string(), err=err)
             # TODO (vshnayder): do modules need error handlers too?
             # We shouldn't be switching on DEBUG.
             if self.runtime.DEBUG:
@@ -239,7 +239,7 @@ class CapaMixin(CapaFields):
                 # create a dummy problem with error message instead of failing
                 problem_text = (u'<problem><text><span class="inline-error">'
                                 u'Problem {url} has an error:</span>{msg}</text></problem>'.format(
-                                    url=self.location.url(),
+                                    url=self.location.to_deprecated_string(),
                                     msg=msg)
                                 )
                 self.lcp = self.new_lcp(self.get_state_for_lcp(), text=problem_text)
@@ -259,7 +259,7 @@ class CapaMixin(CapaFields):
             self.seed = 1
         elif self.rerandomize == "per_student" and hasattr(self.runtime, 'seed'):
             # see comment on randomization_bin
-            self.seed = randomization_bin(self.runtime.seed, self.location.url)
+            self.seed = randomization_bin(self.runtime.seed, unicode(self.location).encode('utf-8'))
         else:
             self.seed = struct.unpack('i', os.urandom(4))[0]
 
@@ -370,7 +370,7 @@ class CapaMixin(CapaFields):
         progress = self.get_progress()
         return self.runtime.render_template('problem_ajax.html', {
             'element_id': self.location.html_id(),
-            'id': self.id,
+            'id': self.location.to_deprecated_string(),
             'ajax_url': self.runtime.ajax_url,
             'progress_status': Progress.to_js_status_str(progress),
             'progress_detail': Progress.to_js_detail_str(progress),
@@ -510,7 +510,7 @@ class CapaMixin(CapaFields):
             msg = (
                 u'[courseware.capa.capa_module] <font size="+1" color="red">'
                 u'Failed to generate HTML for problem {url}</font>'.format(
-                    url=cgi.escape(self.location.url()))
+                    url=cgi.escape(self.location.to_deprecated_string()))
             )
             msg += u'<p>Error:</p><p><pre>{msg}</pre></p>'.format(msg=cgi.escape(err.message))
             msg += u'<p><pre>{tb}</pre></p>'.format(tb=cgi.escape(traceback.format_exc()))
@@ -598,7 +598,7 @@ class CapaMixin(CapaFields):
 
         context = {
             'problem': content,
-            'id': self.id,
+            'id': self.location.to_deprecated_string(),
             'check_button': check_button,
             'check_button_checking': check_button_checking,
             'reset_button': self.should_show_reset_button(),
@@ -763,7 +763,7 @@ class CapaMixin(CapaFields):
         Returns the answers: {'answers' : answers}
         """
         event_info = dict()
-        event_info['problem_id'] = self.location.url()
+        event_info['problem_id'] = self.location.to_deprecated_string()
         self.track_function_unmask('showanswer', event_info)
         if not self.answer_available():
             raise NotFoundError('Answer is not available')
@@ -906,7 +906,7 @@ class CapaMixin(CapaFields):
         """
         event_info = dict()
         event_info['state'] = self.lcp.get_state()
-        event_info['problem_id'] = self.location.url()
+        event_info['problem_id'] = self.location.to_deprecated_string()
 
         answers = self.make_dict_of_responses(data)
         answers_without_files = convert_files_to_filenames(answers)
@@ -1218,7 +1218,7 @@ class CapaMixin(CapaFields):
         Returns the error messages for exceptions occurring while performing
         the rescoring, rather than throwing them.
         """
-        event_info = {'state': self.lcp.get_state(), 'problem_id': self.location.url()}
+        event_info = {'state': self.lcp.get_state(), 'problem_id': self.location.to_deprecated_string()}
 
         _ = self.runtime.service(self, "i18n").ugettext
 
@@ -1293,7 +1293,7 @@ class CapaMixin(CapaFields):
         """
         event_info = dict()
         event_info['state'] = self.lcp.get_state()
-        event_info['problem_id'] = self.location.url()
+        event_info['problem_id'] = self.location.to_deprecated_string()
 
         answers = self.make_dict_of_responses(data)
         event_info['answers'] = answers
@@ -1346,7 +1346,7 @@ class CapaMixin(CapaFields):
         """
         event_info = dict()
         event_info['old_state'] = self.lcp.get_state()
-        event_info['problem_id'] = self.location.url()
+        event_info['problem_id'] = self.location.to_deprecated_string()
         _ = self.runtime.service(self, "i18n").ugettext
 
         if self.closed():

common/lib/xmodule/xmodule/conditional_module.py

@@ -9,7 +9,6 @@ from lxml import etree
 from pkg_resources import resource_string
 
 from xmodule.x_module import XModule
-from xmodule.modulestore import Location
 from xmodule.seq_module import SequenceDescriptor
 from xblock.fields import Scope, ReferenceList
 from xmodule.modulestore.exceptions import ItemNotFoundError
@@ -144,7 +143,6 @@ class ConditionalModule(ConditionalFields, XModule):
 
         return self.system.render_template('conditional_ajax.html', {
             'element_id': self.location.html_id(),
-            'id': self.id,
             'ajax_url': self.system.ajax_url,
             'depends': ';'.join(self.required_html_ids)
         })
@@ -199,20 +197,14 @@ class ConditionalDescriptor(ConditionalFields, SequenceDescriptor):
         # substitution can be done.
         if not self.sources_list:
             if 'sources' in self.xml_attributes and isinstance(self.xml_attributes['sources'], basestring):
-                sources = ConditionalDescriptor.parse_sources(self.xml_attributes)
-                self.sources_list = sources
+                self.sources_list = ConditionalDescriptor.parse_sources(self.xml_attributes)
 
     @staticmethod
     def parse_sources(xml_element):
         """ Parse xml_element 'sources' attr and return a list of location strings. """
-        result = []
         sources = xml_element.get('sources')
         if sources:
-            locations = [location.strip() for location in sources.split(';')]
-            for location in locations:
-                if Location.is_valid(location):  # Check valid location url.
-                    result.append(location)
-        return result
+            return [location.strip() for location in sources.split(';')]
 
     def get_required_module_descriptors(self):
         """Returns a list of XModuleDescriptor instances upon
@@ -221,7 +213,7 @@ class ConditionalDescriptor(ConditionalFields, SequenceDescriptor):
         descriptors = []
         for location in self.sources_list:
             try:
-                descriptor = self.system.load_item(Location(location))
+                descriptor = self.system.load_item(location)
                 descriptors.append(descriptor)
             except ItemNotFoundError:
                 msg = "Invalid module by location."
@@ -238,7 +230,7 @@ class ConditionalDescriptor(ConditionalFields, SequenceDescriptor):
             if child.tag == 'show':
                 locations = ConditionalDescriptor.parse_sources(child)
                 for location in locations:
-                    children.append(Location(location))
+                    children.append(location)
                     show_tag_list.append(location)
             else:
                 try:
@@ -251,22 +243,18 @@ class ConditionalDescriptor(ConditionalFields, SequenceDescriptor):
         return {'show_tag_list': show_tag_list}, children
 
     def definition_to_xml(self, resource_fs):
-        def to_string(string_list):
-            """ Convert List of strings to a single string with "; " as the separator. """
-            return "; ".join(string_list)
-
         xml_object = etree.Element(self._tag_name)
         for child in self.get_children():
-            location = str(child.location)
-            if location not in self.show_tag_list:
+            if child.location not in self.show_tag_list:
                 self.runtime.add_block_as_child_node(child, xml_object)
 
         if self.show_tag_list:
             show_str = u'<{tag_name} sources="{sources}" />'.format(
-                tag_name='show', sources=to_string(self.show_tag_list))
+                tag_name='show', sources=';'.join(location.to_deprecated_string() for location in self.show_tag_list))
             xml_object.append(etree.fromstring(show_str))
 
         # Overwrite the original sources attribute with the value from sources_list, as
         # Locations may have been changed to Locators.
-        self.xml_attributes['sources'] = to_string(self.sources_list)
+        stringified_sources_list = map(lambda loc: loc.to_deprecated_string(), self.sources_list)
+        self.xml_attributes['sources'] = ';'.join(stringified_sources_list)
         return xml_object

common/lib/xmodule/xmodule/contentstore/content.py

+import bson.son
+import re
 XASSET_LOCATION_TAG = 'c4x'
 XASSET_SRCREF_PREFIX = 'xasset:'
 
@@ -8,7 +10,7 @@ import logging
 import StringIO
 from urlparse import urlparse, urlunparse
 
-from xmodule.modulestore import Location
+from xmodule.modulestore.locations import AssetLocation, SlashSeparatedCourseKey
 from .django import contentstore
 from PIL import Image
 
@@ -22,7 +24,7 @@ class StaticContent(object):
         self._data = data
         self.length = length
         self.last_modified_at = last_modified_at
-        self.thumbnail_location = Location(thumbnail_location) if thumbnail_location is not None else None
+        self.thumbnail_location = thumbnail_location
         # optional information about where this file was imported from. This is needed to support import/export
         # cycles
         self.import_path = import_path
@@ -39,44 +41,48 @@ class StaticContent(object):
             extension=XASSET_THUMBNAIL_TAIL_NAME,)
 
     @staticmethod
-    def compute_location(org, course, name, revision=None, is_thumbnail=False):
-        name = name.replace('/', '_')
-        return Location([XASSET_LOCATION_TAG, org, course, 'asset' if not is_thumbnail else 'thumbnail',
-                         Location.clean_keeping_underscores(name), revision])
+    def compute_location(course_key, path, revision=None, is_thumbnail=False):
+        """
+        Constructs a location object for static content.
+
+        - course_key: the course that this asset belongs to
+        - path: is the name of the static asset
+        - revision: is the object's revision information
+        - is_tumbnail: is whether or not we want the thumbnail version of this
+            asset
+        """
+        path = path.replace('/', '_')
+        return AssetLocation(
+            course_key.org, course_key.course, course_key.run,
+            'asset' if not is_thumbnail else 'thumbnail',
+            AssetLocation.clean_keeping_underscores(path),
+            revision
+        )
 
     def get_id(self):
         return StaticContent.get_id_from_location(self.location)
 
     def get_url_path(self):
-        return StaticContent.get_url_path_from_location(self.location)
+        return self.location.to_deprecated_string()
 
     @property
     def data(self):
         return self._data
 
-    @staticmethod
-    def get_url_path_from_location(location):
-        if location is not None:
-            return u"/{tag}/{org}/{course}/{category}/{name}".format(**location.dict())
-        else:
-            return None
+    ASSET_URL_RE = re.compile(r"""
+        /?c4x/
+        (?P<org>[^/]+)/
+        (?P<course>[^/]+)/
+        (?P<category>[^/]+)/
+        (?P<name>[^/]+)
+    """, re.VERBOSE | re.IGNORECASE)
 
     @staticmethod
     def is_c4x_path(path_string):
         """
         Returns a boolean if a path is believed to be a c4x link based on the leading element
         """
-        return path_string.startswith(u'/{0}/'.format(XASSET_LOCATION_TAG))
-
-    @staticmethod
-    def renamespace_c4x_path(path_string, target_location):
-        """
-        Returns an updated string which incorporates a new org/course in order to remap an asset path
-        to a new namespace
-        """
-        location = StaticContent.get_location_from_path(path_string)
-        location = location.replace(org=target_location.org, course=target_location.course)
-        return StaticContent.get_url_path_from_location(location)
+        return StaticContent.ASSET_URL_RE.match(path_string) is not None
 
     @staticmethod
     def get_static_path_from_location(location):
@@ -88,28 +94,35 @@ class StaticContent(object):
         the actual /c4x/... path which the client needs to reference static content
         """
         if location is not None:
-            return u"/static/{name}".format(**location.dict())
+            return u"/static/{name}".format(name=location.name)
         else:
             return None
 
     @staticmethod
-    def get_base_url_path_for_course_assets(loc):
-        if loc is not None:
-            return u"/c4x/{org}/{course}/asset".format(**loc.dict())
+    def get_base_url_path_for_course_assets(course_key):
+        if course_key is None:
+            return None
+
+        assert(isinstance(course_key, SlashSeparatedCourseKey))
+        return course_key.make_asset_key('asset', '').to_deprecated_string()
 
     @staticmethod
     def get_id_from_location(location):
-        return {'tag': location.tag, 'org': location.org, 'course': location.course,
-                'category': location.category, 'name': location.name,
-                'revision': location.revision}
+        """
+        Get the doc store's primary key repr for this location
+        """
+        return bson.son.SON([
+            ('tag', 'c4x'), ('org', location.org), ('course', location.course),
+            ('category', location.category), ('name', location.name),
+            ('revision', location.revision),
+        ])
 
     @staticmethod
     def get_location_from_path(path):
-        # remove leading / character if it is there one
-        if path.startswith('/'):
-            path = path[1:]
-
-        return Location(path.split('/'))
+        """
+        Generate an AssetKey for the given path (old c4x/org/course/asset/name syntax)
+        """
+        return AssetLocation.from_deprecated_string(path)
 
     @staticmethod
     def convert_legacy_static_url_with_course_id(path, course_id):
@@ -117,12 +130,10 @@ class StaticContent(object):
         Returns a path to a piece of static content when we are provided with a filepath and
         a course_id
         """
-
         # Generate url of urlparse.path component
         scheme, netloc, orig_path, params, query, fragment = urlparse(path)
-        course_id_dict = Location.parse_course_id(course_id)
-        loc = StaticContent.compute_location(course_id_dict['org'], course_id_dict['course'], orig_path)
-        loc_url = StaticContent.get_url_path_from_location(loc)
+        loc = StaticContent.compute_location(course_id, orig_path)
+        loc_url = loc.to_deprecated_string()
 
         # Reconstruct with new path
         return urlunparse((scheme, netloc, loc_url, params, query, fragment))
@@ -167,7 +178,7 @@ class ContentStore(object):
     def find(self, filename):
         raise NotImplementedError
 
-    def get_all_content_for_course(self, location, start=0, maxresults=-1, sort=None):
+    def get_all_content_for_course(self, course_key, start=0, maxresults=-1, sort=None):
         '''
         Returns a list of static assets for a course, followed by the total number of assets.
         By default all assets are returned, but start and maxresults can be provided to limit the query.
@@ -192,13 +203,21 @@ class ContentStore(object):
         '''
         raise NotImplementedError
 
+    def delete_all_course_assets(self, course_key):
+        """
+        Delete all of the assets which use this course_key as an identifier
+        :param course_key:
+        """
+        raise NotImplementedError
+
     def generate_thumbnail(self, content, tempfile_path=None):
         thumbnail_content = None
         # use a naming convention to associate originals with the thumbnail
         thumbnail_name = StaticContent.generate_thumbnail_name(content.location.name)
 
-        thumbnail_file_location = StaticContent.compute_location(content.location.org, content.location.course,
-                                                                 thumbnail_name, is_thumbnail=True)
+        thumbnail_file_location = StaticContent.compute_location(
+            content.location.course_key, thumbnail_name, is_thumbnail=True
+        )
 
         # if we're uploading an image, then let's generate a thumbnail so that we can
         # serve it up when needed without having to rescale on the fly

common/lib/xmodule/xmodule/contentstore/utils.py

-from xmodule.modulestore import Location
 from xmodule.contentstore.content import StaticContent
 from .django import contentstore
 
@@ -13,18 +12,14 @@ def empty_asset_trashcan(course_locs):
         # first delete all of the thumbnails
         thumbs = store.get_all_content_thumbnails_for_course(course_loc)
         for thumb in thumbs:
-            thumb_loc = Location(thumb["_id"])
-            id = StaticContent.get_id_from_location(thumb_loc)
-            print "Deleting {0}...".format(id)
-            store.delete(id)
+            print "Deleting {0}...".format(thumb)
+            store.delete(thumb['_id'])
 
         # then delete all of the assets
         assets, __ = store.get_all_content_for_course(course_loc)
         for asset in assets:
-            asset_loc = Location(asset["_id"])
-            id = StaticContent.get_id_from_location(asset_loc)
-            print "Deleting {0}...".format(id)
-            store.delete(id)
+            print "Deleting {0}...".format(asset)
+            store.delete(asset['_id'])
 
 
 def restore_asset_from_trashcan(location):

common/lib/xmodule/xmodule/course_module.py

@@ -438,7 +438,7 @@ class CourseDescriptor(CourseFields, SequenceDescriptor):
             if isinstance(self.location, Location):
                 self.wiki_slug = self.location.course
             elif isinstance(self.location, CourseLocator):
-                self.wiki_slug = self.location.package_id or self.display_name
+                self.wiki_slug = self.id.offering or self.display_name
 
         if self.due_date_display_format is None and self.show_timezone is False:
             # For existing courses with show_timezone set to False (and no due_date_display_format specified),
@@ -810,32 +810,10 @@ class CourseDescriptor(CourseFields, SequenceDescriptor):
     def make_id(org, course, url_name):
         return '/'.join([org, course, url_name])
 
-    @staticmethod
-    def id_to_location(course_id):
-        '''Convert the given course_id (org/course/name) to a location object.
-        Throws ValueError if course_id is of the wrong format.
-        '''
-        course_id_dict = Location.parse_course_id(course_id)
-        course_id_dict['tag'] = 'i4x'
-        course_id_dict['category'] = 'course'
-        return Location(course_id_dict)
-
-    @staticmethod
-    def location_to_id(location):
-        '''Convert a location of a course to a course_id.  If location category
-        is not "course", raise a ValueError.
-
-        location: something that can be passed to Location
-        '''
-        loc = Location(location)
-        if loc.category != "course":
-            raise ValueError("{0} is not a course location".format(loc))
-        return "/".join([loc.org, loc.course, loc.name])
-
     @property
     def id(self):
         """Return the course_id for this course"""
-        return self.location_to_id(self.location)
+        return self.location.course_key
 
     @property
     def start_date_text(self):

common/lib/xmodule/xmodule/error_module.py

@@ -11,7 +11,6 @@ import sys
 from lxml import etree
 from xmodule.x_module import XModule, XModuleDescriptor
 from xmodule.errortracker import exc_info_to_str
-from xmodule.modulestore import Location
 from xblock.fields import String, Scope, ScopeIds
 from xblock.field_data import DictFieldData
 
@@ -81,7 +80,6 @@ class ErrorDescriptor(ErrorFields, XModuleDescriptor):
 
     @classmethod
     def _construct(cls, system, contents, error_msg, location):
-        location = Location(location)
 
         if error_msg is None:
             # this string is not marked for translation because we don't have

common/lib/xmodule/xmodule/foldit_module.py

@@ -108,7 +108,7 @@ class FolditModule(FolditFields, XModule):
         from foldit.models import Score
 
         if courses is None:
-            courses = [self.location.course_id]
+            courses = [self.location.course_key]
 
         leaders = [(leader['username'], leader['score']) for leader in Score.get_tops_n(10, course_list=courses)]
         leaders.sort(key=lambda x: -x[1])

common/lib/xmodule/xmodule/html_module.py

@@ -121,7 +121,7 @@ class HtmlDescriptor(HtmlFields, XmlDescriptor, EditingDescriptor):
         # Add some specific HTML rendering context when editing HTML modules where we pass
         # the root /c4x/ url for assets. This allows client-side substitutions to occur.
         _context.update({
-            'base_asset_url': StaticContent.get_base_url_path_for_course_assets(self.location) + '/',
+            'base_asset_url': StaticContent.get_base_url_path_for_course_assets(self.location.course_key),
             'enable_latex_compiler': self.use_latex_compiler,
             'editor': self.editor
         })

common/lib/xmodule/xmodule/lti_module.py

@@ -347,9 +347,7 @@ class LTIModule(LTIFields, XModule):
         """
         Return course by course id.
         """
-        course_location = CourseDescriptor.id_to_location(self.course_id)
-        course = self.descriptor.runtime.modulestore.get_item(course_location)
-        return course
+        return self.descriptor.runtime.modulestore.get_course(self.course_id)
 
     @property
     def context_id(self):
@@ -359,7 +357,7 @@ class LTIModule(LTIFields, XModule):
         context_id is an opaque identifier that uniquely identifies the context (e.g., a course)
         that contains the link being launched.
         """
-        return self.course_id
+        return self.course_id.to_deprecated_string()
 
     @property
     def role(self):

common/lib/xmodule/xmodule/modulestore/django.py

@@ -66,7 +66,6 @@ def create_modulestore_instance(engine, doc_store_config, options, i18n_service=
     return class_(
         metadata_inheritance_cache_subsystem=metadata_inheritance_cache,
         request_cache=request_cache,
-        modulestore_update_signal=Signal(providing_args=['modulestore', 'course_id', 'location']),
         xblock_mixins=getattr(settings, 'XBLOCK_MIXINS', ()),
         xblock_select=getattr(settings, 'XBLOCK_SELECT_FUNCTION', None),
         doc_store_config=doc_store_config,

common/lib/xmodule/xmodule/modulestore/exceptions.py

@@ -37,6 +37,13 @@ class DuplicateItemError(Exception):
         self.store = store
         self.collection = collection
 
+    def __str__(self, *args, **kwargs):
+        """
+        Print info about what's duplicated
+        """
+        return '{0.store}[{0.collection}] already has {0.element_id}'.format(
+            self, Exception.__str__(self, *args, **kwargs)
+        )
 
 class VersionConflictError(Exception):
     """

common/lib/xmodule/xmodule/modulestore/parsers.py

@@ -7,15 +7,15 @@ BLOCK_PREFIX = r"block/"
 # Prefix for the version portion of a locator URL, when it is preceded by a course ID
 VERSION_PREFIX = r"version/"
 
-ALLOWED_ID_CHARS = r'[\w\-~.:]'
-
+ALLOWED_ID_CHARS = r'[\w\-~.:+]'
+ALLOWED_ID_RE = re.compile(r'^{}+$'.format(ALLOWED_ID_CHARS), re.UNICODE)
 
+# NOTE: if we need to support period in place of +, make it aggressive (take the first period in the string)
 URL_RE_SOURCE = r"""
-    (?P<tag>edx://)?
-    ((?P<package_id>{ALLOWED_ID_CHARS}+)/?)?
+    ((?P<org>{ALLOWED_ID_CHARS}+)\+(?P<offering>{ALLOWED_ID_CHARS}+)/?)?
     ({BRANCH_PREFIX}(?P<branch>{ALLOWED_ID_CHARS}+)/?)?
     ({VERSION_PREFIX}(?P<version_guid>[A-F0-9]+)/?)?
-    ({BLOCK_PREFIX}(?P<block>{ALLOWED_ID_CHARS}+))?
+    ({BLOCK_PREFIX}(?P<block_id>{ALLOWED_ID_CHARS}+))?
     """.format(
         ALLOWED_ID_CHARS=ALLOWED_ID_CHARS, BRANCH_PREFIX=BRANCH_PREFIX,
         VERSION_PREFIX=VERSION_PREFIX, BLOCK_PREFIX=BLOCK_PREFIX
@@ -24,40 +24,33 @@ URL_RE_SOURCE = r"""
 URL_RE = re.compile('^' + URL_RE_SOURCE + '$', re.IGNORECASE | re.VERBOSE | re.UNICODE)
 
 
-def parse_url(string, tag_optional=False):
+def parse_url(string):
     """
-    A url usually begins with 'edx://' (case-insensitive match),
-    followed by either a version_guid or a package_id. If tag_optional, then
+    followed by either a version_guid or a org + offering pair. If tag_optional, then
     the url does not have to start with the tag and edx will be assumed.
 
     Examples:
-        'edx://version/0123FFFF'
-        'edx://mit.eecs.6002x'
-        'edx://mit.eecs.6002x/branch/published'
-        'edx://mit.eecs.6002x/branch/published/block/HW3'
-        'edx://mit.eecs.6002x/branch/published/version/000eee12345/block/HW3'
+        'edx:version/0123FFFF'
+        'edx:mit.eecs.6002x'
+        '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.
 
-    If it can be parsed as a version_guid with no preceding package_id, returns a dict
+    If it can be parsed as a version_guid with no preceding org + offering, returns a dict
     with key 'version_guid' and the value,
 
-    If it can be parsed as a package_id, returns a dict
+    If it can be parsed as a org + offering, returns a dict
     with key 'id' and optional keys 'branch' and 'version_guid'.
-
     """
     match = URL_RE.match(string)
     if not match:
         return None
     matched_dict = match.groupdict()
-    if matched_dict['tag'] is None and not tag_optional:
-        return None
     return matched_dict
 
 
-BLOCK_RE = re.compile(r'^' + ALLOWED_ID_CHARS + r'+$', re.IGNORECASE | re.UNICODE)
-
-
 def parse_block_ref(string):
     r"""
     A block_ref is a string of url safe characters (see ALLOWED_ID_CHARS)
@@ -65,46 +58,6 @@ def parse_block_ref(string):
     If string is a block_ref, returns a dict with key 'block_ref' and the value,
     otherwise returns None.
     """
-    if len(string) > 0 and BLOCK_RE.match(string):
-        return {'block': string}
+    if ALLOWED_ID_RE.match(string):
+        return {'block_id': string}
     return None
-
-
-def parse_package_id(string):
-    r"""
-
-    A package_id has a main id component.
-    There may also be an optional branch (/branch/published or /branch/draft).
-    There may also be an optional version (/version/519665f6223ebd6980884f2b).
-    There may also be an optional block (/block/HW3 or /block/Quiz2).
-
-    Examples of valid package_ids:
-
-      'mit.eecs.6002x'
-      'mit.eecs.6002x/branch/published'
-      'mit.eecs.6002x/block/HW3'
-      'mit.eecs.6002x/branch/published/block/HW3'
-      'mit.eecs.6002x/branch/published/version/519665f6223ebd6980884f2b/block/HW3'
-
-
-    Syntax:
-
-      package_id = main_id [/branch/ branch] [/version/ version ] [/block/ block]
-
-      main_id = name [. name]*
-
-      branch = name
-
-      block = name
-
-      name = ALLOWED_ID_CHARS
-
-    If string is a package_id, returns a dict with keys 'id', 'branch', and 'block'.
-    Revision is optional: if missing returned_dict['branch'] is None.
-    Block is optional: if missing returned_dict['block'] is None.
-    Else returns None.
-    """
-    match = URL_RE.match(string)
-    if not match:
-        return None
-    return match.groupdict()

common/lib/xmodule/xmodule/modulestore/search.py

 from itertools import repeat
-
-from xmodule.course_module import CourseDescriptor
-
 from .exceptions import (ItemNotFoundError, NoPathToItem)
-from . import Location
 
 
-def path_to_location(modulestore, course_id, location):
+def path_to_location(modulestore, usage_key):
     '''
     Try to find a course_id/chapter/section[/position] path to location in
     modulestore.  The courseware insists that the first level in the course is
     chapter, but any kind of module can be a "section".
 
-    location: something that can be passed to Location
-    course_id: Search for paths in this course.
-
-    raise ItemNotFoundError if the location doesn't exist.
-
-    raise NoPathToItem if the location exists, but isn't accessible via
-    a chapter/section path in the course(s) being searched.
+    Args:
+        modulestore: which store holds the relevant objects
+        usage_key: :class:`UsageKey` the id of the location to which to generate the path
 
-    Return a tuple (course_id, chapter, section, position) suitable for the
-    courseware index view.
+    Raises
+        ItemNotFoundError if the location doesn't exist.
+        NoPathToItem if the location exists, but isn't accessible via
+            a chapter/section path in the course(s) being searched.
 
-    A location may be accessible via many paths. This method may
-    return any valid path.
+    Returns:
+        a tuple (course_id, chapter, section, position) suitable for the
+        courseware index view.
 
-    If the section is a sequential or vertical, position will be the position
-    of this location in that sequence.  Otherwise, position will
-    be None. TODO (vshnayder): Not true yet.
+    If the section is a sequential or vertical, position will be the children index
+    of this location under that sequence.
     '''
 
     def flatten(xs):
@@ -55,41 +49,38 @@ def path_to_location(modulestore, course_id, location):
         # tuples (location, path-so-far).  To avoid lots of
         # copying, the path-so-far is stored as a lisp-style
         # list--nested hd::tl tuples, and flattened at the end.
-        queue = [(location, ())]
+        queue = [(usage_key, ())]
         while len(queue) > 0:
-            (loc, path) = queue.pop()  # Takes from the end
-            loc = Location(loc)
+            (next_usage, path) = queue.pop()  # Takes from the end
 
             # get_parent_locations should raise ItemNotFoundError if location
             # isn't found so we don't have to do it explicitly.  Call this
             # first to make sure the location is there (even if it's a course, and
             # we would otherwise immediately exit).
-            parents = modulestore.get_parent_locations(loc, course_id)
+            parents = modulestore.get_parent_locations(next_usage)
 
-            # print 'Processing loc={0}, path={1}'.format(loc, path)
-            if loc.category == "course":
-                # confirm that this is the right course
-                if course_id == CourseDescriptor.location_to_id(loc):
-                    # Found it!
-                    path = (loc, path)
-                    return flatten(path)
+            # print 'Processing loc={0}, path={1}'.format(next_usage, path)
+            if next_usage.definition_key.block_type == "course":
+                # Found it!
+                path = (next_usage, path)
+                return flatten(path)
 
             # otherwise, add parent locations at the end
-            newpath = (loc, path)
+            newpath = (next_usage, path)
             queue.extend(zip(parents, repeat(newpath)))
 
         # If we're here, there is no path
         return None
 
-    if not modulestore.has_item(course_id, location):
-        raise ItemNotFoundError
+    if not modulestore.has_item(usage_key):
+        raise ItemNotFoundError(usage_key)
 
     path = find_path_to_course()
     if path is None:
-        raise NoPathToItem(location)
+        raise NoPathToItem(usage_key)
 
     n = len(path)
-    course_id = CourseDescriptor.location_to_id(path[0])
+    course_id = path[0].course_key
     # pull out the location names
     chapter = path[1].name if n > 1 else None
     section = path[2].name if n > 2 else None
@@ -105,9 +96,9 @@ def path_to_location(modulestore, course_id, location):
     if n > 3:
         position_list = []
         for path_index in range(2, n - 1):
-            category = path[path_index].category
+            category = path[path_index].definition_key.block_type
             if category == 'sequential' or category == 'videosequence':
-                section_desc = modulestore.get_instance(course_id, path[path_index])
+                section_desc = modulestore.get_item(path[path_index])
                 child_locs = [c.location for c in section_desc.get_children()]
                 # positions are 1-indexed, and should be strings to be consistent with
                 # url parsing.

common/lib/xmodule/xmodule/modulestore/split_mongo/caching_descriptor_system.py

 import sys
 import logging
 from xmodule.mako_module import MakoDescriptorSystem
-from xmodule.modulestore.locator import BlockUsageLocator, LocalId
+from xmodule.modulestore.locator import BlockUsageLocator, LocalId, CourseLocator
 from xmodule.error_module import ErrorDescriptor
 from xmodule.errortracker import exc_info_to_str
-from xblock.runtime import KvsFieldData, IdReader
+from xblock.runtime import KvsFieldData
 from ..exceptions import ItemNotFoundError
 from .split_mongo_kvs import SplitMongoKVS
 from xblock.fields import ScopeIds
@@ -13,23 +13,6 @@ from xmodule.modulestore.loc_mapper_store import LocMapperStore
 log = logging.getLogger(__name__)
 
 
-class SplitMongoIdReader(IdReader):
-    """
-    An :class:`~xblock.runtime.IdReader` associated with a particular
-    :class:`.CachingDescriptorSystem`.
-    """
-    def __init__(self, system):
-        self.system = system
-
-    def get_definition_id(self, usage_id):
-        usage = self.system.load_item(usage_id)
-        return usage.definition_locator
-
-    def get_block_type(self, def_id):
-        definition = self.system.modulestore.db_connection.get_definition(def_id)
-        return definition['category']
-
-
 class CachingDescriptorSystem(MakoDescriptorSystem):
     """
     A system that has a cache of a course version's json that it will use to load modules
@@ -44,15 +27,14 @@ class CachingDescriptorSystem(MakoDescriptorSystem):
         modulestore: the module store that can be used to retrieve additional
         modules
 
-        course_entry: the originally fetched enveloped course_structure w/ branch and package_id info.
+        course_entry: the originally fetched enveloped course_structure w/ branch and course id info.
         Callers to _load_item provide an override but that function ignores the provided structure and
-        only looks at the branch and package_id
+        only looks at the branch and course id
 
         module_data: a dict mapping Location -> json that was cached from the
             underlying modulestore
         """
         super(CachingDescriptorSystem, self).__init__(
-            id_reader=SplitMongoIdReader(self),
             field_data=None,
             load_item=self._load_item,
             **kwargs
@@ -72,11 +54,14 @@ class CachingDescriptorSystem(MakoDescriptorSystem):
         self.local_modules = {}
 
     def _load_item(self, block_id, course_entry_override=None):
-        if isinstance(block_id, BlockUsageLocator) and isinstance(block_id.block_id, LocalId):
-            try:
-                return self.local_modules[block_id]
-            except KeyError:
-                raise ItemNotFoundError
+        if isinstance(block_id, BlockUsageLocator):
+            if isinstance(block_id.block_id, LocalId):
+                try:
+                    return self.local_modules[block_id]
+                except KeyError:
+                    raise ItemNotFoundError
+            else:
+                block_id = block_id.block_id
 
         json_data = self.module_data.get(block_id)
         if json_data is None:
@@ -99,14 +84,15 @@ class CachingDescriptorSystem(MakoDescriptorSystem):
     # the thread is working with more than one named container pointing to the same specific structure is
     # 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
-    # is the intended one when not given a course_entry_override; thus, the caching of the last branch/package_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_, block_id, json_data, course_entry_override=None):
         if course_entry_override is None:
             course_entry_override = self.course_entry
         else:
             # most recent retrieval is most likely the right one for next caller (see comment above fn)
             self.course_entry['branch'] = course_entry_override['branch']
-            self.course_entry['package_id'] = course_entry_override['package_id']
+            self.course_entry['org'] = course_entry_override['org']
+            self.course_entry['offering'] = course_entry_override['offering']
         # most likely a lazy loader or the id directly
         definition = json_data.get('definition', {})
         definition_id = self.modulestore.definition_locator(definition)
@@ -116,10 +102,13 @@ class CachingDescriptorSystem(MakoDescriptorSystem):
             block_id = LocalId()
 
         block_locator = BlockUsageLocator(
-            version_guid=course_entry_override['structure']['_id'],
+            CourseLocator(
+                version_guid=course_entry_override['structure']['_id'],
+                org=course_entry_override.get('org'),
+                offering=course_entry_override.get('offering'),
+                branch=course_entry_override.get('branch'),
+            ),
             block_id=block_id,
-            package_id=course_entry_override.get('package_id'),
-            branch=course_entry_override.get('branch')
         )
 
         kvs = SplitMongoKVS(
@@ -141,7 +130,7 @@ class CachingDescriptorSystem(MakoDescriptorSystem):
                 json_data,
                 self,
                 BlockUsageLocator(
-                    version_guid=course_entry_override['structure']['_id'],
+                    CourseLocator(version_guid=course_entry_override['structure']['_id']),
                     block_id=block_id
                 ),
                 error_msg=exc_info_to_str(sys.exc_info())

common/lib/xmodule/xmodule/modulestore/split_mongo/mongo_connection.py

 """
 Segregation of pymongo functions from the data modeling mechanisms for split modulestore.
 """
+import re
 import pymongo
+from bson import son
 
 class MongoConnection(object):
     """
@@ -18,6 +20,7 @@ class MongoConnection(object):
                 host=host,
                 port=port,
                 tz_aware=tz_aware,
+                document_class=son.SON,
                 **kwargs
             ),
             db
@@ -63,11 +66,17 @@ class MongoConnection(object):
         """
         self.structures.update({'_id': structure['_id']}, structure)
 
-    def get_course_index(self, key):
+    def get_course_index(self, key, ignore_case=False):
         """
         Get the course_index from the persistence mechanism whose id is the given key
         """
-        return self.course_index.find_one({'_id': key})
+        case_regex = r"(?i)^{}$" if ignore_case else r"{}"
+        return self.course_index.find_one(
+            son.SON([
+                (key_attr, re.compile(case_regex.format(getattr(key, key_attr))))
+                for key_attr in ('org', 'offering')
+            ])
+        )
 
     def find_matching_course_indexes(self, query):
         """
@@ -86,13 +95,16 @@ class MongoConnection(object):
         """
         Update the db record for course_index
         """
-        self.course_index.update({'_id': course_index['_id']}, course_index)
+        self.course_index.update(
+            son.SON([('org', course_index['org']), ('offering', course_index['offering'])]),
+            course_index
+        )
 
-    def delete_course_index(self, key):
+    def delete_course_index(self, course_index):
         """
-        Delete the course_index from the persistence mechanism whose id is the given key
+        Delete the course_index from the persistence mechanism whose id is the given course_index
         """
-        return self.course_index.remove({'_id': key})
+        return self.course_index.remove(son.SON([('org', course_index['org']), ('offering', course_index['offering'])]))
 
     def get_definition(self, key):
         """

common/lib/xmodule/xmodule/modulestore/tests/django_utils.py

@@ -205,7 +205,7 @@ class ModuleStoreTestCase(TestCase):
         """
         store = editable_modulestore()
         store.update_item(course, '**replace_user**')
-        updated_course = store.get_instance(course.id, course.location)
+        updated_course = store.get_course(course.id)
         return updated_course
 
     @staticmethod

common/lib/xmodule/xmodule/modulestore/tests/factories.py

@@ -2,7 +2,8 @@ from factory import Factory, lazy_attribute_sequence, lazy_attribute
 from factory.containers import CyclicDefinitionError
 from uuid import uuid4
 
-from xmodule.modulestore import Location, prefer_xmodules
+from xmodule.modulestore import prefer_xmodules
+from xmodule.modulestore.locations import Location
 from xblock.core import XBlock
 
 
@@ -36,6 +37,7 @@ class CourseFactory(XModuleFactory):
     number = '999'
     display_name = 'Robot Super Course'
 
+    # pylint: disable=unused-argument
     @classmethod
     def _create(cls, target_class, **kwargs):
 
@@ -46,8 +48,10 @@ class CourseFactory(XModuleFactory):
         # because the factory provides a default 'number' arg, prefer the non-defaulted 'course' arg if any
         number = kwargs.pop('course', kwargs.pop('number', None))
         store = kwargs.pop('modulestore')
+        name = kwargs.get('name', kwargs.get('run', Location.clean(kwargs.get('display_name'))))
+        run = kwargs.get('run', name)
 
-        location = Location('i4x', org, number, 'course', Location.clean(kwargs.get('display_name')))
+        location = Location(org, number, run, 'course', name)
 
         # Write the data to the mongo datastore
         new_course = store.create_xmodule(location, metadata=kwargs.get('metadata', None))
@@ -82,11 +86,15 @@ class ItemFactory(XModuleFactory):
         else:
             dest_name = self.display_name.replace(" ", "_")
 
-        return self.parent_location.replace(category=self.category, name=dest_name)
+        new_location = self.parent_location.course_key.make_usage_key(
+            self.category,
+            dest_name
+        )
+        return new_location
 
     @lazy_attribute
     def parent_location(self):
-        default_location = Location('i4x://MITx/999/course/Robot_Super_Course')
+        default_location = Location('MITx', '999', 'Robot_Super_Course', 'course', 'Robot_Super_Course', None)
         try:
             parent = self.parent
         # This error is raised if the caller hasn't provided either parent or parent_location
@@ -127,12 +135,14 @@ class ItemFactory(XModuleFactory):
 
         # catch any old style users before they get into trouble
         assert 'template' not in kwargs
-        parent_location = Location(kwargs.pop('parent_location', None))
+        parent_location = kwargs.pop('parent_location', None)
         data = kwargs.pop('data', None)
         category = kwargs.pop('category', None)
         display_name = kwargs.pop('display_name', None)
         metadata = kwargs.pop('metadata', {})
         location = kwargs.pop('location')
+
+        assert isinstance(location, Location)
         assert location != parent_location
 
         store = kwargs.pop('modulestore')
@@ -164,7 +174,7 @@ class ItemFactory(XModuleFactory):
         store.update_item(module)
 
         if 'detached' not in module._class_tags:
-            parent.children.append(location.url())
+            parent.children.append(location)
             store.update_item(parent, '**replace_user**')
 
         return store.get_item(location)

common/lib/xmodule/xmodule/modulestore/tests/test_modulestore.py

-from nose.tools import assert_equals, assert_raises  # pylint: disable=E0611
+from nose.tools import assert_equals, assert_raises, assert_true, assert_false  # pylint: disable=E0611
 
 from xmodule.modulestore.exceptions import ItemNotFoundError
 from xmodule.modulestore.search import path_to_location
+from xmodule.modulestore.locations import SlashSeparatedCourseKey
+
 
 def check_path_to_location(modulestore):
     """
     Make sure that path_to_location works: should be passed a modulestore
     with the toy and simple courses loaded.
     """
+    course_id = SlashSeparatedCourseKey("edX", "toy", "2012_Fall")
+
     should_work = (
-        ("i4x://edX/toy/video/Welcome",
-         ("edX/toy/2012_Fall", "Overview", "Welcome", None)),
-        ("i4x://edX/toy/chapter/Overview",
-         ("edX/toy/2012_Fall", "Overview", None, None)),
+        (course_id.make_usage_key('video', 'Welcome'),
+         (course_id, "Overview", "Welcome", None)),
+        (course_id.make_usage_key('chapter', 'Overview'),
+         (course_id, "Overview", None, None)),
     )
-    course_id = "edX/toy/2012_Fall"
 
     for location, expected in should_work:
-        assert_equals(path_to_location(modulestore, course_id, location), expected)
+        assert_equals(path_to_location(modulestore, location), expected)
 
     not_found = (
-        "i4x://edX/toy/video/WelcomeX", "i4x://edX/toy/course/NotHome"
+        course_id.make_usage_key('video', 'WelcomeX'),
+        course_id.make_usage_key('course', 'NotHome'),
     )
     for location in not_found:
-        assert_raises(ItemNotFoundError, path_to_location, modulestore, course_id, location)
+        with assert_raises(ItemNotFoundError):
+            path_to_location(modulestore, location)
+
+
+def check_has_course_method(modulestore, locator, locator_key_fields):
+    error_message = "Called has_course with query {0} and ignore_case is {1}."
+
+    for ignore_case in [True, False]:
+
+        # should find the course with exact locator
+        assert_true(modulestore.has_course(locator, ignore_case))
+
+        for key_field in locator_key_fields:
+            locator_changes_that_should_not_be_found = [  # pylint: disable=invalid-name
+                # replace value for one of the keys
+                {key_field: 'fake'},
+                # add a character at the end
+                {key_field: getattr(locator, key_field) + 'X'},
+                # add a character in the beginning
+                {key_field: 'X' + getattr(locator, key_field)},
+            ]
+            for changes in locator_changes_that_should_not_be_found:
+                search_locator = locator.replace(**changes)
+                assert_false(
+                    modulestore.has_course(search_locator),
+                    error_message.format(search_locator, ignore_case)
+                )
+
+            # test case [in]sensitivity
+            locator_case_changes = [
+                {key_field: getattr(locator, key_field).upper()},
+                {key_field: getattr(locator, key_field).capitalize()},
+                {key_field: getattr(locator, key_field).capitalize().swapcase()},
+            ]
+            for changes in locator_case_changes:
+                search_locator = locator.replace(**changes)
+                assert_equals(
+                    modulestore.has_course(search_locator, ignore_case),
+                    ignore_case,
+                    error_message.format(search_locator, ignore_case)
+                )

common/lib/xmodule/xmodule/modulestore/tests/test_orphan.py

-import uuid
-import mock
-import unittest
-import random
-import datetime
+from xmodule.modulestore.tests.test_split_w_old_mongo import SplitWMongoCourseBoostrapper
 
-from xmodule.modulestore.inheritance import InheritanceMixin
-from xmodule.modulestore.mongo import MongoModuleStore
-from xmodule.modulestore.split_mongo import SplitMongoModuleStore
-from xmodule.modulestore import Location
-from xmodule.fields import Date
-from xmodule.modulestore.locator import BlockUsageLocator, CourseLocator
 
-
-class TestOrphan(unittest.TestCase):
+class TestOrphan(SplitWMongoCourseBoostrapper):
     """
     Test the orphan finding code
     """
 
-    # Snippet of what would be in the django settings envs file
-    db_config = {
-        'host': 'localhost',
-        'db': 'test_xmodule',
-    }
-
-    modulestore_options = {
-        'default_class': 'xmodule.raw_module.RawDescriptor',
-        'fs_root': '',
-        'render_template': mock.Mock(return_value=""),
-        'xblock_mixins': (InheritanceMixin,)
-    }
-
-    split_package_id = 'test_org.test_course.runid'
-
-    def setUp(self):
-        self.db_config['collection'] = 'modulestore{0}'.format(uuid.uuid4().hex[:5])
-
-        self.userid = random.getrandbits(32)
-        super(TestOrphan, self).setUp()
-        self.split_mongo = SplitMongoModuleStore(
-            self.db_config,
-            **self.modulestore_options
-        )
-        self.addCleanup(self.tear_down_split)
-        self.old_mongo = MongoModuleStore(self.db_config, **self.modulestore_options)
-        self.addCleanup(self.tear_down_mongo)
-        self.course_location = None
-        self._create_course()
-
-    def tear_down_split(self):
-        """
-        Remove the test collections, close the db connection
-        """
-        split_db = self.split_mongo.db
-        split_db.drop_collection(split_db.course_index)
-        split_db.drop_collection(split_db.structures)
-        split_db.drop_collection(split_db.definitions)
-        split_db.connection.close()
-
-    def tear_down_mongo(self):
-        """
-        Remove the test collections, close the db connection
-        """
-        split_db = self.split_mongo.db
-        # old_mongo doesn't give a db attr, but all of the dbs are the same
-        split_db.drop_collection(self.old_mongo.collection)
-
-    def _create_item(self, category, name, data, metadata, parent_category, parent_name, runtime):
-        """
-        Create the item of the given category and block id in split and old mongo, add it to the optional
-        parent. The parent category is only needed because old mongo requires it for the id.
-        """
-        location = Location('i4x', 'test_org', 'test_course', category, name)
-        self.old_mongo.create_and_save_xmodule(location, data, metadata, runtime)
-        if isinstance(data, basestring):
-            fields = {'data': data}
-        else:
-            fields = data.copy()
-        fields.update(metadata)
-        if parent_name:
-            # add child to parent in mongo
-            parent_location = Location('i4x', 'test_org', 'test_course', parent_category, parent_name)
-            parent = self.old_mongo.get_item(parent_location)
-            parent.children.append(location.url())
-            self.old_mongo.update_item(parent, self.userid)
-            # create pointer for split
-            course_or_parent_locator = BlockUsageLocator(
-                package_id=self.split_package_id,
-                branch='draft',
-                block_id=parent_name
-            )
-        else:
-            course_or_parent_locator = CourseLocator(
-                package_id='test_org.test_course.runid',
-                branch='draft',
-            )
-        self.split_mongo.create_item(course_or_parent_locator, category, self.userid, block_id=name, fields=fields)
-
     def _create_course(self):
         """
         * some detached items
         * some attached children
         * some orphans
         """
-        date_proxy = Date()
-        metadata = {
-            'start': date_proxy.to_json(datetime.datetime(2000, 3, 13, 4)),
-            'display_name': 'Migration test course',
-        }
-        data = {
-            'wiki_slug': 'test_course_slug'
-        }
-        fields = metadata.copy()
-        fields.update(data)
-        # split requires the course to be created separately from creating items
-        self.split_mongo.create_course(
-            self.split_package_id, 'test_org', self.userid, fields=fields, root_block_id='runid'
-        )
-        self.course_location = Location('i4x', 'test_org', 'test_course', 'course', 'runid')
-        self.old_mongo.create_and_save_xmodule(self.course_location, data, metadata)
-        runtime = self.old_mongo.get_item(self.course_location).runtime
+        super(TestOrphan, self)._create_course()
 
-        self._create_item('chapter', 'Chapter1', {}, {'display_name': 'Chapter 1'}, 'course', 'runid', runtime)
-        self._create_item('chapter', 'Chapter2', {}, {'display_name': 'Chapter 2'}, 'course', 'runid', runtime)
-        self._create_item('chapter', 'OrphanChapter', {}, {'display_name': 'Orphan Chapter'}, None, None, runtime)
-        self._create_item('vertical', 'Vert1', {}, {'display_name': 'Vertical 1'}, 'chapter', 'Chapter1', runtime)
-        self._create_item('vertical', 'OrphanVert', {}, {'display_name': 'Orphan Vertical'}, None, None, runtime)
-        self._create_item('html', 'Html1', "<p>Goodbye</p>", {'display_name': 'Parented Html'}, 'vertical', 'Vert1', runtime)
-        self._create_item('html', 'OrphanHtml', "<p>Hello</p>", {'display_name': 'Orphan html'}, None, None, runtime)
-        self._create_item('static_tab', 'staticuno', "<p>tab</p>", {'display_name': 'Tab uno'}, None, None, runtime)
-        self._create_item('about', 'overview', "<p>overview</p>", {}, None, None, runtime)
-        self._create_item('course_info', 'updates', "<ol><li><h2>Sep 22</h2><p>test</p></li></ol>", {}, None, None, runtime)
+        self._create_item('chapter', 'Chapter1', {}, {'display_name': 'Chapter 1'}, 'course', 'runid')
+        self._create_item('chapter', 'Chapter2', {}, {'display_name': 'Chapter 2'}, 'course', 'runid')
+        self._create_item('chapter', 'OrphanChapter', {}, {'display_name': 'Orphan Chapter'}, None, None)
+        self._create_item('vertical', 'Vert1', {}, {'display_name': 'Vertical 1'}, 'chapter', 'Chapter1')
+        self._create_item('vertical', 'OrphanVert', {}, {'display_name': 'Orphan Vertical'}, None, None)
+        self._create_item('html', 'Html1', "<p>Goodbye</p>", {'display_name': 'Parented Html'}, 'vertical', 'Vert1')
+        self._create_item('html', 'OrphanHtml', "<p>Hello</p>", {'display_name': 'Orphan html'}, None, None)
+        self._create_item('static_tab', 'staticuno', "<p>tab</p>", {'display_name': 'Tab uno'}, None, None)
+        self._create_item('about', 'overview', "<p>overview</p>", {}, None, None)
+        self._create_item('course_info', 'updates', "<ol><li><h2>Sep 22</h2><p>test</p></li></ol>", {}, None, None)
 
     def test_mongo_orphan(self):
         """
         Test that old mongo finds the orphans
         """
-        orphans = self.old_mongo.get_orphans(self.course_location, None)
+        orphans = self.old_mongo.get_orphans(self.old_course_key)
         self.assertEqual(len(orphans), 3, "Wrong # {}".format(orphans))
-        location = self.course_location.replace(category='chapter', name='OrphanChapter')
-        self.assertIn(location.url(), orphans)
-        location = self.course_location.replace(category='vertical', name='OrphanVert')
-        self.assertIn(location.url(), orphans)
-        location = self.course_location.replace(category='html', name='OrphanHtml')
-        self.assertIn(location.url(), orphans)
+        location = self.old_course_key.make_usage_key('chapter', name='OrphanChapter')
+        self.assertIn(location.to_deprecated_string(), orphans)
+        location = self.old_course_key.make_usage_key('vertical', name='OrphanVert')
+        self.assertIn(location.to_deprecated_string(), orphans)
+        location = self.old_course_key.make_usage_key('html', 'OrphanHtml')
+        self.assertIn(location.to_deprecated_string(), orphans)
 
     def test_split_orphan(self):
         """
-        Test that old mongo finds the orphans
+        Test that split mongo finds the orphans
         """
-        orphans = self.split_mongo.get_orphans(self.split_package_id, 'draft')
+        orphans = self.split_mongo.get_orphans(self.split_course_key)
         self.assertEqual(len(orphans), 3, "Wrong # {}".format(orphans))
-        location = BlockUsageLocator(package_id=self.split_package_id, branch='draft', block_id='OrphanChapter')
+        location = self.split_course_key.make_usage_key('chapter', 'OrphanChapter')
         self.assertIn(location, orphans)
-        location = BlockUsageLocator(package_id=self.split_package_id, branch='draft', block_id='OrphanVert')
+        location = self.split_course_key.make_usage_key('vertical', 'OrphanVert')
         self.assertIn(location, orphans)
-        location = BlockUsageLocator(package_id=self.split_package_id, branch='draft', block_id='OrphanHtml')
+        location = self.split_course_key.make_usage_key('html', 'OrphanHtml')
         self.assertIn(location, orphans)

common/lib/xmodule/xmodule/modulestore/tests/test_split_w_old_mongo.py

+import unittest
+import mock
+import datetime
+import uuid
+import random
+
+from xmodule.modulestore.inheritance import InheritanceMixin
+from xmodule.modulestore.locator import CourseLocator, BlockUsageLocator
+from xmodule.modulestore.split_mongo.split import SplitMongoModuleStore
+from xmodule.modulestore.mongo import MongoModuleStore, DraftMongoModuleStore
+from xmodule.modulestore.mongo.draft import DIRECT_ONLY_CATEGORIES
+
+
+class SplitWMongoCourseBoostrapper(unittest.TestCase):
+    """
+    Helper for tests which need to construct split mongo & old mongo based courses to get interesting internal structure.
+    Override _create_course and after invoking the super() _create_course, have it call _create_item for
+    each xblock you want in the course.
+    This class ensures the db gets created, opened, and cleaned up in addition to creating the course
+
+    Defines the following attrs on self:
+    * userid: a random non-registered mock user id
+    * split_mongo: a pointer to the split mongo instance
+    * old_mongo: a pointer to the old_mongo instance
+    * draft_mongo: a pointer to the old draft instance
+    * split_course_key (CourseLocator): of the new course
+    * old_course_key: the SlashSpecifiedCourseKey for the course
+    """
+        # Snippet of what would be in the django settings envs file
+    db_config = {
+        'host': 'localhost',
+        'db': 'test_xmodule',
+    }
+
+    modulestore_options = {
+        'default_class': 'xmodule.raw_module.RawDescriptor',
+        'fs_root': '',
+        'render_template': mock.Mock(return_value=""),
+        'xblock_mixins': (InheritanceMixin,)
+    }
+
+    split_course_key = CourseLocator('test_org', 'test_course.runid', branch='draft')
+
+    def setUp(self):
+        self.db_config['collection'] = 'modulestore{0}'.format(uuid.uuid4().hex[:5])
+
+        self.userid = random.getrandbits(32)
+        super(SplitWMongoCourseBoostrapper, self).setUp()
+        self.split_mongo = SplitMongoModuleStore(
+            self.db_config,
+            **self.modulestore_options
+        )
+        self.addCleanup(self.split_mongo.db.connection.close)
+        self.addCleanup(self.tear_down_split)
+        self.old_mongo = MongoModuleStore(self.db_config, **self.modulestore_options)
+        self.draft_mongo = DraftMongoModuleStore(self.db_config, **self.modulestore_options)
+        self.addCleanup(self.tear_down_mongo)
+        self.old_course_key = None
+        self.runtime = None
+        self._create_course()
+
+    def tear_down_split(self):
+        """
+        Remove the test collections, close the db connection
+        """
+        split_db = self.split_mongo.db
+        split_db.drop_collection(split_db.course_index)
+        split_db.drop_collection(split_db.structures)
+        split_db.drop_collection(split_db.definitions)
+
+    def tear_down_mongo(self):
+        """
+        Remove the test collections, close the db connection
+        """
+        split_db = self.split_mongo.db
+        # old_mongo doesn't give a db attr, but all of the dbs are the same
+        split_db.drop_collection(self.old_mongo.collection)
+
+    def _create_item(self, category, name, data, metadata, parent_category, parent_name, draft=True, split=True):
+        """
+        Create the item of the given category and block id in split and old mongo, add it to the optional
+        parent. The parent category is only needed because old mongo requires it for the id.
+        """
+        location = self.old_course_key.make_usage_key(category, name)
+        if not draft or category in DIRECT_ONLY_CATEGORIES:
+            mongo = self.old_mongo
+        else:
+            mongo = self.draft_mongo
+        mongo.create_and_save_xmodule(location, data, metadata, self.runtime)
+        if isinstance(data, basestring):
+            fields = {'data': data}
+        else:
+            fields = data.copy()
+        fields.update(metadata)
+        if parent_name:
+            # add child to parent in mongo
+            parent_location = self.old_course_key.make_usage_key(parent_category, parent_name)
+            if not draft or parent_category in DIRECT_ONLY_CATEGORIES:
+                mongo = self.old_mongo
+            else:
+                mongo = self.draft_mongo
+            parent = mongo.get_item(parent_location)
+            parent.children.append(location)
+            mongo.update_item(parent, self.userid)
+            # create pointer for split
+            course_or_parent_locator = BlockUsageLocator(
+                course_key=self.split_course_key,
+                block_id=parent_name
+            )
+        else:
+            course_or_parent_locator = self.split_course_key
+        if split:
+            self.split_mongo.create_item(course_or_parent_locator, category, self.userid, block_id=name, fields=fields)
+
+    def _create_course(self, split=True):
+        """
+        * some detached items
+        * some attached children
+        * some orphans
+        """
+        metadata = {
+            'start': datetime.datetime(2000, 3, 13, 4),
+            'display_name': 'Migration test course',
+        }
+        data = {
+            'wiki_slug': 'test_course_slug'
+        }
+        fields = metadata.copy()
+        fields.update(data)
+        if split:
+            # split requires the course to be created separately from creating items
+            self.split_mongo.create_course(
+                self.split_course_key.org, self.split_course_key.offering, self.userid, fields=fields, root_block_id='runid'
+            )
+        old_course = self.old_mongo.create_course(self.split_course_key.org, 'test_course/runid', fields=fields)
+        self.old_course_key = old_course.id
+        self.runtime = old_course.runtime

common/lib/xmodule/xmodule/modulestore/tests/test_xml.py

@@ -7,12 +7,13 @@ import unittest
 from glob import glob
 from mock import patch
 
-from xmodule.course_module import CourseDescriptor
 from xmodule.modulestore.xml import XMLModuleStore
 from xmodule.modulestore import Location, XML_MODULESTORE_TYPE
 
 from .test_modulestore import check_path_to_location
 from xmodule.tests import DATA_DIR
+from xmodule.modulestore.locations import SlashSeparatedCourseKey
+from xmodule.modulestore.tests.test_modulestore import check_has_course_method
 
 
 def glob_tildes_at_end(path):
@@ -58,22 +59,16 @@ class TestXMLModuleStore(unittest.TestCase):
         modulestore = XMLModuleStore(DATA_DIR, course_dirs=['toy'], load_error_modules=False)
 
         # Look up the errors during load. There should be none.
-        location = CourseDescriptor.id_to_location("edX/toy/2012_Fall")
-        errors = modulestore.get_item_errors(location)
+        errors = modulestore.get_course_errors(SlashSeparatedCourseKey("edX", "toy", "2012_Fall"))
         assert errors == []
 
     @patch("xmodule.modulestore.xml.glob.glob", side_effect=glob_tildes_at_end)
     def test_tilde_files_ignored(self, _fake_glob):
         modulestore = XMLModuleStore(DATA_DIR, course_dirs=['tilde'], load_error_modules=False)
-        course_module = modulestore.modules['edX/tilde/2012_Fall']
-        about_location = Location({
-            'tag': 'i4x',
-            'org': 'edX',
-            'course': 'tilde',
-            'category': 'about',
-            'name': 'index',
-        })
-        about_module = course_module[about_location]
+        about_location = SlashSeparatedCourseKey('edX', 'tilde', '2012_Fall').make_usage_key(
+            'about', 'index',
+        )
+        about_module = modulestore.get_item(about_location)
         self.assertIn("GREEN", about_module.data)
         self.assertNotIn("RED", about_module.data)
 
@@ -85,13 +80,13 @@ class TestXMLModuleStore(unittest.TestCase):
         for course in store.get_courses():
             course_locations = store.get_courses_for_wiki(course.wiki_slug)
             self.assertEqual(len(course_locations), 1)
-            self.assertIn(Location('i4x', 'edX', course.location.course, 'course', '2012_Fall'), course_locations)
+            self.assertIn(course.location, course_locations)
 
         course_locations = store.get_courses_for_wiki('no_such_wiki')
         self.assertEqual(len(course_locations), 0)
 
         # now set toy course to share the wiki with simple course
-        toy_course = store.get_course('edX/toy/2012_Fall')
+        toy_course = store.get_course(SlashSeparatedCourseKey('edX', 'toy', '2012_Fall'))
         toy_course.wiki_slug = 'simple'
 
         course_locations = store.get_courses_for_wiki('toy')
@@ -100,4 +95,14 @@ class TestXMLModuleStore(unittest.TestCase):
         course_locations = store.get_courses_for_wiki('simple')
         self.assertEqual(len(course_locations), 2)
         for course_number in ['toy', 'simple']:
-            self.assertIn(Location('i4x', 'edX', course_number, 'course', '2012_Fall'), course_locations)
+            self.assertIn(Location('edX', course_number, '2012_Fall', 'course', '2012_Fall'), course_locations)
+
+    def test_has_course(self):
+        """
+        Test the has_course method
+        """
+        check_has_course_method(
+            XMLModuleStore(DATA_DIR, course_dirs=['toy', 'simple']),
+            SlashSeparatedCourseKey('edX', 'toy', '2012_Fall'),
+            locator_key_fields=SlashSeparatedCourseKey.KEY_FIELDS
+        )

common/lib/xmodule/xmodule/modulestore/tests/test_xml_importer.py

 """
 Tests for XML importer.
 """
-from unittest import TestCase
 import mock
 from xblock.core import XBlock
 from xblock.fields import String, Scope, ScopeIds
@@ -9,7 +8,93 @@ from xblock.runtime import Runtime, KvsFieldData, DictKeyValueStore
 from xmodule.x_module import XModuleMixin
 from xmodule.modulestore import Location
 from xmodule.modulestore.inheritance import InheritanceMixin
-from xmodule.modulestore.xml_importer import remap_namespace
+from xmodule.modulestore.xml_importer import import_module
+from xmodule.modulestore.locations import SlashSeparatedCourseKey
+from xmodule.tests import DATA_DIR
+from uuid import uuid4
+import unittest
+import importlib
+
+
+class ModuleStoreNoSettings(unittest.TestCase):
+    """
+    A mixin to create a mongo modulestore that avoids settings
+    """
+    HOST = 'localhost'
+    PORT = 27017
+    DB = 'test_mongo_%s' % uuid4().hex[:5]
+    COLLECTION = 'modulestore'
+    FS_ROOT = DATA_DIR
+    DEFAULT_CLASS = 'xmodule.modulestore.tests.test_xml_importer.StubXBlock'
+    RENDER_TEMPLATE = lambda t_n, d, ctx = None, nsp = 'main': ''
+
+    modulestore_options = {
+        'default_class': DEFAULT_CLASS,
+        'fs_root': DATA_DIR,
+        'render_template': RENDER_TEMPLATE,
+    }
+    DOC_STORE_CONFIG = {
+        'host': HOST,
+        'db': DB,
+        'collection': COLLECTION,
+    }
+    MODULESTORE = {
+        'ENGINE': 'xmodule.modulestore.mongo.MongoModuleStore',
+        'DOC_STORE_CONFIG': DOC_STORE_CONFIG,
+        'OPTIONS': modulestore_options
+    }
+
+    modulestore = None
+
+    def cleanup_modulestore(self):
+        """
+        cleanup
+        """
+        if modulestore:
+            connection = self.modulestore.database.connection
+            connection.drop_database(self.modulestore.database)
+            connection.close()
+
+    def setUp(self):
+        """
+        Add cleanups
+        """
+        self.addCleanup(self.cleanup_modulestore)
+        super(ModuleStoreNoSettings, self).setUp()
+
+
+#===========================================
+def modulestore():
+    """
+    Mock the django dependent global modulestore function to disentangle tests from django
+    """
+    def load_function(engine_path):
+        """
+        Load the given engine
+        """
+        module_path, _, name = engine_path.rpartition('.')
+        return getattr(importlib.import_module(module_path), name)
+
+    if ModuleStoreNoSettings.modulestore is None:
+        class_ = load_function(ModuleStoreNoSettings.MODULESTORE['ENGINE'])
+
+        options = {}
+
+        options.update(ModuleStoreNoSettings.MODULESTORE['OPTIONS'])
+        options['render_template'] = render_to_template_mock
+
+        # pylint: disable=W0142
+        ModuleStoreNoSettings.modulestore = class_(
+            ModuleStoreNoSettings.MODULESTORE['DOC_STORE_CONFIG'],
+            **options
+        )
+
+    return ModuleStoreNoSettings.modulestore
+
+
+# pylint: disable=W0613
+def render_to_template_mock(*args):
+    pass
 
 
 class StubXBlock(XBlock, XModuleMixin, InheritanceMixin):
@@ -29,7 +114,7 @@ class StubXBlock(XBlock, XModuleMixin, InheritanceMixin):
     )
 
 
-class RemapNamespaceTest(TestCase):
+class RemapNamespaceTest(ModuleStoreNoSettings):
     """
     Test that remapping the namespace from import to the actual course location.
     """
@@ -42,81 +127,99 @@ class RemapNamespaceTest(TestCase):
         self.field_data = KvsFieldData(kvs=DictKeyValueStore())
         self.scope_ids = ScopeIds('Bob', 'stubxblock', '123', 'import')
         self.xblock = StubXBlock(self.runtime, self.field_data, self.scope_ids)
+        super(RemapNamespaceTest, self).setUp()
 
     def test_remap_namespace_native_xblock(self):
 
         # Set the XBlock's location
-        self.xblock.location = Location("i4x://import/org/run/stubxblock")
+        self.xblock.location = Location("org", "import", "run", "category", "stubxblock")
 
         # Explicitly set the content and settings fields
         self.xblock.test_content_field = "Explicitly set"
         self.xblock.test_settings_field = "Explicitly set"
         self.xblock.save()
 
-        # Remap the namespace
-        target_location_namespace = Location("i4x://course/org/run/stubxblock")
-        remap_namespace(self.xblock, target_location_namespace)
+        # Move to different runtime w/ different course id
+        target_location_namespace = SlashSeparatedCourseKey("org", "course", "run")
+        new_version = import_module(
+            self.xblock,
+            modulestore(),
+            self.xblock.location.course_key,
+            target_location_namespace,
+            do_import_static=False
+        )
 
         # Check the XBlock's location
-        self.assertEqual(self.xblock.location, target_location_namespace)
+        self.assertEqual(new_version.location.course_key, target_location_namespace)
 
         # Check the values of the fields.
         # The content and settings fields should be preserved
-        self.assertEqual(self.xblock.test_content_field, 'Explicitly set')
-        self.assertEqual(self.xblock.test_settings_field, 'Explicitly set')
+        self.assertEqual(new_version.test_content_field, 'Explicitly set')
+        self.assertEqual(new_version.test_settings_field, 'Explicitly set')
 
         # Expect that these fields are marked explicitly set
         self.assertIn(
             'test_content_field',
-            self.xblock.get_explicitly_set_fields_by_scope(scope=Scope.content)
+            new_version.get_explicitly_set_fields_by_scope(scope=Scope.content)
         )
         self.assertIn(
             'test_settings_field',
-            self.xblock.get_explicitly_set_fields_by_scope(scope=Scope.settings)
+            new_version.get_explicitly_set_fields_by_scope(scope=Scope.settings)
         )
 
     def test_remap_namespace_native_xblock_default_values(self):
 
         # Set the XBlock's location
-        self.xblock.location = Location("i4x://import/org/run/stubxblock")
+        self.xblock.location = Location("org", "import", "run", "category", "stubxblock")
 
         # Do NOT set any values, so the fields should use the defaults
         self.xblock.save()
 
         # Remap the namespace
-        target_location_namespace = Location("i4x://course/org/run/stubxblock")
-        remap_namespace(self.xblock, target_location_namespace)
+        target_location_namespace = Location("org", "course", "run", "category", "stubxblock")
+        new_version = import_module(
+            self.xblock,
+            modulestore(),
+            self.xblock.location.course_key,
+            target_location_namespace.course_key,
+            do_import_static=False
+        )
 
         # Check the values of the fields.
         # The content and settings fields should be the default values
-        self.assertEqual(self.xblock.test_content_field, 'default value')
-        self.assertEqual(self.xblock.test_settings_field, 'default value')
+        self.assertEqual(new_version.test_content_field, 'default value')
+        self.assertEqual(new_version.test_settings_field, 'default value')
 
         # The fields should NOT appear in the explicitly set fields
         self.assertNotIn(
             'test_content_field',
-            self.xblock.get_explicitly_set_fields_by_scope(scope=Scope.content)
+            new_version.get_explicitly_set_fields_by_scope(scope=Scope.content)
         )
         self.assertNotIn(
             'test_settings_field',
-            self.xblock.get_explicitly_set_fields_by_scope(scope=Scope.settings)
+            new_version.get_explicitly_set_fields_by_scope(scope=Scope.settings)
         )
 
     def test_remap_namespace_native_xblock_inherited_values(self):
 
         # Set the XBlock's location
-        self.xblock.location = Location("i4x://import/org/run/stubxblock")
+        self.xblock.location = Location("org", "import", "run", "category", "stubxblock")
         self.xblock.save()
 
         # Remap the namespace
-        target_location_namespace = Location("i4x://course/org/run/stubxblock")
-        remap_namespace(self.xblock, target_location_namespace)
+        target_location_namespace = Location("org", "course", "run", "category", "stubxblock")
+        new_version = import_module(
+            self.xblock,
+            modulestore(),
+            self.xblock.location.course_key,
+            target_location_namespace.course_key,
+            do_import_static=False
+        )
 
         # Inherited fields should NOT be explicitly set
         self.assertNotIn(
-            'start', self.xblock.get_explicitly_set_fields_by_scope(scope=Scope.settings)
+            'start', new_version.get_explicitly_set_fields_by_scope(scope=Scope.settings)
         )
         self.assertNotIn(
-            'graded', self.xblock.get_explicitly_set_fields_by_scope(scope=Scope.settings)
+            'graded', new_version.get_explicitly_set_fields_by_scope(scope=Scope.settings)
         )
-

common/lib/xmodule/xmodule/modulestore/xml_exporter.py

@@ -32,7 +32,7 @@ class EdxJSONEncoder(json.JSONEncoder):
     """
     def default(self, obj):
         if isinstance(obj, Location):
-            return obj.url()
+            return obj.to_deprecated_string()
         elif isinstance(obj, datetime.datetime):
             if obj.tzinfo is not None:
                 if obj.utcoffset() is None:
@@ -45,24 +45,23 @@ class EdxJSONEncoder(json.JSONEncoder):
             return super(EdxJSONEncoder, self).default(obj)
 
 
-def export_to_xml(modulestore, contentstore, course_location, root_dir, course_dir, draft_modulestore=None):
+def export_to_xml(modulestore, contentstore, course_key, root_dir, course_dir, draft_modulestore=None):
     """
     Export all modules from `modulestore` and content from `contentstore` as xml to `root_dir`.
 
     `modulestore`: A `ModuleStore` object that is the source of the modules to export
     `contentstore`: A `ContentStore` object that is the source of the content to export, can be None
-    `course_location`: The `Location` of the `CourseModuleDescriptor` to export
+    `course_key`: The `CourseKey` of the `CourseModuleDescriptor` to export
     `root_dir`: The directory to write the exported xml to
     `course_dir`: The name of the directory inside `root_dir` to write the course content to
     `draft_modulestore`: An optional `DraftModuleStore` that contains draft content, which will be exported
         alongside the public content in the course.
     """
 
-    course_id = course_location.course_id
-    course = modulestore.get_course(course_id)
+    course = modulestore.get_course(course_key)
 
-    fs = OSFS(root_dir)
-    export_fs = course.runtime.export_fs = fs.makeopendir(course_dir)
+    fsm = OSFS(root_dir)
+    export_fs = course.runtime.export_fs = fsm.makeopendir(course_dir)
 
     root = lxml.etree.Element('unknown')
     course.add_xml_to_node(root)
@@ -74,22 +73,22 @@ def export_to_xml(modulestore, contentstore, course_location, root_dir, course_d
     policies_dir = export_fs.makeopendir('policies')
     if contentstore:
         contentstore.export_all_for_course(
-            course_location,
+            course_key,
             root_dir + '/' + course_dir + '/static/',
             root_dir + '/' + course_dir + '/policies/assets.json',
         )
 
     # export the static tabs
-    export_extra_content(export_fs, modulestore, course_id, course_location, 'static_tab', 'tabs', '.html')
+    export_extra_content(export_fs, modulestore, course_key, 'static_tab', 'tabs', '.html')
 
     # export the custom tags
-    export_extra_content(export_fs, modulestore, course_id, course_location, 'custom_tag_template', 'custom_tags')
+    export_extra_content(export_fs, modulestore, course_key, 'custom_tag_template', 'custom_tags')
 
     # export the course updates
-    export_extra_content(export_fs, modulestore, course_id, course_location, 'course_info', 'info', '.html')
+    export_extra_content(export_fs, modulestore, course_key, 'course_info', 'info', '.html')
 
     # export the 'about' data (e.g. overview, etc.)
-    export_extra_content(export_fs, modulestore, course_id, course_location, 'about', 'about', '.html')
+    export_extra_content(export_fs, modulestore, course_key, 'about', 'about', '.html')
 
     # export the grading policy
     course_run_policy_dir = policies_dir.makeopendir(course.location.name)
@@ -106,18 +105,17 @@ def export_to_xml(modulestore, contentstore, course_location, root_dir, course_d
     # should we change the application, then this assumption will no longer
     # be valid
     if draft_modulestore is not None:
-        draft_verticals = draft_modulestore.get_items([None, course_location.org, course_location.course,
-                                                       'vertical', None, 'draft'])
+        draft_verticals = draft_modulestore.get_items(course_key, category='vertical')
         if len(draft_verticals) > 0:
             draft_course_dir = export_fs.makeopendir(DRAFT_DIR)
             for draft_vertical in draft_verticals:
-                parent_locs = draft_modulestore.get_parent_locations(draft_vertical.location, course.location.course_id)
+                parent_locs = draft_modulestore.get_parent_locations(draft_vertical.location)
                 # Don't try to export orphaned items.
                 if len(parent_locs) > 0:
                     logging.debug('parent_locs = {0}'.format(parent_locs))
-                    draft_vertical.xml_attributes['parent_sequential_url'] = Location(parent_locs[0]).url()
-                    sequential = modulestore.get_item(Location(parent_locs[0]))
-                    index = sequential.children.index(draft_vertical.location.url())
+                    draft_vertical.xml_attributes['parent_sequential_url'] = parent_locs[0].to_deprecated_string()
+                    sequential = modulestore.get_item(parent_locs[0])
+                    index = sequential.children.index(draft_vertical.location)
                     draft_vertical.xml_attributes['index_in_children_list'] = str(index)
                     draft_vertical.runtime.export_fs = draft_course_dir
                     node = lxml.etree.Element('unknown')
@@ -138,9 +136,8 @@ def _export_field_content(xblock_item, item_dir):
                     field_content_file.write(dumps(module_data.get(field_name, {}), cls=EdxJSONEncoder))
 
 
-def export_extra_content(export_fs, modulestore, course_id, course_location, category_type, dirname, file_suffix=''):
-    query_loc = Location('i4x', course_location.org, course_location.course, category_type, None)
-    items = modulestore.get_items(query_loc, course_id)
+def export_extra_content(export_fs, modulestore, course_key, category_type, dirname, file_suffix=''):
+    items = modulestore.get_items(course_key, category=category_type)
 
     if len(items) > 0:
         item_dir = export_fs.makeopendir(dirname)

common/lib/xmodule/xmodule/open_ended_grading_classes/combined_open_ended_modulev1.py

@@ -412,7 +412,7 @@ class CombinedOpenEndedV1Module():
         :param message: A message to put in the log.
         :return: None
         """
-        info_message = "Combined open ended user state for user {0} in location {1} was invalid.  It has been reset, and you now have a new attempt. {2}".format(self.system.anonymous_student_id, self.location.url(), message)
+        info_message = "Combined open ended user state for user {0} in location {1} was invalid.  It has been reset, and you now have a new attempt. {2}".format(self.system.anonymous_student_id, self.location.to_deprecated_string(), message)
         self.current_task_number = 0
         self.student_attempts = 0
         self.old_task_states.append(self.task_states)
@@ -800,7 +800,7 @@ class CombinedOpenEndedV1Module():
         success = False
         allowed_to_submit = True
         try:
-            response = self.peer_gs.get_data_for_location(self.location.url(), student_id)
+            response = self.peer_gs.get_data_for_location(self.location.to_deprecated_string(), student_id)
             count_graded = response['count_graded']
             count_required = response['count_required']
             student_sub_count = response['student_sub_count']

common/lib/xmodule/xmodule/open_ended_grading_classes/combined_open_ended_rubric.py

@@ -96,7 +96,7 @@ class CombinedOpenEndedRubric(object):
         if not success:
             #This is a staff_facing_error
             error_message = "Could not parse rubric : {0} for location {1}. Contact the learning sciences group for assistance.".format(
-                rubric_string, location.url())
+                rubric_string, location.to_deprecated_string())
             log.error(error_message)
             raise RubricParsingError(error_message)
 

common/lib/xmodule/xmodule/open_ended_grading_classes/open_ended_module.py

@@ -105,7 +105,7 @@ class OpenEndedModule(openendedchild.OpenEndedChild):
             # NOTE: self.system.location is valid because the capa_module
             # __init__ adds it (easiest way to get problem location into
             # response types)
-        except TypeError, ValueError:
+        except (TypeError, ValueError):
             # This is a dev_facing_error
             log.exception(
                 "Grader payload from external open ended grading server is not a json object! Object: {0}".format(
@@ -116,7 +116,7 @@ class OpenEndedModule(openendedchild.OpenEndedChild):
 
         parsed_grader_payload.update({
             'location': self.location_string,
-            'course_id': system.course_id,
+            'course_id': system.course_id.to_deprecated_string(),
             'prompt': prompt_string,
             'rubric': rubric_string,
             'initial_display': self.initial_display,

common/lib/xmodule/xmodule/open_ended_grading_classes/openendedchild.py

@@ -157,7 +157,7 @@ class OpenEndedChild(object):
 
         self.location_string = location
         try:
-            self.location_string = self.location_string.url()
+            self.location_string = self.location_string.to_deprecated_string()
         except:
             pass
 

common/lib/xmodule/xmodule/open_ended_grading_classes/peer_grading_service.py

@@ -87,6 +87,11 @@ class PeerGradingService(GradingService):
     def get_problem_list(self, course_id, grader_id):
         params = {'course_id': course_id, 'student_id': grader_id}
         result = self.get(self.get_problem_list_url, params)
+
+        if 'problem_list' in result:
+            for problem in result['problem_list']:
+                problem['location'] = course_id.make_usage_key_from_deprecated_string(problem['location'])
+
         self._record_result('get_problem_list', result)
         dog_stats_api.histogram(
             self._metric_name('get_problem_list.result.length'),

common/lib/xmodule/xmodule/peer_grading_module.py

@@ -8,7 +8,6 @@ from xblock.fields import Dict, String, Scope, Boolean, Float, Reference
 
 from xmodule.capa_module import ComplexEncoder
 from xmodule.fields import Date, Timedelta
-from xmodule.modulestore import Location
 from xmodule.modulestore.exceptions import ItemNotFoundError, NoPathToItem
 from xmodule.raw_module import RawDescriptor
 from xmodule.timeinfo import TimeInfo
@@ -261,7 +260,7 @@ class PeerGradingModule(PeerGradingFields, XModule):
             if not success:
                 log.exception(
                     "No instance data found and could not get data from controller for loc {0} student {1}".format(
-                        self.system.location.url(), self.system.anonymous_student_id
+                        self.system.location.to_deprecated_string(), self.system.anonymous_student_id
                     ))
                 return None
             count_graded = response['count_graded']
@@ -563,7 +562,7 @@ class PeerGradingModule(PeerGradingFields, XModule):
 
         good_problem_list = []
         for problem in problem_list:
-            problem_location = Location(problem['location'])
+            problem_location = problem['location']
             try:
                 descriptor = self._find_corresponding_module_for_location(problem_location)
             except (NoPathToItem, ItemNotFoundError):
@@ -588,7 +587,6 @@ class PeerGradingModule(PeerGradingFields, XModule):
 
         ajax_url = self.ajax_url
         html = self.system.render_template('peer_grading/peer_grading.html', {
-            'course_id': self.course_id,
             'ajax_url': ajax_url,
             'success': success,
             'problem_list': good_problem_list,
@@ -611,10 +609,10 @@ class PeerGradingModule(PeerGradingFields, XModule):
                 log.error(
                     "Peer grading problem in peer_grading_module called with no get parameters, but use_for_single_location is False.")
                 return {'html': "", 'success': False}
-            problem_location = Location(self.link_to_location)
+            problem_location = self.link_to_location
 
         elif data.get('location') is not None:
-            problem_location = Location(data.get('location'))
+            problem_location = self.course_id.make_usage_key_from_deprecated_string(data.get('location'))
 
         module = self._find_corresponding_module_for_location(problem_location)
 

common/lib/xmodule/xmodule/seq_module.py

@@ -96,7 +96,7 @@ class SequenceModule(SequenceFields, XModule):
                 'progress_status': Progress.to_js_status_str(progress),
                 'progress_detail': Progress.to_js_detail_str(progress),
                 'type': child.get_icon_class(),
-                'id': child.id,
+                'id': child.scope_ids.usage_id.to_deprecated_string(),
             }
             if childinfo['title'] == '':
                 childinfo['title'] = child.display_name_with_default
@@ -104,7 +104,7 @@ class SequenceModule(SequenceFields, XModule):
 
         params = {'items': contents,
                   'element_id': self.location.html_id(),
-                  'item_id': self.id,
+                  'item_id': self.location.to_deprecated_string(),
                   'position': self.position,
                   'tag': self.location.category,
                   'ajax_url': self.system.ajax_url,

common/lib/xmodule/xmodule/split_test_module.py

@@ -82,7 +82,7 @@ class SplitTestModule(SplitTestFields, XModule):
         # we've picked a choice.  Use self.descriptor.get_children() instead.
 
         for child in self.descriptor.get_children():
-            if child.location.url() == location:
+            if child.location == location:
                 return child
 
         return None
@@ -182,7 +182,7 @@ class SplitTestModule(SplitTestFields, XModule):
             fragment.add_frag_resources(rendered_child)
 
             contents.append({
-                'id': child.id,
+                'id': child.location.to_deprecated_string(),
                 'content': rendered_child.content
             })
 
@@ -252,7 +252,11 @@ class SplitTestDescriptor(SplitTestFields, SequenceDescriptor):
     def definition_to_xml(self, resource_fs):
 
         xml_object = etree.Element('split_test')
-        xml_object.set('group_id_to_child', json.dumps(self.group_id_to_child))
+        renderable_groups = {}
+        # json.dumps doesn't know how to handle Location objects
+        for group in self.group_id_to_child:
+            renderable_groups[group] = self.group_id_to_child[group].to_deprecated_string()
+        xml_object.set('group_id_to_child', json.dumps(renderable_groups))
         xml_object.set('user_partition_id', str(self.user_partition_id))
         for child in self.get_children():
             self.runtime.add_block_as_child_node(child, xml_object)

common/lib/xmodule/xmodule/tabs.py

@@ -456,7 +456,7 @@ class StaticTab(CourseTab):
         super(StaticTab, self).__init__(
             name=tab_dict['name'] if tab_dict else name,
             tab_id='static_tab_{0}'.format(self.url_slug),
-            link_func=lambda course, reverse_func: reverse_func(self.type, args=[course.id, self.url_slug]),
+            link_func=lambda course, reverse_func: reverse_func(self.type, args=[course.id.to_deprecated_string(), self.url_slug]),
         )
 
     def __getitem__(self, key):
@@ -537,7 +537,7 @@ class TextbookTabs(TextbookTabsBase):
             yield SingleTextbookTab(
                 name=textbook.title,
                 tab_id='textbook/{0}'.format(index),
-                link_func=lambda course, reverse_func: reverse_func('book', args=[course.id, index]),
+                link_func=lambda course, reverse_func: reverse_func('book', args=[course.id.to_deprecated_string(), index]),
             )
 
 
@@ -557,7 +557,7 @@ class PDFTextbookTabs(TextbookTabsBase):
             yield SingleTextbookTab(
                 name=textbook['tab_title'],
                 tab_id='pdftextbook/{0}'.format(index),
-                link_func=lambda course, reverse_func: reverse_func('pdf_book', args=[course.id, index]),
+                link_func=lambda course, reverse_func: reverse_func('pdf_book', args=[course.id.to_deprecated_string(), index]),
             )
 
 
@@ -577,7 +577,7 @@ class HtmlTextbookTabs(TextbookTabsBase):
             yield SingleTextbookTab(
                 name=textbook['tab_title'],
                 tab_id='htmltextbook/{0}'.format(index),
-                link_func=lambda course, reverse_func: reverse_func('html_book', args=[course.id, index]),
+                link_func=lambda course, reverse_func: reverse_func('html_book', args=[course.id.to_deprecated_string(), index]),
             )
 
 
@@ -884,7 +884,7 @@ def link_reverse_func(reverse_name):
     Returns a function that takes in a course and reverse_url_func,
     and calls the reverse_url_func with the given reverse_name and course' ID.
     """
-    return lambda course, reverse_url_func: reverse_url_func(reverse_name, args=[course.id])
+    return lambda course, reverse_url_func: reverse_url_func(reverse_name, args=[course.id.to_deprecated_string()])
 
 
 def link_value_func(value):

common/lib/xmodule/xmodule/tests/__init__.py

@@ -16,12 +16,13 @@ from mock import Mock
 from path import path
 
 from xblock.field_data import DictFieldData
+from xblock.fields import ScopeIds
 
 from xmodule.x_module import ModuleSystem, XModuleDescriptor, XModuleMixin
 from xmodule.modulestore.inheritance import InheritanceMixin
+from xmodule.modulestore.locations import SlashSeparatedCourseKey
 from xmodule.mako_module import MakoDescriptorSystem
 from xmodule.error_module import ErrorDescriptor
-from xmodule.modulestore.xml import LocationReader
 
 
 MODULE_DIR = path(__file__).dirname()
@@ -45,13 +46,21 @@ class TestModuleSystem(ModuleSystem):  # pylint: disable=abstract-method
     ModuleSystem for testing
     """
     def handler_url(self, block, handler, suffix='', query='', thirdparty=False):
-        return str(block.scope_ids.usage_id) + '/' + handler + '/' + suffix + '?' + query
+        return '{usage_id}/{handler}{suffix}?{query}'.format(
+            usage_id=block.scope_ids.usage_id.to_deprecated_string(),
+            handler=handler,
+            suffix=suffix,
+            query=query,
+        )
 
     def local_resource_url(self, block, uri):
-        return 'resource/' + str(block.scope_ids.block_type) + '/' + uri
+        return 'resource/{usage_id}/{uri}'.format(
+            usage_id=block.scope_ids.usage_id.to_deprecated_string(),
+            uri=uri,
+        )
 
 
-def get_test_system(course_id=''):
+def get_test_system(course_id=SlashSeparatedCourseKey('org', 'course', 'run')):
     """
     Construct a test ModuleSystem instance.
 
@@ -96,7 +105,6 @@ def get_test_descriptor_system():
         render_template=mock_render_template,
         mixins=(InheritanceMixin, XModuleMixin),
         field_data=DictFieldData({}),
-        id_reader=LocationReader(),
     )
 
 
@@ -131,12 +139,15 @@ class LogicTest(unittest.TestCase):
             url_name = ''
             category = 'test'
 
-        self.system = get_test_system(course_id='test/course/id')
+        self.system = get_test_system()
         self.descriptor = EmptyClass()
 
         self.xmodule_class = self.descriptor_class.module_class
+        usage_key = self.system.course_id.make_usage_key(self.descriptor.category, 'test_loc')
+        # ScopeIds has 4 fields: user_id, block_type, def_id, usage_id
+        scope_ids = ScopeIds(1, self.descriptor.category, usage_key, usage_key)
         self.xmodule = self.xmodule_class(
-            self.descriptor, self.system, DictFieldData(self.raw_field_data), Mock()
+            self.descriptor, self.system, DictFieldData(self.raw_field_data), scope_ids
         )
 
     def ajax_request(self, dispatch, data):

common/lib/xmodule/xmodule/tests/test_annotatable_module.py

@@ -35,7 +35,7 @@ class AnnotatableModuleTestCase(unittest.TestCase):
             Mock(),
             get_test_system(),
             DictFieldData({'data': self.sample_xml}),
-            ScopeIds(None, None, None, None)
+            ScopeIds(None, None, None, Location('org', 'course', 'run', 'category', 'name', None))
         )
 
     def test_annotation_data_attr(self):

common/lib/xmodule/xmodule/tests/test_capa_module.py

@@ -101,8 +101,14 @@ class CapaFactory(object):
 
             attempts: also added to instance state.  Will be converted to an int.
         """
-        location = Location(["i4x", "edX", "capa_test", "problem",
-                             "SampleProblem{0}".format(cls.next_num())])
+        location = Location(
+            "edX",
+            "capa_test",
+            "2012_Fall",
+            "problem",
+            "SampleProblem{0}".format(cls.next_num()),
+            None
+        )
         if xml is None:
             xml = cls.sample_problem_xml
         field_data = {'data': xml}

common/lib/xmodule/xmodule/tests/test_conditional.py

-from ast import literal_eval
 import json
 import unittest
 
@@ -8,7 +7,7 @@ from mock import Mock, patch
 from xblock.field_data import DictFieldData
 from xblock.fields import ScopeIds
 from xmodule.error_module import NonStaffErrorDescriptor
-from xmodule.modulestore import Location
+from xmodule.modulestore.locations import SlashSeparatedCourseKey, Location
 from xmodule.modulestore.xml import ImportSystem, XMLModuleStore, CourseLocationGenerator
 from xmodule.conditional_module import ConditionalDescriptor
 from xmodule.tests import DATA_DIR, get_test_system, get_test_descriptor_system
@@ -54,13 +53,13 @@ class ConditionalFactory(object):
         descriptor_system = get_test_descriptor_system()
 
         # construct source descriptor and module:
-        source_location = Location(["i4x", "edX", "conditional_test", "problem", "SampleProblem"])
+        source_location = Location("edX", "conditional_test", "test_run", "problem", "SampleProblem", None)
         if source_is_error_module:
             # Make an error descriptor and module
             source_descriptor = NonStaffErrorDescriptor.from_xml(
                 'some random xml data',
                 system,
-                id_generator=CourseLocationGenerator(source_location.org, source_location.course),
+                id_generator=CourseLocationGenerator(SlashSeparatedCourseKey('edX', 'conditional_test', 'test_run')),
                 error_msg='random error message'
             )
         else:
@@ -78,15 +77,19 @@ class ConditionalFactory(object):
         child_descriptor.runtime = descriptor_system
         child_descriptor.xmodule_runtime = get_test_system()
         child_descriptor.render = lambda view, context=None: descriptor_system.render(child_descriptor, view, context)
+        child_descriptor.location = source_location.replace(category='html', name='child')
 
-        descriptor_system.load_item = {'child': child_descriptor, 'source': source_descriptor}.get
+        descriptor_system.load_item = {
+            child_descriptor.location: child_descriptor,
+            source_location: source_descriptor
+        }.get
 
         # construct conditional module:
-        cond_location = Location(["i4x", "edX", "conditional_test", "conditional", "SampleConditional"])
+        cond_location = Location("edX", "conditional_test", "test_run", "conditional", "SampleConditional", None)
         field_data = DictFieldData({
             'data': '<conditional/>',
             'xml_attributes': {'attempted': 'true'},
-            'children': ['child'],
+            'children': [child_descriptor.location],
         })
 
         cond_descriptor = ConditionalDescriptor(
@@ -130,7 +133,6 @@ class ConditionalModuleBasicTest(unittest.TestCase):
         expected = modules['cond_module'].xmodule_runtime.render_template('conditional_ajax.html', {
             'ajax_url': modules['cond_module'].xmodule_runtime.ajax_url,
             'element_id': u'i4x-edX-conditional_test-conditional-SampleConditional',
-            'id': u'i4x://edX/conditional_test/conditional/SampleConditional',
             'depends': u'i4x-edX-conditional_test-problem-SampleProblem',
         })
         self.assertEquals(expected, html)
@@ -198,14 +200,14 @@ class ConditionalModuleXmlTest(unittest.TestCase):
         def inner_get_module(descriptor):
             if isinstance(descriptor, Location):
                 location = descriptor
-                descriptor = self.modulestore.get_instance(course.id, location, depth=None)
+                descriptor = self.modulestore.get_item(location, depth=None)
             descriptor.xmodule_runtime = get_test_system()
             descriptor.xmodule_runtime.get_module = inner_get_module
             return descriptor
 
         # edx - HarvardX
         # cond_test - ER22x
-        location = Location(["i4x", "HarvardX", "ER22x", "conditional", "condone"])
+        location = Location("HarvardX", "ER22x", "2013_Spring", "conditional", "condone")
 
         def replace_urls(text, staticfiles_prefix=None, replace_prefix='/static/', course_namespace=None):
             return text
@@ -224,9 +226,8 @@ class ConditionalModuleXmlTest(unittest.TestCase):
             'conditional_ajax.html',
             {
                 # Test ajax url is just usage-id / handler_name
-                'ajax_url': 'i4x://HarvardX/ER22x/conditional/condone/xmodule_handler',
+                'ajax_url': '{}/xmodule_handler'.format(location.to_deprecated_string()),
                 'element_id': u'i4x-HarvardX-ER22x-conditional-condone',
-                'id': u'i4x://HarvardX/ER22x/conditional/condone',
                 'depends': u'i4x-HarvardX-ER22x-problem-choiceprob'
             }
         )
@@ -242,7 +243,7 @@ class ConditionalModuleXmlTest(unittest.TestCase):
         self.assertFalse(any(['This is a secret' in item for item in html]))
 
         # Now change state of the capa problem to make it completed
-        inner_module = inner_get_module(Location('i4x://HarvardX/ER22x/problem/choiceprob'))
+        inner_module = inner_get_module(location.replace(category="problem", name='choiceprob'))
         inner_module.attempts = 1
         # Save our modifications to the underlying KeyValueStore so they can be persisted
         inner_module.save()

common/lib/xmodule/xmodule/tests/test_content.py

 import unittest
 from xmodule.contentstore.content import StaticContent
 from xmodule.contentstore.content import ContentStore
-from xmodule.modulestore import Location
+from xmodule.modulestore.locations import SlashSeparatedCourseKey, AssetLocation
 
 
 class Content:
@@ -21,18 +21,28 @@ class ContentTest(unittest.TestCase):
         self.assertIsNone(content.thumbnail_location)
 
     def test_static_url_generation_from_courseid(self):
-        url = StaticContent.convert_legacy_static_url_with_course_id('images_course_image.jpg', 'foo/bar/bz')
+        course_key = SlashSeparatedCourseKey('foo', 'bar', 'bz')
+        url = StaticContent.convert_legacy_static_url_with_course_id('images_course_image.jpg', course_key)
         self.assertEqual(url, '/c4x/foo/bar/asset/images_course_image.jpg')
 
     def test_generate_thumbnail_image(self):
         contentStore = ContentStore()
-        content = Content(Location(u'c4x', u'mitX', u'800', u'asset', u'monsters__.jpg'), None)
+        content = Content(AssetLocation(u'mitX', u'800', u'ignore_run', u'asset', u'monsters__.jpg'), None)
         (thumbnail_content, thumbnail_file_location) = contentStore.generate_thumbnail(content)
         self.assertIsNone(thumbnail_content)
-        self.assertEqual(Location(u'c4x', u'mitX', u'800', u'thumbnail', u'monsters__.jpg'), thumbnail_file_location)
+        self.assertEqual(AssetLocation(u'mitX', u'800', u'ignore_run', u'thumbnail', u'monsters__.jpg'), thumbnail_file_location)
 
     def test_compute_location(self):
         # We had a bug that __ got converted into a single _. Make sure that substitution of INVALID_CHARS (like space)
         # still happen.
-        asset_location = StaticContent.compute_location('mitX', '400', 'subs__1eo_jXvZnE .srt.sjson')
-        self.assertEqual(Location(u'c4x', u'mitX', u'400', u'asset', u'subs__1eo_jXvZnE_.srt.sjson', None), asset_location)
+        asset_location = StaticContent.compute_location(
+            SlashSeparatedCourseKey('mitX', '400', 'ignore'), 'subs__1eo_jXvZnE .srt.sjson'
+        )
+        self.assertEqual(AssetLocation(u'mitX', u'400', u'ignore', u'asset', u'subs__1eo_jXvZnE_.srt.sjson', None), asset_location)
+
+    def test_get_location_from_path(self):
+        asset_location = StaticContent.get_location_from_path(u'/c4x/foo/bar/asset/images_course_image.jpg')
+        self.assertEqual(
+            AssetLocation(u'foo', u'bar', None, u'asset', u'images_course_image.jpg', None),
+            asset_location
+        )

common/lib/xmodule/xmodule/tests/test_course_module.py

@@ -8,7 +8,8 @@ from mock import Mock, patch
 from xblock.runtime import KvsFieldData, DictKeyValueStore
 
 import xmodule.course_module
-from xmodule.modulestore.xml import ImportSystem, XMLModuleStore, LocationReader
+from xmodule.modulestore.xml import ImportSystem, XMLModuleStore
+from xmodule.modulestore.locations import SlashSeparatedCourseKey
 from django.utils.timezone import UTC
 
 
@@ -32,7 +33,7 @@ class DummySystem(ImportSystem):
 
         xmlstore = XMLModuleStore("data_dir", course_dirs=[],
                                   load_error_modules=load_error_modules)
-        course_id = "/".join([ORG, COURSE, 'test_run'])
+        course_id = SlashSeparatedCourseKey(ORG, COURSE, 'test_run')
         course_dir = "test_dir"
         error_tracker = Mock()
         parent_tracker = Mock()
@@ -45,7 +46,6 @@ class DummySystem(ImportSystem):
             parent_tracker=parent_tracker,
             load_error_modules=load_error_modules,
             field_data=KvsFieldData(DictKeyValueStore()),
-            id_reader=LocationReader(),
         )
 
 

common/lib/xmodule/xmodule/tests/test_delay_between_attempts.py

@@ -84,8 +84,7 @@ class CapaFactoryWithDelay(object):
         """
         Optional parameters here are cut down to what we actually use vs. the regular CapaFactory.
         """
-        location = Location(["i4x", "edX", "capa_test", "problem",
-                             "SampleProblem{0}".format(cls.next_num())])
+        location = Location("edX", "capa_test", "run", "problem", "SampleProblem{0}".format(cls.next_num()))
         field_data = {'data': cls.sample_problem_xml}
 
         if max_attempts is not None:

common/lib/xmodule/xmodule/tests/test_editing_module.py

@@ -5,6 +5,7 @@ import logging
 
 from mock import Mock
 from pkg_resources import resource_string
+from xmodule.modulestore.locations import Location
 from xmodule.editing_module import TabsEditingDescriptor
 from xblock.field_data import DictFieldData
 from xblock.fields import ScopeIds
@@ -46,7 +47,7 @@ class TabsEditingDescriptorTestCase(unittest.TestCase):
         TabsEditingDescriptor.tabs = self.tabs
         self.descriptor = system.construct_xblock_from_class(
             TabsEditingDescriptor,
-            scope_ids=ScopeIds(None, None, None, None),
+            scope_ids=ScopeIds(None, None, None, Location('org', 'course', 'run', 'category', 'name', 'revision')),
             field_data=DictFieldData({}),
         )
 

common/lib/xmodule/xmodule/tests/test_error_module.py

@@ -4,8 +4,8 @@ Tests for ErrorModule and NonStaffErrorModule
 import unittest
 from xmodule.tests import get_test_system
 from xmodule.error_module import ErrorDescriptor, ErrorModule, NonStaffErrorDescriptor
-from xmodule.modulestore import Location
 from xmodule.modulestore.xml import CourseLocationGenerator
+from xmodule.modulestore.locations import SlashSeparatedCourseKey, Location
 from xmodule.x_module import XModuleDescriptor, XModule
 from mock import MagicMock, Mock, patch
 from xblock.runtime import Runtime, IdReader
@@ -17,9 +17,8 @@ from xblock.test.tools import unabc
 class SetupTestErrorModules():
     def setUp(self):
         self.system = get_test_system()
-        self.org = "org"
-        self.course = "course"
-        self.location = Location(['i4x', self.org, self.course, None, None])
+        self.course_id = SlashSeparatedCourseKey('org', 'course', 'run')
+        self.location = self.course_id.make_usage_key('foo', 'bar')
         self.valid_xml = u"<problem>ABC \N{SNOWMAN}</problem>"
         self.error_msg = "Error"
 
@@ -35,7 +34,7 @@ class TestErrorModule(unittest.TestCase, SetupTestErrorModules):
         descriptor = ErrorDescriptor.from_xml(
             self.valid_xml,
             self.system,
-            CourseLocationGenerator(self.org, self.course),
+            CourseLocationGenerator(self.course_id),
             self.error_msg
         )
         self.assertIsInstance(descriptor, ErrorDescriptor)
@@ -70,7 +69,7 @@ class TestNonStaffErrorModule(unittest.TestCase, SetupTestErrorModules):
         descriptor = NonStaffErrorDescriptor.from_xml(
             self.valid_xml,
             self.system,
-            CourseLocationGenerator(self.org, self.course)
+            CourseLocationGenerator(self.course_id)
         )
         self.assertIsInstance(descriptor, NonStaffErrorDescriptor)
 
@@ -78,7 +77,7 @@ class TestNonStaffErrorModule(unittest.TestCase, SetupTestErrorModules):
         descriptor = NonStaffErrorDescriptor.from_xml(
             self.valid_xml,
             self.system,
-            CourseLocationGenerator(self.org, self.course)
+            CourseLocationGenerator(self.course_id)
         )
         descriptor.xmodule_runtime = self.system
         context_repr = self.system.render(descriptor, 'student_view').content
@@ -130,7 +129,7 @@ class TestErrorModuleConstruction(unittest.TestCase):
         self.descriptor = BrokenDescriptor(
             TestRuntime(Mock(spec=IdReader), field_data),
             field_data,
-            ScopeIds(None, None, None, 'i4x://org/course/broken/name')
+            ScopeIds(None, None, None, Location('org', 'course', 'run', 'broken', 'name', None))
         )
         self.descriptor.xmodule_runtime = TestRuntime(Mock(spec=IdReader), field_data)
         self.descriptor.xmodule_runtime.error_descriptor_class = ErrorDescriptor

common/lib/xmodule/xmodule/tests/test_export.py

@@ -36,7 +36,7 @@ def strip_filenames(descriptor):
     """
     Recursively strips 'filename' from all children's definitions.
     """
-    print("strip filename from {desc}".format(desc=descriptor.location.url()))
+    print("strip filename from {desc}".format(desc=descriptor.location.to_deprecated_string()))
     if descriptor._field_data.has(descriptor, 'filename'):
         descriptor._field_data.delete(descriptor, 'filename')
 
@@ -173,11 +173,11 @@ class TestEdxJsonEncoder(unittest.TestCase):
         self.null_utc_tz = NullTZ()
 
     def test_encode_location(self):
-        loc = Location('i4x', 'org', 'course', 'category', 'name')
-        self.assertEqual(loc.url(), self.encoder.default(loc))
+        loc = Location('org', 'course', 'run', 'category', 'name', None)
+        self.assertEqual(loc.to_deprecated_string(), self.encoder.default(loc))
 
-        loc = Location('i4x', 'org', 'course', 'category', 'name', 'version')
-        self.assertEqual(loc.url(), self.encoder.default(loc))
+        loc = Location('org', 'course', 'run', 'category', 'name', 'version')
+        self.assertEqual(loc.to_deprecated_string(), self.encoder.default(loc))
 
     def test_encode_naive_datetime(self):
         self.assertEqual(

common/lib/xmodule/xmodule/tests/test_import.py

@@ -12,12 +12,13 @@ from django.utils.timezone import UTC
 
 from xmodule.xml_module import is_pointer_tag
 from xmodule.modulestore import Location, only_xmodules
-from xmodule.modulestore.xml import ImportSystem, XMLModuleStore, LocationReader
+from xmodule.modulestore.xml import ImportSystem, XMLModuleStore
 from xmodule.modulestore.inheritance import compute_inherited_metadata
 from xmodule.x_module import XModuleMixin
 from xmodule.fields import Date
 from xmodule.tests import DATA_DIR
 from xmodule.modulestore.inheritance import InheritanceMixin
+from xmodule.modulestore.locations import SlashSeparatedCourseKey
 
 from xblock.core import XBlock
 from xblock.fields import Scope, String, Integer
@@ -34,7 +35,7 @@ class DummySystem(ImportSystem):
     def __init__(self, load_error_modules):
 
         xmlstore = XMLModuleStore("data_dir", course_dirs=[], load_error_modules=load_error_modules)
-        course_id = "/".join([ORG, COURSE, 'test_run'])
+        course_id = SlashSeparatedCourseKey(ORG, COURSE, 'test_run')
         course_dir = "test_dir"
         error_tracker = Mock()
         parent_tracker = Mock()
@@ -48,7 +49,6 @@ class DummySystem(ImportSystem):
             load_error_modules=load_error_modules,
             mixins=(InheritanceMixin, XModuleMixin),
             field_data=KvsFieldData(DictKeyValueStore()),
-            id_reader=LocationReader(),
         )
 
     def render_template(self, _template, _context):
@@ -343,7 +343,7 @@ class ImportTestCase(BaseCourseTestCase):
 
         def check_for_key(key, node, value):
             "recursive check for presence of key"
-            print("Checking {0}".format(node.location.url()))
+            print("Checking {0}".format(node.location.to_deprecated_string()))
             self.assertEqual(getattr(node, key), value)
             for c in node.get_children():
                 check_for_key(key, c, value)
@@ -383,12 +383,10 @@ class ImportTestCase(BaseCourseTestCase):
 
         modulestore = XMLModuleStore(DATA_DIR, course_dirs=['toy', 'two_toys'])
 
-        toy_id = "edX/toy/2012_Fall"
-        two_toy_id = "edX/toy/TT_2012_Fall"
-
-        location = Location(["i4x", "edX", "toy", "video", "Welcome"])
-        toy_video = modulestore.get_instance(toy_id, location)
-        two_toy_video = modulestore.get_instance(two_toy_id, location)
+        location = Location("edX", "toy", "2012_Fall", "video", "Welcome", None)
+        toy_video = modulestore.get_item(location)
+        location_two = Location("edX", "toy", "TT_2012_Fall", "video", "Welcome", None)
+        two_toy_video = modulestore.get_item(location_two)
         self.assertEqual(toy_video.youtube_id_1_0, "p2Q6BrNhdh8")
         self.assertEqual(two_toy_video.youtube_id_1_0, "p2Q6BrNhdh9")
 
@@ -401,10 +399,9 @@ class ImportTestCase(BaseCourseTestCase):
         courses = modulestore.get_courses()
         self.assertEquals(len(courses), 1)
         course = courses[0]
-        course_id = course.id
 
         print("course errors:")
-        for (msg, err) in modulestore.get_item_errors(course.location):
+        for (msg, err) in modulestore.get_course_errors(course.id):
             print(msg)
             print(err)
 
@@ -416,13 +413,12 @@ class ImportTestCase(BaseCourseTestCase):
 
         print("Ch2 location: ", ch2.location)
 
-        also_ch2 = modulestore.get_instance(course_id, ch2.location)
+        also_ch2 = modulestore.get_item(ch2.location)
         self.assertEquals(ch2, also_ch2)
 
         print("making sure html loaded")
-        cloc = course.location
-        loc = Location(cloc.tag, cloc.org, cloc.course, 'html', 'secret:toylab')
-        html = modulestore.get_instance(course_id, loc)
+        loc = course.id.make_usage_key('html', 'secret:toylab')
+        html = modulestore.get_item(loc)
         self.assertEquals(html.display_name, "Toy lab")
 
     def test_unicode(self):
@@ -442,12 +438,16 @@ class ImportTestCase(BaseCourseTestCase):
 
         # Expect to find an error/exception about characters in "®esources"
         expect = "Invalid characters"
-        errors = [(msg.encode("utf-8"), err.encode("utf-8"))
-                    for msg, err in
-                    modulestore.get_item_errors(course.location)]
-
-        self.assertTrue(any(expect in msg or expect in err
-            for msg, err in errors))
+        errors = [
+            (msg.encode("utf-8"), err.encode("utf-8"))
+            for msg, err
+            in modulestore.get_course_errors(course.id)
+        ]
+
+        self.assertTrue(any(
+            expect in msg or expect in err
+            for msg, err in errors
+        ))
         chapters = course.get_children()
         self.assertEqual(len(chapters), 4)
 
@@ -458,7 +458,7 @@ class ImportTestCase(BaseCourseTestCase):
 
         modulestore = XMLModuleStore(DATA_DIR, course_dirs=['toy'])
 
-        toy_id = "edX/toy/2012_Fall"
+        toy_id = SlashSeparatedCourseKey('edX', 'toy', '2012_Fall')
 
         course = modulestore.get_course(toy_id)
         chapters = course.get_children()
@@ -484,20 +484,12 @@ class ImportTestCase(BaseCourseTestCase):
 
         self.assertEqual(len(sections), 1)
 
-        location = course.location
-
-        conditional_location = Location(
-            location.tag, location.org, location.course,
-            'conditional', 'condone'
-        )
-        module = modulestore.get_instance(course.id, conditional_location)
+        conditional_location = course.id.make_usage_key('conditional', 'condone')
+        module = modulestore.get_item(conditional_location)
         self.assertEqual(len(module.children), 1)
 
-        poll_location = Location(
-            location.tag, location.org, location.course,
-            'poll_question', 'first_poll'
-        )
-        module = modulestore.get_instance(course.id, poll_location)
+        poll_location = course.id.make_usage_key('poll_question', 'first_poll')
+        module = modulestore.get_item(poll_location)
         self.assertEqual(len(module.get_children()), 0)
         self.assertEqual(module.voted, False)
         self.assertEqual(module.poll_answer, '')
@@ -527,9 +519,9 @@ class ImportTestCase(BaseCourseTestCase):
         '''
         modulestore = XMLModuleStore(DATA_DIR, course_dirs=['graphic_slider_tool'])
 
-        sa_id = "edX/gst_test/2012_Fall"
-        location = Location(["i4x", "edX", "gst_test", "graphical_slider_tool", "sample_gst"])
-        gst_sample = modulestore.get_instance(sa_id, location)
+        sa_id = SlashSeparatedCourseKey("edX", "gst_test", "2012_Fall")
+        location = sa_id.make_usage_key("graphical_slider_tool", "sample_gst")
+        gst_sample = modulestore.get_item(location)
         render_string_from_sample_gst_xml = """
         <slider var="a" style="width:400px;float:left;"/>\
 <plot style="margin-top:15px;margin-bottom:15px;"/>""".strip()
@@ -545,12 +537,8 @@ class ImportTestCase(BaseCourseTestCase):
 
         self.assertEqual(len(sections), 1)
 
-        location = course.location
-        location = Location(
-            location.tag, location.org, location.course,
-            'word_cloud', 'cloud1'
-        )
-        module = modulestore.get_instance(course.id, location)
+        location = course.id.make_usage_key('word_cloud', 'cloud1')
+        module = modulestore.get_item(location)
         self.assertEqual(len(module.get_children()), 0)
         self.assertEqual(module.num_inputs, 5)
         self.assertEqual(module.num_top_words, 250)
@@ -561,7 +549,7 @@ class ImportTestCase(BaseCourseTestCase):
         """
         modulestore = XMLModuleStore(DATA_DIR, course_dirs=['toy'])
 
-        toy_id = "edX/toy/2012_Fall"
+        toy_id = SlashSeparatedCourseKey('edX', 'toy', '2012_Fall')
 
         course = modulestore.get_course(toy_id)
 

common/lib/xmodule/xmodule/tests/test_import_static.py

@@ -3,8 +3,8 @@ Tests that check that we ignore the appropriate files when importing courses.
 """
 import unittest
 from mock import Mock
-from xmodule.modulestore import Location
 from xmodule.modulestore.xml_importer import import_static_content
+from xmodule.modulestore.locations import SlashSeparatedCourseKey
 from xmodule.tests import DATA_DIR
 
 
@@ -12,10 +12,10 @@ class IgnoredFilesTestCase(unittest.TestCase):
     "Tests for ignored files"
     def test_ignore_tilde_static_files(self):
         course_dir = DATA_DIR / "tilde"
-        loc = Location("edX", "tilde", "Fall_2012")
+        course_id = SlashSeparatedCourseKey("edX", "tilde", "Fall_2012")
         content_store = Mock()
         content_store.generate_thumbnail.return_value = ("content", "location")
-        import_static_content(Mock(), Mock(), course_dir, content_store, loc)
+        import_static_content(course_dir, content_store, course_id)
         saved_static_content = [call[0][0] for call in content_store.save.call_args_list]
         name_val = {sc.name: sc.data for sc in saved_static_content}
         self.assertIn("example.txt", name_val)

common/lib/xmodule/xmodule/tests/test_lti_unit.py

@@ -262,26 +262,22 @@ class LTIModuleTest(LogicTest):
             self.assertEqual(real_resource_link_id, expected_resource_link_id)
 
     def test_lis_result_sourcedid(self):
-        with patch('xmodule.lti_module.LTIModule.location', new_callable=PropertyMock) as mock_location:
-            self.xmodule.location.html_id = lambda: 'i4x-2-3-lti-31de800015cf4afb973356dbe81496df'
-            expected_sourcedId = u':'.join(urllib.quote(i) for i in (
-                self.system.course_id,
-                urllib.quote(self.unquoted_resource_link_id),
-                self.user_id
-            ))
-            real_lis_result_sourcedid = self.xmodule.get_lis_result_sourcedid()
-            self.assertEqual(real_lis_result_sourcedid, expected_sourcedId)
-
+        expected_sourcedId = u':'.join(urllib.quote(i) for i in (
+            self.system.course_id.to_deprecated_string(),
+            self.xmodule.get_resource_link_id(),
+            self.user_id
+        ))
+        real_lis_result_sourcedid = self.xmodule.get_lis_result_sourcedid()
+        self.assertEqual(real_lis_result_sourcedid, expected_sourcedId)
 
-    @patch('xmodule.course_module.CourseDescriptor.id_to_location')
-    def test_client_key_secret(self, test):
+    def test_client_key_secret(self):
         """
         LTI module gets client key and secret provided.
         """
         #this adds lti passports to system
         mocked_course = Mock(lti_passports = ['lti_id:test_client:test_secret'])
         modulestore = Mock()
-        modulestore.get_item.return_value = mocked_course
+        modulestore.get_course.return_value = mocked_course
         runtime = Mock(modulestore=modulestore)
         self.xmodule.descriptor.runtime = runtime
         self.xmodule.lti_id = "lti_id"
@@ -289,8 +285,7 @@ class LTIModuleTest(LogicTest):
         expected = ('test_client', 'test_secret')
         self.assertEqual(expected, (key, secret))
 
-    @patch('xmodule.course_module.CourseDescriptor.id_to_location')
-    def test_client_key_secret_not_provided(self, test):
+    def test_client_key_secret_not_provided(self):
         """
         LTI module attempts to get client key and secret provided in cms.
 
@@ -300,7 +295,7 @@ class LTIModuleTest(LogicTest):
         #this adds lti passports to system
         mocked_course = Mock(lti_passports = ['test_id:test_client:test_secret'])
         modulestore = Mock()
-        modulestore.get_item.return_value = mocked_course
+        modulestore.get_course.return_value = mocked_course
         runtime = Mock(modulestore=modulestore)
         self.xmodule.descriptor.runtime = runtime
         #set another lti_id
@@ -309,8 +304,7 @@ class LTIModuleTest(LogicTest):
         expected = ('','')
         self.assertEqual(expected, key_secret)
 
-    @patch('xmodule.course_module.CourseDescriptor.id_to_location')
-    def test_bad_client_key_secret(self, test):
+    def test_bad_client_key_secret(self):
         """
         LTI module attempts to get client key and secret provided in cms.
 
@@ -319,16 +313,16 @@ class LTIModuleTest(LogicTest):
         #this adds lti passports to system
         mocked_course = Mock(lti_passports = ['test_id_test_client_test_secret'])
         modulestore = Mock()
-        modulestore.get_item.return_value = mocked_course
+        modulestore.get_course.return_value = mocked_course
         runtime = Mock(modulestore=modulestore)
         self.xmodule.descriptor.runtime = runtime
         self.xmodule.lti_id = 'lti_id'
         with self.assertRaises(LTIError):
             self.xmodule.get_client_key_secret()
 
-    @patch('xmodule.lti_module.signature.verify_hmac_sha1', return_value=True)
-    @patch('xmodule.lti_module.LTIModule.get_client_key_secret', return_value=('test_client_key', u'test_client_secret'))
-    def test_successful_verify_oauth_body_sign(self, get_key_secret, mocked_verify):
+    @patch('xmodule.lti_module.signature.verify_hmac_sha1', Mock(return_value=True))
+    @patch('xmodule.lti_module.LTIModule.get_client_key_secret', Mock(return_value=('test_client_key', u'test_client_secret')))
+    def test_successful_verify_oauth_body_sign(self):
         """
         Test if OAuth signing was successful.
         """
@@ -337,9 +331,9 @@ class LTIModuleTest(LogicTest):
         except LTIError:
             self.fail("verify_oauth_body_sign() raised LTIError unexpectedly!")
 
-    @patch('xmodule.lti_module.signature.verify_hmac_sha1', return_value=False)
-    @patch('xmodule.lti_module.LTIModule.get_client_key_secret', return_value=('test_client_key', u'test_client_secret'))
-    def test_failed_verify_oauth_body_sign(self, get_key_secret, mocked_verify):
+    @patch('xmodule.lti_module.signature.verify_hmac_sha1', Mock(return_value=False))
+    @patch('xmodule.lti_module.LTIModule.get_client_key_secret', Mock(return_value=('test_client_key', u'test_client_secret')))
+    def test_failed_verify_oauth_body_sign(self):
         """
         Oauth signing verify fail.
         """
@@ -411,4 +405,4 @@ class LTIModuleTest(LogicTest):
         """
         Tests that LTI parameter context_id is equal to course_id.
         """
-        self.assertEqual(self.system.course_id, self.xmodule.context_id)
+        self.assertEqual(self.system.course_id.to_deprecated_string(), self.xmodule.context_id)

common/lib/xmodule/xmodule/tests/test_peer_grading.py

@@ -7,7 +7,7 @@ from webob.multidict import MultiDict
 from xblock.field_data import DictFieldData
 from xblock.fields import ScopeIds
 
-from xmodule.modulestore import Location
+from xmodule.modulestore.locations import Location, SlashSeparatedCourseKey
 from xmodule.tests import get_test_system, get_test_descriptor_system
 from xmodule.tests.test_util_open_ended import DummyModulestore
 from xmodule.open_ended_grading_classes.peer_grading_service import MockPeerGradingService
@@ -16,20 +16,17 @@ from xmodule.modulestore.exceptions import ItemNotFoundError, NoPathToItem
 
 log = logging.getLogger(__name__)
 
-ORG = "edX"
-COURSE = "open_ended"
-
 
 class PeerGradingModuleTest(unittest.TestCase, DummyModulestore):
     """
     Test peer grading xmodule at the unit level.  More detailed tests are difficult, as the module relies on an
     external grading service.
     """
-    problem_location = Location(["i4x", "edX", "open_ended", "peergrading",
-                                 "PeerGradingSample"])
-    coe_location = Location(["i4x", "edX", "open_ended", "combinedopenended", "SampleQuestion"])
+    course_id = SlashSeparatedCourseKey('edX', 'open_ended', '2012_Fall')
+    problem_location = course_id.make_usage_key("peergrading", "PeerGradingSample")
+    coe_location = course_id.make_usage_key("combinedopenended", "SampleQuestion")
     calibrated_dict = {'location': "blah"}
-    coe_dict = {'location': coe_location.url()}
+    coe_dict = {'location': coe_location.to_deprecated_string()}
     save_dict = MultiDict({
         'location': "blah",
         'submission_id': 1,
@@ -42,7 +39,7 @@ class PeerGradingModuleTest(unittest.TestCase, DummyModulestore):
     save_dict.extend(('rubric_scores[]', val) for val in (0, 1))
 
     def get_module_system(self, descriptor):
-        test_system = get_test_system()
+        test_system = get_test_system(self.course_id)
         test_system.open_ended_grading_interface = None
         return test_system
 
@@ -51,9 +48,9 @@ class PeerGradingModuleTest(unittest.TestCase, DummyModulestore):
         Create a peer grading module from a test system
         @return:
         """
-        self.setup_modulestore(COURSE)
-        self.peer_grading = self.get_module_from_location(self.problem_location, COURSE)
-        self.coe = self.get_module_from_location(self.coe_location, COURSE)
+        self.setup_modulestore(self.course_id.course)
+        self.peer_grading = self.get_module_from_location(self.problem_location)
+        self.coe = self.get_module_from_location(self.coe_location)
 
     def test_module_closed(self):
         """
@@ -75,7 +72,7 @@ class PeerGradingModuleTest(unittest.TestCase, DummyModulestore):
         Try getting data from the external grading service
         @return:
         """
-        success, _data = self.peer_grading.query_data_for_location(self.problem_location.url())
+        success, _data = self.peer_grading.query_data_for_location(self.problem_location.to_deprecated_string())
         self.assertTrue(success)
 
     def test_get_score_none(self):
@@ -149,8 +146,11 @@ class PeerGradingModuleTest(unittest.TestCase, DummyModulestore):
         Mainly for diff coverage
         @return:
         """
+        # pylint: disable=protected-access
         with self.assertRaises(ItemNotFoundError):
-            self.peer_grading._find_corresponding_module_for_location(Location('i4x', 'a', 'b', 'c', 'd'))
+            self.peer_grading._find_corresponding_module_for_location(
+                Location('org', 'course', 'run', 'category', 'name', 'revision')
+            )
 
     def test_get_instance_state(self):
         """
@@ -235,7 +235,13 @@ class MockPeerGradingServiceProblemList(MockPeerGradingService):
     def get_problem_list(self, course_id, grader_id):
         return {'success': True,
                 'problem_list': [
-                    {"num_graded": 3, "num_pending": 681, "num_required": 3, "location": "i4x://edX/open_ended/combinedopenended/SampleQuestion", "problem_name": "Peer-Graded Essay"},
+                    {
+                        "num_graded": 3,
+                        "num_pending": 681,
+                        "num_required": 3,
+                        "location": course_id.make_usage_key('combinedopenended', 'SampleQuestion'),
+                        "problem_name": "Peer-Graded Essay"
+                    },
                 ]}
 
 
@@ -244,12 +250,12 @@ class PeerGradingModuleScoredTest(unittest.TestCase, DummyModulestore):
     Test peer grading xmodule at the unit level.  More detailed tests are difficult, as the module relies on an
     external grading service.
     """
-    problem_location = Location(
-        ["i4x", "edX", "open_ended", "peergrading", "PeerGradingScored"]
-    )
+
+    course_id = SlashSeparatedCourseKey('edX', 'open_ended', '2012_Fall')
+    problem_location = course_id.make_usage_key("peergrading", "PeerGradingScored")
 
     def get_module_system(self, descriptor):
-        test_system = get_test_system()
+        test_system = get_test_system(self.course_id)
         test_system.open_ended_grading_interface = None
         return test_system
 
@@ -258,10 +264,10 @@ class PeerGradingModuleScoredTest(unittest.TestCase, DummyModulestore):
         Create a peer grading module from a test system
         @return:
         """
-        self.setup_modulestore(COURSE)
+        self.setup_modulestore(self.course_id.course)
 
     def test_metadata_load(self):
-        peer_grading = self.get_module_from_location(self.problem_location, COURSE)
+        peer_grading = self.get_module_from_location(self.problem_location)
         self.assertFalse(peer_grading.closed())
 
     def test_problem_list(self):
@@ -270,7 +276,7 @@ class PeerGradingModuleScoredTest(unittest.TestCase, DummyModulestore):
         """
 
         # Initialize peer grading module.
-        peer_grading = self.get_module_from_location(self.problem_location, COURSE)
+        peer_grading = self.get_module_from_location(self.problem_location)
 
         # Ensure that it cannot find any peer grading.
         html = peer_grading.peer_grading()
@@ -286,13 +292,12 @@ class PeerGradingModuleLinkedTest(unittest.TestCase, DummyModulestore):
     """
     Test peer grading that is linked to an open ended module.
     """
-    problem_location = Location(["i4x", "edX", "open_ended", "peergrading",
-                                 "PeerGradingLinked"])
-    coe_location = Location(["i4x", "edX", "open_ended", "combinedopenended",
-                             "SampleQuestion"])
+    course_id = SlashSeparatedCourseKey('edX', 'open_ended', '2012_Fall')
+    problem_location = course_id.make_usage_key("peergrading", "PeerGradingLinked")
+    coe_location = course_id.make_usage_key("combinedopenended", "SampleQuestion")
 
     def get_module_system(self, descriptor):
-        test_system = get_test_system()
+        test_system = get_test_system(self.course_id)
         test_system.open_ended_grading_interface = None
         return test_system
 
@@ -300,7 +305,7 @@ class PeerGradingModuleLinkedTest(unittest.TestCase, DummyModulestore):
         """
         Create a peer grading module from a test system.
         """
-        self.setup_modulestore(COURSE)
+        self.setup_modulestore(self.course_id.course)
 
     @property
     def field_data(self):
@@ -312,7 +317,7 @@ class PeerGradingModuleLinkedTest(unittest.TestCase, DummyModulestore):
             'data': '<peergrading/>',
             'location': self.problem_location,
             'use_for_single_location': True,
-            'link_to_location': self.coe_location.url(),
+            'link_to_location': self.coe_location.to_deprecated_string(),
             'graded': True,
             })
 
@@ -424,7 +429,7 @@ class PeerGradingModuleLinkedTest(unittest.TestCase, DummyModulestore):
         peer_grading = self._create_peer_grading_with_linked_problem(self.coe_location)
 
         # If we specify a location, it will render the problem for that location.
-        data = peer_grading.handle_ajax('problem', {'location': self.coe_location})
+        data = peer_grading.handle_ajax('problem', {'location': self.coe_location.to_deprecated_string()})
         self.assertTrue(json.loads(data)['success'])
 
         # If we don't specify a location, it should use the linked location.

common/lib/xmodule/xmodule/tests/test_self_assessment.py

@@ -4,6 +4,7 @@ import unittest
 from mock import Mock, MagicMock
 from webob.multidict import MultiDict
 from pytz import UTC
+from xblock.fields import ScopeIds
 from xmodule.open_ended_grading_classes.self_assessment_module import SelfAssessmentModule
 from xmodule.modulestore import Location
 from lxml import etree
@@ -29,8 +30,7 @@ class SelfAssessmentTest(unittest.TestCase):
         'hintprompt': 'Consider this...',
     }
 
-    location = Location(["i4x", "edX", "sa_test", "selfassessment",
-                         "SampleQuestion"])
+    location = Location("edX", "sa_test", "run", "selfassessment", "SampleQuestion", None)
 
     descriptor = Mock()
 
@@ -56,7 +56,10 @@ class SelfAssessmentTest(unittest.TestCase):
         }
 
         system = get_test_system()
-        system.xmodule_instance = Mock(scope_ids=Mock(usage_id='dummy-usage-id'))
+
+        usage_key = system.course_id.make_usage_key('combinedopenended', 'test_loc')
+        scope_ids = ScopeIds(1, 'combinedopenended', usage_key, usage_key)
+        system.xmodule_instance = Mock(scope_ids=scope_ids)
         self.module = SelfAssessmentModule(
             system,
             self.location,

common/lib/xmodule/xmodule/tests/test_tabs.py

@@ -2,6 +2,7 @@
 from mock import MagicMock
 import xmodule.tabs as tabs
 import unittest
+from xmodule.modulestore.locations import SlashSeparatedCourseKey
 
 
 class TabTestCase(unittest.TestCase):
@@ -9,7 +10,7 @@ class TabTestCase(unittest.TestCase):
     def setUp(self):
 
         self.course = MagicMock()
-        self.course.id = 'edX/toy/2012_Fall'
+        self.course.id = SlashSeparatedCourseKey('edX', 'toy', '2012_Fall')
         self.fake_dict_tab = {'fake_key': 'fake_value'}
         self.settings = MagicMock()
         self.settings.FEATURES = {}
@@ -137,7 +138,7 @@ class ProgressTestCase(TabTestCase):
         return self.check_tab(
             tab_class=tabs.ProgressTab,
             dict_tab={'type': tabs.ProgressTab.type, 'name': 'same'},
-            expected_link=self.reverse('progress', args=[self.course.id]),
+            expected_link=self.reverse('progress', args=[self.course.id.to_deprecated_string()]),
             expected_tab_id=tabs.ProgressTab.type,
             invalid_dict_tab=None,
         )
@@ -161,7 +162,7 @@ class WikiTestCase(TabTestCase):
         return self.check_tab(
             tab_class=tabs.WikiTab,
             dict_tab={'type': tabs.WikiTab.type, 'name': 'same'},
-            expected_link=self.reverse('course_wiki', args=[self.course.id]),
+            expected_link=self.reverse('course_wiki', args=[self.course.id.to_deprecated_string()]),
             expected_tab_id=tabs.WikiTab.type,
             invalid_dict_tab=self.fake_dict_tab,
         )
@@ -220,7 +221,7 @@ class StaticTabTestCase(TabTestCase):
         tab = self.check_tab(
             tab_class=tabs.StaticTab,
             dict_tab={'type': tabs.StaticTab.type, 'name': 'same', 'url_slug': url_slug},
-            expected_link=self.reverse('static_tab', args=[self.course.id, url_slug]),
+            expected_link=self.reverse('static_tab', args=[self.course.id.to_deprecated_string(), url_slug]),
             expected_tab_id='static_tab_schmug',
             invalid_dict_tab=self.fake_dict_tab,
         )
@@ -257,7 +258,10 @@ class TextbooksTestCase(TabTestCase):
             # verify all textbook type tabs
             if isinstance(tab, tabs.SingleTextbookTab):
                 book_type, book_index = tab.tab_id.split("/", 1)
-                expected_link = self.reverse(type_to_reverse_name[book_type], args=[self.course.id, book_index])
+                expected_link = self.reverse(
+                    type_to_reverse_name[book_type],
+                    args=[self.course.id.to_deprecated_string(), book_index]
+                )
                 self.assertEqual(tab.link_func(self.course, self.reverse), expected_link)
                 self.assertTrue(tab.name.startswith('Book{0}'.format(book_index)))
                 num_textbooks_found = num_textbooks_found + 1
@@ -279,7 +283,7 @@ class GradingTestCase(TabTestCase):
             tab_class=tab_class,
             dict_tab={'type': tab_class.type, 'name': name},
             expected_name=name,
-            expected_link=self.reverse(link_value, args=[self.course.id]),
+            expected_link=self.reverse(link_value, args=[self.course.id.to_deprecated_string()]),
             expected_tab_id=tab_class.type,
             invalid_dict_tab=None,
         )
@@ -314,7 +318,7 @@ class NotesTestCase(TabTestCase):
         return self.check_tab(
             tab_class=tabs.NotesTab,
             dict_tab={'type': tabs.NotesTab.type, 'name': 'same'},
-            expected_link=self.reverse('notes', args=[self.course.id]),
+            expected_link=self.reverse('notes', args=[self.course.id.to_deprecated_string()]),
             expected_tab_id=tabs.NotesTab.type,
             invalid_dict_tab=self.fake_dict_tab,
         )
@@ -341,7 +345,7 @@ class SyllabusTestCase(TabTestCase):
             tab_class=tabs.SyllabusTab,
             dict_tab={'type': tabs.SyllabusTab.type, 'name': name},
             expected_name=name,
-            expected_link=self.reverse('syllabus', args=[self.course.id]),
+            expected_link=self.reverse('syllabus', args=[self.course.id.to_deprecated_string()]),
             expected_tab_id=tabs.SyllabusTab.type,
             invalid_dict_tab=None,
         )
@@ -365,7 +369,7 @@ class InstructorTestCase(TabTestCase):
             tab_class=tabs.InstructorTab,
             dict_tab={'type': tabs.InstructorTab.type, 'name': name},
             expected_name=name,
-            expected_link=self.reverse('instructor_dashboard', args=[self.course.id]),
+            expected_link=self.reverse('instructor_dashboard', args=[self.course.id.to_deprecated_string()]),
             expected_tab_id=tabs.InstructorTab.type,
             invalid_dict_tab=None,
         )
@@ -603,7 +607,7 @@ class DiscussionLinkTestCase(TabTestCase):
         """Custom reverse function"""
         def reverse_discussion_link(viewname, args):
             """reverse lookup for discussion link"""
-            if viewname == "django_comment_client.forum.views.forum_form_discussion" and args == [course.id]:
+            if viewname == "django_comment_client.forum.views.forum_form_discussion" and args == [course.id.to_deprecated_string()]:
                 return "default_discussion_link"
         return reverse_discussion_link
 

common/lib/xmodule/xmodule/tests/test_util_open_ended.py

@@ -92,11 +92,8 @@ class DummyModulestore(object):
         courses = self.modulestore.get_courses()
         return courses[0]
 
-    def get_module_from_location(self, location, course):
-        course = self.get_course(course)
-        if not isinstance(location, Location):
-            location = Location(location)
-        descriptor = self.modulestore.get_instance(course.id, location, depth=None)
+    def get_module_from_location(self, usage_key):
+        descriptor = self.modulestore.get_item(usage_key, depth=None)
         descriptor.xmodule_runtime = self.get_module_system(descriptor)
         return descriptor
 

common/lib/xmodule/xmodule/tests/test_video.py

@@ -125,7 +125,7 @@ class VideoDescriptorTest(unittest.TestCase):
 
     def setUp(self):
         system = get_test_descriptor_system()
-        location = Location('i4x://org/course/video/name')
+        location = Location('org', 'course', 'run', 'video', 'name', None)
         self.descriptor = system.construct_xblock_from_class(
             VideoDescriptor,
             scope_ids=ScopeIds(None, None, location, location),
@@ -138,7 +138,7 @@ class VideoDescriptorTest(unittest.TestCase):
         back out to XML.
         """
         system = DummySystem(load_error_modules=True)
-        location = Location(["i4x", "edX", "video", "default", "SampleProblem1"])
+        location = Location("edX", 'course', 'run', "video", 'SampleProblem1', None)
         field_data = DictFieldData({'location': location})
         descriptor = VideoDescriptor(system, field_data, Mock())
         descriptor.youtube_id_0_75 = 'izygArpw-Qo'
@@ -154,7 +154,7 @@ class VideoDescriptorTest(unittest.TestCase):
         in the output string.
         """
         system = DummySystem(load_error_modules=True)
-        location = Location(["i4x", "edX", "video", "default", "SampleProblem1"])
+        location = Location("edX", 'course', 'run', "video", "SampleProblem1", None)
         field_data = DictFieldData({'location': location})
         descriptor = VideoDescriptor(system, field_data, Mock())
         descriptor.youtube_id_0_75 = 'izygArpw-Qo'
@@ -194,8 +194,7 @@ class VideoDescriptorImportTestCase(unittest.TestCase):
               <transcript language="ge" src="german_translation.srt" />
             </video>
         '''
-        location = Location(["i4x", "edX", "video", "default",
-                             "SampleProblem1"])
+        location = Location("edX", 'course', 'run', "video", "SampleProblem1", None)
         field_data = DictFieldData({
             'data': sample_xml,
             'location': location
@@ -498,6 +497,9 @@ class VideoExportTestCase(unittest.TestCase):
     Make sure that VideoDescriptor can export itself to XML
     correctly.
     """
+    def setUp(self):
+        self.location = Location("edX", 'course', 'run', "video", "SampleProblem1", None)
+
     def assertXmlEqual(self, expected, xml):
         for attr in ['tag', 'attrib', 'text', 'tail']:
             self.assertEqual(getattr(expected, attr), getattr(xml, attr))
@@ -507,8 +509,7 @@ class VideoExportTestCase(unittest.TestCase):
     def test_export_to_xml(self):
         """Test that we write the correct XML on export."""
         module_system = DummySystem(load_error_modules=True)
-        location = Location(["i4x", "edX", "video", "default", "SampleProblem1"])
-        desc = VideoDescriptor(module_system, DictFieldData({}), ScopeIds(None, None, location, location))
+        desc = VideoDescriptor(module_system, DictFieldData({}), ScopeIds(None, None, self.location, self.location))
 
         desc.youtube_id_0_75 = 'izygArpw-Qo'
         desc.youtube_id_1_0 = 'p2Q6BrNhdh8'
@@ -540,8 +541,7 @@ class VideoExportTestCase(unittest.TestCase):
     def test_export_to_xml_empty_end_time(self):
         """Test that we write the correct XML on export."""
         module_system = DummySystem(load_error_modules=True)
-        location = Location(["i4x", "edX", "video", "default", "SampleProblem1"])
-        desc = VideoDescriptor(module_system, DictFieldData({}), ScopeIds(None, None, location, location))
+        desc = VideoDescriptor(module_system, DictFieldData({}), ScopeIds(None, None, self.location, self.location))
 
         desc.youtube_id_0_75 = 'izygArpw-Qo'
         desc.youtube_id_1_0 = 'p2Q6BrNhdh8'
@@ -569,8 +569,7 @@ class VideoExportTestCase(unittest.TestCase):
     def test_export_to_xml_empty_parameters(self):
         """Test XML export with defaults."""
         module_system = DummySystem(load_error_modules=True)
-        location = Location(["i4x", "edX", "video", "default", "SampleProblem1"])
-        desc = VideoDescriptor(module_system, DictFieldData({}), ScopeIds(None, None, location, location))
+        desc = VideoDescriptor(module_system, DictFieldData({}), ScopeIds(None, None, self.location, self.location))
 
         xml = desc.definition_to_xml(None)
         expected = '<video url_name="SampleProblem1"/>\n'

common/lib/xmodule/xmodule/tests/test_xblock_wrappers.py

@@ -190,7 +190,7 @@ class LeafDescriptorFactory(Factory):
 
     @lazy_attribute
     def location(self):
-        return Location('i4x://org/course/category/{}'.format(self.url_name))
+        return Location('org', 'course', 'run', 'category', self.url_name, None)
 
     @lazy_attribute
     def block_type(self):

common/lib/xmodule/xmodule/tests/xml/__init__.py

@@ -7,8 +7,8 @@ from unittest import TestCase
 
 from xmodule.x_module import XMLParsingSystem, policy_key
 from xmodule.mako_module import MakoDescriptorSystem
-from xmodule.modulestore.xml import create_block_from_xml, LocationReader, CourseLocationGenerator
-from xmodule.modulestore import Location
+from xmodule.modulestore.xml import create_block_from_xml, CourseLocationGenerator
+from xmodule.modulestore.locations import SlashSeparatedCourseKey, Location
 
 from xblock.runtime import KvsFieldData, DictKeyValueStore
 
@@ -18,8 +18,7 @@ class InMemorySystem(XMLParsingSystem, MakoDescriptorSystem):  # pylint: disable
     The simplest possible XMLParsingSystem
     """
     def __init__(self, xml_import_data):
-        self.org = xml_import_data.org
-        self.course = xml_import_data.course
+        self.course_id = SlashSeparatedCourseKey.from_deprecated_string(xml_import_data.course_id)
         self.default_class = xml_import_data.default_class
         self._descriptors = {}
 
@@ -37,7 +36,6 @@ class InMemorySystem(XMLParsingSystem, MakoDescriptorSystem):  # pylint: disable
             select=xml_import_data.xblock_select,
             render_template=lambda template, context: pprint.pformat((template, context)),
             field_data=KvsFieldData(DictKeyValueStore()),
-            id_reader=LocationReader(),
         )
 
     def process_xml(self, xml):  # pylint: disable=method-hidden
@@ -45,14 +43,14 @@ class InMemorySystem(XMLParsingSystem, MakoDescriptorSystem):  # pylint: disable
         descriptor = create_block_from_xml(
             xml,
             self,
-            CourseLocationGenerator(self.org, self.course),
+            CourseLocationGenerator(self.course_id),
         )
-        self._descriptors[descriptor.location.url()] = descriptor
+        self._descriptors[descriptor.location.to_deprecated_string()] = descriptor
         return descriptor
 
     def load_item(self, location):  # pylint: disable=method-hidden
         """Return the descriptor loaded for `location`"""
-        return self._descriptors[Location(location).url()]
+        return self._descriptors[location.to_deprecated_string()]
 
 
 class XModuleXmlImportTest(TestCase):

common/lib/xmodule/xmodule/tests/xml/factories.py

@@ -17,15 +17,14 @@ class XmlImportData(object):
     Class to capture all of the data needed to actually run an XML import,
     so that the Factories have something to generate
     """
-    def __init__(self, xml_node, xml=None, org=None, course=None,
+    def __init__(self, xml_node, xml=None, course_id=None,
                  default_class=None, policy=None,
                  filesystem=None, parent=None,
                  xblock_mixins=(), xblock_select=None):
 
         self._xml_node = xml_node
         self._xml_string = xml
-        self.org = org
-        self.course = course
+        self.course_id = course_id
         self.default_class = default_class
         self.filesystem = filesystem
         self.xblock_mixins = xblock_mixins
@@ -47,8 +46,8 @@ class XmlImportData(object):
 
     def __repr__(self):
         return u"XmlImportData{!r}".format((
-            self._xml_node, self._xml_string, self.org,
-            self.course, self.default_class, self.policy,
+            self._xml_node, self._xml_string, self.course_id,
+            self.default_class, self.policy,
             self.filesystem, self.parent, self.xblock_mixins,
             self.xblock_select,
         ))
@@ -74,6 +73,7 @@ class XmlImportFactory(Factory):
     policy = {}
     inline_xml = True
     tag = 'unknown'
+    course_id = 'edX/xml_test_course/101'
 
     @classmethod
     def _adjust_kwargs(cls, **kwargs):
@@ -136,8 +136,6 @@ class XmlImportFactory(Factory):
 class CourseFactory(XmlImportFactory):
     """Factory for <course> nodes"""
     tag = 'course'
-    org = 'edX'
-    course = 'xml_test_course'
     name = '101'
     static_asset_path = 'xml_test_course'
 

common/lib/xmodule/xmodule/vertical_module.py

@@ -25,7 +25,7 @@ class VerticalModule(VerticalFields, XModule):
             fragment.add_frag_resources(rendered_child)
 
             contents.append({
-                'id': child.id,
+                'id': child.location.to_deprecated_string(),
                 'content': rendered_child.content
             })
 

common/lib/xmodule/xmodule/video_module/transcripts_utils.py

@@ -289,9 +289,7 @@ def copy_or_rename_transcript(new_name, old_name, item, delete_old=False, user=N
     If `delete_old` is True, removes `old_name` files from storage.
     """
     filename = 'subs_{0}.srt.sjson'.format(old_name)
-    content_location = StaticContent.compute_location(
-        item.location.org, item.location.course, filename
-    )
+    content_location = StaticContent.compute_location(item.location.course_key, filename)
     transcripts = contentstore().find(content_location).data
     save_subs_to_store(json.loads(transcripts), new_name, item)
     item.sub = new_name
@@ -532,7 +530,7 @@ class Transcript(object):
         """
         Return asset location. `location` is module location.
         """
-        return StaticContent.compute_location(location.org, location.course, filename)
+        return StaticContent.compute_location(location.course_key, filename)
 
     @staticmethod
     def delete_asset(location, filename):
@@ -545,4 +543,5 @@ class Transcript(object):
             log.info("Transcript asset %s was removed from store.", filename)
         except NotFoundError:
             pass
+        return StaticContent.compute_location(location.course_key, filename)
 

common/lib/xmodule/xmodule/x_module.py

@@ -18,14 +18,12 @@ from webob.multidict import MultiDict
 from xblock.core import XBlock
 from xblock.fields import Scope, Integer, Float, List, XBlockMixin, String, Dict
 from xblock.fragment import Fragment
-from xblock.plugin import default_select
 from xblock.runtime import Runtime
 from xmodule.fields import RelativeTime
 
 from xmodule.errortracker import exc_info_to_str
-from xmodule.modulestore import Location
-from xmodule.modulestore.exceptions import ItemNotFoundError, InsufficientSpecificationError, InvalidLocationError
-from xmodule.modulestore.locator import BlockUsageLocator
+from xmodule.modulestore.exceptions import ItemNotFoundError
+from xmodule.modulestore.keys import OpaqueKeyReader, UsageKey
 from xmodule.exceptions import UndefinedContext
 from dogapi import dog_stats_api
 
@@ -156,11 +154,7 @@ class XModuleMixin(XBlockMixin):
 
     @property
     def course_id(self):
-        return self.runtime.course_id
-
-    @property
-    def id(self):
-        return self.location.url()
+        return self.location.course_key
 
     @property
     def category(self):
@@ -168,16 +162,11 @@ class XModuleMixin(XBlockMixin):
 
     @property
     def location(self):
-        try:
-            return Location(self.scope_ids.usage_id)
-        except InvalidLocationError:
-            if isinstance(self.scope_ids.usage_id, BlockUsageLocator):
-                return self.scope_ids.usage_id
-            else:
-                return BlockUsageLocator(self.scope_ids.usage_id)
+        return self.scope_ids.usage_id
 
     @location.setter
     def location(self, value):
+        assert isinstance(value, UsageKey)
         self.scope_ids = self.scope_ids._replace(
             def_id=value,
             usage_id=value,
@@ -185,12 +174,7 @@ class XModuleMixin(XBlockMixin):
 
     @property
     def url_name(self):
-        if isinstance(self.location, Location):
-            return self.location.name
-        elif isinstance(self.location, BlockUsageLocator):
-            return self.location.block_id
-        else:
-            raise InsufficientSpecificationError()
+        return self.location.name
 
     @property
     def display_name_with_default(self):
@@ -203,6 +187,17 @@ class XModuleMixin(XBlockMixin):
             name = self.url_name.replace('_', ' ')
         return name
 
+    @property
+    def xblock_kvs(self):
+        """
+        Retrieves the internal KeyValueStore for this XModule.
+
+        Should only be used by the persistence layer. Use with caution.
+        """
+        # if caller wants kvs, caller's assuming it's up to date; so, decache it
+        self.save()
+        return self._field_data._kvs  # pylint: disable=protected-access
+
     def get_explicitly_set_fields_by_scope(self, scope=Scope.content):
         """
         Get a dictionary of the fields for the given scope which are set explicitly on this xblock. (Including
@@ -214,15 +209,6 @@ class XModuleMixin(XBlockMixin):
                 result[field.name] = field.read_json(self)
         return result
 
-    @property
-    def xblock_kvs(self):
-        """
-        Use w/ caution. Really intended for use by the persistence layer.
-        """
-        # if caller wants kvs, caller's assuming it's up to date; so, decache it
-        self.save()
-        return self._field_data._kvs  # pylint: disable=protected-access
-
     def get_content_titles(self):
         """
         Returns list of content titles for all of self's children.
@@ -684,7 +670,6 @@ class XModuleDescriptor(XModuleMixin, HTMLSnippet, ResourceTemplates, XBlock):
         Interpret the parsed XML in `node`, creating an XModuleDescriptor.
         """
         xml = etree.tostring(node)
-        # TODO: change from_xml to not take org and course, it can use self.system.
         block = cls.from_xml(xml, runtime, id_generator)
         return block
 
@@ -1023,7 +1008,7 @@ class DescriptorSystem(MetricsMixin, ConfigurableFragmentWrapper, Runtime):  # p
         local_resource_url: an implementation of :meth:`xblock.runtime.Runtime.local_resource_url`
 
         """
-        super(DescriptorSystem, self).__init__(**kwargs)
+        super(DescriptorSystem, self).__init__(id_reader=OpaqueKeyReader(), **kwargs)
 
         # This is used by XModules to write out separate files during xml export
         self.export_fs = None
@@ -1217,7 +1202,7 @@ class ModuleSystem(MetricsMixin,ConfigurableFragmentWrapper, Runtime):  # pylint
 
         # Usage_store is unused, and field_data is often supplanted with an
         # explicit field_data during construct_xblock.
-        super(ModuleSystem, self).__init__(id_reader=None, field_data=field_data, **kwargs)
+        super(ModuleSystem, self).__init__(id_reader=OpaqueKeyReader(), field_data=field_data, **kwargs)
 
         self.STATIC_URL = static_url
         self.xqueue = xqueue