Fixed some isinstance errors with opaque-keys

cms/djangoapps/contentstore/utils.py

@@ -14,7 +14,8 @@ from xmodule.contentstore.content import StaticContent
 from xmodule.modulestore import ModuleStoreEnum
 from xmodule.modulestore.django import modulestore
 from xmodule.modulestore.exceptions import ItemNotFoundError
-from opaque_keys.edx.locations import SlashSeparatedCourseKey, Location
+from opaque_keys.edx.keys import UsageKey, CourseKey
+from xmodule.modulestore.store_utilities import delete_course
 from student.roles import CourseInstructorRole, CourseStaffRole
 
 
@@ -54,7 +55,7 @@ def get_lms_link_for_item(location, preview=False):
     :param location: the location to jump to
     :param preview: True if the preview version of LMS should be returned. Default value is false.
     """
-    assert(isinstance(location, Location))
+    assert(isinstance(location, UsageKey))
 
     if settings.LMS_BASE is None:
         return None
@@ -76,7 +77,7 @@ def get_lms_link_for_about_page(course_id):
     Returns the url to the course about page from the location tuple.
     """
 
-    assert(isinstance(course_id, SlashSeparatedCourseKey))
+    assert(isinstance(course_id, CourseKey))
 
     if settings.FEATURES.get('ENABLE_MKTG_SITE', False):
         if not hasattr(settings, 'MKTG_URLS'):

common/djangoapps/student/models.py

@@ -738,7 +738,7 @@ class CourseEnrollment(models.Model):
 
         try:
             context = contexts.course_context_from_course_id(self.course_id)
-            assert(isinstance(self.course_id, SlashSeparatedCourseKey))
+            assert(isinstance(self.course_id, CourseKey))
             data = {
                 'user_id': self.user.id,
                 'course_id': self.course_id.to_deprecated_string(),
@@ -884,7 +884,7 @@ class CourseEnrollment(models.Model):
 
         `course_id_partial` (CourseKey) is missing the run component
         """
-        assert isinstance(course_id_partial, SlashSeparatedCourseKey)
+        assert isinstance(course_id_partial, CourseKey)
         assert not course_id_partial.run  # None or empty string
         course_key = SlashSeparatedCourseKey(course_id_partial.org, course_id_partial.course, '')
         querystring = unicode(course_key.to_deprecated_string())

common/djangoapps/track/contexts.py

@@ -2,6 +2,7 @@
 import logging
 
 from opaque_keys.edx.locations import SlashSeparatedCourseKey
+from opaque_keys.edx.keys import CourseKey
 from opaque_keys import InvalidKeyError
 from util.request import COURSE_REGEX
 
@@ -48,7 +49,7 @@ def course_context_from_course_id(course_id):
         return {'course_id': '', 'org_id': ''}
 
     # TODO: Make this accept any CourseKey, and serialize it using .to_string
-    assert(isinstance(course_id, SlashSeparatedCourseKey))
+    assert(isinstance(course_id, CourseKey))
     return {
         'course_id': course_id.to_deprecated_string(),
         'org_id': course_id.org,

common/djangoapps/xmodule_django/models.py

 from django.db import models
 from django.core.exceptions import ValidationError
 from opaque_keys.edx.locations import SlashSeparatedCourseKey, Location
+from opaque_keys.edx.keys import CourseKey, UsageKey
 
 from south.modelsinspector import add_introspection_rules
 add_introspection_rules([], ["^xmodule_django\.models\.CourseKeyField"])
@@ -54,7 +55,7 @@ class CourseKeyField(models.CharField):
         if value is self.Empty or value is None:
             return None
 
-        assert isinstance(value, (basestring, SlashSeparatedCourseKey))
+        assert isinstance(value, (basestring, CourseKey))
         if value == '':
             # handle empty string for models being created w/o fields populated
             return None
@@ -74,7 +75,7 @@ class CourseKeyField(models.CharField):
         if value is self.Empty or value is None:
             return ''  # CharFields should use '' as their empty value, rather than None
 
-        assert isinstance(value, SlashSeparatedCourseKey)
+        assert isinstance(value, CourseKey)
         return value.to_deprecated_string()
 
     def validate(self, value, model_instance):
@@ -104,7 +105,7 @@ class LocationKeyField(models.CharField):
         if value is self.Empty or value is None:
             return value
 
-        assert isinstance(value, (basestring, Location))
+        assert isinstance(value, (basestring, UsageKey))
 
         if value == '':
             return None
@@ -124,7 +125,7 @@ class LocationKeyField(models.CharField):
         if value is self.Empty:
             return ''
 
-        assert isinstance(value, Location)
+        assert isinstance(value, UsageKey)
         return value.to_deprecated_string()
 
     def validate(self, value, model_instance):

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

@@ -12,6 +12,7 @@ from urllib import urlencode
 
 from opaque_keys.edx.locations import AssetLocation
 from opaque_keys.edx.keys import CourseKey
+from .django import contentstore
 from PIL import Image
 
 

common/lib/xmodule/xmodule/contentstore/mongo.py

@@ -12,7 +12,8 @@ from fs.osfs import OSFS
 import os
 import json
 from bson.son import SON
-from opaque_keys.edx.locations import AssetLocation, SlashSeparatedCourseKey
+from opaque_keys.edx.locator import AssetLocator
+from opaque_keys.edx.locations import AssetLocation
 
 
 class MongoContentStore(ContentStore):
@@ -78,9 +79,8 @@ class MongoContentStore(ContentStore):
         return content
 
     def delete(self, location_or_id):
-        if isinstance(location_or_id, AssetLocation):
-            location_or_id, __ = self.asset_db_key(location_or_id)
-
+        if isinstance(location_or_id, AssetLocator):
+            location_or_id, _ = self.asset_db_key(location_or_id)
         # Deletes of non-existent files are considered successful
         self.fs.delete(location_or_id)
 
@@ -151,12 +151,12 @@ class MongoContentStore(ContentStore):
             # TODO: On 6/19/14, I had to put a try/except around this
             # to export a course. The course failed on JSON files in
             # the /static/ directory placed in it with an import.
-            # 
-            # If this hasn't been looked at in a while, remove this comment. 
+            #
+            # If this hasn't been looked at in a while, remove this comment.
             #
             # When debugging course exports, this might be a good place
             # to look. -- pmitros
-            self.export(asset_location, output_directory) 
+            self.export(asset_location, output_directory)
             for attr, value in asset.iteritems():
                 if attr not in ['_id', 'md5', 'uploadDate', 'length', 'chunkSize']:
                     policy.setdefault(asset_location.name, {})[attr] = value

common/lib/xmodule/xmodule/course_module.py

@@ -9,6 +9,7 @@ import dateutil.parser
 from lazy import lazy
 
 from opaque_keys.edx.locations import Location
+from opaque_keys.edx.locator import UsageKey
 from xmodule.seq_module import SequenceDescriptor, SequenceModule
 from xmodule.graders import grader_from_conf
 from xmodule.tabs import CourseTabList
@@ -550,7 +551,7 @@ class CourseDescriptor(CourseFields, SequenceDescriptor):
         _ = self.runtime.service(self, "i18n").ugettext
 
         if self.wiki_slug is None:
-            if isinstance(self.location, Location):
+            if isinstance(self.location, UsageKey):
                 self.wiki_slug = self.location.course
             elif isinstance(self.location, CourseLocator):
                 self.wiki_slug = self.id.offering or self.display_name

common/lib/xmodule/xmodule/modulestore/loc_mapper_store.py

@@ -11,6 +11,7 @@ from xmodule.modulestore import ModuleStoreEnum
 from xmodule.modulestore.exceptions import InvalidLocationError, ItemNotFoundError
 from opaque_keys.edx.locator import BlockUsageLocator, CourseLocator
 from opaque_keys.edx.locations import SlashSeparatedCourseKey
+from opaque_keys.edx.keys import CourseKey
 
 
 class LocMapperStore(object):
@@ -349,7 +350,7 @@ class LocMapperStore(object):
         """
         Construct the SON needed to repr the course_key for either a query or an insertion
         """
-        assert(isinstance(course_key, SlashSeparatedCourseKey))
+        assert(isinstance(course_key, CourseKey))
         return bson.son.SON([
             ('org', course_key.org),
             ('course', course_key.course),

common/lib/xmodule/xmodule/modulestore/mixed.py

@@ -184,7 +184,7 @@ class MixedModuleStore(ModuleStoreWriteBase):
             for course in store.get_courses():
                 course_id = self._clean_course_id_for_mapping(course.id)
                 if course_id not in courses:
-                    if has_locators and isinstance(course_id, SlashSeparatedCourseKey):
+                    if has_locators and isinstance(course_id, CourseKey):
 
                         # see if a locator version of course is in the result
                         try:

common/lib/xmodule/xmodule/modulestore/mongo/base.py

@@ -39,6 +39,7 @@ from xmodule.modulestore.exceptions import ItemNotFoundError, InvalidLocationErr
 from xmodule.modulestore.inheritance import own_metadata, InheritanceMixin, inherit_metadata, InheritanceKeyValueStore
 from xblock.core import XBlock
 from opaque_keys.edx.locations import SlashSeparatedCourseKey
+from opaque_keys.edx.keys import UsageKey, CourseKey
 from xmodule.exceptions import HeartbeatFailure
 
 log = logging.getLogger(__name__)
@@ -174,7 +175,7 @@ class CachingDescriptorSystem(MakoDescriptorSystem):
         """
         Return an XModule instance for the specified location
         """
-        assert isinstance(location, Location)
+        assert isinstance(location, UsageKey)
         json_data = self.module_data.get(location)
         if json_data is None:
             module = self.modulestore.get_item(location)
@@ -264,6 +265,9 @@ class CachingDescriptorSystem(MakoDescriptorSystem):
         key = Location.from_deprecated_string(ref_string)
         return key.replace(run=self.modulestore._fill_in_run(key.course_key).run)
 
+    def __setattr__(self, name, value):
+        return super(CachingDescriptorSystem, self).__setattr__(name, value)
+
     def _convert_reference_fields_to_keys(self, class_, course_key, jsonfields):
         """
         Find all fields of type reference and convert the payload into UsageKeys
@@ -693,7 +697,7 @@ class MongoModuleStore(ModuleStoreWriteBase):
         '''Look for a given location in the collection. If the item is not present, raise
         ItemNotFoundError.
         '''
-        assert isinstance(location, Location)
+        assert isinstance(location, UsageKey)
         item = self.collection.find_one(
             {'_id': location.to_deprecated_son()}
         )
@@ -705,7 +709,7 @@ class MongoModuleStore(ModuleStoreWriteBase):
         """
         Get the course with the given courseid (org/course/run)
         """
-        assert(isinstance(course_key, SlashSeparatedCourseKey))
+        assert(isinstance(course_key, CourseKey))
         course_key = self._fill_in_run(course_key)
         location = course_key.make_usage_key('course', course_key.run)
         try:
@@ -722,7 +726,7 @@ class MongoModuleStore(ModuleStoreWriteBase):
         If ignore_case is True, do a case insensitive search,
         otherwise, do a case sensitive search
         """
-        assert(isinstance(course_key, SlashSeparatedCourseKey))
+        assert(isinstance(course_key, CourseKey))
         course_key = self._fill_in_run(course_key)
         location = course_key.make_usage_key('course', course_key.run)
         if ignore_case:
@@ -1056,7 +1060,7 @@ class MongoModuleStore(ModuleStoreWriteBase):
                     ]
                 elif isinstance(xblock.fields[field_name], ReferenceValueDict):
                     for key, subvalue in value.iteritems():
-                        assert isinstance(subvalue, Location)
+                        assert isinstance(subvalue, UsageKey)
                         value[key] = subvalue.to_deprecated_string()
         return jsonfields
 

common/lib/xmodule/xmodule/modulestore/mongoengine_fields.py

@@ -4,7 +4,7 @@ Custom field types for mongoengine
 import mongoengine
 from opaque_keys.edx.locations import SlashSeparatedCourseKey, Location
 from types import NoneType
-from opaque_keys.edx.keys import CourseKey
+from opaque_keys.edx.keys import CourseKey, UsageKey
 
 
 class CourseKeyField(mongoengine.StringField):
@@ -19,7 +19,7 @@ class CourseKeyField(mongoengine.StringField):
         """
         For now saves the course key in the deprecated form
         """
-        assert isinstance(course_key, (NoneType, SlashSeparatedCourseKey))
+        assert isinstance(course_key, (NoneType, CourseKey))
         if course_key:
             # don't call super as base.BaseField.to_mongo calls to_python() for some odd reason
             return course_key.to_deprecated_string()
@@ -32,7 +32,7 @@ class CourseKeyField(mongoengine.StringField):
         """
         # calling super b/c it decodes utf (and doesn't have circularity of from_python)
         course_key = super(CourseKeyField, self).to_python(course_key)
-        assert isinstance(course_key, (NoneType, basestring, SlashSeparatedCourseKey))
+        assert isinstance(course_key, (NoneType, basestring, CourseKey))
         if course_key == '':
             return None
         if isinstance(course_key, basestring):
@@ -41,7 +41,7 @@ class CourseKeyField(mongoengine.StringField):
             return course_key
 
     def validate(self, value):
-        assert isinstance(value, (NoneType, basestring, SlashSeparatedCourseKey))
+        assert isinstance(value, (NoneType, basestring, CourseKey))
         if isinstance(value, CourseKey):
             return super(CourseKeyField, self).validate(value.to_deprecated_string())
         else:
@@ -59,7 +59,7 @@ class UsageKeyField(mongoengine.StringField):
         """
         For now saves the usage key in the deprecated location i4x/c4x form
         """
-        assert isinstance(location, (NoneType, Location))
+        assert isinstance(location, (NoneType, UsageKey))
         if location is None:
             return None
         return super(UsageKeyField, self).to_mongo(location.to_deprecated_string())
@@ -68,7 +68,7 @@ class UsageKeyField(mongoengine.StringField):
         """
         Deserialize to a UsageKey instance: for now it's a location missing the run
         """
-        assert isinstance(location, (NoneType, basestring, Location))
+        assert isinstance(location, (NoneType, basestring, UsageKey))
         if location == '':
             return None
         if isinstance(location, basestring):
@@ -78,8 +78,8 @@ class UsageKeyField(mongoengine.StringField):
             return location
 
     def validate(self, value):
-        assert isinstance(value, (NoneType, basestring, Location))
-        if isinstance(value, Location):
+        assert isinstance(value, (NoneType, basestring, UsageKey))
+        if isinstance(value, UsageKey):
             return super(UsageKeyField, self).validate(value.to_deprecated_string())
         else:
             return super(UsageKeyField, self).validate(value)

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

@@ -4,6 +4,7 @@ from uuid import uuid4
 
 from xmodule.modulestore import prefer_xmodules, ModuleStoreEnum
 from opaque_keys.edx.locations import Location
+from opaque_keys.edx.keys import UsageKey
 from xblock.core import XBlock
 from xmodule.tabs import StaticTab
 from decorator import contextmanager
@@ -150,7 +151,7 @@ class ItemFactory(XModuleFactory):
         user_id = kwargs.pop('user_id', ModuleStoreEnum.UserID.test)
         publish_item = kwargs.pop('publish_item', True)
 
-        assert isinstance(location, Location)
+        assert isinstance(location, UsageKey)
         assert location != parent_location
 
         store = kwargs.pop('modulestore')

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

@@ -26,6 +26,7 @@ from xmodule.modulestore import ModuleStoreEnum
 from xmodule.modulestore.mongo import MongoModuleStore, MongoKeyValueStore
 from xmodule.modulestore.draft import DraftModuleStore
 from opaque_keys.edx.locations import SlashSeparatedCourseKey, AssetLocation
+from opaque_keys.edx.keys import UsageKey, CourseKey
 from xmodule.modulestore.xml_exporter import export_to_xml
 from xmodule.modulestore.xml_importer import import_from_xml, perform_xlint
 from xmodule.contentstore.mongo import MongoContentStore
@@ -407,20 +408,20 @@ class TestMongoModuleStore(unittest.TestCase):
         def check_xblock_fields():
             def check_children(xblock):
                 for child in xblock.children:
-                    assert_is_instance(child, Location)
+                    assert_is_instance(child, UsageKey)
 
             course = self.draft_store.get_course(course_key)
             check_children(course)
 
             refele = self.draft_store.get_item(self.refloc)
             check_children(refele)
-            assert_is_instance(refele.reference_link, Location)
+            assert_is_instance(refele.reference_link, UsageKey)
             assert_greater(len(refele.reference_list), 0)
             for ref in refele.reference_list:
-                assert_is_instance(ref, Location)
+                assert_is_instance(ref, UsageKey)
             assert_greater(len(refele.reference_dict), 0)
             for ref in refele.reference_dict.itervalues():
-                assert_is_instance(ref, Location)
+                assert_is_instance(ref, UsageKey)
 
         def check_mongo_fields():
             def get_item(location):

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

@@ -438,12 +438,15 @@ class ImportTestCase(BaseCourseTestCase):
         print("course errors:")
 
         # Expect to find an error/exception about characters in "®esources"
-        expect = "Invalid characters"
+        expect = "UnicodeEncodeError"
         errors = [
             (msg.encode("utf-8"), err.encode("utf-8"))
             for msg, err
             in modulestore.get_course_errors(course.id)
         ]
+        for msg, err in errors:
+            print("msg: " + str(msg))
+            print("err: " + str(err))
 
         self.assertTrue(any(
             expect in msg or expect in err

lms/djangoapps/bulk_email/tests/test_forms.py

@@ -78,7 +78,7 @@ class CourseAuthorizationFormTest(ModuleStoreTestCase):
     @patch.dict(settings.FEATURES, {'ENABLE_INSTRUCTOR_EMAIL': True, 'REQUIRE_COURSE_EMAIL_AUTH': True})
     def test_form_typo(self):
         # Munge course id
-        bad_id = SlashSeparatedCourseKey(u'Broken{}'.format(self.course.id.org), '', self.course.id.run + '_typo')
+        bad_id = SlashSeparatedCourseKey(u'Broken{}'.format(self.course.id.org), 'hello', self.course.id.run + '_typo')
 
         form_data = {'course_id': bad_id.to_deprecated_string(), 'email_enabled': True}
         form = CourseAuthorizationAdminForm(data=form_data)

lms/djangoapps/courseware/access.py

@@ -9,7 +9,6 @@ from django.contrib.auth.models import AnonymousUser
 
 from xmodule.course_module import CourseDescriptor
 from xmodule.error_module import ErrorDescriptor
-from opaque_keys.edx.locations import Location
 from xmodule.x_module import XModule
 
 from xblock.core import XBlock
@@ -23,7 +22,7 @@ from student.roles import (
     GlobalStaff, CourseStaffRole, CourseInstructorRole,
     OrgStaffRole, OrgInstructorRole, CourseBetaTesterRole
 )
-from opaque_keys.edx.keys import CourseKey
+from opaque_keys.edx.keys import CourseKey, UsageKey
 DEBUG_ACCESS = False
 
 log = logging.getLogger(__name__)
@@ -85,7 +84,7 @@ def has_access(user, action, obj, course_key=None):
     if isinstance(obj, CourseKey):
         return _has_access_course_key(user, action, obj)
 
-    if isinstance(obj, Location):
+    if isinstance(obj, UsageKey):
         return _has_access_location(user, action, obj, course_key)
 
     if isinstance(obj, basestring):

lms/djangoapps/courseware/model_data.py

@@ -13,6 +13,7 @@ from .models import (
 )
 import logging
 from opaque_keys.edx.locations import SlashSeparatedCourseKey, Location
+from opaque_keys.edx.keys import CourseKey, UsageKey
 
 from django.db import DatabaseError
 from django.contrib.auth.models import User
@@ -61,7 +62,7 @@ class FieldDataCache(object):
         self.descriptors = descriptors
         self.select_for_update = select_for_update
 
-        assert isinstance(course_id, SlashSeparatedCourseKey)
+        assert isinstance(course_id, CourseKey)
         self.course_id = course_id
         self.user = user
 
@@ -238,7 +239,7 @@ class FieldDataCache(object):
         if key.scope == Scope.user_state:
             # When we start allowing block_scope_ids to be either Locations or Locators,
             # this assertion will fail. Fix the code here when that happens!
-            assert(isinstance(key.block_scope_id, Location))
+            assert(isinstance(key.block_scope_id, UsageKey))
             field_object, _ = StudentModule.objects.get_or_create(
                 course_id=self.course_id,
                 student=User.objects.get(id=key.user_id),

lms/djangoapps/django_comment_client/utils.py

@@ -16,7 +16,8 @@ import pystache_custom as pystache
 
 from xmodule.modulestore.django import modulestore
 from django.utils.timezone import UTC
-from opaque_keys.edx.locations import i4xEncoder, SlashSeparatedCourseKey
+from opaque_keys.edx.locations import i4xEncoder
+from opaque_keys.edx.keys import CourseKey
 import json
 
 log = logging.getLogger(__name__)
@@ -314,7 +315,7 @@ def render_mustache(template_name, dictionary, *args, **kwargs):
 
 
 def permalink(content):
-    if isinstance(content['course_id'], SlashSeparatedCourseKey):
+    if isinstance(content['course_id'], CourseKey):
         course_id = content['course_id'].to_deprecated_string()
     else:
         course_id = content['course_id']

lms/djangoapps/instructor_task/api_helper.py

@@ -14,7 +14,7 @@ from celery.states import READY_STATES, SUCCESS, FAILURE, REVOKED
 from courseware.module_render import get_xqueue_callback_url_prefix
 
 from xmodule.modulestore.django import modulestore
-from opaque_keys.edx.locations import Location
+from opaque_keys.edx.keys import UsageKey
 from instructor_task.models import InstructorTask, PROGRESS
 
 
@@ -262,7 +262,7 @@ def encode_problem_and_student_input(usage_key, student=None):  # pylint: disabl
         student (User): the student affected
     """
 
-    assert isinstance(usage_key, Location)
+    assert isinstance(usage_key, UsageKey)
     if student is not None:
         task_input = {'problem_url': usage_key.to_deprecated_string(), 'student': student.username}
         task_key_stub = "{student}_{problem}".format(student=student.id, problem=usage_key.to_deprecated_string())