diff --git a/cms/djangoapps/contentstore/management/commands/clone_course.py b/cms/djangoapps/contentstore/management/commands/clone_course.py index 5c961a9767..be9b0bcb52 100644 --- a/cms/djangoapps/contentstore/management/commands/clone_course.py +++ b/cms/djangoapps/contentstore/management/commands/clone_course.py @@ -2,10 +2,8 @@ Script for cloning a course """ from django.core.management.base import BaseCommand, CommandError -from xmodule.modulestore.store_utilities import clone_course from xmodule.modulestore.django import modulestore from xmodule.modulestore.mixed import store_bulk_write_operations_on_course -from xmodule.contentstore.django import contentstore from student.roles import CourseInstructorRole, CourseStaffRole from opaque_keys.edx.keys import CourseKey from opaque_keys import InvalidKeyError @@ -37,12 +35,11 @@ class Command(BaseCommand): dest_course_id = self.course_key_from_arg(args[1]) mstore = modulestore() - cstore = contentstore() print("Cloning course {0} to {1}".format(source_course_id, dest_course_id)) with store_bulk_write_operations_on_course(mstore, dest_course_id): - if clone_course(mstore, cstore, source_course_id, dest_course_id, None): + if mstore.clone_course(source_course_id, dest_course_id, None): print("copying User permissions...") # purposely avoids auth.add_user b/c it doesn't have a caller to authorize CourseInstructorRole(dest_course_id).add_users( diff --git a/cms/djangoapps/contentstore/management/commands/migrate_to_split.py b/cms/djangoapps/contentstore/management/commands/migrate_to_split.py index ede707079f..7b1845ea4d 100644 --- a/cms/djangoapps/contentstore/management/commands/migrate_to_split.py +++ b/cms/djangoapps/contentstore/management/commands/migrate_to_split.py @@ -1,3 +1,5 @@ +# pylint: disable=W0212 + """ Django management command to migrate a course from the old Mongo modulestore to the new split-Mongo modulestore. @@ -10,6 +12,7 @@ from xmodule.modulestore.django import loc_mapper from opaque_keys.edx.keys import CourseKey from opaque_keys import InvalidKeyError from opaque_keys.edx.locations import SlashSeparatedCourseKey +from xmodule.modulestore import ModuleStoreEnum def user_from_str(identifier): @@ -66,9 +69,8 @@ class Command(BaseCommand): course_key, user, org, offering = self.parse_args(*args) migrator = SplitMigrator( - draft_modulestore=modulestore('default'), - direct_modulestore=modulestore('direct'), - split_modulestore=modulestore('split'), + draft_modulestore=modulestore()._get_modulestore_by_type(ModuleStoreEnum.Type.mongo), + split_modulestore=modulestore()._get_modulestore_by_type(ModuleStoreEnum.Type.split), loc_mapper=loc_mapper(), ) diff --git a/cms/djangoapps/contentstore/management/commands/tests/test_rollback_split_course.py b/cms/djangoapps/contentstore/management/commands/tests/test_rollback_split_course.py index 3eacdaf672..4083f4a4f8 100644 --- a/cms/djangoapps/contentstore/management/commands/tests/test_rollback_split_course.py +++ b/cms/djangoapps/contentstore/management/commands/tests/test_rollback_split_course.py @@ -14,7 +14,9 @@ from xmodule.modulestore.tests.factories import CourseFactory from xmodule.modulestore.django import modulestore, loc_mapper from xmodule.modulestore.exceptions import ItemNotFoundError from xmodule.modulestore.split_migrator import SplitMigrator +from xmodule.modulestore import ModuleStoreEnum # pylint: disable=E1101 +# pylint: disable=W0212 @unittest.skip("Not fixing split mongo until we land opaque-keys 0.9") @@ -87,9 +89,8 @@ class TestRollbackSplitCourse(ModuleStoreTestCase): # migrate old course to split migrator = SplitMigrator( - draft_modulestore=modulestore('default'), - direct_modulestore=modulestore('direct'), - split_modulestore=modulestore('split'), + draft_modulestore=modulestore()._get_modulestore_by_type(ModuleStoreEnum.Type.mongo), + split_modulestore=modulestore()._get_modulestore_by_type(ModuleStoreEnum.Type.split), loc_mapper=loc_mapper(), ) migrator.migrate_mongo_course(self.old_course.location, self.user) diff --git a/cms/djangoapps/contentstore/tests/test_clone_course.py b/cms/djangoapps/contentstore/tests/test_clone_course.py new file mode 100644 index 0000000000..ac6d5821a5 --- /dev/null +++ b/cms/djangoapps/contentstore/tests/test_clone_course.py @@ -0,0 +1,44 @@ +""" +Unit tests for cloning a course between the same and different module stores. +""" +from django.utils.unittest.case import skipIf +from opaque_keys.edx.locations import SlashSeparatedCourseKey +from opaque_keys.edx.locator import CourseLocator +from xmodule.modulestore import ModuleStoreEnum +from contentstore.tests.utils import CourseTestCase + + +@skipIf( + not 'run' in CourseLocator.KEY_FIELDS, + "Pending integration with latest opaque-keys library - need removal of offering, make_asset_key on CourseLocator, etc." +) +class CloneCourseTest(CourseTestCase): + """ + Unit tests for cloning a course + """ + def test_clone_course(self): + """Tests cloning of a course as follows: XML -> Mongo (+ data) -> Mongo -> Split -> Split""" + # 1. import and populate test toy course + mongo_course1_id = self.import_and_populate_course() + self.check_populated_course(mongo_course1_id) + + # 2. clone course (mongo -> mongo) + # TODO - This is currently failing since clone_course doesn't handle Private content - fails on Publish + mongo_course2_id = SlashSeparatedCourseKey('edX2', 'toy2', '2013_Fall') + self.store.clone_course(mongo_course1_id, mongo_course2_id, self.user.id) + self.assertCoursesEqual(mongo_course1_id, mongo_course2_id) + + # 3. clone course (mongo -> split) + with self.store.set_default_store(ModuleStoreEnum.Type.split): + split_course3_id = CourseLocator( + org="edx3", course="split3", run="2013_Fall", branch=ModuleStoreEnum.BranchName.draft + ) + self.store.clone_course(mongo_course2_id, split_course3_id, self.user.id) + self.assertCoursesEqual(mongo_course2_id, split_course3_id) + + # 4. clone course (split -> split) + split_course4_id = CourseLocator( + org="edx4", course="split4", run="2013_Fall", branch=ModuleStoreEnum.BranchName.draft + ) + self.store.clone_course(split_course3_id, split_course4_id, self.user.id) + self.assertCoursesEqual(split_course3_id, split_course4_id) diff --git a/cms/djangoapps/contentstore/tests/test_contentstore.py b/cms/djangoapps/contentstore/tests/test_contentstore.py index 2259d81ba8..795a372824 100644 --- a/cms/djangoapps/contentstore/tests/test_contentstore.py +++ b/cms/djangoapps/contentstore/tests/test_contentstore.py @@ -1,16 +1,15 @@ # -*- coding: utf-8 -*- # pylint: disable=E1101 +# pylint: disable=W0212 import copy import mock -import re import shutil from datetime import timedelta from fs.osfs import OSFS from json import loads from path import path -from pymongo import MongoClient from tempdir import mkdtemp_clean from textwrap import dedent from uuid import uuid4 @@ -20,23 +19,19 @@ from django.contrib.auth.models import User from django.test import TestCase from django.test.utils import override_settings -from contentstore.tests.utils import parse_json, AjaxEnabledTestClient +from contentstore.tests.utils import parse_json, AjaxEnabledTestClient, CourseTestCase from contentstore.views.component import ADVANCED_COMPONENT_TYPES -from xmodule.contentstore.content import StaticContent from xmodule.contentstore.django import contentstore, _CONTENTSTORE from xmodule.contentstore.utils import restore_asset_from_trashcan, empty_asset_trashcan from xmodule.exceptions import NotFoundError, InvalidVersionError -from xmodule.modulestore import mongo, PublishState, ModuleStoreEnum -from xmodule.modulestore.mongo.base import MongoRevisionKey +from xmodule.modulestore import ModuleStoreEnum from xmodule.modulestore.mixed import store_branch_setting -from xmodule.modulestore.django import modulestore from xmodule.modulestore.exceptions import ItemNotFoundError from xmodule.modulestore.inheritance import own_metadata from opaque_keys.edx.keys import UsageKey from opaque_keys.edx.locations import SlashSeparatedCourseKey, AssetLocation -from xmodule.modulestore.store_utilities import clone_course, delete_course -from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase +from xmodule.modulestore.store_utilities import delete_course from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory from xmodule.modulestore.xml_exporter import export_to_xml from xmodule.modulestore.xml_importer import import_from_xml, perform_xlint @@ -52,6 +47,7 @@ from student import auth from student.models import CourseEnrollment from student.roles import CourseCreatorRole, CourseInstructorRole from opaque_keys import InvalidKeyError +from contentstore.tests.utils import get_url TEST_DATA_CONTENTSTORE = copy.deepcopy(settings.CONTENTSTORE) @@ -68,37 +64,12 @@ class MongoCollectionFindWrapper(object): return self.original(query, *args, **kwargs) -def get_url(handler_name, key_value, key_name='usage_key_string', kwargs=None): - # Helper function for getting HTML for a page in Studio and - # checking that it does not error. - return reverse_url(handler_name, key_name, key_value, kwargs) - - @override_settings(CONTENTSTORE=TEST_DATA_CONTENTSTORE) -class ContentStoreTestCase(ModuleStoreTestCase): +class ContentStoreTestCase(CourseTestCase): """ Base class for Content Store Test Cases """ - def setUp(self): - uname = 'testuser' - email = 'test+courses@edx.org' - password = 'foo' - - # Create the use so we can log them in. - self.user = User.objects.create_user(uname, email, password) - - # Note that we do not actually need to do anything - # for registration if we directly mark them active. - self.user.is_active = True - # Staff has access to view all courses - self.user.is_staff = True - - # Save the data that we've just changed to the db. - self.user.save() - - self.client = AjaxEnabledTestClient() - self.client.login(username=uname, password=password) - + pass class ContentStoreToyCourseTest(ContentStoreTestCase): """ @@ -106,7 +77,7 @@ class ContentStoreToyCourseTest(ContentStoreTestCase): TODO: refactor using CourseFactory so they do not. """ def tearDown(self): - MongoClient().drop_database(TEST_DATA_CONTENTSTORE['DOC_STORE_CONFIG']['db']) + contentstore().drop_database() _CONTENTSTORE.clear() def check_components_on_page(self, component_types, expected_types): @@ -121,7 +92,7 @@ class ContentStoreToyCourseTest(ContentStoreTestCase): exactly the same -- for example, 'video' in component_types should cause 'Video' to be present. """ - store = modulestore() + store = self.store _, course_items = import_from_xml(store, self.user.id, 'common/test/data/', ['simple']) course = course_items[0] course.advanced_modules = component_types @@ -148,7 +119,7 @@ class ContentStoreToyCourseTest(ContentStoreTestCase): self.check_components_on_page(['word_cloud'], ['Word cloud']) def test_malformed_edit_unit_request(self): - store = modulestore() + store = self.store _, course_items = import_from_xml(store, self.user.id, 'common/test/data/', ['simple']) # just pick one vertical @@ -158,23 +129,12 @@ class ContentStoreToyCourseTest(ContentStoreTestCase): self.assertEqual(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]) + """Verifies the editing HTML in all the verticals in the given test course""" + _, course_items = import_from_xml(self.store, self.user.id, 'common/test/data/', [test_course_name]) - items = modulestore().get_items(course_items[0].id, category='vertical') + items = self.store.get_items(course_items[0].id, category='vertical') self._check_verticals(items) - def _lock_an_asset(self, content_store, course_id): - """ - Lock an arbitrary asset in the course - :param course_location: - """ - course_assets, __ = content_store.get_all_content_for_course(course_id) - self.assertGreater(len(course_assets), 0, "No assets to lock") - asset_id = course_assets[0]['_id'] - asset_key = StaticContent.compute_location(course_id, asset_id['name']) - content_store.set_attr(asset_key, 'locked', True) - return asset_key - def test_edit_unit_toy(self): self.check_edit_unit('toy') @@ -191,7 +151,7 @@ class ContentStoreToyCourseTest(ContentStoreTestCase): Unfortunately, None = published for the revision field, so get_items() would return both draft and non-draft copies. ''' - store = modulestore() + store = self.store _, course_items = import_from_xml(store, self.user.id, 'common/test/data/', ['simple']) course_key = course_items[0].id html_usage_key = course_key.make_usage_key('html', 'test_html') @@ -218,7 +178,7 @@ class ContentStoreToyCourseTest(ContentStoreTestCase): module as 'own-metadata' when publishing. Also verifies the metadata inheritance is properly computed ''' - draft_store = modulestore() + draft_store = self.store import_from_xml(draft_store, self.user.id, 'common/test/data/', ['simple']) course_key = SlashSeparatedCourseKey('edX', 'simple', '2012_Fall') @@ -279,7 +239,7 @@ class ContentStoreToyCourseTest(ContentStoreTestCase): self.assertEqual(html_module.graceperiod, new_graceperiod) def test_get_depth_with_drafts(self): - store = modulestore() + store = self.store import_from_xml(store, self.user.id, 'common/test/data/', ['simple']) course_key = SlashSeparatedCourseKey('edX', 'simple', '2012_Fall') @@ -307,16 +267,15 @@ class ContentStoreToyCourseTest(ContentStoreTestCase): self.assertEqual(num_drafts, 1) def test_no_static_link_rewrites_on_import(self): - module_store = modulestore() - _, course_items = import_from_xml(module_store, self.user.id, 'common/test/data/', ['toy']) + _, course_items = import_from_xml(self.store, self.user.id, 'common/test/data/', ['toy']) course = course_items[0] handouts_usage_key = course.id.make_usage_key('course_info', 'handouts') - handouts = module_store.get_item(handouts_usage_key) + handouts = self.store.get_item(handouts_usage_key) self.assertIn('/static/', handouts.data) handouts_usage_key = course.id.make_usage_key('html', 'toyhtml') - handouts = module_store.get_item(handouts_usage_key) + handouts = self.store.get_item(handouts_usage_key) self.assertIn('/static/', handouts.data) @mock.patch('xmodule.course_module.requests.get') @@ -327,17 +286,15 @@ class ContentStoreToyCourseTest(ContentStoreTestCase): """).strip() - module_store = modulestore() - import_from_xml(module_store, self.user.id, 'common/test/data/', ['toy']) - course = module_store.get_course(SlashSeparatedCourseKey('edX', 'toy', '2012_Fall')) + import_from_xml(self.store, self.user.id, 'common/test/data/', ['toy']) + course = self.store.get_course(SlashSeparatedCourseKey('edX', 'toy', '2012_Fall')) self.assertGreater(len(course.textbooks), 0) def test_import_polls(self): - module_store = modulestore() - _, course_items = import_from_xml(module_store, self.user.id, 'common/test/data/', ['toy']) + _, course_items = import_from_xml(self.store, self.user.id, 'common/test/data/', ['toy']) course_key = course_items[0].id - items = module_store.get_items(course_key, category='poll_question') + items = self.store.get_items(course_key, category='poll_question') found = len(items) > 0 self.assertTrue(found) @@ -353,7 +310,7 @@ class ContentStoreToyCourseTest(ContentStoreTestCase): """ Tests the ajax callback to render an XModule """ - direct_store = modulestore() + direct_store = self.store _, course_items = import_from_xml(direct_store, self.user.id, 'common/test/data/', ['toy']) usage_key = course_items[0].id.make_usage_key('vertical', 'vertical_test') @@ -370,7 +327,7 @@ class ContentStoreToyCourseTest(ContentStoreTestCase): self.assertContains(resp, 'edX+toy+2012_Fall+poll_question+T1_changemind_poll_foo_2') def test_delete(self): - store = modulestore() + store = self.store course = CourseFactory.create(org='edX', course='999', display_name='Robot Super Course') chapterloc = ItemFactory.create(parent_location=course.location, display_name="Chapter").location @@ -405,14 +362,13 @@ class ContentStoreToyCourseTest(ContentStoreTestCase): This test case verifies that a course can use specialized override for about data, e.g. /about/Fall_2012/effort.html while there is a base definition in /about/effort.html ''' - module_store = modulestore() - _, course_items = import_from_xml(module_store, self.user.id, 'common/test/data/', ['toy']) + _, course_items = import_from_xml(self.store, self.user.id, 'common/test/data/', ['toy']) course_key = course_items[0].id - effort = module_store.get_item(course_key.make_usage_key('about', 'effort')) + effort = self.store.get_item(course_key.make_usage_key('about', 'effort')) self.assertEqual(effort.data, '6 hours') # this one should be in a non-override folder - effort = module_store.get_item(course_key.make_usage_key('about', 'end_date')) + effort = self.store.get_item(course_key.make_usage_key('about', 'end_date')) self.assertEqual(effort.data, 'TBD') def test_asset_import(self): @@ -421,10 +377,9 @@ class ContentStoreToyCourseTest(ContentStoreTestCase): ''' content_store = contentstore() - module_store = modulestore() - import_from_xml(module_store, self.user.id, 'common/test/data/', ['toy'], static_content_store=content_store, verbose=True) + import_from_xml(self.store, self.user.id, 'common/test/data/', ['toy'], static_content_store=content_store, verbose=True) - course = module_store.get_course(SlashSeparatedCourseKey('edX', 'toy', '2012_Fall')) + course = self.store.get_course(SlashSeparatedCourseKey('edX', 'toy', '2012_Fall')) self.assertIsNotNone(course) @@ -510,8 +465,7 @@ class ContentStoreToyCourseTest(ContentStoreTestCase): content_store = contentstore() trash_store = contentstore('trashcan') - module_store = modulestore() - _, course_items = import_from_xml(module_store, self.user.id, 'common/test/data/', ['toy'], static_content_store=content_store) + _, course_items = import_from_xml(self.store, self.user.id, 'common/test/data/', ['toy'], static_content_store=content_store) # look up original (and thumbnail) in content store, should be there after import location = AssetLocation.from_deprecated_string('/c4x/edX/toy/asset/sample_static.txt') @@ -542,17 +496,16 @@ class ContentStoreToyCourseTest(ContentStoreTestCase): Test that course info updates are imported and exported with all content fields ('data', 'items') """ content_store = contentstore() - module_store = modulestore() data_dir = "common/test/data/" - import_from_xml(module_store, self.user.id, data_dir, ['course_info_updates'], + import_from_xml(self.store, self.user.id, data_dir, ['course_info_updates'], static_content_store=content_store, verbose=True) course_id = SlashSeparatedCourseKey('edX', 'course_info_updates', '2014_T1') - course = module_store.get_course(course_id) + course = self.store.get_course(course_id) self.assertIsNotNone(course) - course_updates = module_store.get_item(course_id.make_usage_key('course_info', 'updates')) + course_updates = self.store.get_item(course_id.make_usage_key('course_info', 'updates')) self.assertIsNotNone(course_updates) @@ -576,7 +529,7 @@ class ContentStoreToyCourseTest(ContentStoreTestCase): # with same content as in course 'info' directory root_dir = path(mkdtemp_clean()) print 'Exporting to tempdir = {0}'.format(root_dir) - export_to_xml(module_store, content_store, course_id, root_dir, 'test_export') + export_to_xml(self.store, content_store, course_id, root_dir, 'test_export') # check that exported course has files 'updates.html' and 'updates.items.json' filesystem = OSFS(root_dir / 'test_export/info') @@ -622,119 +575,8 @@ class ContentStoreToyCourseTest(ContentStoreTestCase): all_thumbnails = trash_store.get_all_content_thumbnails_for_course(course_id) self.assertEqual(len(all_thumbnails), 0) - def test_clone_course(self): - - course_data = { - 'org': 'MITx', - 'number': '999', - 'display_name': 'Robot Super Course', - 'run': '2013_Spring', - } - - module_store = modulestore() - _, course_items = import_from_xml(module_store, self.user.id, 'common/test/data/', ['toy']) - - source_course_id = course_items[0].id - dest_course_id = _get_course_id(course_data) - - # get a vertical (and components in it) to put into DRAFT - # this is to assert that draft content is also cloned over - vertical = module_store.get_item( - source_course_id.make_usage_key('vertical', 'vertical_test'), - depth=1 - ) - - module_store.convert_to_draft(vertical.location, self.user.id) - - items = module_store.get_items(source_course_id) - self.assertGreater(len(items), 0) - - _create_course(self, dest_course_id, course_data) - - content_store = contentstore() - - # now do the actual cloning - clone_course(module_store, content_store, source_course_id, dest_course_id, self.user.id) - - # first assert that all draft content got cloned as well - draft_items = module_store.get_items(source_course_id, revision=ModuleStoreEnum.RevisionOption.draft_only) - self.assertGreater(len(draft_items), 0) - draft_clone_items = module_store.get_items(dest_course_id, revision=ModuleStoreEnum.RevisionOption.draft_only) - self.assertGreater(len(draft_clone_items), 0) - self.assertEqual(len(draft_items), len(draft_clone_items)) - - # now loop through all the units in the course and verify that the clone can render them, which - # means the objects are at least present - items = module_store.get_items(source_course_id) - self.assertGreater(len(items), 0) - clone_items = module_store.get_items(dest_course_id) - self.assertGreater(len(clone_items), 0) - - for descriptor in items: - source_item = module_store.get_item(descriptor.location) - new_loc = descriptor.location.map_into_course(dest_course_id) - if descriptor.location.category == 'course': - new_loc = new_loc.replace(name=new_loc.run) - print "Checking {0} should now also be at {1}".format(descriptor.location, new_loc) - lookup_item = module_store.get_item(new_loc) - - if hasattr(source_item, 'data') and hasattr(lookup_item, 'data'): - self.assertEqual(source_item.data, lookup_item.data) - - # also make sure that metadata was cloned over and filtered with own_metadata, i.e. inherited - # values were not explicitly set - self.assertEqual(own_metadata(source_item), own_metadata(lookup_item)) - - # check that the children are as expected - self.assertEqual(source_item.has_children, lookup_item.has_children) - if source_item.has_children: - expected_children = [] - for child_loc in source_item.children: - child_loc = child_loc.map_into_course(dest_course_id) - expected_children.append(child_loc) - self.assertEqual(expected_children, lookup_item.children) - - def test_portable_link_rewrites_during_clone_course(self): - course_data = { - 'org': 'MITx', - 'number': '999', - 'display_name': 'Robot Super Course', - 'run': '2013_Spring' - } - - module_store = modulestore() - content_store = contentstore() - - import_from_xml(module_store, self.user.id, 'common/test/data/', ['toy']) - - source_course_id = SlashSeparatedCourseKey('edX', 'toy', '2012_Fall') - dest_course_id = _get_course_id(course_data) - - # let's force a non-portable link in the clone source - # as a final check, make sure that any non-portable links are rewritten during cloning - html_module = module_store.get_item(source_course_id.make_usage_key('html', 'nonportable')) - - self.assertIsInstance(html_module.data, basestring) - new_data = html_module.data = html_module.data.replace('/static/', '/c4x/{0}/{1}/asset/'.format( - source_course_id.org, source_course_id.run)) - module_store.update_item(html_module, self.user.id) - - html_module = module_store.get_item(html_module.location) - self.assertEqual(new_data, html_module.data) - - # create the destination course - _create_course(self, dest_course_id, course_data) - - # do the actual cloning - clone_course(module_store, content_store, source_course_id, dest_course_id, self.user.id) - - # make sure that any non-portable links are rewritten during cloning - html_module = module_store.get_item(dest_course_id.make_usage_key('html', 'nonportable')) - - self.assertIn('/asset/foo.jpg', html_module.data) - def test_illegal_draft_crud_ops(self): - draft_store = modulestore() + draft_store = self.store course = CourseFactory.create(org='MITx', course='999', display_name='Robot Super Course') @@ -759,20 +601,19 @@ class ContentStoreToyCourseTest(ContentStoreTestCase): self.assertEqual(resp.status_code, 400) def test_rewrite_nonportable_links_on_import(self): - module_store = modulestore() content_store = contentstore() - import_from_xml(module_store, self.user.id, 'common/test/data/', ['toy'], static_content_store=content_store) + import_from_xml(self.store, self.user.id, 'common/test/data/', ['toy'], static_content_store=content_store) # first check a static asset link course_key = SlashSeparatedCourseKey('edX', 'toy', 'run') html_module_location = course_key.make_usage_key('html', 'nonportable') - html_module = module_store.get_item(html_module_location) + html_module = self.store.get_item(html_module_location) self.assertIn('/static/foo.jpg', html_module.data) # then check a intra courseware link html_module_location = course_key.make_usage_key('html', 'nonportable_link') - html_module = module_store.get_item(html_module_location) + html_module = self.store.get_item(html_module_location) self.assertIn('/jump_to_id/nonportable_link', html_module.data) def test_delete_course(self): @@ -780,24 +621,23 @@ class ContentStoreToyCourseTest(ContentStoreTestCase): This test will import a course, make a draft item, and delete it. This will also assert that the draft content is also deleted """ - module_store = modulestore() content_store = contentstore() - _, course_items = import_from_xml(module_store, self.user.id, 'common/test/data/', ['toy'], static_content_store=content_store) + _, course_items = import_from_xml(self.store, self.user.id, 'common/test/data/', ['toy'], static_content_store=content_store) course_id = course_items[0].id # get a vertical (and components in it) to put into DRAFT - vertical = module_store.get_item(course_id.make_usage_key('vertical', 'vertical_test'), depth=1) + vertical = self.store.get_item(course_id.make_usage_key('vertical', 'vertical_test'), depth=1) - module_store.convert_to_draft(vertical.location, self.user.id) + self.store.convert_to_draft(vertical.location, self.user.id) # delete the course - delete_course(module_store, content_store, course_id, commit=True) + delete_course(self.store, content_store, course_id, commit=True) # assert that there's absolutely no non-draft modules in the course # this should also include all draft items - items = module_store.get_items(course_id) + items = self.store.get_items(course_id) self.assertEqual(len(items), 0) # assert that all content in the asset library is also deleted @@ -823,74 +663,26 @@ class ContentStoreToyCourseTest(ContentStoreTestCase): """).strip() - module_store = modulestore() content_store = contentstore() - - import_from_xml(module_store, self.user.id, 'common/test/data/', ['toy'], static_content_store=content_store) - course_id = SlashSeparatedCourseKey('edX', 'toy', '2012_Fall') - - # get a vertical (and components in it) to copy into an orphan sub dag - vertical = module_store.get_item(course_id.make_usage_key('vertical', 'vertical_test'), depth=1) - # We had a bug where orphaned draft nodes caused export to fail. This is here to cover that case. - vertical.location = vertical.location.replace(name='no_references') - - module_store.update_item(vertical, self.user.id, allow_not_found=True) - orphan_vertical = module_store.get_item(vertical.location) - self.assertEqual(orphan_vertical.location.name, 'no_references') - - # get the original vertical (and components in it) to put into DRAFT - vertical = module_store.get_item(course_id.make_usage_key('vertical', 'vertical_test'), depth=1) - self.assertEqual(len(orphan_vertical.children), len(vertical.children)) - draft_vertical = module_store.convert_to_draft(vertical.location, self.user.id) - self.assertEqual(module_store.compute_publish_state(draft_vertical), PublishState.draft) + course_id = self.import_and_populate_course() root_dir = path(mkdtemp_clean()) - - # now create a new/different private (draft only) vertical - vertical.location = mongo.draft.as_draft(course_id.make_usage_key('vertical', 'a_private_vertical')) - vertical = module_store.create_and_save_xmodule(vertical.location, self.user.id) - self.assertEqual(module_store.compute_publish_state(vertical), PublishState.private) - private_vertical = module_store.get_item(vertical.location) - vertical = None # blank out b/c i destructively manipulated its location 2 lines above - - # now create a new/different published (no draft) vertical - public_vertical_location = course_id.make_usage_key('vertical', 'a_published_vertical') - module_store.create_and_save_xmodule(public_vertical_location, self.user.id) - public_vertical = module_store.publish(public_vertical_location, self.user.id) - self.assertEqual(module_store.compute_publish_state(public_vertical), PublishState.public) - - # add the new private and new public to list of children - sequential = module_store.get_item(course_id.make_usage_key('sequential', 'vertical_sequential')) - private_location_no_draft = private_vertical.location.replace(revision=MongoRevisionKey.published) - sequential.children.append(private_location_no_draft) - sequential.children.append(public_vertical_location) - module_store.update_item(sequential, self.user.id) - - # read back the sequential, to make sure we have a pointer to - sequential = module_store.get_item(course_id.make_usage_key('sequential', 'vertical_sequential')) - self.assertIn(private_location_no_draft, sequential.children) - - locked_asset_key = self._lock_an_asset(content_store, course_id) - locked_asset_attrs = content_store.get_attrs(locked_asset_key) - # the later import will reupload - del locked_asset_attrs['uploadDate'] - print 'Exporting to tempdir = {0}'.format(root_dir) # export out to a tempdir - export_to_xml(module_store, content_store, course_id, root_dir, 'test_export') + export_to_xml(self.store, content_store, course_id, root_dir, 'test_export') # check for static tabs - self.verify_content_existence(module_store, root_dir, course_id, 'tabs', 'static_tab', '.html') + self.verify_content_existence(self.store, root_dir, course_id, 'tabs', 'static_tab', '.html') # check for about content - self.verify_content_existence(module_store, root_dir, course_id, 'about', 'about', '.html') + self.verify_content_existence(self.store, root_dir, course_id, 'about', 'about', '.html') # check for grading_policy.json filesystem = OSFS(root_dir / 'test_export/policies/2012_Fall') self.assertTrue(filesystem.exists('grading_policy.json')) - course = module_store.get_course(course_id) + course = self.store.get_course(course_id) # compare what's on disk compared to what we have in our course with filesystem.open('grading_policy.json', 'r') as grading_policy: on_disk = loads(grading_policy.read()) @@ -906,25 +698,23 @@ class ContentStoreToyCourseTest(ContentStoreTestCase): self.assertEqual(on_disk['course/2012_Fall'], own_metadata(course)) # remove old course - delete_course(module_store, content_store, course_id, commit=True) + delete_course(self.store, content_store, course_id, commit=True) + # reimport over old course - self.check_import( - module_store, root_dir, content_store, course_id, - locked_asset_key, locked_asset_attrs - ) + self.check_import(root_dir, content_store, course_id) + # import to different course id - self.check_import( - module_store, root_dir, content_store, SlashSeparatedCourseKey('anotherX', 'anotherToy', 'Someday'), - locked_asset_key, locked_asset_attrs - ) + new_course_id = SlashSeparatedCourseKey('anotherX', 'anotherToy', 'Someday') + self.check_import(root_dir, content_store, new_course_id) + self.assertCoursesEqual(course_id, new_course_id) shutil.rmtree(root_dir) - def check_import(self, module_store, root_dir, content_store, course_id, - locked_asset_key, locked_asset_attrs): + def check_import(self, root_dir, content_store, course_id): + """Imports the course in root_dir into the given course_id and verifies its content""" # reimport import_from_xml( - module_store, + self.store, self.user.id, root_dir, ['test_export'], @@ -932,74 +722,33 @@ class ContentStoreToyCourseTest(ContentStoreTestCase): target_course_id=course_id, ) - items = module_store.get_items( - course_id, - category='vertical', - revision=ModuleStoreEnum.RevisionOption.published_only - ) - self._check_verticals(items) + # verify content of the course + self.check_populated_course(course_id) - def verify_item_publish_state(item, publish_state): - if publish_state in (PublishState.private, PublishState.draft): - self.assertTrue(getattr(item, 'is_draft', False)) - else: - self.assertFalse(getattr(item, 'is_draft', False)) - self.assertEqual(module_store.compute_publish_state(item), publish_state) + # verify additional export attributes + def verify_export_attrs_removed(attributes): + """Verifies all temporary attributes added during export are removed""" + self.assertNotIn('index_in_children_list', attributes) + self.assertNotIn('parent_sequential_url', attributes) - def get_and_verify_item_publish_state(item_type, item_name, publish_state): - item = module_store.get_item(course_id.make_usage_key(item_type, item_name)) - verify_item_publish_state(item, publish_state) - return item - - # verify that the draft vertical is draft - vertical = get_and_verify_item_publish_state('vertical', 'vertical_test', PublishState.draft) - self.assertNotIn('index_in_children_list', vertical.xml_attributes) - self.assertNotIn('parent_sequential_url', vertical.xml_attributes) + vertical = self.store.get_item(course_id.make_usage_key('vertical', self.TEST_VERTICAL)) + verify_export_attrs_removed(vertical.xml_attributes) for child in vertical.get_children(): - verify_item_publish_state(child, PublishState.draft) - self.assertNotIn('index_in_children_list', child.xml_attributes) + verify_export_attrs_removed(child.xml_attributes) if hasattr(child, 'data'): - self.assertNotIn('index_in_children_list', child.data) - self.assertNotIn('parent_sequential_url', child.xml_attributes) - if hasattr(child, 'data'): - self.assertNotIn('parent_sequential_url', child.data) - - # make sure that we don't have a sequential that is not in draft mode - get_and_verify_item_publish_state('sequential', 'vertical_sequential', PublishState.public) - - # verify that we have the private vertical - get_and_verify_item_publish_state('vertical', 'a_private_vertical', PublishState.private) - - # verify that we have the public vertical - get_and_verify_item_publish_state('vertical', 'a_published_vertical', PublishState.public) - - # make sure the textbook survived the export/import - course = module_store.get_course(course_id) - - self.assertGreater(len(course.textbooks), 0) - - locked_asset_key = locked_asset_key.map_into_course(course_id) - new_attrs = content_store.get_attrs(locked_asset_key) - for key, value in locked_asset_attrs.iteritems(): - if key == '_id': - self.assertEqual(value['name'], new_attrs[key]['name']) - elif key == 'filename': - pass - else: - self.assertEqual(value, new_attrs[key]) + verify_export_attrs_removed(child.data) def test_export_course_with_metadata_only_video(self): - module_store = modulestore() content_store = contentstore() - import_from_xml(module_store, self.user.id, 'common/test/data/', ['toy']) + import_from_xml(self.store, self.user.id, 'common/test/data/', ['toy']) course_id = SlashSeparatedCourseKey('edX', 'toy', '2012_Fall') # create a new video module and add it as a child to a vertical # this re-creates a bug whereby since the video template doesn't have # anything in 'data' field, the export was blowing up - verticals = module_store.get_items(course_id, category='vertical') + verticals = self.store.get_items(course_id, category='vertical') self.assertGreater(len(verticals), 0) @@ -1012,7 +761,7 @@ class ContentStoreToyCourseTest(ContentStoreTestCase): print 'Exporting to tempdir = {0}'.format(root_dir) # export out to a tempdir - export_to_xml(module_store, content_store, course_id, root_dir, 'test_export') + export_to_xml(self.store, content_store, course_id, root_dir, 'test_export') shutil.rmtree(root_dir) @@ -1020,13 +769,12 @@ class ContentStoreToyCourseTest(ContentStoreTestCase): """ Similar to `test_export_course_with_metadata_only_video`. """ - module_store = modulestore() content_store = contentstore() - import_from_xml(module_store, self.user.id, 'common/test/data/', ['word_cloud']) + import_from_xml(self.store, self.user.id, 'common/test/data/', ['word_cloud']) course_id = SlashSeparatedCourseKey('HarvardX', 'ER22x', '2013_Spring') - verticals = module_store.get_items(course_id, category='vertical') + verticals = self.store.get_items(course_id, category='vertical') self.assertGreater(len(verticals), 0) @@ -1039,7 +787,7 @@ class ContentStoreToyCourseTest(ContentStoreTestCase): print 'Exporting to tempdir = {0}'.format(root_dir) # export out to a tempdir - export_to_xml(module_store, content_store, course_id, root_dir, 'test_export') + export_to_xml(self.store, content_store, course_id, root_dir, 'test_export') shutil.rmtree(root_dir) @@ -1048,13 +796,12 @@ class ContentStoreToyCourseTest(ContentStoreTestCase): Test that an empty `data` field is preserved through export/import. """ - module_store = modulestore() content_store = contentstore() - import_from_xml(module_store, self.user.id, 'common/test/data/', ['toy']) + import_from_xml(self.store, self.user.id, 'common/test/data/', ['toy']) course_id = SlashSeparatedCourseKey('edX', 'toy', '2012_Fall') - verticals = module_store.get_items(course_id, category='vertical') + verticals = self.store.get_items(course_id, category='vertical') self.assertGreater(len(verticals), 0) @@ -1067,11 +814,11 @@ class ContentStoreToyCourseTest(ContentStoreTestCase): # Export the course root_dir = path(mkdtemp_clean()) - export_to_xml(module_store, content_store, course_id, root_dir, 'test_roundtrip') + export_to_xml(self.store, content_store, course_id, root_dir, 'test_roundtrip') # Reimport and get the video back - import_from_xml(module_store, self.user.id, root_dir) - imported_word_cloud = module_store.get_item(course_id.make_usage_key('word_cloud', 'untitled')) + import_from_xml(self.store, self.user.id, root_dir) + imported_word_cloud = self.store.get_item(course_id.make_usage_key('word_cloud', 'untitled')) # It should now contain empty data self.assertEquals(imported_word_cloud.data, '') @@ -1080,33 +827,30 @@ class ContentStoreToyCourseTest(ContentStoreTestCase): """ Test that a course which has HTML that has style formatting is preserved in export/import """ - module_store = modulestore() content_store = contentstore() - import_from_xml(module_store, self.user.id, 'common/test/data/', ['toy']) + import_from_xml(self.store, self.user.id, 'common/test/data/', ['toy']) course_id = SlashSeparatedCourseKey('edX', 'toy', '2012_Fall') # Export the course root_dir = path(mkdtemp_clean()) - export_to_xml(module_store, content_store, course_id, root_dir, 'test_roundtrip') + export_to_xml(self.store, content_store, course_id, root_dir, 'test_roundtrip') # Reimport and get the video back - import_from_xml(module_store, self.user.id, root_dir) + import_from_xml(self.store, self.user.id, root_dir) # get the sample HTML with styling information - html_module = module_store.get_item(course_id.make_usage_key('html', 'with_styling')) + html_module = self.store.get_item(course_id.make_usage_key('html', 'with_styling')) self.assertIn('

', html_module.data) # get the sample HTML with just a simple tag information - html_module = module_store.get_item(course_id.make_usage_key('html', 'just_img')) + html_module = self.store.get_item(course_id.make_usage_key('html', 'just_img')) self.assertIn('', html_module.data) def test_course_handouts_rewrites(self): - module_store = modulestore() - # import a test course - _, course_items = import_from_xml(module_store, self.user.id, 'common/test/data/', ['toy']) + _, course_items = import_from_xml(self.store, self.user.id, 'common/test/data/', ['toy']) course_id = course_items[0].id handouts_location = course_id.make_usage_key('course_info', 'handouts') @@ -1121,8 +865,8 @@ class ContentStoreToyCourseTest(ContentStoreTestCase): self.assertContains(resp, '/c4x/edX/toy/asset/handouts_sample_handout.txt') def test_prefetch_children(self): - mongo_store = modulestore()._get_modulestore_by_type(ModuleStoreEnum.Type.mongo) - import_from_xml(modulestore(), self.user.id, 'common/test/data/', ['toy']) + mongo_store = self.store._get_modulestore_by_type(ModuleStoreEnum.Type.mongo) + import_from_xml(self.store, self.user.id, 'common/test/data/', ['toy']) course_id = SlashSeparatedCourseKey('edX', 'toy', '2012_Fall') wrapper = MongoCollectionFindWrapper(mongo_store.collection.find) @@ -1152,32 +896,31 @@ class ContentStoreToyCourseTest(ContentStoreTestCase): def test_export_course_without_content_store(self): - module_store = modulestore() content_store = contentstore() # Create toy course - _, course_items = import_from_xml(module_store, self.user.id, 'common/test/data/', ['toy']) + _, course_items = import_from_xml(self.store, self.user.id, 'common/test/data/', ['toy']) course_id = course_items[0].id root_dir = path(mkdtemp_clean()) print 'Exporting to tempdir = {0}'.format(root_dir) - export_to_xml(module_store, None, course_id, root_dir, 'test_export_no_content_store') + export_to_xml(self.store, None, course_id, root_dir, 'test_export_no_content_store') # Delete the course from module store and reimport it - delete_course(module_store, content_store, course_id, commit=True) + delete_course(self.store, content_store, course_id, commit=True) import_from_xml( - module_store, self.user.id, root_dir, ['test_export_no_content_store'], + self.store, self.user.id, root_dir, ['test_export_no_content_store'], static_content_store=None, target_course_id=course_id ) # Verify reimported course - items = module_store.get_items( + items = self.store.get_items( course_id, category='sequential', name='vertical_sequential' @@ -1198,39 +941,17 @@ class ContentStoreTest(ContentStoreTestCase): Tests for the CMS ContentStore application. """ def setUp(self): - """ - These tests need a user in the DB so that the django Test Client - can log them in. - They inherit from the ModuleStoreTestCase class so that the mongodb collection - will be cleared out before each test case execution and deleted - afterwards. - """ - uname = 'testuser' - email = 'test+courses@edx.org' - password = 'foo' - - # Create the use so we can log them in. - self.user = User.objects.create_user(uname, email, password) - - # Note that we do not actually need to do anything - # for registration if we directly mark them active. - self.user.is_active = True - # Staff has access to view all courses - self.user.is_staff = True - self.user.save() - - self.client = AjaxEnabledTestClient() - self.client.login(username=uname, password=password) + super(ContentStoreTest, self).setUp() self.course_data = { 'org': 'MITx', - 'number': '999', + 'number': '111', 'display_name': 'Robot Super Course', 'run': '2013_Spring' } def tearDown(self): - MongoClient().drop_database(TEST_DATA_CONTENTSTORE['DOC_STORE_CONFIG']['db']) + contentstore().drop_database() _CONTENTSTORE.clear() def assert_created_course(self, number_suffix=None): @@ -1535,7 +1256,7 @@ class ContentStoreTest(ContentStoreTestCase): self.assertEqual(resp.status_code, 200) payload = parse_json(resp) problem_loc = UsageKey.from_string(payload['locator']) - problem = modulestore().get_item(problem_loc) + problem = self.store.get_item(problem_loc) # should be a CapaDescriptor self.assertIsInstance(problem, CapaDescriptor, "New problem is not a CapaDescriptor") context = problem.get_context() @@ -1555,7 +1276,7 @@ class ContentStoreTest(ContentStoreTestCase): ) self.assertEqual(resp.status_code, 200) - _, course_items = import_from_xml(modulestore(), self.user.id, 'common/test/data/', ['simple']) + _, course_items = import_from_xml(self.store, self.user.id, 'common/test/data/', ['simple']) course_key = course_items[0].id resp = self._show_course_overview(course_key) @@ -1604,13 +1325,12 @@ class ContentStoreTest(ContentStoreTestCase): delete_item(category='chapter', name='chapter_2') def test_import_into_new_course_id(self): - module_store = modulestore() target_course_id = _get_course_id(self.course_data) _create_course(self, target_course_id, self.course_data) - import_from_xml(module_store, self.user.id, 'common/test/data/', ['toy'], target_course_id=target_course_id) + import_from_xml(self.store, self.user.id, 'common/test/data/', ['toy'], target_course_id=target_course_id) - modules = module_store.get_items(target_course_id) + modules = self.store.get_items(target_course_id) # we should have a number of modules in there # we can't specify an exact number since it'll always be changing @@ -1621,7 +1341,7 @@ class ContentStoreTest(ContentStoreTestCase): # # first check PDF textbooks, to make sure the url paths got updated - course_module = module_store.get_course(target_course_id) + course_module = self.store.get_course(target_course_id) self.assertEqual(len(course_module.pdf_textbooks), 1) self.assertEqual(len(course_module.pdf_textbooks[0]["chapters"]), 2) @@ -1629,8 +1349,6 @@ class ContentStoreTest(ContentStoreTestCase): self.assertEqual(course_module.pdf_textbooks[0]["chapters"][1]["url"], '/static/Chapter2.pdf') def test_import_into_new_course_id_wiki_slug_renamespacing(self): - module_store = modulestore() - # If reimporting into the same course do not change the wiki_slug. target_course_id = SlashSeparatedCourseKey('edX', 'toy', '2012_Fall') course_data = { @@ -1640,17 +1358,17 @@ class ContentStoreTest(ContentStoreTestCase): 'run': target_course_id.run } _create_course(self, target_course_id, course_data) - course_module = module_store.get_course(target_course_id) + course_module = self.store.get_course(target_course_id) course_module.wiki_slug = 'toy' course_module.save() # Import a course with wiki_slug == location.course - import_from_xml(module_store, self.user.id, 'common/test/data/', ['toy'], target_course_id=target_course_id) - course_module = module_store.get_course(target_course_id) + import_from_xml(self.store, self.user.id, 'common/test/data/', ['toy'], target_course_id=target_course_id) + course_module = self.store.get_course(target_course_id) self.assertEquals(course_module.wiki_slug, 'toy') # But change the wiki_slug if it is a different course. - target_course_id = SlashSeparatedCourseKey('MITx', '999', '2013_Spring') + target_course_id = SlashSeparatedCourseKey('MITx', '111', '2013_Spring') course_data = { 'org': target_course_id.org, 'number': target_course_id.course, @@ -1660,23 +1378,22 @@ class ContentStoreTest(ContentStoreTestCase): _create_course(self, target_course_id, course_data) # Import a course with wiki_slug == location.course - import_from_xml(module_store, self.user.id, 'common/test/data/', ['toy'], target_course_id=target_course_id) - course_module = module_store.get_course(target_course_id) - self.assertEquals(course_module.wiki_slug, 'MITx.999.2013_Spring') + import_from_xml(self.store, self.user.id, 'common/test/data/', ['toy'], target_course_id=target_course_id) + course_module = self.store.get_course(target_course_id) + self.assertEquals(course_module.wiki_slug, 'MITx.111.2013_Spring') # Now try importing a course with wiki_slug == '{0}.{1}.{2}'.format(location.org, location.course, location.run) - import_from_xml(module_store, self.user.id, 'common/test/data/', ['two_toys'], target_course_id=target_course_id) - course_module = module_store.get_course(target_course_id) - self.assertEquals(course_module.wiki_slug, 'MITx.999.2013_Spring') + import_from_xml(self.store, self.user.id, 'common/test/data/', ['two_toys'], target_course_id=target_course_id) + course_module = self.store.get_course(target_course_id) + self.assertEquals(course_module.wiki_slug, 'MITx.111.2013_Spring') def test_import_metadata_with_attempts_empty_string(self): - module_store = modulestore() - import_from_xml(module_store, self.user.id, 'common/test/data/', ['simple']) + import_from_xml(self.store, self.user.id, 'common/test/data/', ['simple']) did_load_item = False try: course_key = SlashSeparatedCourseKey('edX', 'simple', 'problem') usage_key = course_key.make_usage_key('problem', 'ps01-simple') - module_store.get_item(usage_key) + self.store.get_item(usage_key) did_load_item = True except ItemNotFoundError: pass @@ -1685,23 +1402,21 @@ class ContentStoreTest(ContentStoreTestCase): self.assertTrue(did_load_item) def test_forum_id_generation(self): - module_store = modulestore() course = CourseFactory.create(org='edX', course='999', display_name='Robot Super Course') new_component_location = course.id.make_usage_key('discussion', 'new_component') # crate a new module and add it as a child to a vertical - module_store.create_and_save_xmodule(new_component_location, self.user.id) + self.store.create_and_save_xmodule(new_component_location, self.user.id) - new_discussion_item = module_store.get_item(new_component_location) + new_discussion_item = self.store.get_item(new_component_location) self.assertNotEquals(new_discussion_item.discussion_id, '$$GUID$$') def test_metadata_inheritance(self): - module_store = modulestore() - _, course_items = import_from_xml(module_store, self.user.id, 'common/test/data/', ['toy']) + _, course_items = import_from_xml(self.store, self.user.id, 'common/test/data/', ['toy']) course = course_items[0] - verticals = module_store.get_items(course.id, category='vertical') + verticals = self.store.get_items(course.id, category='vertical') # let's assert on the metadata_inheritance on an existing vertical for vertical in verticals: @@ -1713,14 +1428,14 @@ class ContentStoreTest(ContentStoreTestCase): new_component_location = course.id.make_usage_key('html', 'new_component') # crate a new module and add it as a child to a vertical - new_object = module_store.create_xmodule(new_component_location) - module_store.update_item(new_object, self.user.id, allow_not_found=True) + new_object = self.store.create_xmodule(new_component_location) + self.store.update_item(new_object, self.user.id, allow_not_found=True) parent = verticals[0] parent.children.append(new_component_location) - module_store.update_item(parent, self.user.id) + self.store.update_item(parent, self.user.id) # flush the cache - new_module = module_store.get_item(new_component_location) + new_module = self.store.get_item(new_component_location) # check for grace period definition which should be defined at the course level self.assertEqual(parent.graceperiod, new_module.graceperiod) @@ -1733,10 +1448,10 @@ class ContentStoreTest(ContentStoreTestCase): # now let's define an override at the leaf node level # new_module.graceperiod = timedelta(1) - module_store.update_item(new_module, self.user.id) + self.store.update_item(new_module, self.user.id) # flush the cache and refetch - new_module = module_store.get_item(new_component_location) + new_module = self.store.get_item(new_component_location) self.assertEqual(timedelta(1), new_module.graceperiod) @@ -1753,9 +1468,8 @@ class ContentStoreTest(ContentStoreTestCase): self.assertGreaterEqual(len(course.checklists), 4) # by fetching - module_store = modulestore() - fetched_course = module_store.get_item(course.location) - fetched_item = module_store.get_item(vertical.location) + fetched_course = self.store.get_item(course.location) + fetched_item = self.store.get_item(vertical.location) self.assertIsNotNone(fetched_course.start) self.assertEqual(course.start, fetched_course.start) self.assertEqual(fetched_course.start, fetched_item.start) @@ -1765,20 +1479,18 @@ class ContentStoreTest(ContentStoreTestCase): def test_image_import(self): """Test backwards compatibilty of course image.""" - module_store = modulestore() - content_store = contentstore() # Use conditional_and_poll, as it's got an image already import_from_xml( - module_store, + self.store, self.user.id, 'common/test/data/', ['conditional_and_poll'], static_content_store=content_store ) - course = module_store.get_courses()[0] + course = self.store.get_courses()[0] # Make sure the course image is set to the right place self.assertEqual(course.course_image, 'images_course_image.jpg') @@ -1799,8 +1511,8 @@ class ContentStoreTest(ContentStoreTestCase): course_key = _get_course_id(self.course_data) _create_course(self, course_key, self.course_data) - course_module = modulestore().get_course(course_key) - self.assertEquals(course_module.wiki_slug, 'MITx.999.2013_Spring') + course_module = self.store.get_course(course_key) + self.assertEquals(course_module.wiki_slug, 'MITx.111.2013_Spring') class MetadataSaveTestCase(ContentStoreTestCase): @@ -1852,8 +1564,8 @@ class MetadataSaveTestCase(ContentStoreTestCase): delattr(self.video_descriptor, field_name) self.assertNotIn('html5_sources', own_metadata(self.video_descriptor)) - modulestore().update_item(self.video_descriptor, self.user.id) - module = modulestore().get_item(location) + self.store.update_item(self.video_descriptor, self.user.id) + module = self.store.get_item(location) self.assertNotIn('html5_sources', own_metadata(module)) diff --git a/cms/djangoapps/contentstore/tests/test_core_caching.py b/cms/djangoapps/contentstore/tests/test_core_caching.py index f93e1222d2..16b15d127f 100644 --- a/cms/djangoapps/contentstore/tests/test_core_caching.py +++ b/cms/djangoapps/contentstore/tests/test_core_caching.py @@ -1,6 +1,5 @@ from cache_toolbox.core import get_cached_content, set_cached_content, del_cached_content from opaque_keys.edx.locations import Location -from xmodule.contentstore.content import StaticContent from django.test import TestCase diff --git a/cms/djangoapps/contentstore/tests/test_export_git.py b/cms/djangoapps/contentstore/tests/test_export_git.py index b023470bbf..4d38b29618 100644 --- a/cms/djangoapps/contentstore/tests/test_export_git.py +++ b/cms/djangoapps/contentstore/tests/test_export_git.py @@ -10,7 +10,6 @@ from uuid import uuid4 from django.conf import settings from django.test.utils import override_settings -from pymongo import MongoClient from .utils import CourseTestCase import contentstore.git_export_utils as git_export_utils @@ -37,7 +36,7 @@ class TestExportGit(CourseTestCase): self.test_url = reverse_course_url('export_git', self.course.id) def tearDown(self): - MongoClient().drop_database(TEST_DATA_CONTENTSTORE['DOC_STORE_CONFIG']['db']) + modulestore().contentstore.drop_database() _CONTENTSTORE.clear() def test_giturl_missing(self): diff --git a/cms/djangoapps/contentstore/tests/test_import.py b/cms/djangoapps/contentstore/tests/test_import.py index 22bab5db4c..3f8ed0b05d 100644 --- a/cms/djangoapps/contentstore/tests/test_import.py +++ b/cms/djangoapps/contentstore/tests/test_import.py @@ -7,7 +7,6 @@ Tests for import_from_xml using the mongo modulestore. from django.test.client import Client from django.test.utils import override_settings from django.conf import settings -from path import path import copy from django.contrib.auth.models import User @@ -22,7 +21,6 @@ from xmodule.contentstore.django import _CONTENTSTORE from xmodule.exceptions import NotFoundError from uuid import uuid4 -from pymongo import MongoClient TEST_DATA_CONTENTSTORE = copy.deepcopy(settings.CONTENTSTORE) TEST_DATA_CONTENTSTORE['DOC_STORE_CONFIG']['db'] = 'test_xcontent_%s' % uuid4().hex @@ -56,7 +54,7 @@ class ContentStoreImportTest(ModuleStoreTestCase): self.client.login(username=uname, password=password) def tearDown(self): - MongoClient().drop_database(TEST_DATA_CONTENTSTORE['DOC_STORE_CONFIG']['db']) + contentstore().drop_database() _CONTENTSTORE.clear() def load_test_import_course(self): diff --git a/cms/djangoapps/contentstore/tests/test_transcripts_utils.py b/cms/djangoapps/contentstore/tests/test_transcripts_utils.py index a739e39263..d7125447d8 100644 --- a/cms/djangoapps/contentstore/tests/test_transcripts_utils.py +++ b/cms/djangoapps/contentstore/tests/test_transcripts_utils.py @@ -151,7 +151,7 @@ class TestSaveSubsToStore(ModuleStoreTestCase): def tearDown(self): self.clear_subs_content() - MongoClient().drop_database(TEST_DATA_CONTENTSTORE['DOC_STORE_CONFIG']['db']) + contentstore().drop_database() _CONTENTSTORE.clear() @@ -190,7 +190,7 @@ class TestDownloadYoutubeSubs(ModuleStoreTestCase): org=self.org, number=self.number, display_name=self.display_name) def tearDown(self): - MongoClient().drop_database(TEST_DATA_CONTENTSTORE['DOC_STORE_CONFIG']['db']) + contentstore().drop_database() _CONTENTSTORE.clear() def test_success_downloading_subs(self): diff --git a/cms/djangoapps/contentstore/tests/utils.py b/cms/djangoapps/contentstore/tests/utils.py index 062f79a2e0..9fddf53bcb 100644 --- a/cms/djangoapps/contentstore/tests/utils.py +++ b/cms/djangoapps/contentstore/tests/utils.py @@ -1,16 +1,25 @@ +# pylint: disable=E1101 ''' Utilities for contentstore tests ''' import json +import re from django.contrib.auth.models import User from django.test.client import Client +from xmodule.contentstore.django import contentstore +from xmodule.contentstore.content import StaticContent +from xmodule.modulestore import PublishState, ModuleStoreEnum from xmodule.modulestore.django import modulestore +from xmodule.modulestore.inheritance import own_metadata from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory +from xmodule.modulestore.xml_importer import import_from_xml from student.models import Registration +from opaque_keys.edx.locations import SlashSeparatedCourseKey, AssetLocation +from contentstore.utils import reverse_url def parse_json(response): @@ -133,3 +142,213 @@ class CourseTestCase(ModuleStoreTestCase): """ self.course.save() self.store.update_item(self.course, self.user.id) + + TEST_VERTICAL = 'vertical_test' + PRIVATE_VERTICAL = 'a_private_vertical' + PUBLISHED_VERTICAL = 'a_published_vertical' + SEQUENTIAL = 'vertical_sequential' + LOCKED_ASSET_KEY = AssetLocation.from_deprecated_string('/c4x/edX/toy/asset/sample_static.txt') + + def import_and_populate_course(self): + """ + Imports the test toy course and populates it with additional test data + """ + content_store = contentstore() + import_from_xml(self.store, self.user.id, 'common/test/data/', ['toy'], static_content_store=content_store) + course_id = SlashSeparatedCourseKey('edX', 'toy', '2012_Fall') + + # create an Orphan + # We had a bug where orphaned draft nodes caused export to fail. This is here to cover that case. + vertical = self.store.get_item(course_id.make_usage_key('vertical', self.TEST_VERTICAL), depth=1) + vertical.location = vertical.location.replace(name='no_references') + self.store.update_item(vertical, self.user.id, allow_not_found=True) + orphan_vertical = self.store.get_item(vertical.location) + self.assertEqual(orphan_vertical.location.name, 'no_references') + self.assertEqual(len(orphan_vertical.children), len(vertical.children)) + + # create a Draft vertical + vertical = self.store.get_item(course_id.make_usage_key('vertical', self.TEST_VERTICAL), depth=1) + draft_vertical = self.store.convert_to_draft(vertical.location, self.user.id) + self.assertEqual(self.store.compute_publish_state(draft_vertical), PublishState.draft) + + # create a Private (draft only) vertical + private_vertical = self.store.create_and_save_xmodule(course_id.make_usage_key('vertical', self.PRIVATE_VERTICAL), self.user.id) + self.assertEqual(self.store.compute_publish_state(private_vertical), PublishState.private) + + # create a Published (no draft) vertical + public_vertical = self.store.create_and_save_xmodule(course_id.make_usage_key('vertical', self.PUBLISHED_VERTICAL), self.user.id) + public_vertical = self.store.publish(public_vertical.location, self.user.id) + self.assertEqual(self.store.compute_publish_state(public_vertical), PublishState.public) + + # add the new private and new public as children of the sequential + sequential = self.store.get_item(course_id.make_usage_key('sequential', self.SEQUENTIAL)) + sequential.children.append(private_vertical.location) + sequential.children.append(public_vertical.location) + self.store.update_item(sequential, self.user.id) + + # lock an asset + content_store.set_attr(self.LOCKED_ASSET_KEY, 'locked', True) + + # create a non-portable link - should be rewritten in new courses + html_module = self.store.get_item(course_id.make_usage_key('html', 'nonportable')) + new_data = html_module.data = html_module.data.replace( + '/static/', + '/c4x/{0}/{1}/asset/'.format(course_id.org, course_id.course) + ) + self.store.update_item(html_module, self.user.id) + + html_module = self.store.get_item(html_module.location) + self.assertEqual(new_data, html_module.data) + + return course_id + + def check_populated_course(self, course_id): + """ + Verifies the content of the given course, per data that was populated in import_and_populate_course + """ + items = self.store.get_items( + course_id, + category='vertical', + revision=ModuleStoreEnum.RevisionOption.published_only + ) + self.check_verticals(items) + + def verify_item_publish_state(item, publish_state): + """Verifies the publish state of the item is as expected.""" + if publish_state in (PublishState.private, PublishState.draft): + self.assertTrue(getattr(item, 'is_draft', False)) + else: + self.assertFalse(getattr(item, 'is_draft', False)) + self.assertEqual(self.store.compute_publish_state(item), publish_state) + + def get_and_verify_publish_state(item_type, item_name, publish_state): + """Gets the given item from the store and verifies the publish state of the item is as expected.""" + item = self.store.get_item(course_id.make_usage_key(item_type, item_name)) + verify_item_publish_state(item, publish_state) + return item + + # verify that the draft vertical is draft + vertical = get_and_verify_publish_state('vertical', self.TEST_VERTICAL, PublishState.draft) + for child in vertical.get_children(): + verify_item_publish_state(child, PublishState.draft) + + # make sure that we don't have a sequential that is not in draft mode + sequential = get_and_verify_publish_state('sequential', self.SEQUENTIAL, PublishState.public) + + # verify that we have the private vertical + private_vertical = get_and_verify_publish_state('vertical', self.PRIVATE_VERTICAL, PublishState.private) + + # verify that we have the public vertical + public_vertical = get_and_verify_publish_state('vertical', self.PUBLISHED_VERTICAL, PublishState.public) + + # verify verticals are children of sequential + for vert in [vertical, private_vertical, public_vertical]: + self.assertIn(vert.location, sequential.children) + + # verify textbook exists + course = self.store.get_course(course_id) + self.assertGreater(len(course.textbooks), 0) + + # verify asset attributes of locked asset key + self.assertAssetsEqual(self.LOCKED_ASSET_KEY, self.LOCKED_ASSET_KEY.course_key, course_id) + + # verify non-portable links are rewritten + html_module = self.store.get_item(course_id.make_usage_key('html', 'nonportable')) + self.assertIn('/static/foo.jpg', html_module.data) + + return course + + def assertCoursesEqual(self, course1_id, course2_id): + """ + Verifies the content of the two given courses are equal + """ + course1_items = self.store.get_items(course1_id) + course2_items = self.store.get_items(course2_id) + self.assertGreater(len(course1_items), 0) # ensure it found content instead of [] == [] + self.assertEqual(len(course1_items), len(course2_items)) + + for course1_item in course1_items: + course2_item_location = course1_item.location.map_into_course(course2_id) + if course1_item.location.category == 'course': + course2_item_location = course2_item_location.replace(name=course2_item_location.run) + course2_item = self.store.get_item(course2_item_location) + + # compare published state + self.assertEqual( + self.store.compute_publish_state(course1_item), + self.store.compute_publish_state(course2_item) + ) + + # compare data + self.assertEqual(hasattr(course1_item, 'data'), hasattr(course2_item, 'data')) + if hasattr(course1_item, 'data'): + self.assertEqual(course1_item.data, course2_item.data) + + # compare meta-data + self.assertEqual(own_metadata(course1_item), own_metadata(course2_item)) + + # compare children + self.assertEqual(course1_item.has_children, course2_item.has_children) + if course1_item.has_children: + expected_children = [] + for course1_item_child in course1_item.children: + expected_children.append( + course1_item_child.map_into_course(course2_id) + ) + self.assertEqual(expected_children, course2_item.children) + + # compare assets + content_store = contentstore() + course1_assets, count_course1_assets = content_store.get_all_content_for_course(course1_id) + _, count_course2_assets = content_store.get_all_content_for_course(course2_id) + self.assertEqual(count_course1_assets, count_course2_assets) + for asset in course1_assets: + asset_id = asset.get('content_son', asset['_id']) + asset_key = StaticContent.compute_location(course1_id, asset_id['name']) + self.assertAssetsEqual(asset_key, course1_id, course2_id) + + def check_verticals(self, items): + """ Test getting the editing HTML for each vertical. """ + # assert is here to make sure that the course being tested actually has verticals (units) to check. + self.assertGreater(len(items), 0, "Course has no verticals (units) to check") + 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) + + def assertAssetsEqual(self, asset_key, course1_id, course2_id): + """Verifies the asset of the given key has the same attributes in both given courses.""" + content_store = contentstore() + course1_asset_attrs = content_store.get_attrs(asset_key.map_into_course(course1_id)) + course2_asset_attrs = content_store.get_attrs(asset_key.map_into_course(course2_id)) + self.assertEqual(len(course1_asset_attrs), len(course2_asset_attrs)) + for key, value in course1_asset_attrs.iteritems(): + if key == '_id': + self.assertEqual(value['name'], course2_asset_attrs[key]['name']) + elif key == 'filename' or key == 'uploadDate' or key == 'content_son' or key == 'thumbnail_location': + pass + else: + self.assertEqual(value, course2_asset_attrs[key]) + + +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"(? 0: items = self.fs_files.find( - location_to_query(course_filter, wildcard=True, tag=XASSET_LOCATION_TAG), + query_for_course(course_key, "asset" if not get_thumbnails else "thumbnail"), skip=start, limit=maxresults, sort=sort ) else: - items = self.fs_files.find(location_to_query(course_filter, wildcard=True, tag=XASSET_LOCATION_TAG), sort=sort) + items = self.fs_files.find( + query_for_course(course_key, "asset" if not get_thumbnails else "thumbnail"), sort=sort + ) count = items.count() return list(items), count @@ -243,7 +242,7 @@ class MongoContentStore(ContentStore): for attr in attr_dict.iterkeys(): if attr in ['_id', 'md5', 'uploadDate', 'length']: raise AttributeError("{} is a protected attribute.".format(attr)) - asset_db_key = self.asset_db_key(location) + asset_db_key, __ = self.asset_db_key(location) # catch upsert error and raise NotFoundError if asset doesn't exist result = self.fs_files.update({'_id': asset_db_key}, {"$set": attr_dict}, upsert=False) if not result.get('updatedExisting', True): @@ -259,30 +258,117 @@ class MongoContentStore(ContentStore): :param location: a c4x asset location """ - asset_db_key = self.asset_db_key(location) + asset_db_key, __ = self.asset_db_key(location) item = self.fs_files.find_one({'_id': asset_db_key}) if item is None: raise NotFoundError(asset_db_key) return item + def copy_all_course_assets(self, source_course_key, dest_course_key): + """ + See :meth:`.ContentStore.copy_all_course_assets` + + This implementation fairly expensively copies all of the data + """ + source_query = query_for_course(source_course_key) + # it'd be great to figure out how to do all of this on the db server and not pull the bits over + for asset in self.fs_files.find(source_query): + asset_key = self.make_id_son(asset) + # don't convert from string until fs access + source_content = self.fs.get(asset_key) + if isinstance(asset_key, basestring): + asset_key = AssetLocation.from_string(asset_key) + __, asset_key = self.asset_db_key(asset_key) + asset_key['org'] = dest_course_key.org + asset_key['course'] = dest_course_key.course + if getattr(dest_course_key, 'deprecated', False): # remove the run if exists + if 'run' in asset_key: + del asset_key['run'] + asset_id = asset_key + else: # add the run, since it's the last field, we're golden + asset_key['run'] = dest_course_key.run + asset_id = unicode(dest_course_key.make_asset_key(asset_key['category'], asset_key['name'])) + + self.fs.put( + source_content.read(), + _id=asset_id, filename=asset['filename'], content_type=asset['contentType'], + displayname=asset['displayname'], content_son=asset_key, + # thumbnail is not technically correct but will be functionally correct as the code + # only looks at the name which is not course relative. + thumbnail_location=asset['thumbnail_location'], + import_path=asset['import_path'], + # getattr b/c caching may mean some pickled instances don't have attr + locked=asset.get('locked', False) + ) + def delete_all_course_assets(self, course_key): """ Delete all assets identified via this course_key. Dangerous operation which may remove assets referenced by other runs or other courses. :param course_key: """ - course_query = MongoModuleStore._course_key_to_son(course_key, tag=XASSET_LOCATION_TAG) # pylint: disable=protected-access + course_query = query_for_course(course_key) matching_assets = self.fs_files.find(course_query) for asset in matching_assets: - self.fs.delete(asset['_id']) + asset_key = self.make_id_son(asset) + self.fs.delete(asset_key) - @staticmethod - def asset_db_key(location): + # codifying the original order which pymongo used for the dicts coming out of location_to_dict + # stability of order is more important than sanity of order as any changes to order make things + # unfindable + ordered_key_fields = ['category', 'name', 'course', 'tag', 'org', 'revision'] + + @classmethod + def asset_db_key(cls, location): """ - Returns the database query to find the given asset location. + Returns the database _id and son structured lookup to find the given asset location. """ - # codifying the original order which pymongo used for the dicts coming out of location_to_dict - # stability of order is more important than sanity of order as any changes to order make things - # unfindable - ordered_key_fields = ['category', 'name', 'course', 'tag', 'org', 'revision'] - return SON((field_name, getattr(location, field_name)) for field_name in ordered_key_fields) + dbkey = SON((field_name, getattr(location, field_name)) for field_name in cls.ordered_key_fields) + if getattr(location, 'deprecated', False): + content_id = dbkey + else: + # NOTE, there's no need to state that run doesn't exist in the negative case b/c access via + # SON requires equivalence (same keys and values in exact same order) + dbkey['run'] = location.run + content_id = unicode(location) + return content_id, dbkey + + def make_id_son(self, fs_entry): + """ + Change the _id field in fs_entry into the properly ordered SON or string + Args: + fs_entry: the element returned by self.fs_files.find + """ + _id_field = fs_entry.get('_id', fs_entry) + if isinstance(_id_field, basestring): + return _id_field + dbkey = SON((field_name, _id_field.get(field_name)) for field_name in self.ordered_key_fields) + if 'run' in _id_field: + # NOTE, there's no need to state that run doesn't exist in the negative case b/c access via + # SON requires equivalence (same keys and values in exact same order) + dbkey['run'] = _id_field['run'] + fs_entry['_id'] = dbkey + return dbkey + + +def query_for_course(course_key, category=None): + """ + Construct a SON object that will query for all assets possibly limited to the given type + (thumbnail v assets) in the course using the index in mongo_indexes.md + """ + if getattr(course_key, 'deprecated', False): + prefix = '_id' + else: + prefix = 'content_son' + dbkey = SON([ + ('{}.tag'.format(prefix), XASSET_LOCATION_TAG), + ('{}.org'.format(prefix), course_key.org), + ('{}.course'.format(prefix), course_key.course), + ]) + if category: + dbkey['{}.category'.format(prefix)] = category + if getattr(course_key, 'deprecated', False): + dbkey['{}.run'.format(prefix)] = {'$exists': False} + else: + dbkey['{}.run'.format(prefix)] = course_key.run + return dbkey diff --git a/common/lib/xmodule/xmodule/modulestore/__init__.py b/common/lib/xmodule/xmodule/modulestore/__init__.py index 73b5536202..29ffc39c5c 100644 --- a/common/lib/xmodule/xmodule/modulestore/__init__.py +++ b/common/lib/xmodule/xmodule/modulestore/__init__.py @@ -329,6 +329,23 @@ class ModuleStoreWrite(ModuleStoreRead): """ pass + @abstractmethod + def clone_course(self, source_course_id, dest_course_id, user_id): + """ + Sets up source_course_id to point a course with the same content as the desct_course_id. This + operation may be cheap or expensive. It may have to copy all assets and all xblock content or + merely setup new pointers. + + Backward compatibility: this method used to require in some modulestores that dest_course_id + pointed to an empty but already created course. Implementers should support this or should + enable creating the course from scratch. + + Raises: + ItemNotFoundError: if the source course doesn't exist (or any of its xblocks aren't found) + DuplicateItemError: if the destination course already exists (with content in some cases) + """ + pass + @abstractmethod def delete_course(self, course_key, user_id=None): """ @@ -434,8 +451,10 @@ class ModuleStoreWriteBase(ModuleStoreReadBase, ModuleStoreWrite): ''' Implement interface functionality that can be shared. ''' - def __init__(self, **kwargs): + def __init__(self, contentstore, **kwargs): super(ModuleStoreWriteBase, self).__init__(**kwargs) + + self.contentstore = contentstore # TODO: Don't have a runtime just to generate the appropriate mixin classes (cpennington) # This is only used by partition_fields_by_scope, which is only needed because # the split mongo store is used for item creation as well as item persistence @@ -501,6 +520,16 @@ class ModuleStoreWriteBase(ModuleStoreReadBase, ModuleStoreWrite): self.update_item(new_object, user_id, allow_not_found=True) return new_object + def clone_course(self, source_course_id, dest_course_id, user_id): + """ + This base method just copies the assets. The lower level impls must do the actual cloning of + content. + """ + # copy the assets + self.contentstore.copy_all_course_assets(source_course_id, dest_course_id) + super(ModuleStoreWriteBase, self).clone_course(source_course_id, dest_course_id, user_id) + return dest_course_id + def only_xmodules(identifier, entry_points): """Only use entry_points that are supplied by the xmodule package""" diff --git a/common/lib/xmodule/xmodule/modulestore/django.py b/common/lib/xmodule/xmodule/modulestore/django.py index 45994b8219..973833f4f8 100644 --- a/common/lib/xmodule/xmodule/modulestore/django.py +++ b/common/lib/xmodule/xmodule/modulestore/django.py @@ -17,6 +17,7 @@ import threading from xmodule.modulestore.loc_mapper_store import LocMapperStore from xmodule.util.django import get_current_request_hostname import xmodule.modulestore # pylint: disable=unused-import +from xmodule.contentstore.django import contentstore # We may not always have the request_cache module available try: @@ -37,7 +38,7 @@ def load_function(path): return getattr(import_module(module_path), name) -def create_modulestore_instance(engine, doc_store_config, options, i18n_service=None): +def create_modulestore_instance(engine, content_store, doc_store_config, options, i18n_service=None): """ This will return a new instance of a modulestore given an engine and options """ @@ -62,6 +63,7 @@ def create_modulestore_instance(engine, doc_store_config, options, i18n_service= metadata_inheritance_cache = get_cache('default') return class_( + contentstore=content_store, metadata_inheritance_cache_subsystem=metadata_inheritance_cache, request_cache=request_cache, xblock_mixins=getattr(settings, 'XBLOCK_MIXINS', ()), @@ -85,6 +87,7 @@ def modulestore(): if _MIXED_MODULESTORE is None: _MIXED_MODULESTORE = create_modulestore_instance( settings.MODULESTORE['default']['ENGINE'], + contentstore(), settings.MODULESTORE['default'].get('DOC_STORE_CONFIG', {}), settings.MODULESTORE['default'].get('OPTIONS', {}) ) diff --git a/common/lib/xmodule/xmodule/modulestore/mixed.py b/common/lib/xmodule/xmodule/modulestore/mixed.py index 7dfe0f345c..f1efa8ed3b 100644 --- a/common/lib/xmodule/xmodule/modulestore/mixed.py +++ b/common/lib/xmodule/xmodule/modulestore/mixed.py @@ -11,7 +11,7 @@ from contextlib import contextmanager from opaque_keys import InvalidKeyError from . import ModuleStoreWriteBase -from xmodule.modulestore import PublishState +from xmodule.modulestore import PublishState, ModuleStoreEnum, split_migrator from xmodule.modulestore.django import create_modulestore_instance, loc_mapper from opaque_keys.edx.locator import CourseLocator, BlockUsageLocator from xmodule.modulestore.exceptions import ItemNotFoundError @@ -29,12 +29,12 @@ class MixedModuleStore(ModuleStoreWriteBase): """ ModuleStore knows how to route requests to the right persistence ms """ - def __init__(self, mappings, stores, i18n_service=None, **kwargs): + def __init__(self, contentstore, mappings, stores, i18n_service=None, **kwargs): """ Initialize a MixedModuleStore. Here we look into our passed in kwargs which should be a collection of other modulestore configuration information """ - super(MixedModuleStore, self).__init__(**kwargs) + super(MixedModuleStore, self).__init__(contentstore, **kwargs) self.modulestores = [] self.mappings = {} @@ -61,6 +61,7 @@ class MixedModuleStore(ModuleStoreWriteBase): ] store = create_modulestore_instance( store_settings['ENGINE'], + self.contentstore, store_settings.get('DOC_STORE_CONFIG', {}), store_settings.get('OPTIONS', {}), i18n_service=i18n_service, @@ -295,6 +296,36 @@ class MixedModuleStore(ModuleStoreWriteBase): return store.create_course(org, offering, user_id, fields, **kwargs) + def clone_course(self, source_course_id, dest_course_id, user_id): + """ + See the superclass for the general documentation. + + If cloning w/in a store, delegates to that store's clone_course which, in order to be self- + sufficient, should handle the asset copying (call the same method as this one does) + If cloning between stores, + * copy the assets + * migrate the courseware + """ + source_modulestore = self._get_modulestore_for_courseid(source_course_id) + # for a temporary period of time, we may want to hardcode dest_modulestore as split if there's a split + # to have only course re-runs go to split. This code, however, uses the config'd priority + dest_modulestore = self._get_modulestore_for_courseid(dest_course_id) + if source_modulestore == dest_modulestore: + return source_modulestore.clone_course(source_course_id, dest_course_id, user_id) + + # ensure super's only called once. The delegation above probably calls it; so, don't move + # the invocation above the delegation call + super(MixedModuleStore, self).clone_course(source_course_id, dest_course_id, user_id) + + if dest_modulestore.get_modulestore_type() == ModuleStoreEnum.Type.split: + if not hasattr(self, 'split_migrator'): + self.split_migrator = split_migrator.SplitMigrator( + dest_modulestore, source_modulestore, loc_mapper() + ) + self.split_migrator.migrate_mongo_course( + source_course_id, user_id, dest_course_id.org, dest_course_id.offering + ) + def create_item(self, course_or_parent_loc, category, user_id=None, **kwargs): """ Create and return the item. If parent_loc is a specific location v a course id, @@ -460,6 +491,24 @@ class MixedModuleStore(ModuleStoreWriteBase): else: raise NotImplementedError(u"Cannot call {} on store {}".format(method, store)) + @contextmanager + def set_default_store(self, store_type): + """ + A context manager for temporarily changing the default store in the Mixed modulestore + """ + previous_store_list = self.modulestores + found = False + try: + for i, store in enumerate(self.modulestores): + if store.get_modulestore_type() == store_type: + self.modulestores.insert(0, self.modulestores.pop(i)) + found = True + yield + if not found: + raise Exception(u"Cannot find store of type {}".format(store_type)) + finally: + self.modulestores = previous_store_list + @contextmanager def store_branch_setting(store, branch_setting): diff --git a/common/lib/xmodule/xmodule/modulestore/mongo/base.py b/common/lib/xmodule/xmodule/modulestore/mongo/base.py index 7ae03cc332..32311b9734 100644 --- a/common/lib/xmodule/xmodule/modulestore/mongo/base.py +++ b/common/lib/xmodule/xmodule/modulestore/mongo/base.py @@ -332,12 +332,12 @@ class MongoModuleStore(ModuleStoreWriteBase): """ A Mongodb backed ModuleStore """ - reference_type = Location + reference_type = SlashSeparatedCourseKey # TODO (cpennington): Enable non-filesystem filestores # pylint: disable=C0103 # pylint: disable=W0201 - def __init__(self, doc_store_config, fs_root, render_template, + def __init__(self, contentstore, doc_store_config, fs_root, render_template, default_class=None, error_tracker=null_error_tracker, i18n_service=None, @@ -346,7 +346,7 @@ class MongoModuleStore(ModuleStoreWriteBase): :param doc_store_config: must have a host, db, and collection entries. Other common entries: port, tz_aware. """ - super(MongoModuleStore, self).__init__(**kwargs) + super(MongoModuleStore, self).__init__(contentstore, **kwargs) def do_connection( db, collection, host, port=27017, tz_aware=True, user=None, password=None, **kwargs @@ -857,7 +857,6 @@ class MongoModuleStore(ModuleStoreWriteBase): Raises: InvalidLocationError: If a course with the same org and offering already exists """ - course, _, run = offering.partition('/') course_id = SlashSeparatedCourseKey(org, course, run) diff --git a/common/lib/xmodule/xmodule/modulestore/mongo/draft.py b/common/lib/xmodule/xmodule/modulestore/mongo/draft.py index 023ada5109..675963850b 100644 --- a/common/lib/xmodule/xmodule/modulestore/mongo/draft.py +++ b/common/lib/xmodule/xmodule/modulestore/mongo/draft.py @@ -7,15 +7,21 @@ and otherwise returns i4x://org/course/cat/name). """ import pymongo +import logging +from opaque_keys.edx.locations import Location from xmodule.exceptions import InvalidVersionError from xmodule.modulestore import PublishState, ModuleStoreEnum -from xmodule.modulestore.exceptions import ItemNotFoundError, DuplicateItemError, InvalidBranchSetting +from xmodule.modulestore.exceptions import ( + ItemNotFoundError, DuplicateItemError, InvalidBranchSetting, DuplicateCourseError +) from xmodule.modulestore.mongo.base import ( MongoModuleStore, MongoRevisionKey, as_draft, as_published, DIRECT_ONLY_CATEGORIES, SORT_REVISION_FAVOR_DRAFT ) -from opaque_keys.edx.locations import Location +from xmodule.modulestore.store_utilities import rewrite_nonportable_content_links + +log = logging.getLogger(__name__) def wrap_draft(item): @@ -138,6 +144,73 @@ class DraftModuleStore(MongoModuleStore): del key['_id.revision'] return self.collection.find(key).count() > 0 + def clone_course(self, source_course_id, dest_course_id, user_id): + """ + Only called if cloning within this store or if env doesn't set up mixed. + * copy the courseware + """ + # check to see if the source course is actually there + if not self.has_course(source_course_id): + raise ItemNotFoundError("Cannot find a course at {0}. Aborting".format(source_course_id)) + + # verify that the dest_location really is an empty course + # b/c we don't want the payload, I'm copying the guts of get_items here + query = self._course_key_to_son(dest_course_id) + query['_id.category'] = {'$nin': ['course', 'about']} + if self.collection.find(query).limit(1).count() > 0: + raise DuplicateCourseError( + dest_course_id, + "Course at destination {0} is not an empty course. You can only clone into an empty course. Aborting...".format( + dest_course_id + ) + ) + + # clone the assets + super(DraftModuleStore, self).clone_course(source_course_id, dest_course_id, user_id) + + # get the whole old course + new_course = self.get_course(dest_course_id) + if new_course is None: + # create_course creates the about overview + new_course = self.create_course(dest_course_id.org, dest_course_id.offering, user_id) + + # Get all modules under this namespace which is (tag, org, course) tuple + modules = self.get_items(source_course_id, revision=ModuleStoreEnum.RevisionOption.published_only) + self._clone_modules(modules, dest_course_id, user_id) + course_location = dest_course_id.make_usage_key('course', dest_course_id.run) + self.publish(course_location, user_id) + + modules = self.get_items(source_course_id, revision=ModuleStoreEnum.RevisionOption.draft_only) + self._clone_modules(modules, dest_course_id, user_id) + + return True + + def _clone_modules(self, modules, dest_course_id, user_id): + """Clones each module into the given course""" + for module in modules: + original_loc = module.location + module.location = module.location.map_into_course(dest_course_id) + if module.location.category == 'course': + module.location = module.location.replace(name=module.location.run) + + log.info("Cloning module %s to %s....", original_loc, module.location) + + if 'data' in module.fields and module.fields['data'].is_set_on(module) and isinstance(module.data, basestring): + module.data = rewrite_nonportable_content_links( + original_loc.course_key, dest_course_id, module.data + ) + + # repoint children + if module.has_children: + new_children = [] + for child_loc in module.children: + child_loc = child_loc.map_into_course(dest_course_id) + new_children.append(child_loc) + + module.children = new_children + + self.update_item(module, user_id, allow_not_found=True) + def _get_raw_parent_locations(self, location, key_revision): """ Get the parents but don't unset the revision in their locations. diff --git a/common/lib/xmodule/xmodule/modulestore/split_migrator.py b/common/lib/xmodule/xmodule/modulestore/split_migrator.py index e1bdf4c5ab..b24e3088ef 100644 --- a/common/lib/xmodule/xmodule/modulestore/split_migrator.py +++ b/common/lib/xmodule/xmodule/modulestore/split_migrator.py @@ -15,10 +15,9 @@ class SplitMigrator(object): Copies courses from old mongo to split mongo and sets up location mapping so any references to the old name will be able to find the new elements. """ - def __init__(self, split_modulestore, direct_modulestore, draft_modulestore, loc_mapper): + def __init__(self, split_modulestore, draft_modulestore, loc_mapper): super(SplitMigrator, self).__init__() self.split_modulestore = split_modulestore - self.direct_modulestore = direct_modulestore self.draft_modulestore = draft_modulestore self.loc_mapper = loc_mapper @@ -43,7 +42,7 @@ class SplitMigrator(object): # locations are in location, children, conditionals, course.tab # create the course: set fields to explicitly_set for each scope, id_root = new_course_locator, master_branch = 'production' - original_course = self.direct_modulestore.get_course(course_key) + original_course = self.draft_modulestore.get_course(course_key) new_course_root_locator = self.loc_mapper.translate_location(original_course.location) new_course = self.split_modulestore.create_course( new_course_root_locator.org, new_course_root_locator.offering, user.id, @@ -65,7 +64,7 @@ class SplitMigrator(object): # iterate over published course elements. Wildcarding rather than descending b/c some elements are orphaned (e.g., # course about pages, conditionals) - for module in self.direct_modulestore.get_items(course_key): + for module in self.draft_modulestore.get_items(course_key, revision=ModuleStoreEnum.RevisionOption.published_only): # don't copy the course again. No drafts should get here if module.location != old_course_loc: # create split_xblock using split.create_item diff --git a/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py b/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py index ccc1123f6f..0875170bc5 100644 --- a/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py +++ b/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py @@ -105,7 +105,8 @@ class SplitMongoModuleStore(ModuleStoreWriteBase): SCHEMA_VERSION = 1 reference_type = Locator - def __init__(self, doc_store_config, fs_root, render_template, + + def __init__(self, contentstore, doc_store_config, fs_root, render_template, default_class=None, error_tracker=null_error_tracker, loc_mapper=None, @@ -115,7 +116,7 @@ class SplitMongoModuleStore(ModuleStoreWriteBase): :param doc_store_config: must have a host, db, and collection entries. Other common entries: port, tz_aware. """ - super(SplitMongoModuleStore, self).__init__(**kwargs) + super(SplitMongoModuleStore, self).__init__(contentstore, **kwargs) self.loc_mapper = loc_mapper self.db_connection = MongoConnection(**doc_store_config) @@ -870,6 +871,20 @@ class SplitMongoModuleStore(ModuleStoreWriteBase): # reconstruct the new_item from the cache return self.get_item(item_loc) + def clone_course(self, source_course_id, dest_course_id, user_id): + """ + See :meth: `.ModuleStoreWrite.clone_course` for documentation. + + In split, other than copying the assets, this is cheap as it merely creates a new version of the + existing course. + """ + super(SplitMongoModuleStore, self).clone_course(source_course_id, dest_course_id, user_id) + source_index = self.get_course_index_info(source_course_id) + return self.create_course( + dest_course_id.org, dest_course_id.offering, user_id, fields=None, # override start_date? + versions_dict=source_index['versions'] + ) + def create_course( self, org, offering, user_id, fields=None, master_branch=ModuleStoreEnum.BranchName.draft, versions_dict=None, root_category='course', diff --git a/common/lib/xmodule/xmodule/modulestore/store_utilities.py b/common/lib/xmodule/xmodule/modulestore/store_utilities.py index dd48f1d60c..d1e40ade9c 100644 --- a/common/lib/xmodule/xmodule/modulestore/store_utilities.py +++ b/common/lib/xmodule/xmodule/modulestore/store_utilities.py @@ -2,7 +2,6 @@ import re import logging from xmodule.contentstore.content import StaticContent -from xmodule.modulestore import ModuleStoreEnum def _prefix_only_url_replace_regex(prefix): @@ -88,91 +87,6 @@ def rewrite_nonportable_content_links(source_course_id, dest_course_id, text): return text -def _clone_modules(modulestore, modules, source_course_id, dest_course_id, user_id): - for module in modules: - original_loc = module.location - module.location = module.location.map_into_course(dest_course_id) - if module.location.category == 'course': - module.location = module.location.replace(name=module.location.run) - - print "Cloning module {0} to {1}....".format(original_loc, module.location) - - if 'data' in module.fields and module.fields['data'].is_set_on(module) and isinstance(module.data, basestring): - module.data = rewrite_nonportable_content_links( - source_course_id, dest_course_id, module.data - ) - - # repoint children - if module.has_children: - new_children = [] - for child_loc in module.children: - child_loc = child_loc.map_into_course(dest_course_id) - new_children.append(child_loc) - - module.children = new_children - - modulestore.update_item(module, user_id, allow_not_found=True) - - -def clone_course(modulestore, contentstore, source_course_id, dest_course_id, user_id): - # check to see if the dest_location exists as an empty course - # we need an empty course because the app layers manage the permissions and users - if not modulestore.has_course(dest_course_id): - raise Exception(u"An empty course at {0} must have already been created. Aborting...".format(dest_course_id)) - - # verify that the dest_location really is an empty course, which means only one with an optional 'overview' - dest_modules = modulestore.get_items(dest_course_id) - - for module in dest_modules: - if module.location.category == 'course' or ( - module.location.category == 'about' and module.location.name == 'overview' - ): - continue - # only course and about overview allowed - raise Exception("Course at destination {0} is not an empty course. You can only clone into an empty course. Aborting...".format(dest_course_id)) - - # check to see if the source course is actually there - if not modulestore.has_course(source_course_id): - raise Exception("Cannot find a course at {0}. Aborting".format(source_course_id)) - - # Get all modules under this namespace which is (tag, org, course) tuple - modules = modulestore.get_items(source_course_id, revision=ModuleStoreEnum.RevisionOption.published_only) - _clone_modules(modulestore, modules, source_course_id, dest_course_id, user_id) - course_location = dest_course_id.make_usage_key('course', dest_course_id.run) - modulestore.publish(course_location, user_id) - - modules = modulestore.get_items(source_course_id, revision=ModuleStoreEnum.RevisionOption.draft_only) - _clone_modules(modulestore, modules, source_course_id, dest_course_id, user_id) - - # now iterate through all of the assets and clone them - # first the thumbnails - thumb_keys = contentstore.get_all_content_thumbnails_for_course(source_course_id) - for thumb_key in thumb_keys: - content = contentstore.find(thumb_key) - content.location = content.location.map_into_course(dest_course_id) - - print "Cloning thumbnail {0} to {1}".format(thumb_key, content.location) - - contentstore.save(content) - - # now iterate through all of the assets, also updating the thumbnail pointer - - asset_keys, __ = contentstore.get_all_content_for_course(source_course_id) - for asset_key in asset_keys: - content = contentstore.find(asset_key) - content.location = content.location.map_into_course(dest_course_id) - - # be sure to update the pointer to the thumbnail - if content.thumbnail_location is not None: - content.thumbnail_location = content.thumbnail_location.map_into_course(dest_course_id) - - print "Cloning asset {0} to {1}".format(asset_key, content.location) - - contentstore.save(content) - - return True - - def delete_course(modulestore, contentstore, course_key, commit=False): """ This method will actually do the work to delete all content in a course in a MongoDB backed diff --git a/common/lib/xmodule/xmodule/modulestore/tests/django_utils.py b/common/lib/xmodule/xmodule/modulestore/tests/django_utils.py index 3acbb29249..8792f3b439 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/django_utils.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/django_utils.py @@ -7,7 +7,6 @@ from django.test import TestCase from xmodule.modulestore.django import ( modulestore, clear_existing_modulestores, loc_mapper) from xmodule.modulestore import ModuleStoreEnum -from xmodule.contentstore.django import contentstore def mixed_store_config(data_dir, mappings): @@ -160,10 +159,8 @@ class ModuleStoreTestCase(TestCase): connection.drop_database(store.db.name) connection.close() - if contentstore().fs_files: - db = contentstore().fs_files.database - db.connection.drop_database(db) - db.connection.close() + if hasattr(store, 'contentstore'): + store.contentstore.drop_database() location_mapper = loc_mapper() if location_mapper.db: diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_contentstore.py b/common/lib/xmodule/xmodule/modulestore/tests/test_contentstore.py new file mode 100644 index 0000000000..f85c88d60b --- /dev/null +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_contentstore.py @@ -0,0 +1,213 @@ +""" + Test contentstore.mongo functionality +""" +import logging +from uuid import uuid4 +import unittest +import mimetypes +from tempfile import mkdtemp +import path +import shutil + +from opaque_keys.edx.locations import SlashSeparatedCourseKey, AssetLocation +from xmodule.tests import DATA_DIR +from xmodule.contentstore.mongo import MongoContentStore +from xmodule.contentstore.content import StaticContent +from xmodule.exceptions import NotFoundError +import ddt +from __builtin__ import delattr + + +log = logging.getLogger(__name__) + +HOST = 'localhost' +PORT = 27017 +DB = 'test_mongo_%s' % uuid4().hex[:5] + + +@ddt.ddt +class TestContentstore(unittest.TestCase): + """ + Test the methods in contentstore.mongo using deprecated and non-deprecated keys + """ + + # don't use these 2 class vars as they restore behavior once the tests are done + asset_deprecated = None + ssck_deprecated = None + + @classmethod + def tearDownClass(cls): + """ + Restores deprecated values + """ + if cls.asset_deprecated is not None: + setattr(AssetLocation, 'deprecated', cls.asset_deprecated) + else: + delattr(AssetLocation, 'deprecated') + if cls.ssck_deprecated is not None: + setattr(SlashSeparatedCourseKey, 'deprecated', cls.ssck_deprecated) + else: + delattr(SlashSeparatedCourseKey, 'deprecated') + return super(TestContentstore, cls).tearDownClass() + + def set_up_assets(self, deprecated): + """ + Setup contentstore w/ proper overriding of deprecated. + """ + # since MongoModuleStore and MongoContentStore are basically assumed to be together, create this class + # as well + self.contentstore = MongoContentStore(HOST, DB, port=PORT) + self.addCleanup(self.contentstore.drop_database) + + setattr(AssetLocation, 'deprecated', deprecated) + setattr(SlashSeparatedCourseKey, 'deprecated', deprecated) + + self.course1_key = SlashSeparatedCourseKey('test', 'asset_test', '2014_07') + self.course2_key = SlashSeparatedCourseKey('test', 'asset_test2', '2014_07') + + self.course1_files = ['contains.sh', 'picture1.jpg', 'picture2.jpg'] + self.course2_files = ['picture1.jpg', 'picture3.jpg', 'door_2.ogg'] + + def load_assets(course_key, files): + locked = False + for filename in files: + asset_key = course_key.make_asset_key('asset', filename) + self.save_asset(filename, asset_key, filename, locked) + locked = not locked + + load_assets(self.course1_key, self.course1_files) + load_assets(self.course2_key, self.course2_files) + + def save_asset(self, filename, asset_key, displayname, locked): + """ + Load and save the given file. + """ + with open("{}/static/{}".format(DATA_DIR, filename), "rb") as f: + content = StaticContent( + asset_key, displayname, mimetypes.guess_type(filename)[0], f.read(), + locked=locked + ) + self.contentstore.save(content) + + @ddt.data(True, False) + def test_delete(self, deprecated): + """ + Test that deleting assets works + """ + self.set_up_assets(deprecated) + asset_key = self.course1_key.make_asset_key('asset', self.course1_files[0]) + self.contentstore.delete(asset_key) + + with self.assertRaises(NotFoundError): + self.contentstore.find(asset_key) + + # ensure deleting a non-existent file is a noop + self.contentstore.delete(asset_key) + + @ddt.data(True, False) + def test_find(self, deprecated): + """ + Test using find + """ + self.set_up_assets(deprecated) + asset_key = self.course1_key.make_asset_key('asset', self.course1_files[0]) + self.assertIsNotNone(self.contentstore.find(asset_key), "Could not find {}".format(asset_key)) + + self.assertIsNotNone(self.contentstore.find(asset_key, as_stream=True), "Could not find {}".format(asset_key)) + + unknown_asset = self.course1_key.make_asset_key('asset', 'no_such_file.gif') + with self.assertRaises(NotFoundError): + self.contentstore.find(unknown_asset) + self.assertIsNone( + self.contentstore.find(unknown_asset, throw_on_not_found=False), + "Found unknown asset {}".format(unknown_asset) + ) + + @ddt.data(True, False) + def test_export_for_course(self, deprecated): + """ + Test export + """ + self.set_up_assets(deprecated) + root_dir = path.path(mkdtemp()) + try: + self.contentstore.export_all_for_course( + self.course1_key, root_dir, + path.path(root_dir / "policy.json"), + ) + for filename in self.course1_files: + filepath = path.path(root_dir / filename) + self.assertTrue(filepath.isfile(), "{} is not a file".format(filepath)) + for filename in self.course2_files: + if filename not in self.course1_files: + filepath = path.path(root_dir / filename) + self.assertFalse(filepath.isfile(), "{} is unexpected exported a file".format(filepath)) + finally: + shutil.rmtree(root_dir) + + @ddt.data(True, False) + def test_get_all_content(self, deprecated): + """ + Test get_all_content_for_course + """ + self.set_up_assets(deprecated) + course1_assets, count = self.contentstore.get_all_content_for_course(self.course1_key) + self.assertEqual(count, len(self.course1_files), course1_assets) + for asset in course1_assets: + if deprecated: + parsed = AssetLocation.from_deprecated_string(asset['filename']) + else: + parsed = AssetLocation.from_string(asset['filename']) + self.assertIn(parsed.name, self.course1_files) + + course1_assets, __ = self.contentstore.get_all_content_for_course(self.course1_key, 1, 1) + self.assertEqual(len(course1_assets), 1, course1_assets) + + fake_course = SlashSeparatedCourseKey('test', 'fake', 'non') + course_assets, count = self.contentstore.get_all_content_for_course(fake_course) + self.assertEqual(count, 0) + self.assertEqual(course_assets, []) + + @ddt.data(True, False) + def test_attrs(self, deprecated): + """ + Test setting and getting attrs + """ + self.set_up_assets(deprecated) + for filename in self.course1_files: + asset_key = self.course1_key.make_asset_key('asset', filename) + prelocked = self.contentstore.get_attr(asset_key, 'locked', False) + self.contentstore.set_attr(asset_key, 'locked', not prelocked) + self.assertEqual(self.contentstore.get_attr(asset_key, 'locked', False), not prelocked) + + @ddt.data(True, False) + def test_copy_assets(self, deprecated): + """ + copy_all_course_assets + """ + self.set_up_assets(deprecated) + dest_course = SlashSeparatedCourseKey('test', 'destination', 'copy') + self.contentstore.copy_all_course_assets(self.course1_key, dest_course) + for filename in self.course1_files: + asset_key = self.course1_key.make_asset_key('asset', filename) + dest_key = dest_course.make_asset_key('asset', filename) + source = self.contentstore.find(asset_key) + copied = self.contentstore.find(dest_key) + for propname in ['name', 'content_type', 'length', 'locked']: + self.assertEqual(getattr(source, propname), getattr(copied, propname)) + + __, count = self.contentstore.get_all_content_for_course(dest_course) + self.assertEqual(count, len(self.course1_files)) + + @ddt.data(True, False) + def test_delete_assets(self, deprecated): + """ + delete_all_course_assets + """ + self.set_up_assets(deprecated) + self.contentstore.delete_all_course_assets(self.course1_key) + __, count = self.contentstore.get_all_content_for_course(self.course1_key) + self.assertEqual(count, 0) + # ensure it didn't remove any from other course + __, count = self.contentstore.get_all_content_for_course(self.course2_key) + self.assertEqual(count, len(self.course2_files)) diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py b/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py index 6266793651..b7a957d944 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py @@ -210,7 +210,7 @@ class TestMixedModuleStore(LocMapperSetupSansDjango): if index > 0: store_configs[index], store_configs[0] = store_configs[0], store_configs[index] break - self.store = MixedModuleStore(**self.options) + self.store = MixedModuleStore(None, **self.options) self.addCleanup(self.store.close_all_connections) # convert to CourseKeys @@ -518,7 +518,7 @@ def load_function(path): # pylint: disable=unused-argument -def create_modulestore_instance(engine, doc_store_config, options, i18n_service=None): +def create_modulestore_instance(engine, contentstore, doc_store_config, options, i18n_service=None): """ This will return a new instance of a modulestore given an engine and options """ @@ -526,6 +526,7 @@ def create_modulestore_instance(engine, doc_store_config, options, i18n_service= return class_( doc_store_config=doc_store_config, + contentstore=contentstore, branch_setting_func=lambda: ModuleStoreEnum.Branch.draft_preferred, **options ) diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py b/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py index ea50d75492..ca9647b7a5 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py @@ -1,3 +1,5 @@ +# pylint: disable=E1101 +# pylint: disable=W0212 # pylint: disable=E0611 from nose.tools import assert_equals, assert_raises, \ assert_not_equals, assert_false, assert_true, assert_greater, assert_is_instance, assert_is_none @@ -101,6 +103,7 @@ class TestMongoModuleStore(unittest.TestCase): # Also test draft store imports # draft_store = DraftModuleStore( + content_store, doc_store_config, FS_ROOT, RENDER_TEMPLATE, default_class=DEFAULT_CLASS, branch_setting_func=lambda: ModuleStoreEnum.Branch.draft_preferred @@ -145,6 +148,7 @@ class TestMongoModuleStore(unittest.TestCase): def test_mongo_modulestore_type(self): store = MongoModuleStore( + None, {'host': HOST, 'db': DB, 'collection': COLLECTION}, FS_ROOT, RENDER_TEMPLATE, default_class=DEFAULT_CLASS ) @@ -289,7 +293,7 @@ class TestMongoModuleStore(unittest.TestCase): # a bit overkill, could just do for content[0] for content in course_content: assert not content.get('locked', False) - asset_key = AssetLocation._from_deprecated_son(content['_id'], location.run) + asset_key = AssetLocation._from_deprecated_son(content.get('content_son', content['_id']), location.run) assert not TestMongoModuleStore.content_store.get_attr(asset_key, 'locked', False) attrs = TestMongoModuleStore.content_store.get_attrs(asset_key) assert_in('uploadDate', attrs) @@ -302,7 +306,10 @@ class TestMongoModuleStore(unittest.TestCase): TestMongoModuleStore.content_store.set_attrs(asset_key, {'miscel': 99}) assert_equals(TestMongoModuleStore.content_store.get_attr(asset_key, 'miscel'), 99) - asset_key = AssetLocation._from_deprecated_son(course_content[0]['_id'], location.run) + asset_key = AssetLocation._from_deprecated_son( + course_content[0].get('content_son', course_content[0]['_id']), + location.run + ) assert_raises( AttributeError, TestMongoModuleStore.content_store.set_attr, asset_key, 'md5', 'ff1532598830e3feac91c2449eaa60d6' diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_split_migrator.py b/common/lib/xmodule/xmodule/modulestore/tests/test_split_migrator.py index c842b657a7..46c9c60e31 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_split_migrator.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_split_migrator.py @@ -23,7 +23,7 @@ class TestMigration(SplitWMongoCourseBoostrapper): # pylint: disable=W0142 self.loc_mapper = LocMapperStore(test_location_mapper.TrivialCache(), **self.db_config) self.split_mongo.loc_mapper = self.loc_mapper - self.migrator = SplitMigrator(self.split_mongo, self.old_mongo, self.draft_mongo, self.loc_mapper) + self.migrator = SplitMigrator(self.split_mongo, self.draft_mongo, self.loc_mapper) def tearDown(self): dbref = self.loc_mapper.db diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_split_modulestore.py b/common/lib/xmodule/xmodule/modulestore/tests/test_split_modulestore.py index 64af2b81ef..3345827215 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_split_modulestore.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_split_modulestore.py @@ -1759,6 +1759,7 @@ def modulestore(): # pylint: disable=W0142 SplitModuleTest.modulestore = class_( + None, # contentstore SplitModuleTest.MODULESTORE['DOC_STORE_CONFIG'], **options ) diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_split_w_old_mongo.py b/common/lib/xmodule/xmodule/modulestore/tests/test_split_w_old_mongo.py index e7c4d3fb59..1b6c72a3fa 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_split_w_old_mongo.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_split_w_old_mongo.py @@ -49,14 +49,15 @@ class SplitWMongoCourseBoostrapper(unittest.TestCase): self.userid = random.getrandbits(32) super(SplitWMongoCourseBoostrapper, self).setUp() self.split_mongo = SplitMongoModuleStore( + None, 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.old_mongo = MongoModuleStore(None, self.db_config, **self.modulestore_options) self.draft_mongo = DraftMongoModuleStore( - self.db_config, branch_setting_func=lambda: ModuleStoreEnum.Branch.draft_preferred, **self.modulestore_options + None, self.db_config, branch_setting_func=lambda: ModuleStoreEnum.Branch.draft_preferred, **self.modulestore_options ) self.addCleanup(self.tear_down_mongo) self.old_course_key = None diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_xml_importer.py b/common/lib/xmodule/xmodule/modulestore/tests/test_xml_importer.py index 66759b6725..f3451d453a 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_xml_importer.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_xml_importer.py @@ -85,6 +85,7 @@ def modulestore(): # pylint: disable=W0142 ModuleStoreNoSettings.modulestore = class_( + None, # contentstore ModuleStoreNoSettings.MODULESTORE['DOC_STORE_CONFIG'], **options ) diff --git a/common/lib/xmodule/xmodule/modulestore/xml.py b/common/lib/xmodule/xmodule/modulestore/xml.py index e3c58f531f..f661048ff9 100644 --- a/common/lib/xmodule/xmodule/modulestore/xml.py +++ b/common/lib/xmodule/xmodule/modulestore/xml.py @@ -19,15 +19,14 @@ from xmodule.errortracker import make_error_tracker, exc_info_to_str from xmodule.mako_module import MakoDescriptorSystem from xmodule.x_module import XMLParsingSystem, policy_key from xmodule.modulestore.xml_exporter import DEFAULT_CONTENT_FIELDS -from xmodule.modulestore import ModuleStoreEnum +from xmodule.modulestore import ModuleStoreEnum, ModuleStoreReadBase from xmodule.tabs import CourseTabList from opaque_keys.edx.keys import UsageKey -from opaque_keys.edx.locations import SlashSeparatedCourseKey +from opaque_keys.edx.locations import SlashSeparatedCourseKey, Location from xblock.field_data import DictFieldData from xblock.runtime import DictKeyValueStore, IdGenerator -from . import ModuleStoreReadBase, Location, ModuleStoreEnum from .exceptions import ItemNotFoundError from .inheritance import compute_inherited_metadata, inheriting_field_data @@ -720,7 +719,7 @@ class XMLModuleStore(ModuleStoreReadBase): except KeyError: raise ItemNotFoundError(usage_key) - def get_items(self, course_id, settings=None, content=None, **kwargs): + def get_items(self, course_id, settings=None, content=None, revision=None, **kwargs): """ Returns: list of XModuleDescriptor instances for the matching items within the course with @@ -745,6 +744,9 @@ class XMLModuleStore(ModuleStoreReadBase): you can search dates by providing either a datetime for == (probably useless) or a tuple (">"|"<" datetime) for after or before, etc. """ + if revision == ModuleStoreEnum.RevisionOption.draft_only: + return [] + items = [] category = kwargs.pop('category', None) diff --git a/common/lib/xmodule/xmodule/video_module/transcripts_utils.py b/common/lib/xmodule/xmodule/video_module/transcripts_utils.py index 1d80f56945..3a1fa48948 100644 --- a/common/lib/xmodule/xmodule/video_module/transcripts_utils.py +++ b/common/lib/xmodule/xmodule/video_module/transcripts_utils.py @@ -500,8 +500,7 @@ class Transcript(object): Delete asset by location and filename. """ try: - content = Transcript.get_asset(location, filename) - contentstore().delete(content.get_id()) + contentstore().delete(Transcript.asset_location(location, filename)) log.info("Transcript asset %s was removed from store.", filename) except NotFoundError: pass diff --git a/common/test/data/static/contains.sh b/common/test/data/static/contains.sh new file mode 100644 index 0000000000..6aab7a14fe --- /dev/null +++ b/common/test/data/static/contains.sh @@ -0,0 +1,2 @@ +#!/usr/bin/env zsh +git log --all ^opaque-keys-merge-base --format=%H $1 | while read f; do git branch --contains $f; done | sort -u diff --git a/common/test/data/static/door_2.ogg b/common/test/data/static/door_2.ogg new file mode 100644 index 0000000000..ae6b966a05 Binary files /dev/null and b/common/test/data/static/door_2.ogg differ diff --git a/common/test/data/static/picture1.jpg b/common/test/data/static/picture1.jpg new file mode 100644 index 0000000000..1708d1bcca Binary files /dev/null and b/common/test/data/static/picture1.jpg differ diff --git a/common/test/data/static/picture2.jpg b/common/test/data/static/picture2.jpg new file mode 100644 index 0000000000..9285e05ced Binary files /dev/null and b/common/test/data/static/picture2.jpg differ diff --git a/common/test/data/static/picture3.jpg b/common/test/data/static/picture3.jpg new file mode 100644 index 0000000000..b3738a7f38 Binary files /dev/null and b/common/test/data/static/picture3.jpg differ diff --git a/lms/djangoapps/courseware/tests/test_video_handlers.py b/lms/djangoapps/courseware/tests/test_video_handlers.py index 89afc336bf..77670568bb 100644 --- a/lms/djangoapps/courseware/tests/test_video_handlers.py +++ b/lms/djangoapps/courseware/tests/test_video_handlers.py @@ -65,7 +65,10 @@ def _clear_assets(location): assets, __ = store.get_all_content_for_course(location.course_key) for asset in assets: - asset_location = AssetLocation._from_deprecated_son(asset["_id"], location.course_key.run) + asset_location = AssetLocation._from_deprecated_son( + asset.get('content_son', asset["_id"]), + location.course_key.run + ) del_cached_content(asset_location) store.delete(asset_location) diff --git a/mongo_indexes.md b/mongo_indexes.md index 3eb57438df..639743d7ed 100644 --- a/mongo_indexes.md +++ b/mongo_indexes.md @@ -26,10 +26,16 @@ fs.files: Index needed thru 'category' by `_get_all_content_for_course` and others. That query also takes a sort which can be `uploadDate`, `display_name`, +Replace existing index which leaves out `run` with this one: ``` -ensureIndex({'_id.tag': 1, '_id.org': 1, '_id.course': 1, '_id.category': 1}) +ensureIndex({'_id.tag': 1, '_id.org': 1, '_id.course': 1, '_id.category': 1, '_id.run': 1}) +ensureIndex({'content_son.tag': 1, 'content_son.org': 1, 'content_son.course': 1, 'content_son.category': 1, 'content_son.run': 1}) ``` +Note: I'm not advocating adding one which leaves out `category` for now because that would only be +used for `delete_all_course_assets` which in the future should not actually delete the assets except +when doing garbage collection. + Remove index on `displayname` modulestore: