Moved library key normalization into library_tools module, fixed library update task test

cms/djangoapps/contentstore/courseware_index.py

@@ -8,6 +8,7 @@ from django.conf import settings
 from django.utils.translation import ugettext as _
 from eventtracking import tracker
 from xmodule.modulestore import ModuleStoreEnum
+from xmodule.library_tools import normalize_key_for_search
 from search.search_engine_base import SearchEngine
 
 # REINDEX_AGE is the default amount of time that we look back for changes
@@ -50,7 +51,7 @@ class SearchIndexerBase(object):
         return settings.FEATURES.get(cls.ENABLE_INDEXING_KEY, False)
 
     @classmethod
-    def _normalize_structure_key(cls, structure_key):
+    def normalize_structure_key(cls, structure_key):
         """ Normalizes structure key for use in indexing """
         raise NotImplementedError("Should be overridden in child classes")
 
@@ -107,7 +108,7 @@ class SearchIndexerBase(object):
         if not searcher:
             return
 
-        structure_key = cls._normalize_structure_key(structure_key)
+        structure_key = cls.normalize_structure_key(structure_key)
         location_info = cls._get_location_info(structure_key)
 
         # Wrap counter in dictionary - otherwise we seem to lose scope inside the embedded function `index_item`
@@ -235,7 +236,7 @@ class CoursewareSearchIndexer(SearchIndexerBase):
     }
 
     @classmethod
-    def _normalize_structure_key(cls, structure_key):
+    def normalize_structure_key(cls, structure_key):
         """ Normalizes structure key for use in indexing """
         return structure_key
 
@@ -271,9 +272,9 @@ class LibrarySearchIndexer(SearchIndexerBase):
     }
 
     @classmethod
-    def _normalize_structure_key(cls, structure_key):
+    def normalize_structure_key(cls, structure_key):
         """ Normalizes structure key for use in indexing """
-        return structure_key.replace(version_guid=None, branch=None)
+        return normalize_key_for_search(structure_key)
 
     @classmethod
     def _fetch_top_level(cls, modulestore, structure_key):

cms/djangoapps/contentstore/tasks.py

@@ -82,17 +82,21 @@ 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(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))
@@ -105,12 +109,7 @@ def update_library_index(library_id, triggered_time_isoformat):
     """ Updates course search index. """
     try:
         library_key = CourseKey.from_string(library_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)
-        LibrarySearchIndexer.index(modulestore(), library_key, triggered_at=triggered_time)
+        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))

cms/djangoapps/contentstore/tests/test_courseware_index.py

@@ -10,6 +10,7 @@ from pytz import UTC
 from uuid import uuid4
 from unittest import skip
 
+from xmodule.library_tools import normalize_key_for_search
 from xmodule.modulestore import ModuleStoreEnum
 from xmodule.modulestore.django import SignalHandler
 from xmodule.modulestore.edit_info import EditInfoMixin
@@ -29,7 +30,6 @@ from contentstore.courseware_index import CoursewareSearchIndexer, LibrarySearch
 from contentstore.signals import listen_for_course_publish, listen_for_library_update
 
 
-
 COURSE_CHILD_STRUCTURE = {
     "course": "chapter",
     "chapter": "sequential",
@@ -531,7 +531,7 @@ class TestLargeCourseDeletions(MixedWithOptionsTestCase):
 class TestTaskExecution(ModuleStoreTestCase):
     """
     Set of tests to ensure that the task code will do the right thing when
-    executed directly. The test course gets created without the listener
+    executed directly. The test course and library gets created without the listeners
     being present, which allows us to ensure that when the listener is
     executed, it is done as expected.
     """
@@ -593,7 +593,7 @@ class TestTaskExecution(ModuleStoreTestCase):
         response = searcher.search(field_dictionary={"course": unicode(self.course.id)})
         self.assertEqual(response["total"], 0)
 
-        #update_search_index(unicode(self.course.id), datetime.now(UTC).isoformat())
+        # update_search_index(unicode(self.course.id), datetime.now(UTC).isoformat())
         listen_for_course_publish(self, self.course.id)
 
         # Note that this test will only succeed if celery is working in inline mode
@@ -602,16 +602,16 @@ class TestTaskExecution(ModuleStoreTestCase):
 
     def test_task_library_update(self):
         """ Making sure that the receiver correctly fires off the task when invoked by signal """
-        searcher = SearchEngine.get_search_engine(CoursewareSearchIndexer.INDEX_NAME)
-        library_search_key = unicode(self.library.location.library_key.replace(version_guid=None, branch=None))
+        searcher = SearchEngine.get_search_engine(LibrarySearchIndexer.INDEX_NAME)
+        library_search_key = unicode(normalize_key_for_search(self.library.location.library_key))
         response = searcher.search(field_dictionary={"library": library_search_key})
         self.assertEqual(response["total"], 0)
 
-        #update_search_index(unicode(self.course.id), datetime.now(UTC).isoformat())
-        listen_for_library_update(self, self.library.location)
+        # update_search_index(unicode(self.library.location.library_key), datetime.now(UTC).isoformat())
+        listen_for_library_update(self, self.library.location.library_key)
 
         # Note that this test will only succeed if celery is working in inline mode
-        response = response = searcher.search(field_dictionary={"library": library_search_key})
+        response = searcher.search(field_dictionary={"library": library_search_key})
         self.assertEqual(response["total"], 2)
 
 

common/lib/xmodule/xmodule/library_tools.py

@@ -10,6 +10,11 @@ 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
@@ -92,6 +97,7 @@ class LibraryToolsService(object):
         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
             }