diff --git a/cms/djangoapps/contentstore/management/commands/reindex_course.py b/cms/djangoapps/contentstore/management/commands/reindex_course.py new file mode 100644 index 0000000000000000000000000000000000000000..56cb1008af84058e3761f4f46dbc8adce1e32d08 --- /dev/null +++ b/cms/djangoapps/contentstore/management/commands/reindex_course.py @@ -0,0 +1,115 @@ +""" Management command to update courses' search index """ +import logging +from django.core.management import BaseCommand, CommandError +from optparse import make_option +from textwrap import dedent + +from contentstore.courseware_index import CoursewareSearchIndexer +from search.search_engine_base import SearchEngine +from elasticsearch import exceptions + +from opaque_keys.edx.keys import CourseKey +from opaque_keys import InvalidKeyError +from opaque_keys.edx.locator import CourseLocator + +from .prompt import query_yes_no + +from xmodule.modulestore.django import modulestore + + +class Command(BaseCommand): + """ + Command to re-index courses + + Examples: + + ./manage.py reindex_course <course_id_1> <course_id_2> - reindexes courses with keys course_id_1 and course_id_2 + ./manage.py reindex_course --all - reindexes all available courses + ./manage.py reindex_course --setup - reindexes all courses for devstack setup + """ + help = dedent(__doc__) + + can_import_settings = True + + args = "<course_id course_id ...>" + + all_option = make_option('--all', + action='store_true', + dest='all', + default=False, + help='Reindex all courses') + + setup_option = make_option('--setup', + action='store_true', + dest='setup', + default=False, + help='Reindex all courses on developers stack setup') + + option_list = BaseCommand.option_list + (all_option, setup_option) + + CONFIRMATION_PROMPT = u"Re-indexing all courses might be a time consuming operation. Do you want to continue?" + + def _parse_course_key(self, raw_value): + """ Parses course key from string """ + try: + result = CourseKey.from_string(raw_value) + except InvalidKeyError: + raise CommandError("Invalid course_key: '%s'." % raw_value) + + if not isinstance(result, CourseLocator): + raise CommandError(u"Argument {0} is not a course key".format(raw_value)) + + return result + + def handle(self, *args, **options): + """ + By convention set by Django developers, this method actually executes command's actions. + So, there could be no better docstring than emphasize this once again. + """ + all_option = options.get('all', False) + setup_option = options.get('setup', False) + index_all_courses_option = all_option or setup_option + + if len(args) == 0 and not index_all_courses_option: + raise CommandError(u"reindex_course requires one or more arguments: <course_id>") + + store = modulestore() + + if index_all_courses_option: + index_name = CoursewareSearchIndexer.INDEX_NAME + doc_type = CoursewareSearchIndexer.DOCUMENT_TYPE + if setup_option: + try: + # try getting the ElasticSearch engine + searcher = SearchEngine.get_search_engine(index_name) + except exceptions.ElasticsearchException as exc: + logging.exception('Search Engine error - %s', unicode(exc)) + return + + index_exists = searcher._es.indices.exists(index=index_name) # pylint: disable=protected-access + doc_type_exists = searcher._es.indices.exists_type( # pylint: disable=protected-access + index=index_name, + doc_type=doc_type + ) + + index_mapping = searcher._es.indices.get_mapping( # pylint: disable=protected-access + index=index_name, + doc_type=doc_type + ) if index_exists and doc_type_exists else {} + + if index_exists and index_mapping: + return + + # if reindexing is done during devstack setup step, don't prompt the user + if setup_option or query_yes_no(self.CONFIRMATION_PROMPT, default="no"): + # in case of --setup or --all, get the list of course keys from all courses + # that are stored in the modulestore + course_keys = [course.id for course in modulestore().get_courses()] + else: + return + else: + # in case course keys are provided as arguments + course_keys = map(self._parse_course_key, args) + + for course_key in course_keys: + CoursewareSearchIndexer.do_course_reindex(store, course_key) diff --git a/cms/djangoapps/contentstore/management/commands/tests/test_reindex_courses.py b/cms/djangoapps/contentstore/management/commands/tests/test_reindex_courses.py new file mode 100644 index 0000000000000000000000000000000000000000..6383d046b8b9c7912fd63555fefa5401a4584a07 --- /dev/null +++ b/cms/djangoapps/contentstore/management/commands/tests/test_reindex_courses.py @@ -0,0 +1,131 @@ +""" Tests for course reindex command """ +import ddt +from django.core.management import call_command, CommandError +import mock + +from xmodule.modulestore import ModuleStoreEnum +from xmodule.modulestore.django import modulestore +from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase +from common.test.utils import nostderr +from xmodule.modulestore.tests.factories import CourseFactory, LibraryFactory + +from contentstore.management.commands.reindex_course import Command as ReindexCommand +from contentstore.courseware_index import SearchIndexingError + + +@ddt.ddt +class TestReindexCourse(ModuleStoreTestCase): + """ Tests for course reindex command """ + def setUp(self): + """ Setup method - create courses """ + super(TestReindexCourse, self).setUp() + self.store = modulestore() + self.first_lib = LibraryFactory.create( + org="test", library="lib1", display_name="run1", default_store=ModuleStoreEnum.Type.split + ) + self.second_lib = LibraryFactory.create( + org="test", library="lib2", display_name="run2", default_store=ModuleStoreEnum.Type.split + ) + + self.first_course = CourseFactory.create( + org="test", course="course1", display_name="run1" + ) + self.second_course = CourseFactory.create( + org="test", course="course2", display_name="run1" + ) + + REINDEX_PATH_LOCATION = 'contentstore.management.commands.reindex_course.CoursewareSearchIndexer.do_course_reindex' + MODULESTORE_PATCH_LOCATION = 'contentstore.management.commands.reindex_course.modulestore' + YESNO_PATCH_LOCATION = 'contentstore.management.commands.reindex_course.query_yes_no' + + def _get_lib_key(self, library): + """ Get's library key as it is passed to indexer """ + return library.location.library_key + + def _build_calls(self, *courses): + """ Builds a list of mock.call instances representing calls to reindexing method """ + return [mock.call(self.store, course.id) for course in courses] + + def test_given_no_arguments_raises_command_error(self): + """ Test that raises CommandError for incorrect arguments """ + with self.assertRaises(SystemExit), nostderr(): + with self.assertRaisesRegexp(CommandError, ".* requires one or more arguments .*"): + call_command('reindex_course') + + @ddt.data('qwerty', 'invalid_key', 'xblock-v1:qwe+rty') + def test_given_invalid_course_key_raises_not_found(self, invalid_key): + """ Test that raises InvalidKeyError for invalid keys """ + errstring = "Invalid course_key: '%s'." % invalid_key + with self.assertRaises(SystemExit) as ex: + with self.assertRaisesRegexp(CommandError, errstring): + call_command('reindex_course', invalid_key) + self.assertEqual(ex.exception.code, 1) + + def test_given_library_key_raises_command_error(self): + """ Test that raises CommandError if library key is passed """ + with self.assertRaises(SystemExit), nostderr(): + with self.assertRaisesRegexp(SearchIndexingError, ".* is not a course key"): + call_command('reindex_course', unicode(self._get_lib_key(self.first_lib))) + + with self.assertRaises(SystemExit), nostderr(): + with self.assertRaisesRegexp(SearchIndexingError, ".* is not a course key"): + call_command('reindex_course', unicode(self._get_lib_key(self.second_lib))) + + with self.assertRaises(SystemExit), nostderr(): + with self.assertRaisesRegexp(SearchIndexingError, ".* is not a course key"): + call_command( + 'reindex_course', + unicode(self.second_course.id), + unicode(self._get_lib_key(self.first_lib)) + ) + + def test_given_id_list_indexes_courses(self): + """ Test that reindexes courses when given single course key or a list of course keys """ + with mock.patch(self.REINDEX_PATH_LOCATION) as patched_index, \ + mock.patch(self.MODULESTORE_PATCH_LOCATION, mock.Mock(return_value=self.store)): + call_command('reindex_course', unicode(self.first_course.id)) + self.assertEqual(patched_index.mock_calls, self._build_calls(self.first_course)) + patched_index.reset_mock() + + call_command('reindex_course', unicode(self.second_course.id)) + self.assertEqual(patched_index.mock_calls, self._build_calls(self.second_course)) + patched_index.reset_mock() + + call_command( + 'reindex_course', + unicode(self.first_course.id), + unicode(self.second_course.id) + ) + expected_calls = self._build_calls(self.first_course, self.second_course) + self.assertEqual(patched_index.mock_calls, expected_calls) + + def test_given_all_key_prompts_and_reindexes_all_courses(self): + """ Test that reindexes all courses when --all key is given and confirmed """ + with mock.patch(self.YESNO_PATCH_LOCATION) as patched_yes_no: + patched_yes_no.return_value = True + with mock.patch(self.REINDEX_PATH_LOCATION) as patched_index, \ + mock.patch(self.MODULESTORE_PATCH_LOCATION, mock.Mock(return_value=self.store)): + call_command('reindex_course', all=True) + + patched_yes_no.assert_called_once_with(ReindexCommand.CONFIRMATION_PROMPT, default='no') + expected_calls = self._build_calls(self.first_course, self.second_course) + self.assertItemsEqual(patched_index.mock_calls, expected_calls) + + def test_given_all_key_prompts_and_reindexes_all_courses_cancelled(self): + """ Test that does not reindex anything when --all key is given and cancelled """ + with mock.patch(self.YESNO_PATCH_LOCATION) as patched_yes_no: + patched_yes_no.return_value = False + with mock.patch(self.REINDEX_PATH_LOCATION) as patched_index, \ + mock.patch(self.MODULESTORE_PATCH_LOCATION, mock.Mock(return_value=self.store)): + call_command('reindex_course', all=True) + + patched_yes_no.assert_called_once_with(ReindexCommand.CONFIRMATION_PROMPT, default='no') + patched_index.assert_not_called() + + def test_fail_fast_if_reindex_fails(self): + """ Test that fails on first reindexing exception """ + with mock.patch(self.REINDEX_PATH_LOCATION) as patched_index: + patched_index.side_effect = SearchIndexingError("message", []) + + with self.assertRaises(SearchIndexingError): + call_command('reindex_course', unicode(self.second_course.id)) diff --git a/lms/envs/common.py b/lms/envs/common.py index 73dfd5f03cb0f77ab7ae5eae4b70f2e2f88888a2..c1ebcfbb0431124fca9465291f2c18ec0c731e9c 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -395,6 +395,9 @@ FEATURES = { # Course discovery feature 'ENABLE_COURSE_DISCOVERY': False, + # Setting for overriding default filtering facets for Course discovery + # COURSE_DISCOVERY_FILTERS = ["org", "language", "modes"] + # Software secure fake page feature flag 'ENABLE_SOFTWARE_SECURE_FAKE': False, diff --git a/lms/envs/devstack.py b/lms/envs/devstack.py index 3d90ef36a93b04dc30793bb8ef62769e6166341d..8d3fba9362ca30d5a305820c5c13bf1fcb0550c3 100644 --- a/lms/envs/devstack.py +++ b/lms/envs/devstack.py @@ -138,7 +138,7 @@ FEATURES['LICENSING'] = True ########################## Courseware Search ####################### -FEATURES['ENABLE_COURSEWARE_SEARCH'] = False +FEATURES['ENABLE_COURSEWARE_SEARCH'] = True SEARCH_ENGINE = "search.elastic.ElasticSearchEngine" @@ -167,7 +167,9 @@ COURSE_DISCOVERY_MEANINGS = { 'language': LANGUAGE_MAP, } -FEATURES['ENABLE_COURSE_DISCOVERY'] = False +FEATURES['ENABLE_COURSE_DISCOVERY'] = True +# Setting for overriding default filtering facets for Course discovery +# COURSE_DISCOVERY_FILTERS = ["org", "language", "modes"] FEATURES['COURSES_ARE_BROWSEABLE'] = True HOMEPAGE_COURSE_MAX = 9 diff --git a/pavelib/paver_tests/test_servers.py b/pavelib/paver_tests/test_servers.py index f5a105797540b2940094aae713968d0fd29903f9..3dbf20388eb95a7a16e21478afa61202d91dcd6c 100644 --- a/pavelib/paver_tests/test_servers.py +++ b/pavelib/paver_tests/test_servers.py @@ -30,6 +30,9 @@ EXPECTED_CELERY_COMMAND = ( EXPECTED_RUN_SERVER_COMMAND = ( "python manage.py {system} --settings={settings} runserver --traceback --pythonpath=. 0.0.0.0:{port}" ) +EXPECTED_INDEX_COURSE_COMMAND = ( + "python manage.py {system} --settings={settings} reindex_course --setup" +) @ddt.ddt @@ -83,13 +86,27 @@ class TestPaverServerTasks(PaverTestCase): Test the "devstack" task. """ options = server_options.copy() + is_optimized = options.get("optimized", False) + expected_settings = "devstack_optimized" if is_optimized else options.get("settings", "devstack") # First test with LMS options["system"] = "lms" + options["expected_messages"] = [ + EXPECTED_INDEX_COURSE_COMMAND.format( + system="cms", + settings=expected_settings, + ) + ] self.verify_server_task("devstack", options, contracts_default=True) # Then test with Studio options["system"] = "cms" + options["expected_messages"] = [ + EXPECTED_INDEX_COURSE_COMMAND.format( + system="cms", + settings=expected_settings, + ) + ] self.verify_server_task("devstack", options, contracts_default=True) @ddt.data( @@ -196,7 +213,7 @@ class TestPaverServerTasks(PaverTestCase): call_task("pavelib.servers.devstack", args=args) else: call_task("pavelib.servers.{task_name}".format(task_name=task_name), options=options) - expected_messages = [] + expected_messages = options.get("expected_messages", []) expected_settings = settings if settings else "devstack" expected_asset_settings = asset_settings if asset_settings else expected_settings if is_optimized: diff --git a/pavelib/servers.py b/pavelib/servers.py index d2b0615e7f173edeeabcde5e44f7b88287e144ef..7c0d778bbf1cc04358bf617c302cb0924effe20e 100644 --- a/pavelib/servers.py +++ b/pavelib/servers.py @@ -135,6 +135,7 @@ def devstack(args): if args.optimized: settings = OPTIMIZED_SETTINGS asset_settings = OPTIMIZED_ASSETS_SETTINGS + sh(django_cmd('cms', settings, 'reindex_course', '--setup')) run_server( args.system[0], fast=args.fast,