feat: Rebuild full search document for deletes

Author: mgwozdz-unicon

openedx/core/djangoapps/content/search/api.py

@@ -22,7 +22,12 @@
 from meilisearch.models.task import TaskInfo
 from opaque_keys import OpaqueKey
 from opaque_keys.edx.keys import CourseKey, UsageKey
-from opaque_keys.edx.locator import LibraryCollectionLocator, LibraryContainerLocator, LibraryLocatorV2
+from opaque_keys.edx.locator import (
+    LibraryCollectionLocator,
+    LibraryContainerLocator,
+    LibraryLocatorV2,
+    LibraryUsageLocatorV2,
+)
 from openedx_content import api as content_api
 from openedx_content import models_api as content_models
 from rest_framework.request import Request
@@ -333,6 +338,28 @@ def _update_index_docs(docs) -> None:
     _wait_for_meili_tasks(tasks)
 
 
+def _replace_index_docs(docs) -> None:
+    """
+    Helper function that fully replaces the documents in the search index.
+
+    We use this when nested fields need to be removed from indexed documents, because
+    Meilisearch partial updates do not reliably clear nested facet values.
+    """
+    if not docs:
+        return
+
+    client = _get_meilisearch_client()
+    current_rebuild_index_name = _get_running_rebuild_index_name()
+
+    tasks = []
+    if current_rebuild_index_name:
+        # Keep the temporary rebuild index in sync with the live index.
+        tasks.append(client.index(current_rebuild_index_name).add_documents(docs))
+    tasks.append(client.index(STUDIO_INDEX_NAME).add_documents(docs))
+
+    _wait_for_meili_tasks(tasks)
+
+
 def only_if_meilisearch_enabled(f):
     """
     Only call `f` if meilisearch is enabled
@@ -1030,13 +1057,65 @@ def upsert_content_library_index_docs(library_key: LibraryLocatorV2, full_index:
     _update_index_docs(docs)
 
 
+def _build_full_content_object_index_doc(key: OpaqueKey) -> tuple[dict | None, bool]:
+    """
+    Build the current full search document for an object whose associations changed.
+
+    Returns a tuple of (doc, should_delete_index_doc). The second value is True when the
+    object no longer exists in a form that should remain indexed.
+    """
+    if isinstance(key, LibraryCollectionLocator):
+        doc = searchable_doc_for_collection(key)
+        if doc.get("_disabled") or not doc.get(Fields.type):
+            return None, True
+    elif isinstance(key, LibraryContainerLocator):
+        doc = searchable_doc_for_container(key)
+        if not doc.get(Fields.display_name):
+            return None, True
+        doc.update(searchable_doc_collections(key))
+        match key.container_type:
+            case content_models.Unit.type_code:
+                doc.update(searchable_doc_containers(key, "subsections"))
+            case content_models.Subsection.type_code:
+                doc.update(searchable_doc_containers(key, "sections"))
+    elif isinstance(key, LibraryUsageLocatorV2):
+        try:
+            library_block = lib_api.get_component_from_usage_key(key)
+        except lib_api.ContentLibraryBlockNotFound:
+            return None, True
+        library_block_metadata = lib_api.LibraryXBlockMetadata.from_component(key.context_key, library_block)
+        doc = searchable_doc_for_library_block(library_block_metadata)
+        doc.update(searchable_doc_collections(key))
+        doc.update(searchable_doc_containers(key, "units"))
+    elif isinstance(key, UsageKey):
+        xblock = modulestore().get_item(key)
+        if xblock.scope_ids.block_type in EXCLUDED_XBLOCK_TYPES:
+            return None, False
+        doc = searchable_doc_for_course_block(xblock)
+    else:  # pragma: no cover
+        raise TypeError(f"Unsupported content object key type: {type(key)!r}")
+
+    doc.update(searchable_doc_tags(key))
+    return doc, False
+
+
 def upsert_content_object_tags_index_doc(key: OpaqueKey):
     """
     Updates the tags data in document for the given Course/Library item
     """
-    doc = {Fields.id: meili_id_from_opaque_key(key)}
-    doc.update(searchable_doc_tags(key))
-    _update_index_docs([doc])
+    try:
+        doc, should_delete_doc = _build_full_content_object_index_doc(key)
+    except ItemNotFoundError:
+        should_delete_doc = True
+        doc = None
+
+    if should_delete_doc:
+        delete_index_doc(key)
+        return
+    if doc is None:
+        return
+
+    _replace_index_docs([doc])
 
 
 def upsert_item_collections_index_docs(opaque_key: OpaqueKey):

openedx/core/djangoapps/content/search/tests/test_api.py

@@ -608,22 +608,22 @@ def test_index_xblock_tags(self, mock_meilisearch) -> None:
 
         # Build expected docs with tags at each stage
         doc_sequential_with_tags1 = {
-            "id": self.doc_sequential["id"],
+            **copy.deepcopy(self.doc_sequential),
             "tags": {
                 'taxonomy': ['A'],
                 'level0': ['A > one', 'A > two']
-            }
+            },
         }
         doc_sequential_with_tags2 = {
-            "id": self.doc_sequential["id"],
+            **copy.deepcopy(self.doc_sequential),
             "tags": {
                 'taxonomy': ['A', 'B'],
                 'level0': ['A > one', 'A > two', 'B > four', 'B > three']
-            }
+            },
         }
 
-        assert mock_meilisearch.return_value.index.return_value.update_documents.call_count == 2
-        mock_meilisearch.return_value.index.return_value.update_documents.assert_has_calls(
+        assert mock_meilisearch.return_value.index.return_value.add_documents.call_count == 2
+        mock_meilisearch.return_value.index.return_value.add_documents.assert_has_calls(
             [
                 call([doc_sequential_with_tags1]),
                 call([doc_sequential_with_tags2]),
@@ -667,22 +667,22 @@ def test_index_library_block_tags(self, mock_meilisearch) -> None:
 
         # Build expected docs with tags at each stage
         doc_problem_with_tags1 = {
-            "id": self.doc_problem1["id"],
+            **copy.deepcopy(self.doc_problem1),
             "tags": {
                 'taxonomy': ['A'],
                 'level0': ['A > one', 'A > two']
-            }
+            },
         }
         doc_problem_with_tags2 = {
-            "id": self.doc_problem1["id"],
+            **copy.deepcopy(self.doc_problem1),
             "tags": {
                 'taxonomy': ['A', 'B'],
                 'level0': ['A > one', 'A > two', 'B > four', 'B > three']
-            }
+            },
         }
 
-        assert mock_meilisearch.return_value.index.return_value.update_documents.call_count == 2
-        mock_meilisearch.return_value.index.return_value.update_documents.assert_has_calls(
+        assert mock_meilisearch.return_value.index.return_value.add_documents.call_count == 2
+        mock_meilisearch.return_value.index.return_value.add_documents.assert_has_calls(
             [
                 call([doc_problem_with_tags1]),
                 call([doc_problem_with_tags2]),
@@ -873,22 +873,22 @@ def test_index_tags_in_collections(self, mock_meilisearch) -> None:
 
         # Build expected docs with tags at each stage
         doc_collection_with_tags1 = {
-            "id": "lib-collectionorg1libmycol-5b647617",
+            **copy.deepcopy(self.collection_dict),
             "tags": {
                 'taxonomy': ['A'],
                 'level0': ['A > one', 'A > two']
-            }
+            },
         }
         doc_collection_with_tags2 = {
-            "id": "lib-collectionorg1libmycol-5b647617",
+            **copy.deepcopy(self.collection_dict),
             "tags": {
                 'taxonomy': ['A', 'B'],
                 'level0': ['A > one', 'A > two', 'B > four', 'B > three']
-            }
+            },
         }
 
-        assert mock_meilisearch.return_value.index.return_value.update_documents.call_count == 2
-        mock_meilisearch.return_value.index.return_value.update_documents.assert_has_calls(
+        assert mock_meilisearch.return_value.index.return_value.add_documents.call_count == 2
+        mock_meilisearch.return_value.index.return_value.add_documents.assert_has_calls(
             [
                 call([doc_collection_with_tags1]),
                 call([doc_collection_with_tags2]),
@@ -1144,30 +1144,57 @@ def test_index_tags_in_containers(self, container_type, container_id, mock_meili
         tagging_api.tag_object(container_key, self.taxonomyB, ["three", "four"])
 
         # Build expected docs with tags at each stage
+        container_doc = copy.deepcopy(getattr(self, f"{container_type}_dict"))
+        if container_type == "unit":
+            container_doc["subsections"] = {
+                "display_name": [self.subsection_dict["display_name"]],
+                "key": [self.subsection_key],
+            }
+        elif container_type == "subsection":
+            container_doc["sections"] = {
+                "display_name": [self.section_dict["display_name"]],
+                "key": [self.section_key],
+            }
+
         doc_unit_with_tags1 = {
+            **container_doc,
             "id": container_id,
             "tags": {
                 'taxonomy': ['A'],
                 'level0': ['A > one', 'A > two']
-            }
+            },
         }
         doc_unit_with_tags2 = {
+            **copy.deepcopy(container_doc),
             "id": container_id,
             "tags": {
                 'taxonomy': ['A', 'B'],
                 'level0': ['A > one', 'A > two', 'B > four', 'B > three']
-            }
+            },
         }
 
-        assert mock_meilisearch.return_value.index.return_value.update_documents.call_count == 2
-        mock_meilisearch.return_value.index.return_value.update_documents.assert_has_calls(
+        assert mock_meilisearch.return_value.index.return_value.add_documents.call_count == 2
+        mock_meilisearch.return_value.index.return_value.add_documents.assert_has_calls(
             [
                 call([doc_unit_with_tags1]),
                 call([doc_unit_with_tags2]),
             ],
             any_order=True,
         )
 
+    @override_settings(MEILISEARCH_ENABLED=True)
+    def test_replace_full_doc_when_container_tags_are_cleared(self, mock_meilisearch) -> None:
+        expected_doc = copy.deepcopy(self.unit_dict)
+        expected_doc["subsections"] = {
+            "display_name": [self.subsection_dict["display_name"]],
+            "key": [self.subsection_key],
+        }
+        expected_doc["tags"] = {}
+
+        api.upsert_content_object_tags_index_doc(self.unit.container_key)
+
+        mock_meilisearch.return_value.index.return_value.add_documents.assert_called_once_with([expected_doc])
+
     @override_settings(MEILISEARCH_ENABLED=True)
     def test_block_in_units(self, mock_meilisearch) -> None:
         """Test that search index is updated correctly when we add a problem block to a unit"""