diff --git a/cms/djangoapps/contentstore/tests/test_contentstore.py b/cms/djangoapps/contentstore/tests/test_contentstore.py index 8665d59542..2259d81ba8 100644 --- a/cms/djangoapps/contentstore/tests/test_contentstore.py +++ b/cms/djangoapps/contentstore/tests/test_contentstore.py @@ -131,7 +131,6 @@ class ContentStoreToyCourseTest(ContentStoreTestCase): descriptor = store.get_items(course.id, category='vertical',) resp = self.client.get_html(get_url('unit_handler', descriptor[0].location)) self.assertEqual(resp.status_code, 200) - _test_no_locations(self, resp) for expected in expected_types: self.assertIn(expected, resp.content) @@ -157,7 +156,6 @@ class ContentStoreToyCourseTest(ContentStoreTestCase): resp = self.client.get_html(get_url('unit_handler', usage_key)) self.assertEqual(resp.status_code, 400) - _test_no_locations(self, resp, status_code=400) def check_edit_unit(self, test_course_name): _, course_items = import_from_xml(modulestore(), self.user.id, 'common/test/data/', [test_course_name]) @@ -364,8 +362,6 @@ class ContentStoreToyCourseTest(ContentStoreTestCase): get_url('xblock_view_handler', usage_key, kwargs={'view_name': 'container_preview'}) ) self.assertEqual(resp.status_code, 200) - # TODO: uncomment when preview no longer has locations being returned. - # _test_no_locations(self, resp) # These are the data-ids of the xblocks contained in the vertical. self.assertContains(resp, 'edX+toy+2012_Fall+video+sample_video') @@ -534,7 +530,7 @@ class ContentStoreToyCourseTest(ContentStoreTestCase): url = reverse_course_url( 'assets_handler', course.id, - kwargs={'asset_key_string': course.id.make_asset_key('asset', 'sample_static.txt')} + kwargs={'asset_key_string': unicode(course.id.make_asset_key('asset', 'sample_static.txt'))} ) resp = self.client.delete(url) self.assertEqual(resp.status_code, 204) @@ -761,7 +757,6 @@ class ContentStoreToyCourseTest(ContentStoreTestCase): def test_bad_contentstore_request(self): resp = self.client.get_html('http://localhost:8001/c4x/CDX/123123/asset/&images_circuits_Lab7Solution2.png') self.assertEqual(resp.status_code, 400) - _test_no_locations(self, resp, 400) def test_rewrite_nonportable_links_on_import(self): module_store = modulestore() @@ -1196,7 +1191,6 @@ class ContentStoreToyCourseTest(ContentStoreTestCase): for descriptor in items: resp = self.client.get_html(get_url('unit_handler', descriptor.location)) self.assertEqual(resp.status_code, 200) - _test_no_locations(self, resp) class ContentStoreTest(ContentStoreTestCase): @@ -1475,7 +1469,6 @@ class ContentStoreTest(ContentStoreTestCase): status_code=200, html=True ) - _test_no_locations(self, resp) def test_course_factory(self): """Test that the course factory works correctly.""" @@ -1498,7 +1491,6 @@ class ContentStoreTest(ContentStoreTestCase): status_code=200, html=True ) - _test_no_locations(self, resp) def test_course_overview_view_with_course(self): """Test viewing the course overview page with an existing course""" @@ -1522,7 +1514,6 @@ class ContentStoreTest(ContentStoreTestCase): } resp = self.client.ajax_post(reverse_url('xblock_handler'), section_data) - _test_no_locations(self, resp, html=False) self.assertEqual(resp.status_code, 200) data = parse_json(resp) @@ -1563,7 +1554,6 @@ class ContentStoreTest(ContentStoreTestCase): get_url(handler, course_key, 'course_key_string') ) self.assertEqual(resp.status_code, 200) - _test_no_locations(self, resp) _, course_items = import_from_xml(modulestore(), self.user.id, 'common/test/data/', ['simple']) course_key = course_items[0].id @@ -1589,20 +1579,17 @@ class ContentStoreTest(ContentStoreTestCase): subsection_key = course_key.make_usage_key('sequential', 'test_sequence') resp = self.client.get_html(get_url('subsection_handler', subsection_key)) self.assertEqual(resp.status_code, 200) - _test_no_locations(self, resp) # go look at the Edit page unit_key = course_key.make_usage_key('vertical', 'test_vertical') resp = self.client.get_html(get_url('unit_handler', unit_key)) self.assertEqual(resp.status_code, 200) - _test_no_locations(self, resp) def delete_item(category, name): """ Helper method for testing the deletion of an xblock item. """ item_key = course_key.make_usage_key(category, name) resp = self.client.delete(get_url('xblock_handler', item_key)) self.assertEqual(resp.status_code, 204) - _test_no_locations(self, resp, status_code=204, html=False) # delete a component delete_item(category='html', name='test_html') @@ -1805,7 +1792,6 @@ class ContentStoreTest(ContentStoreTestCase): Show the course overview page. """ resp = self.client.get_html(get_url('course_handler', course_key, 'course_key_string')) - _test_no_locations(self, resp) return resp def test_wiki_slug(self): @@ -1887,7 +1873,6 @@ class EntryPageTestCase(TestCase): def _test_page(self, page, status_code=200): resp = self.client.get_html(page) self.assertEqual(resp.status_code, status_code) - _test_no_locations(self, resp, status_code) def test_how_it_works(self): self._test_page("/howitworks") @@ -1925,19 +1910,3 @@ def _course_factory_create_course(): def _get_course_id(course_data): """Returns the course ID (org/number/run).""" return SlashSeparatedCourseKey(course_data['org'], course_data['number'], course_data['run']) - - -def _test_no_locations(test, resp, status_code=200, html=True): - """ - Verifies that "i4x", which appears in old locations, but not - new locators, does not appear in the HTML response output. - Used to verify that database refactoring is complete. - """ - test.assertNotContains(resp, 'i4x', status_code=status_code, html=html) - if html: - # For HTML pages, it is nice to call the method with html=True because - # it checks that the HTML properly parses. However, it won't find i4x usages - # in JavaScript blocks. - content = resp.content - hits = len(re.findall(r"(?(?:[^/]+/[^/]+/[^/]+)|(?:[^/]+))' +USAGE_KEY_PATTERN = r'(?P(?:i4x://?[^/]+/[^/]+/[^/]+/[^@]+(?:@[^/]+)?)|(?:[^/]+))' +ASSET_KEY_PATTERN = r'(?P(?:/?c4x(:/)?/[^/]+/[^/]+/[^/]+/[^@]+(?:@[^/]+)?)|(?:[^/]+))' + urlpatterns = patterns('', # nopep8 url(r'^transcripts/upload$', 'contentstore.views.upload_transcripts', name='upload_transcripts'), @@ -66,30 +70,30 @@ urlpatterns += patterns( url(r'^signin$', 'login_page', name='login'), url(r'^request_course_creator$', 'request_course_creator'), - url(r'^course_team/(?P[^/]+)/(?P.+)?$', 'course_team_handler'), - url(r'^course_info/(?P[^/]+)$', 'course_info_handler'), + url(r'^course_team/{}/(?P.+)?$'.format(COURSE_KEY_PATTERN), 'course_team_handler'), + url(r'^course_info/{}$'.format(COURSE_KEY_PATTERN), 'course_info_handler'), url( - r'^course_info_update/(?P[^/]+)/(?P\d+)?$', + r'^course_info_update/{}/(?P\d+)?$'.format(COURSE_KEY_PATTERN), 'course_info_update_handler' ), - url(r'^course/(?P[^/]+)?$', 'course_handler', name='course_handler'), - url(r'^subsection/(?P[^/]+)$', 'subsection_handler'), - url(r'^unit/(?P[^/]+)$', 'unit_handler'), - url(r'^container/(?P[^/]+)$', 'container_handler'), - url(r'^checklists/(?P[^/]+)/(?P\d+)?$', 'checklists_handler'), - url(r'^orphan/(?P[^/]+)$', 'orphan_handler'), - url(r'^assets/(?P[^/]+)/(?P.+)?$', 'assets_handler'), - url(r'^import/(?P[^/]+)$', 'import_handler'), - url(r'^import_status/(?P[^/]+)/(?P.+)$', 'import_status_handler'), - url(r'^export/(?P[^/]+)$', 'export_handler'), - url(r'^xblock/(?P[^/]+)/(?P[^/]+)$', 'xblock_view_handler'), - url(r'^xblock/(?P[^/]+)?$', 'xblock_handler'), - url(r'^tabs/(?P[^/]+)$', 'tabs_handler'), - url(r'^settings/details/(?P[^/]+)$', 'settings_handler'), - url(r'^settings/grading/(?P[^/]+)(/)?(?P\d+)?$', 'grading_handler'), - url(r'^settings/advanced/(?P[^/]+)$', 'advanced_settings_handler'), - url(r'^textbooks/(?P[^/]+)$', 'textbooks_list_handler'), - url(r'^textbooks/(?P[^/]+)/(?P\d[^/]*)$', 'textbooks_detail_handler'), + url(r'^course/{}?$'.format(COURSE_KEY_PATTERN), 'course_handler', name='course_handler'), + url(r'^subsection/{}$'.format(USAGE_KEY_PATTERN), 'subsection_handler'), + url(r'^unit/{}$'.format(USAGE_KEY_PATTERN), 'unit_handler'), + url(r'^container/{}$'.format(USAGE_KEY_PATTERN), 'container_handler'), + url(r'^checklists/{}/(?P\d+)?$'.format(COURSE_KEY_PATTERN), 'checklists_handler'), + url(r'^orphan/{}$'.format(COURSE_KEY_PATTERN), 'orphan_handler'), + url(r'^assets/{}/{}?$'.format(COURSE_KEY_PATTERN, ASSET_KEY_PATTERN), 'assets_handler'), + url(r'^import/{}$'.format(COURSE_KEY_PATTERN), 'import_handler'), + url(r'^import_status/{}/(?P.+)$'.format(COURSE_KEY_PATTERN), 'import_status_handler'), + url(r'^export/{}$'.format(COURSE_KEY_PATTERN), 'export_handler'), + url(r'^xblock/{}/(?P[^/]+)$'.format(USAGE_KEY_PATTERN), 'xblock_view_handler'), + url(r'^xblock/{}?$'.format(USAGE_KEY_PATTERN), 'xblock_handler'), + url(r'^tabs/{}$'.format(COURSE_KEY_PATTERN), 'tabs_handler'), + url(r'^settings/details/{}$'.format(COURSE_KEY_PATTERN), 'settings_handler'), + url(r'^settings/grading/{}(/)?(?P\d+)?$'.format(COURSE_KEY_PATTERN), 'grading_handler'), + url(r'^settings/advanced/{}$'.format(COURSE_KEY_PATTERN), 'advanced_settings_handler'), + url(r'^textbooks/{}$'.format(COURSE_KEY_PATTERN), 'textbooks_list_handler'), + url(r'^textbooks/{}/(?P\d[^/]*)$'.format(COURSE_KEY_PATTERN), 'textbooks_detail_handler'), ) js_info_dict = { @@ -105,7 +109,7 @@ urlpatterns += patterns('', if settings.FEATURES.get('ENABLE_EXPORT_GIT'): - urlpatterns += (url(r'^export_git/(?P[^/]+)$', + urlpatterns += (url(r'^export_git/{}$'.format(COURSE_KEY_PATTERN), 'contentstore.views.export_git', name='export_git'),) if settings.FEATURES.get('ENABLE_SERVICE_STATUS'): diff --git a/common/lib/xmodule/xmodule/modulestore/mongo/base.py b/common/lib/xmodule/xmodule/modulestore/mongo/base.py index d9c9270a95..1eddc7afd7 100644 --- a/common/lib/xmodule/xmodule/modulestore/mongo/base.py +++ b/common/lib/xmodule/xmodule/modulestore/mongo/base.py @@ -197,7 +197,7 @@ class CachingDescriptorSystem(MakoDescriptorSystem): del metadata[old_name] children = [ - location.course_key.make_usage_key_from_deprecated_string(childloc) + self._convert_reference_to_key(childloc) for childloc in definition.get('children', []) ] data = definition.get('data', {}) @@ -254,6 +254,13 @@ class CachingDescriptorSystem(MakoDescriptorSystem): error_msg=exc_info_to_str(sys.exc_info()) ) + def _convert_reference_to_key(self, ref_string): + """ + Convert a single serialized UsageKey string in a ReferenceField into a UsageKey. + """ + key = Location.from_deprecated_string(ref_string) + return key.replace(run=self.modulestore._fill_in_run(key.course_key).run) + def _convert_reference_fields_to_keys(self, class_, course_key, jsonfields): """ Find all fields of type reference and convert the payload into UsageKeys @@ -267,15 +274,15 @@ class CachingDescriptorSystem(MakoDescriptorSystem): if field is None: continue elif isinstance(field, Reference): - jsonfields[field_name] = course_key.make_usage_key_from_deprecated_string(value) + jsonfields[field_name] = self._convert_reference_to_key(value) elif isinstance(field, ReferenceList): jsonfields[field_name] = [ - course_key.make_usage_key_from_deprecated_string(ele) for ele in value + self._convert_reference_to_key(ele) for ele in value ] elif isinstance(field, ReferenceValueDict): for key, subvalue in value.iteritems(): assert isinstance(subvalue, basestring) - value[key] = course_key.make_usage_key_from_deprecated_string(subvalue) + value[key] = self._convert_reference_to_key(subvalue) return jsonfields @@ -378,6 +385,7 @@ class MongoModuleStore(ModuleStoreWriteBase): # performance optimization to prevent updating the meta-data inheritance tree during # bulk write operations self.ignore_write_events_on_courses = set() + self._course_run_cache = {} def begin_bulk_write_operation_on_course(self, course_id): """ @@ -394,6 +402,27 @@ class MongoModuleStore(ModuleStoreWriteBase): self.ignore_write_events_on_courses.remove(course_id) self.refresh_cached_metadata_inheritance_tree(course_id) + def _fill_in_run(self, course_key): + if course_key.run is not None: + return course_key + + cache_key = (course_key.org, course_key.course) + if cache_key not in self._course_run_cache: + + matching_courses = list(self.collection.find(SON([ + ('_id.tag', 'i4x'), + ('_id.org', course_key.org), + ('_id.course', course_key.course), + ('_id.category', 'course'), + ])).limit(1)) + + if not matching_courses: + return course_key + + self._course_run_cache[cache_key] = matching_courses[0]['_id']['name'] + + return course_key.replace(run=self._course_run_cache[cache_key]) + def _compute_metadata_inheritance_tree(self, course_id): ''' TODO (cdodge) This method can be deleted when the 'split module store' work has been completed @@ -401,6 +430,7 @@ class MongoModuleStore(ModuleStoreWriteBase): # get all collections in the course, this query should not return any leaf nodes # note this is a bit ugly as when we add new categories of containers, we have to add it here + course_id = self._fill_in_run(course_id) block_types_with_children = set( name for name, class_ in XBlock.load_classes() if getattr(class_, 'has_children', False) ) @@ -476,6 +506,7 @@ class MongoModuleStore(ModuleStoreWriteBase): ''' tree = {} + course_id = self._fill_in_run(course_id) if not force_refresh: # see if we are first in the request cache (if present) if self.request_cache is not None and course_id in self.request_cache.data.get('metadata_inheritance', {}): @@ -554,6 +585,7 @@ class MongoModuleStore(ModuleStoreWriteBase): data = {} to_process = list(items) + course_key = self._fill_in_run(course_key) while to_process and depth is None or depth >= 0: children = [] for item in to_process: @@ -581,6 +613,7 @@ class MongoModuleStore(ModuleStoreWriteBase): """ Load an XModuleDescriptor from item, using the children stored in data_cache """ + course_key = self._fill_in_run(course_key) location = Location._from_deprecated_son(item['location'], course_key.run) data_dir = getattr(item, 'data_dir', location.course) root = self.fs_root / data_dir @@ -617,6 +650,7 @@ class MongoModuleStore(ModuleStoreWriteBase): Load a list of xmodules from the data in items, with children cached up to specified depth """ + course_key = self._fill_in_run(course_key) data_cache = self._cache_children(course_key, items, depth) # if we are loading a course object, if we're not prefetching children (depth != 0) then don't @@ -669,6 +703,7 @@ class MongoModuleStore(ModuleStoreWriteBase): Get the course with the given courseid (org/course/run) """ assert(isinstance(course_key, SlashSeparatedCourseKey)) + course_key = self._fill_in_run(course_key) location = course_key.make_usage_key('course', course_key.run) try: return self.get_item(location, depth=depth) @@ -685,6 +720,7 @@ class MongoModuleStore(ModuleStoreWriteBase): otherwise, do a case sensitive search """ assert(isinstance(course_key, SlashSeparatedCourseKey)) + course_key = self._fill_in_run(course_key) location = course_key.make_usage_key('course', course_key.run) if ignore_case: course_query = location.to_deprecated_son('_id.') @@ -873,6 +909,7 @@ class MongoModuleStore(ModuleStoreWriteBase): :param runtime: if you already have an xblock from the course, the xblock.runtime value :param fields: a dictionary of field names and values for the new xmodule """ + location = location.replace(run=self._fill_in_run(location.course_key).run) # differs from split mongo in that I believe most of this logic should be above the persistence # layer but added it here to enable quick conversion. I'll need to reconcile these. if metadata is None: @@ -1073,6 +1110,7 @@ class MongoModuleStore(ModuleStoreWriteBase): """ Return an array of all of the locations (deprecated string format) for orphans in the course. """ + course_key = self._fill_in_run(course_key) detached_categories = [name for name, __ in XBlock.load_tagged_classes("detached")] query = self._course_key_to_son(course_key) query['_id.category'] = {'$nin': detached_categories}