Merge pull request #7641 from open-craft/eugeny/problem-type-search

Capa problem type filtering using edx-search

cms/djangoapps/contentstore/management/commands/reindex_library.py

+""" Management command to update libraries' search index """
+from django.core.management import BaseCommand, CommandError
+from optparse import make_option
+from textwrap import dedent
+
+from contentstore.courseware_index import LibrarySearchIndexer
+
+from opaque_keys.edx.keys import CourseKey
+from opaque_keys import InvalidKeyError
+from opaque_keys.edx.locations import SlashSeparatedCourseKey
+from opaque_keys.edx.locator import LibraryLocator
+
+from .prompt import query_yes_no
+
+from xmodule.modulestore.django import modulestore
+
+
+class Command(BaseCommand):
+    """
+    Command to reindex content libraries (single, multiple or all available)
+
+    Examples:
+
+        ./manage.py reindex_library lib1 lib2 - reindexes libraries with keys lib1 and lib2
+        ./manage.py reindex_library --all - reindexes all available libraries
+    """
+    help = dedent(__doc__)
+
+    can_import_settings = True
+
+    args = "<library_id library_id ...>"
+
+    option_list = BaseCommand.option_list + (
+        make_option(
+            '--all',
+            action='store_true',
+            dest='all',
+            default=False,
+            help='Reindex all libraries'
+        ),)
+
+    CONFIRMATION_PROMPT = u"Reindexing all libraries might be a time consuming operation. Do you want to continue?"
+
+    def _parse_library_key(self, raw_value):
+        """ Parses library key from string """
+        try:
+            result = CourseKey.from_string(raw_value)
+        except InvalidKeyError:
+            result = SlashSeparatedCourseKey.from_deprecated_string(raw_value)
+
+        if not isinstance(result, LibraryLocator):
+            raise CommandError(u"Argument {0} is not a library 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.
+        """
+        if len(args) == 0 and not options.get('all', False):
+            raise CommandError(u"reindex_library requires one or more arguments: <library_id>")
+
+        store = modulestore()
+
+        if options.get('all', False):
+            if query_yes_no(self.CONFIRMATION_PROMPT, default="no"):
+                library_keys = [library.location.library_key.replace(branch=None) for library in store.get_libraries()]
+            else:
+                return
+        else:
+            library_keys = map(self._parse_library_key, args)
+
+        for library_key in library_keys:
+            LibrarySearchIndexer.do_library_reindex(store, library_key)

cms/djangoapps/contentstore/management/commands/tests/test_reindex_library.py

+""" Tests for library reindex command """
+import sys
+import contextlib
+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 xmodule.modulestore.tests.factories import CourseFactory, LibraryFactory
+
+from opaque_keys import InvalidKeyError
+
+from contentstore.management.commands.reindex_library import Command as ReindexCommand
+from contentstore.courseware_index import SearchIndexingError
+
+
+@contextlib.contextmanager
+def nostderr():
+    """
+    ContextManager to suppress stderr messages
+    http://stackoverflow.com/a/1810086/882918
+    """
+    savestderr = sys.stderr
+
+    class Devnull(object):
+        """ /dev/null incarnation as output-stream-like object """
+        def write(self, _):
+            """ Write method - just does nothing"""
+            pass
+
+    sys.stderr = Devnull()
+    try:
+        yield
+    finally:
+        sys.stderr = savestderr
+
+
+@ddt.ddt
+class TestReindexLibrary(ModuleStoreTestCase):
+    """ Tests for library reindex command """
+    def setUp(self):
+        """ Setup method - create libraries and courses """
+        super(TestReindexLibrary, 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", default_store=ModuleStoreEnum.Type.split
+        )
+        self.second_course = CourseFactory.create(
+            org="test", course="course2", display_name="run1", default_store=ModuleStoreEnum.Type.split
+        )
+
+    REINDEX_PATH_LOCATION = 'contentstore.management.commands.reindex_library.LibrarySearchIndexer.do_library_reindex'
+    MODULESTORE_PATCH_LOCATION = 'contentstore.management.commands.reindex_library.modulestore'
+    YESNO_PATCH_LOCATION = 'contentstore.management.commands.reindex_library.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, *libraries):
+        """ BUilds a list of mock.call instances representing calls to reindexing method """
+        return [mock.call(self.store, self._get_lib_key(lib)) for lib in libraries]
+
+    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_library')
+
+    @ddt.data('qwerty', 'invalid_key', 'xblock-v1:qwe+rty')
+    def test_given_invalid_lib_key_raises_not_found(self, invalid_key):
+        """ Test that raises InvalidKeyError for invalid keys """
+        with self.assertRaises(InvalidKeyError):
+            call_command('reindex_library', invalid_key)
+
+    def test_given_course_key_raises_command_error(self):
+        """ Test that raises CommandError if course key is passed """
+        with self.assertRaises(SystemExit), nostderr():
+            with self.assertRaisesRegexp(CommandError, ".* is not a library key"):
+                call_command('reindex_library', unicode(self.first_course.id))
+
+        with self.assertRaises(SystemExit), nostderr():
+            with self.assertRaisesRegexp(CommandError, ".* is not a library key"):
+                call_command('reindex_library', unicode(self.second_course.id))
+
+        with self.assertRaises(SystemExit), nostderr():
+            with self.assertRaisesRegexp(CommandError, ".* is not a library key"):
+                call_command(
+                    'reindex_library',
+                    unicode(self.second_course.id),
+                    unicode(self._get_lib_key(self.first_lib))
+                )
+
+    def test_given_id_list_indexes_libraries(self):
+        """ Test that reindexes libraries when given single library key or a list of library 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_library', unicode(self._get_lib_key(self.first_lib)))
+            self.assertEqual(patched_index.mock_calls, self._build_calls(self.first_lib))
+            patched_index.reset_mock()
+
+            call_command('reindex_library', unicode(self._get_lib_key(self.second_lib)))
+            self.assertEqual(patched_index.mock_calls, self._build_calls(self.second_lib))
+            patched_index.reset_mock()
+
+            call_command(
+                'reindex_library',
+                unicode(self._get_lib_key(self.first_lib)),
+                unicode(self._get_lib_key(self.second_lib))
+            )
+            expected_calls = self._build_calls(self.first_lib, self.second_lib)
+            self.assertEqual(patched_index.mock_calls, expected_calls)
+
+    def test_given_all_key_prompts_and_reindexes_all_libraries(self):
+        """ Test that reindexes all libraries 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_library', all=True)
+
+                patched_yes_no.assert_called_once_with(ReindexCommand.CONFIRMATION_PROMPT, default='no')
+                expected_calls = self._build_calls(self.first_lib, self.second_lib)
+                self.assertEqual(patched_index.mock_calls, expected_calls)
+
+    def test_given_all_key_prompts_and_reindexes_all_libraries_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_library', 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_library', unicode(self._get_lib_key(self.second_lib)))

cms/djangoapps/contentstore/signals.py

-""" receiver of course_published events in order to trigger indexing task """
+""" receivers of course_published and library_updated events in order to trigger indexing task """
 from datetime import datetime
 from pytz import UTC
 
 from django.dispatch import receiver
 
 from xmodule.modulestore.django import SignalHandler
-from contentstore.courseware_index import indexing_is_enabled
+from contentstore.courseware_index import CoursewareSearchIndexer, LibrarySearchIndexer
 
 
 @receiver(SignalHandler.course_published)
@@ -15,5 +15,16 @@ def listen_for_course_publish(sender, course_key, **kwargs):  # pylint: disable=
     """
     # import here, because signal is registered at startup, but items in tasks are not yet able to be loaded
     from .tasks import update_search_index
-    if indexing_is_enabled():
+    if CoursewareSearchIndexer.indexing_is_enabled():
         update_search_index.delay(unicode(course_key), datetime.now(UTC).isoformat())
+
+
+@receiver(SignalHandler.library_updated)
+def listen_for_library_update(sender, library_key, **kwargs):  # pylint: disable=unused-argument
+    """
+    Receives signal and kicks off celery task to update search index
+    """
+    # import here, because signal is registered at startup, but items in tasks are not yet able to be loaded
+    from .tasks import update_library_index
+    if LibrarySearchIndexer.indexing_is_enabled():
+        update_library_index.delay(unicode(library_key), datetime.now(UTC).isoformat())

cms/djangoapps/contentstore/tasks.py

@@ -10,7 +10,7 @@ from pytz import UTC
 
 from django.contrib.auth.models import User
 
-from contentstore.courseware_index import CoursewareSearchIndexer, SearchIndexingError
+from contentstore.courseware_index import CoursewareSearchIndexer, LibrarySearchIndexer, SearchIndexingError
 from contentstore.utils import initialize_permissions
 from course_action_state.models import CourseRerunState
 from opaque_keys.edx.keys import CourseKey
@@ -82,19 +82,36 @@ def deserialize_fields(json_fields):
     return fields
 
 
+def _parse_time(time_isoformat):
+    """ Parses time from iso format """
+    return datetime.strptime(
+        # remove the +00:00 from the end of the formats generated within the system
+        time_isoformat.split('+')[0],
+        "%Y-%m-%dT%H:%M:%S.%f"
+    ).replace(tzinfo=UTC)
+
+
 @task()
 def update_search_index(course_id, triggered_time_isoformat):
     """ Updates course search index. """
     try:
         course_key = CourseKey.from_string(course_id)
-        triggered_time = datetime.strptime(
-            # remove the +00:00 from the end of the formats generated within the system
-            triggered_time_isoformat.split('+')[0],
-            "%Y-%m-%dT%H:%M:%S.%f"
-        ).replace(tzinfo=UTC)
-        CoursewareSearchIndexer.index_course(modulestore(), course_key, triggered_at=triggered_time)
+        CoursewareSearchIndexer.index(modulestore(), course_key, triggered_at=(_parse_time(triggered_time_isoformat)))
 
     except SearchIndexingError as exc:
         LOGGER.error('Search indexing error for complete course %s - %s', course_id, unicode(exc))
     else:
         LOGGER.debug('Search indexing successful for complete course %s', course_id)
+
+
+@task()
+def update_library_index(library_id, triggered_time_isoformat):
+    """ Updates course search index. """
+    try:
+        library_key = CourseKey.from_string(library_id)
+        LibrarySearchIndexer.index(modulestore(), library_key, triggered_at=(_parse_time(triggered_time_isoformat)))
+
+    except SearchIndexingError as exc:
+        LOGGER.error('Search indexing error for library %s - %s', library_id, unicode(exc))
+    else:
+        LOGGER.debug('Search indexing successful for library %s', library_id)

cms/djangoapps/contentstore/tests/test_libraries.py

@@ -386,6 +386,7 @@ class TestLibraries(LibraryTestCase):
         html_block = modulestore().get_item(lc_block.children[0])
         self.assertEqual(html_block.data, data2)
 
+    @patch("xmodule.library_tools.SearchEngine.get_search_engine", Mock(return_value=None))
     def test_refreshes_children_if_capa_type_change(self):
         """ Tests that children are automatically refreshed if capa type field changes """
         name1, name2 = "Option Problem", "Multiple Choice Problem"

cms/envs/aws.py

@@ -328,7 +328,7 @@ API_DATE_FORMAT = ENV_TOKENS.get('API_DATE_FORMAT', API_DATE_FORMAT)
 # Example: {'CN': 'http://api.xuetangx.com/edx/video?s3_url='}
 VIDEO_CDN_URL = ENV_TOKENS.get('VIDEO_CDN_URL', {})
 
-if FEATURES['ENABLE_COURSEWARE_INDEX']:
+if FEATURES['ENABLE_COURSEWARE_INDEX'] or FEATURES['ENABLE_LIBRARY_INDEX']:
     # Use ElasticSearch for the search engine
     SEARCH_ENGINE = "search.elastic.ElasticSearchEngine"
 

cms/envs/bok_choy.py

@@ -78,6 +78,7 @@ YOUTUBE['TEST_URL'] = "127.0.0.1:{0}/test_youtube/".format(YOUTUBE_PORT)
 YOUTUBE['TEXT_API']['url'] = "127.0.0.1:{0}/test_transcripts_youtube/".format(YOUTUBE_PORT)
 
 FEATURES['ENABLE_COURSEWARE_INDEX'] = True
+FEATURES['ENABLE_LIBRARY_INDEX'] = True
 SEARCH_ENGINE = "search.tests.mock_search_engine.MockSearchEngine"
 # Path at which to store the mock index
 MOCK_SEARCH_BACKING_FILE = (

cms/envs/common.py

@@ -140,6 +140,9 @@ FEATURES = {
     # Enable the courseware search functionality
     'ENABLE_COURSEWARE_INDEX': False,
 
+    # Enable content libraries search functionality
+    'ENABLE_LIBRARY_INDEX': False,
+
     # Enable course reruns, which will always use the split modulestore
     'ALLOW_COURSE_RERUNS': True,
 

cms/envs/devstack.py

@@ -80,6 +80,7 @@ FEATURES['ENTRANCE_EXAMS'] = True
 
 ################################ SEARCH INDEX ################################
 FEATURES['ENABLE_COURSEWARE_INDEX'] = True
+FEATURES['ENABLE_LIBRARY_INDEX'] = True
 SEARCH_ENGINE = "search.elastic.ElasticSearchEngine"
 
 ###############################################################################

cms/envs/test.py

@@ -267,6 +267,7 @@ VIDEO_CDN_URL = {
 
 # Courseware Search Index
 FEATURES['ENABLE_COURSEWARE_INDEX'] = True
+FEATURES['ENABLE_LIBRARY_INDEX'] = True
 SEARCH_ENGINE = "search.tests.mock_search_engine.MockSearchEngine"
 
 # Dummy secret key for dev/test

common/lib/xmodule/xmodule/capa_module.py

@@ -113,6 +113,7 @@ class CapaDescriptor(CapaFields, RawDescriptor):
     Module implementing problems in the LON-CAPA format,
     as implemented by capa.capa_problem
     """
+    INDEX_CONTENT_TYPE = 'CAPA'
 
     module_class = CapaModule
 
@@ -186,6 +187,21 @@ class CapaDescriptor(CapaFields, RawDescriptor):
         registered_tags = responsetypes.registry.registered_tags()
         return set([node.tag for node in tree.iter() if node.tag in registered_tags])
 
+    def index_dictionary(self):
+        """
+        Return dictionary prepared with module content and type for indexing.
+        """
+        result = super(CapaDescriptor, self).index_dictionary()
+        if not result:
+            result = {}
+        index = {
+            'content_type': self.INDEX_CONTENT_TYPE,
+            'problem_types': list(self.problem_types),
+            "display_name": self.display_name
+        }
+        result.update(index)
+        return result
+
     # Proxy to CapaModule for access to any of its attributes
     answer_available = module_attr('answer_available')
     check_button_name = module_attr('check_button_name')

common/lib/xmodule/xmodule/library_tools.py

@@ -2,13 +2,19 @@
 XBlock runtime services for LibraryContentModule
 """
 from django.core.exceptions import PermissionDenied
-from opaque_keys.edx.locator import LibraryLocator
+from opaque_keys.edx.locator import LibraryLocator, LibraryUsageLocator
+from search.search_engine_base import SearchEngine
 from xmodule.library_content_module import ANY_CAPA_TYPE_VALUE
 from xmodule.modulestore import ModuleStoreEnum
 from xmodule.modulestore.exceptions import ItemNotFoundError
 from xmodule.capa_module import CapaDescriptor
 
 
+def normalize_key_for_search(library_key):
+    """ Normalizes library key for use with search indexing """
+    return library_key.replace(version_guid=None, branch=None)
+
+
 class LibraryToolsService(object):
     """
     Service that allows LibraryContentModule to interact with libraries in the
@@ -86,13 +92,25 @@ class LibraryToolsService(object):
             result_json.append(info)
         return result_json
 
+    def _problem_type_filter(self, library, capa_type):
+        """ Filters library children by capa type"""
+        search_engine = SearchEngine.get_search_engine(index="library_index")
+        if search_engine:
+            filter_clause = {
+                "library": unicode(normalize_key_for_search(library.location.library_key)),
+                "content_type": CapaDescriptor.INDEX_CONTENT_TYPE,
+                "problem_types": capa_type
+            }
+            search_result = search_engine.search(field_dictionary=filter_clause)
+            results = search_result.get('results', [])
+            return [LibraryUsageLocator.from_string(item['data']['id']) for item in results]
+        else:
+            return [key for key in library.children if self._filter_child(key, capa_type)]
+
     def _filter_child(self, usage_key, capa_type):
         """
         Filters children by CAPA problem type, if configured
         """
-        if capa_type == ANY_CAPA_TYPE_VALUE:
-            return True
-
         if usage_key.block_type != "problem":
             return False
 
@@ -137,7 +155,7 @@ class LibraryToolsService(object):
         filter_children = (dest_block.capa_type != ANY_CAPA_TYPE_VALUE)
         if filter_children:
             # Apply simple filtering based on CAPA problem types:
-            source_blocks.extend([key for key in library.children if self._filter_child(key, dest_block.capa_type)])
+            source_blocks.extend(self._problem_type_filter(library, dest_block.capa_type))
         else:
             source_blocks.extend(library.children)
 

common/lib/xmodule/xmodule/modulestore/__init__.py

@@ -120,6 +120,7 @@ class BulkOpsRecord(object):
     def __init__(self):
         self._active_count = 0
         self.has_publish_item = False
+        self.has_library_updated_item = False
 
     @property
     def active(self):
@@ -248,7 +249,7 @@ class BulkOperationsMixin(object):
         if bulk_ops_record.is_root:
             self._start_outermost_bulk_operation(bulk_ops_record, course_key)
 
-    def _end_outermost_bulk_operation(self, bulk_ops_record, course_key, emit_signals=True):
+    def _end_outermost_bulk_operation(self, bulk_ops_record, structure_key, emit_signals=True):
         """
         The outermost nested bulk_operation call: do the actual end of the bulk operation.
 
@@ -256,12 +257,12 @@ class BulkOperationsMixin(object):
         """
         pass
 
-    def _end_bulk_operation(self, course_key, emit_signals=True):
+    def _end_bulk_operation(self, structure_key, emit_signals=True):
         """
-        End the active bulk operation on course_key.
+        End the active bulk operation on structure_key (course or library key).
         """
         # If no bulk op is active, return
-        bulk_ops_record = self._get_bulk_ops_record(course_key)
+        bulk_ops_record = self._get_bulk_ops_record(structure_key)
         if not bulk_ops_record.active:
             return
 
@@ -272,9 +273,9 @@ class BulkOperationsMixin(object):
         if bulk_ops_record.active:
             return
 
-        self._end_outermost_bulk_operation(bulk_ops_record, course_key, emit_signals)
+        self._end_outermost_bulk_operation(bulk_ops_record, structure_key, emit_signals)
 
-        self._clear_bulk_ops_record(course_key)
+        self._clear_bulk_ops_record(structure_key)
 
     def _is_in_bulk_operation(self, course_key, ignore_case=False):
         """
@@ -291,6 +292,15 @@ class BulkOperationsMixin(object):
             signal_handler.send("course_published", course_key=course_id)
             bulk_ops_record.has_publish_item = False
 
+    def send_bulk_library_updated_signal(self, bulk_ops_record, library_id):
+        """
+        Sends out the signal that library have been updated.
+        """
+        signal_handler = getattr(self, 'signal_handler', None)
+        if signal_handler and bulk_ops_record.has_library_updated_item:
+            signal_handler.send("library_updated", library_key=library_id)
+            bulk_ops_record.has_library_updated_item = False
+
 
 class EditInfo(object):
     """
@@ -1326,6 +1336,23 @@ class ModuleStoreWriteBase(ModuleStoreReadBase, ModuleStoreWrite):
             else:
                 signal_handler.send("course_published", course_key=course_key)
 
+    def _flag_library_updated_event(self, library_key):
+        """
+        Wrapper around calls to fire the library_updated signal
+        Unless we're nested in an active bulk operation, this simply fires the signal
+        otherwise a publish will be signalled at the end of the bulk operation
+
+        Arguments:
+            library_updated - library_updated to which the signal applies
+        """
+        signal_handler = getattr(self, 'signal_handler', None)
+        if signal_handler:
+            bulk_record = self._get_bulk_ops_record(library_key) if isinstance(self, BulkOperationsMixin) else None
+            if bulk_record and bulk_record.active:
+                bulk_record.has_library_updated_item = True
+            else:
+                signal_handler.send("library_updated", library_key=library_key)
+
 
 def only_xmodules(identifier, entry_points):
     """Only use entry_points that are supplied by the xmodule package"""

common/lib/xmodule/xmodule/modulestore/django.py

@@ -80,9 +80,11 @@ class SignalHandler(object):
 
     """
     course_published = django.dispatch.Signal(providing_args=["course_key"])
+    library_updated = django.dispatch.Signal(providing_args=["library_key"])
 
     _mapping = {
-        "course_published": course_published
+        "course_published": course_published,
+        "library_updated": library_updated
     }
 
     def __init__(self, modulestore_class):

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

@@ -466,16 +466,17 @@ class MongoBulkOpsMixin(BulkOperationsMixin):
         # ensure it starts clean
         bulk_ops_record.dirty = False
 
-    def _end_outermost_bulk_operation(self, bulk_ops_record, course_id, emit_signals=True):
+    def _end_outermost_bulk_operation(self, bulk_ops_record, structure_key, emit_signals=True):
         """
-        Restart updating the meta-data inheritance cache for the given course.
+        Restart updating the meta-data inheritance cache for the given course or library.
         Refresh the meta-data inheritance cache now since it was temporarily disabled.
         """
         if bulk_ops_record.dirty:
-            self.refresh_cached_metadata_inheritance_tree(course_id)
+            self.refresh_cached_metadata_inheritance_tree(structure_key)
 
             if emit_signals:
-                self.send_bulk_published_signal(bulk_ops_record, course_id)
+                self.send_bulk_published_signal(bulk_ops_record, structure_key)
+                self.send_bulk_library_updated_signal(bulk_ops_record, structure_key)
 
             bulk_ops_record.dirty = False  # brand spanking clean now
 

common/lib/xmodule/xmodule/modulestore/split_mongo/split.py

@@ -229,9 +229,9 @@ class SplitBulkWriteMixin(BulkOperationsMixin):
         # Ensure that any edits to the index don't pollute the initial_index
         bulk_write_record.index = copy.deepcopy(bulk_write_record.initial_index)
 
-    def _end_outermost_bulk_operation(self, bulk_write_record, course_key, emit_signals=True):
+    def _end_outermost_bulk_operation(self, bulk_write_record, structure_key, emit_signals=True):
         """
-        End the active bulk write operation on course_key.
+        End the active bulk write operation on structure_key (course or library key).
         """
 
         dirty = False
@@ -268,7 +268,8 @@ class SplitBulkWriteMixin(BulkOperationsMixin):
                 self.db_connection.update_course_index(bulk_write_record.index, from_index=bulk_write_record.initial_index)
 
         if dirty and emit_signals:
-            self.send_bulk_published_signal(bulk_write_record, course_key)
+            self.send_bulk_published_signal(bulk_write_record, structure_key)
+            self.send_bulk_library_updated_signal(bulk_write_record, structure_key)
 
     def get_course_index(self, course_key, ignore_case=False):
         """
@@ -1536,6 +1537,9 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase):
                     block_id=block_key.id,
                 )
 
+            if isinstance(course_key, LibraryLocator):
+                self._flag_library_updated_event(course_key)
+
             # reconstruct the new_item from the cache
             return self.get_item(item_loc)
 
@@ -1891,6 +1895,9 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase):
                 else:
                     course_key = CourseLocator(version_guid=new_id)
 
+                if isinstance(course_key, LibraryLocator):
+                    self._flag_library_updated_event(course_key)
+
                 # fetch and return the new item--fetching is unnecessary but a good qc step
                 new_locator = course_key.make_usage_key(block_key.type, block_key.id)
                 return self.get_item(new_locator, **kwargs)
@@ -2392,6 +2399,9 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase):
             else:
                 result = CourseLocator(version_guid=new_id)
 
+            if isinstance(usage_locator.course_key, LibraryLocator):
+                self._flag_library_updated_event(usage_locator.course_key)
+
             return result
 
     @contract(block_key=BlockKey, blocks='dict(BlockKey: BlockData)')

common/lib/xmodule/xmodule/tests/test_capa_module.py

@@ -1659,18 +1659,26 @@ class CapaModuleTest(unittest.TestCase):
 
 @ddt.ddt
 class CapaDescriptorTest(unittest.TestCase):
-    def _create_descriptor(self, xml):
+    def _create_descriptor(self, xml, name=None):
         """ Creates a CapaDescriptor to run test against """
         descriptor = CapaDescriptor(get_test_system(), scope_ids=1)
         descriptor.data = xml
+        if name:
+            descriptor.display_name = name
         return descriptor
 
     @ddt.data(*responsetypes.registry.registered_tags())
     def test_all_response_types(self, response_tag):
         """ Tests that every registered response tag is correctly returned """
         xml = "<problem><{response_tag}></{response_tag}></problem>".format(response_tag=response_tag)
-        descriptor = self._create_descriptor(xml)
+        name = "Some Capa Problem"
+        descriptor = self._create_descriptor(xml, name=name)
         self.assertEquals(descriptor.problem_types, {response_tag})
+        self.assertEquals(descriptor.index_dictionary(), {
+            'content_type': CapaDescriptor.INDEX_CONTENT_TYPE,
+            'display_name': name,
+            'problem_types': [response_tag]
+        })
 
     def test_response_types_ignores_non_response_tags(self):
         xml = textwrap.dedent("""
@@ -1687,8 +1695,14 @@ class CapaDescriptorTest(unittest.TestCase):
             </multiplechoiceresponse>
             </problem>
         """)
-        descriptor = self._create_descriptor(xml)
+        name = "Test Capa Problem"
+        descriptor = self._create_descriptor(xml, name=name)
         self.assertEquals(descriptor.problem_types, {"multiplechoiceresponse"})
+        self.assertEquals(descriptor.index_dictionary(), {
+            'content_type': CapaDescriptor.INDEX_CONTENT_TYPE,
+            'display_name': name,
+            'problem_types': ["multiplechoiceresponse"]
+        })
 
     def test_response_types_multiple_tags(self):
         xml = textwrap.dedent("""
@@ -1710,8 +1724,16 @@ class CapaDescriptorTest(unittest.TestCase):
                 </optionresponse>
             </problem>
         """)
-        descriptor = self._create_descriptor(xml)
+        name = "Other Test Capa Problem"
+        descriptor = self._create_descriptor(xml, name=name)
         self.assertEquals(descriptor.problem_types, {"multiplechoiceresponse", "optionresponse"})
+        self.assertEquals(
+            descriptor.index_dictionary(), {
+                'content_type': CapaDescriptor.INDEX_CONTENT_TYPE,
+                'display_name': name,
+                'problem_types': ["optionresponse", "multiplechoiceresponse"]
+            }
+        )
 
 
 class ComplexEncoderTest(unittest.TestCase):

common/lib/xmodule/xmodule/tests/test_library_content.py

@@ -18,6 +18,7 @@ from xmodule.modulestore.tests.utils import MixedSplitTestCase
 from xmodule.tests import get_test_system
 from xmodule.validation import StudioValidationMessage
 from xmodule.x_module import AUTHOR_VIEW
+from search.search_engine_base import SearchEngine
 
 dummy_render = lambda block, _: Fragment(block.data)  # pylint: disable=invalid-name
 
@@ -66,10 +67,17 @@ class LibraryContentTest(MixedSplitTestCase):
         module.xmodule_runtime = module_system
 
 
-class TestLibraryContentModule(LibraryContentTest):
+class LibraryContentModuleTestMixin(object):
     """
     Basic unit tests for LibraryContentModule
     """
+    problem_types = [
+        ["multiplechoiceresponse"], ["optionresponse"], ["optionresponse", "coderesponse"],
+        ["coderesponse", "optionresponse"]
+    ]
+
+    problem_type_lookup = {}
+
     def _get_capa_problem_type_xml(self, *args):
         """ Helper function to create empty CAPA problem definition """
         problem = "<problem>"
@@ -84,12 +92,10 @@ class TestLibraryContentModule(LibraryContentTest):
 
         Creates four blocks total.
         """
-        problem_types = [
-            ["multiplechoiceresponse"], ["optionresponse"], ["optionresponse", "coderesponse"],
-            ["coderesponse", "optionresponse"]
-        ]
-        for problem_type in problem_types:
-            self.make_block("problem", self.library, data=self._get_capa_problem_type_xml(*problem_type))
+        self.problem_type_lookup = {}
+        for problem_type in self.problem_types:
+            block = self.make_block("problem", self.library, data=self._get_capa_problem_type_xml(*problem_type))
+            self.problem_type_lookup[block.location] = problem_type
 
     def test_lib_content_block(self):
         """
@@ -236,6 +242,42 @@ class TestLibraryContentModule(LibraryContentTest):
         self.assertNotIn(LibraryContentDescriptor.display_name, non_editable_metadata_fields)
 
 
+@patch('xmodule.library_tools.SearchEngine.get_search_engine', Mock(return_value=None))
+class TestLibraryContentModuleNoSearchIndex(LibraryContentModuleTestMixin, LibraryContentTest):
+    """
+    Tests for library container when no search index is available.
+    Tests fallback low-level CAPA problem introspection
+    """
+    pass
+
+
+search_index_mock = Mock(spec=SearchEngine)  # pylint: disable=invalid-name
+
+
+@patch('xmodule.library_tools.SearchEngine.get_search_engine', Mock(return_value=search_index_mock))
+class TestLibraryContentModuleWithSearchIndex(LibraryContentModuleTestMixin, LibraryContentTest):
+    """
+    Tests for library container with mocked search engine response.
+    """
+    def _get_search_response(self, field_dictionary=None):
+        """ Mocks search response as returned by search engine """
+        target_type = field_dictionary.get('problem_types')
+        matched_block_locations = [
+            key for key, problem_types in
+            self.problem_type_lookup.items() if target_type in problem_types
+        ]
+        return {
+            'results': [
+                {'data': {'id': str(location)}} for location in matched_block_locations
+            ]
+        }
+
+    def setUp(self):
+        """ Sets up search engine mock """
+        super(TestLibraryContentModuleWithSearchIndex, self).setUp()
+        search_index_mock.search = Mock(side_effect=self._get_search_response)
+
+
 @patch(
     'xmodule.modulestore.split_mongo.caching_descriptor_system.CachingDescriptorSystem.render', VanillaRuntime.render
 )

common/test/acceptance/tests/helpers.py

@@ -417,3 +417,17 @@ def create_user_partition_json(partition_id, name, description, groups, scheme="
     return UserPartition(
         partition_id, name, description, groups, MockUserPartitionScheme(scheme)
     ).to_json()
+
+
+class TestWithSearchIndexMixin(object):
+    """ Mixin encapsulating search index creation """
+    TEST_INDEX_FILENAME = "test_root/index_file.dat"
+
+    def _create_search_index(self):
+        """ Creates search index backing file """
+        with open(self.TEST_INDEX_FILENAME, "w+") as index_file:
+            json.dump({}, index_file)
+
+    def _cleanup_index_file(self):
+        """ Removes search index backing file """
+        os.remove(self.TEST_INDEX_FILENAME)

common/test/acceptance/tests/lms/test_library.py

@@ -6,7 +6,7 @@ import ddt
 import textwrap
 
 from nose.plugins.attrib import attr
-from ..helpers import UniqueCourseTest
+from ..helpers import UniqueCourseTest, TestWithSearchIndexMixin
 from ...pages.studio.auto_auth import AutoAuthPage
 from ...pages.studio.overview import CourseOutlinePage
 from ...pages.studio.library import StudioLibraryContentEditor, StudioLibraryContainerXBlockWrapper
@@ -196,10 +196,19 @@ class LibraryContentTest(LibraryContentTestBase):
 
 @ddt.ddt
 @attr('shard_3')
-class StudioLibraryContainerCapaFilterTest(LibraryContentTestBase):
+class StudioLibraryContainerCapaFilterTest(LibraryContentTestBase, TestWithSearchIndexMixin):
     """
     Test Library Content block in LMS
     """
+    def setUp(self):
+        """ SetUp method """
+        self._create_search_index()
+        super(StudioLibraryContainerCapaFilterTest, self).setUp()
+
+    def tearDown(self):
+        self._cleanup_index_file()
+        super(StudioLibraryContainerCapaFilterTest, self).tearDown()
+
     def _get_problem_choice_group_text(self, name, items):
         """ Generates Choice Group CAPA problem XML """
         items_text = "\n".join([
@@ -231,7 +240,7 @@ class StudioLibraryContainerCapaFilterTest(LibraryContentTestBase):
         """
         Populates library fixture with XBlock Fixtures
         """
-        library_fixture.add_children(
+        items = (
             XBlockFixtureDesc(
                 "problem", "Problem Choice Group 1",
                 data=self._get_problem_choice_group_text("Problem Choice Group 1 Text", [("1", False), ('2', True)])
@@ -249,6 +258,7 @@ class StudioLibraryContainerCapaFilterTest(LibraryContentTestBase):
                 data=self._get_problem_select_text("Problem Select 2 Text", ["Option 3", "Option 4"], "Option 4")
             ),
         )
+        library_fixture.add_children(*items)
 
     @property
     def _problem_headers(self):

common/test/acceptance/tests/studio/test_studio_library_container.py

@@ -7,7 +7,7 @@ import textwrap
 
 from .base_studio_test import StudioLibraryTest
 from ...fixtures.course import CourseFixture
-from ..helpers import UniqueCourseTest
+from ..helpers import UniqueCourseTest, TestWithSearchIndexMixin
 from ...pages.studio.library import StudioLibraryContentEditor, StudioLibraryContainerXBlockWrapper
 from ...pages.studio.overview import CourseOutlinePage
 from ...fixtures.course import XBlockFixtureDesc
@@ -18,7 +18,7 @@ UNIT_NAME = 'Test Unit'
 
 
 @ddt.ddt
-class StudioLibraryContainerTest(StudioLibraryTest, UniqueCourseTest):
+class StudioLibraryContainerTest(StudioLibraryTest, UniqueCourseTest, TestWithSearchIndexMixin):
     """
     Test Library Content block in LMS
     """
@@ -26,6 +26,7 @@ class StudioLibraryContainerTest(StudioLibraryTest, UniqueCourseTest):
         """
         Install library with some content and a course using fixtures
         """
+        self._create_search_index()
         super(StudioLibraryContainerTest, self).setUp()
         # Also create a course:
         self.course_fixture = CourseFixture(
@@ -42,6 +43,11 @@ class StudioLibraryContainerTest(StudioLibraryTest, UniqueCourseTest):
         subsection = self.outline.section(SECTION_NAME).subsection(SUBSECTION_NAME)
         self.unit_page = subsection.expand_subsection().unit(UNIT_NAME).go_to()
 
+    def tearDown(self):
+        """ Tear down method: remove search index backing file """
+        self._cleanup_index_file()
+        super(StudioLibraryContainerTest, self).tearDown()
+
     def populate_library_fixture(self, library_fixture):
         """
         Populate the children of the test course fixture.