test: updated test_import.py and test_course_index.py for split modulestore

This is part of Old Mongo removal. Also in this commit:

* fixed 400 error for cms old_style assets
* fix TEST_DATA_SPLIT_MODULESTORE import for test_course_index

Author: UvgenGen

cms/djangoapps/contentstore/tests/test_import.py

@@ -5,6 +5,7 @@
 
 
 import copy
+from unittest import skip
 from unittest.mock import patch
 from uuid import uuid4
 
@@ -174,29 +175,29 @@ def test_tab_name_imports_correctly(self):
         print(f"course tabs = {course.tabs}")
         self.assertEqual(course.tabs[1]['name'], 'Syllabus')
 
-    @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split)
-    def test_reimport(self, default_ms_type):
-        with modulestore().default_store(default_ms_type):
-            __, __, course = self.load_test_import_course(create_if_not_present=True)
-            self.load_test_import_course(target_id=course.id)
+    def test_reimport(self):
+        __, __, course = self.load_test_import_course(create_if_not_present=True)
+        self.load_test_import_course(target_id=course.id)
 
+    @skip("OldMongo Deprecation")
     def test_rewrite_reference_list(self):
         # This test fails with split modulestore (the HTML component is not in "different_course_id" namespace).
         # More investigation needs to be done.
-        module_store = modulestore()._get_modulestore_by_type(ModuleStoreEnum.Type.mongo)
+        module_store = modulestore()
         target_id = module_store.make_course_key('testX', 'conditional_copy', 'copy_run')
         import_course_from_xml(
             module_store,
             self.user.id,
             TEST_DATA_DIR,
             ['conditional'],
-            target_id=target_id
+            target_id=target_id,
+            create_if_not_present=True
         )
         conditional_block = module_store.get_item(
             target_id.make_usage_key('conditional', 'condone')
         )
         self.assertIsNotNone(conditional_block)
-        different_course_id = module_store.make_course_key('edX', 'different_course', None)
+        different_course_id = module_store.make_course_key('edX', 'different_course', 'course_run')
         self.assertListEqual(
             [
                 target_id.make_usage_key('problem', 'choiceprob'),
@@ -256,22 +257,20 @@ def _verify_split_test_import(self, target_course_name, source_course_name, spli
 
         self.assertEqual(remapped_verticals, split_test_block.group_id_to_child)
 
-    @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split)
-    def test_video_components_present_while_import(self, store):
+    def test_video_components_present_while_import(self):
         """
         Test that video components with same edx_video_id are present while re-importing
         """
-        with modulestore().default_store(store):
-            module_store = modulestore()
-            course_id = module_store.make_course_key('edX', 'test_import_course', '2012_Fall')
+        module_store = modulestore()
+        course_id = module_store.make_course_key('edX', 'test_import_course', '2012_Fall')
 
-            # Import first time
-            __, __, course = self.load_test_import_course(target_id=course_id, module_store=module_store)
+        # Import first time
+        __, __, course = self.load_test_import_course(target_id=course_id, module_store=module_store)
 
-            # Re-import
-            __, __, re_course = self.load_test_import_course(target_id=course.id, module_store=module_store)
+        # Re-import
+        __, __, re_course = self.load_test_import_course(target_id=course.id, module_store=module_store)
 
-            vertical = module_store.get_item(re_course.id.make_usage_key('vertical', 'vertical_test'))
+        vertical = module_store.get_item(re_course.id.make_usage_key('vertical', 'vertical_test'))
 
-            video = module_store.get_item(vertical.children[1])
-            self.assertEqual(video.display_name, 'default')
+        video = module_store.get_item(vertical.children[1])
+        self.assertEqual(video.display_name, 'default')

cms/djangoapps/contentstore/views/tests/test_course_index.py

@@ -5,7 +5,7 @@
 
 import datetime
 import json
-from unittest import SkipTest, mock, skip
+from unittest import mock, skip
 from unittest.mock import patch
 
 import ddt
@@ -35,6 +35,7 @@
 from openedx.core.djangoapps.waffle_utils.testutils import WAFFLE_TABLES
 from xmodule.modulestore.django import modulestore  # lint-amnesty, pylint: disable=wrong-import-order
 from xmodule.modulestore.exceptions import ItemNotFoundError  # lint-amnesty, pylint: disable=wrong-import-order
+from xmodule.modulestore.tests.django_utils import TEST_DATA_SPLIT_MODULESTORE
 from xmodule.modulestore.tests.factories import CourseFactory, BlockFactory, LibraryFactory, check_mongo_calls  # lint-amnesty, pylint: disable=wrong-import-order
 
 from ..course import _deprecated_blocks_info, course_outline_initial_state, reindex_course_and_check_access
@@ -46,6 +47,8 @@ class TestCourseIndex(CourseTestCase):
     Unit tests for getting the list of courses and the course outline.
     """
 
+    MODULESTORE = TEST_DATA_SPLIT_MODULESTORE
+
     def setUp(self):
         """
         Add a course with odd characters in the fields
@@ -333,6 +336,9 @@ class TestCourseIndexArchived(CourseTestCase):
     """
     Unit tests for testing the course index list when there are archived courses.
     """
+
+    MODULESTORE = TEST_DATA_SPLIT_MODULESTORE
+
     NOW = datetime.datetime.now(pytz.utc)
     DAY = datetime.timedelta(days=1)
     YESTERDAY = NOW - DAY
@@ -415,16 +421,15 @@ def check_index_page(self, separate_archived_courses, org):
         archived_course_tab = parsed_html.find_class('archived-courses')
         self.assertEqual(len(archived_course_tab), 1 if separate_archived_courses else 0)
 
-    @skip('Skip test for old mongo course')
     @ddt.data(
         # Staff user has course staff access
         (True, 'staff', None, 0, 20),
         (False, 'staff', None, 0, 20),
         # Base user has global staff access
-        (True, 'user', ORG, 1, 20),
-        (False, 'user', ORG, 1, 20),
-        (True, 'user', None, 1, 20),
-        (False, 'user', None, 1, 20),
+        (True, 'user', ORG, 2, 20),
+        (False, 'user', ORG, 2, 20),
+        (True, 'user', None, 2, 20),
+        (False, 'user', None, 2, 20),
     )
     @ddt.unpack
     def test_separate_archived_courses(self, separate_archived_courses, username, org, mongo_queries, sql_queries):
@@ -452,6 +457,9 @@ class TestCourseOutline(CourseTestCase):
     """
     Unit tests for the course outline.
     """
+
+    MODULESTORE = TEST_DATA_SPLIT_MODULESTORE
+
     ENABLED_SIGNALS = ['course_published']
 
     def setUp(self):
@@ -550,14 +558,16 @@ def _create_test_data(self, course_block, create_blocks=False, publish=True, blo
         if create_blocks:
             for block_type in block_types:
                 BlockFactory.create(
-                    parent_location=self.vertical.location,
+                    parent=self.vertical,
                     category=block_type,
                     display_name=f'{block_type} Problem'
                 )
 
             if not publish:
                 self.store.unpublish(self.vertical.location, self.user.id)
 
+        # get updated vertical
+        self.vertical = modulestore().get_item(self.vertical.location)
         course_block.advanced_modules.extend(block_types)
 
     def _verify_deprecated_info(self, course_id, advanced_modules, info, deprecated_block_types):
@@ -584,6 +594,7 @@ def _verify_deprecated_info(self, course_id, advanced_modules, info, deprecated_
             reverse_course_url('advanced_settings_handler', course_id)
         )
 
+    @skip('OldMongo Deprecation. HiddenDescriptorWithMixins is not created for split.')
     @ddt.data(
         [{'publish': True}, ['notes']],
         [{'publish': False}, ['notes']],
@@ -596,6 +607,9 @@ def test_verify_deprecated_warning_message(self, publish, block_types):
         """
         course_block = modulestore().get_item(self.course.location)
         self._create_test_data(course_block, create_blocks=True, block_types=block_types, publish=publish)
+        # get updated course_block
+        course_block = modulestore().get_item(self.course.location)
+
         info = _deprecated_blocks_info(course_block, block_types)
         self._verify_deprecated_info(
             course_block.id,
@@ -604,6 +618,7 @@ def test_verify_deprecated_warning_message(self, publish, block_types):
             block_types
         )
 
+    @skip('OldMongo Deprecation. HiddenDescriptorWithMixins is not created for split.')
     @ddt.data(
         (["a", "b", "c"], ["a", "b", "c"]),
         (["a", "b", "c"], ["a", "b", "d"]),
@@ -618,6 +633,8 @@ def test_verify_warn_only_on_enabled_blocks(self, enabled_block_types, deprecate
         expected_block_types = list(set(enabled_block_types) & set(deprecated_block_types))
         course_block = modulestore().get_item(self.course.location)
         self._create_test_data(course_block, create_blocks=True, block_types=enabled_block_types)
+        # get updated course_module
+        course_block = modulestore().get_item(self.course.location)
         info = _deprecated_blocks_info(course_block, deprecated_block_types)
         self._verify_deprecated_info(
             course_block.id,
@@ -632,8 +649,6 @@ def test_proctoring_link_is_visible(self, mock_validate_proctoring_settings):
         """
         Test to check proctored exam settings mfe url is rendering properly
         """
-        if self.course.id.deprecated:
-            raise SkipTest("Skip test for old mongo course")
         mock_validate_proctoring_settings.return_value = [
             {
                 'key': 'proctoring_provider',
@@ -655,6 +670,9 @@ class TestCourseReIndex(CourseTestCase):
     """
     Unit tests for the course outline.
     """
+
+    MODULESTORE = TEST_DATA_SPLIT_MODULESTORE
+
     SUCCESSFUL_RESPONSE = _("Course has been successfully reindexed.")
 
     ENABLED_SIGNALS = ['course_published']
@@ -835,7 +853,7 @@ def test_reindex_seq_error_json_responses(self, mock_index_dictionary):
         with self.assertRaises(SearchIndexingError):
             reindex_course_and_check_access(self.course.id, self.user)
 
-    @mock.patch('xmodule.modulestore.mongo.base.MongoModuleStore.get_course')
+    @mock.patch('xmodule.modulestore.split_mongo.split.SplitMongoModuleStore.get_course')
     def test_reindex_no_item(self, mock_get_course):
         """
         Test system logs an error if no item found.
@@ -945,7 +963,7 @@ def test_indexing_seq_error_responses(self, mock_index_dictionary):
         with self.assertRaises(SearchIndexingError):
             CoursewareSearchIndexer.do_course_reindex(modulestore(), self.course.id)
 
-    @mock.patch('xmodule.modulestore.mongo.base.MongoModuleStore.get_course')
+    @mock.patch('xmodule.modulestore.split_mongo.split.SplitMongoModuleStore.get_course')
     def test_indexing_no_item(self, mock_get_course):
         """
         Test system logs an error if no item found.

openedx/core/djangoapps/contentserver/test/test_contentserver.py

@@ -20,7 +20,7 @@
 from xmodule.contentstore.django import contentstore
 from xmodule.contentstore.content import StaticContent, VERSIONED_ASSETS_PREFIX
 from xmodule.modulestore.django import modulestore
-from xmodule.modulestore.tests.django_utils import TEST_DATA_MONGO_MODULESTORE, SharedModuleStoreTestCase
+from xmodule.modulestore.tests.django_utils import TEST_DATA_SPLIT_MODULESTORE, SharedModuleStoreTestCase
 from xmodule.modulestore.xml_importer import import_course_from_xml
 from xmodule.assetstore.assetmgr import AssetManager
 from xmodule.modulestore.exceptions import ItemNotFoundError
@@ -73,7 +73,7 @@ class ContentStoreToyCourseTest(SharedModuleStoreTestCase):
     """
     Tests that use the toy course.
     """
-    MODULESTORE = TEST_DATA_MONGO_MODULESTORE
+    MODULESTORE = TEST_DATA_SPLIT_MODULESTORE
 
     @classmethod
     def setUpClass(cls):
@@ -82,23 +82,23 @@ def setUpClass(cls):
         cls.contentstore = contentstore()
         cls.modulestore = modulestore()
 
-        cls.course_key = cls.modulestore.make_course_key('edX', 'toy', '2012_Fall')
-
-        import_course_from_xml(
+        course_items = import_course_from_xml(
             cls.modulestore, 1, TEST_DATA_DIR, ['toy'],
-            static_content_store=cls.contentstore, verbose=True
+            static_content_store=cls.contentstore, verbose=True, create_if_not_present=True
         )
+        cls.course = course_items[0]
+        cls.course_key = cls.course.id
 
         # A locked asset
         cls.locked_asset = cls.course_key.make_asset_key('asset', 'sample_static.html')
-        cls.url_locked = str(cls.locked_asset)
+        cls.url_locked = '/' + str(cls.locked_asset)
         cls.url_locked_versioned = get_versioned_asset_url(cls.url_locked)
         cls.url_locked_versioned_old_style = get_old_style_versioned_asset_url(cls.url_locked)
         cls.contentstore.set_attr(cls.locked_asset, 'locked', True)
 
         # An unlocked asset
         cls.unlocked_asset = cls.course_key.make_asset_key('asset', 'another_static.txt')
-        cls.url_unlocked = str(cls.unlocked_asset)
+        cls.url_unlocked = '/' + str(cls.unlocked_asset)
         cls.url_unlocked_versioned = get_versioned_asset_url(cls.url_unlocked)
         cls.url_unlocked_versioned_old_style = get_old_style_versioned_asset_url(cls.url_unlocked)
         cls.length_unlocked = cls.contentstore.get_attr(cls.unlocked_asset, 'length')
@@ -134,7 +134,7 @@ def test_unlocked_versioned_asset_old_style(self):
         Test that unlocked assets that are versioned (old-style) are being served.
         """
         self.client.logout()
-        resp = self.client.get(self.url_unlocked_versioned_old_style)
+        resp = self.client.get(self.url_unlocked_versioned_old_style, follow=True)
         assert resp.status_code == 200
 
     def test_unlocked_versioned_asset_with_nonexistent_version(self):
@@ -168,7 +168,7 @@ def test_locked_versioned_old_styleasset(self):
         assert CourseEnrollment.is_enrolled(self.non_staff_usr, self.course_key)
 
         self.client.login(username=self.non_staff_usr, password='test')
-        resp = self.client.get(self.url_locked_versioned_old_style)
+        resp = self.client.get(self.url_locked_versioned_old_style, follow=True)
         assert resp.status_code == 200
 
     def test_locked_asset_not_logged_in(self):

xmodule/contentstore/content.py

@@ -143,9 +143,9 @@ def get_location_from_path(path):
             return AssetKey.from_string(path)
         except InvalidKeyError:
             # TODO - re-address this once LMS-11198 is tackled.
-            if path.startswith('/'):
+            if path.startswith('/') or path.endswith('/'):
                 # try stripping off the leading slash and try again
-                return AssetKey.from_string(path[1:])
+                return AssetKey.from_string(path.strip('/'))
 
     @staticmethod
     def is_versioned_asset_path(path):