Skip to content

Commit 8aaae46

Browse files
authored
fix: index and entity link sync issues on parent block deletion (#37541)
Meilisearch index documents were not synced properly when any block with children blocks like units, subsections, sections etc. were being deleted as the `XBLOCK_DELETED` is only triggered for the deleted block. This PR fixes it by deleting all index documents that contain the deleted block in its `breadcrumbs` field as only blocks that are children of this block will have it its breadcrumbs field. Similarly, the entity links that store links between course and library blocks was not synced properly due to children `ContainerLinks` not being deleted.
1 parent 121fee3 commit 8aaae46

7 files changed

Lines changed: 80 additions & 5 deletions

File tree

cms/djangoapps/contentstore/signals/handlers.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -221,6 +221,7 @@ def handle_item_deleted(**kwargs):
221221
id_list.add(block.location)
222222

223223
ComponentLink.objects.filter(downstream_usage_key__in=id_list).delete()
224+
ContainerLink.objects.filter(downstream_usage_key__in=id_list).delete()
224225

225226

226227
@receiver(GRADING_POLICY_CHANGED)

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

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -638,7 +638,7 @@ def add_with_children(block):
638638
_update_index_docs(docs)
639639

640640

641-
def delete_index_doc(key: OpaqueKey) -> None:
641+
def delete_index_doc(key: OpaqueKey, *, delete_children: bool = False) -> None:
642642
"""
643643
Deletes the document for the given XBlock from the search index
644644
@@ -647,6 +647,37 @@ def delete_index_doc(key: OpaqueKey) -> None:
647647
"""
648648
doc = searchable_doc_for_key(key)
649649
_delete_index_doc(doc[Fields.id])
650+
if delete_children:
651+
_delete_documents(f'{Fields.breadcrumbs}.{Fields.usage_key} = "{key}"')
652+
653+
654+
def delete_docs_with_context_key(key: OpaqueKey) -> None:
655+
"""
656+
Delete all docs for given context key
657+
"""
658+
_delete_documents(f'{Fields.context_key} = "{key}"')
659+
660+
661+
def _delete_documents(filter_query: str) -> None:
662+
"""
663+
Deletes all documents from the search index that match the given filter
664+
665+
Args:
666+
filter (str): The query to use when filtering documents
667+
"""
668+
if not filter_query:
669+
return
670+
671+
client = _get_meilisearch_client()
672+
current_rebuild_index_name = _get_running_rebuild_index_name()
673+
674+
tasks = []
675+
if current_rebuild_index_name:
676+
# If there is a rebuild in progress, the document will also be removed from the new index.
677+
tasks.append(client.index(current_rebuild_index_name).delete_documents(filter=filter_query))
678+
tasks.append(client.index(STUDIO_INDEX_NAME).delete_documents(filter=filter_query))
679+
680+
_wait_for_meili_tasks(tasks)
650681

651682

652683
def _delete_index_doc(doc_id) -> None:

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,8 +48,9 @@ class Fields:
4848
org = "org"
4949
access_id = "access_id" # .models.SearchAccess.id
5050
# breadcrumbs: an array of {"display_name": "..."} entries. First one is the name of the course/library itself.
51-
# After that is the name of any parent Section/Subsection/Unit/etc.
51+
# After that is the name of any parent Section/Subsection/Unit/etc and its usage_key.
5252
# It's a list of dictionaries because for now we just include the name of each but in future we may add their IDs.
53+
# Example: [{"display_name": "My course"}, {"display_name": "Section1", "usage_key": "..."}]}
5354
breadcrumbs = "breadcrumbs"
5455
# tags (dictionary)
5556
# See https://blog.meilisearch.com/nested-hierarchical-facets-guide/

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

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@
4343
from openedx.core.djangoapps.content.course_overviews.models import CourseOverview
4444
from openedx.core.djangoapps.content.search.models import SearchAccess
4545
from openedx.core.djangoapps.content_libraries import api as lib_api
46+
from xmodule.modulestore.django import SignalHandler
4647

4748
from .api import (
4849
only_if_meilisearch_enabled,
@@ -51,6 +52,7 @@
5152
upsert_item_containers_index_docs,
5253
)
5354
from .tasks import (
55+
delete_course_index_docs,
5456
delete_library_block_index_doc,
5557
delete_library_container_index_doc,
5658
delete_xblock_index_doc,
@@ -344,3 +346,12 @@ def handle_reindex_on_signal(**kwargs):
344346
return
345347

346348
upsert_course_blocks_docs.delay(str(course_data.course_key))
349+
350+
351+
@receiver(SignalHandler.course_deleted)
352+
def listen_for_course_delete(sender, course_key, **kwargs): # pylint: disable=unused-argument
353+
"""
354+
Catches the signal that a course has been deleted
355+
and removes its entry from the Course About Search index.
356+
"""
357+
delete_course_index_docs.delay(str(course_key))

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
Fields.last_published,
2727
Fields.content + "." + Fields.problem_types,
2828
Fields.publish_status,
29+
Fields.breadcrumbs + "." + Fields.usage_key,
2930
]
3031

3132
# Mark which attributes are used for keyword search, in order of importance:

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

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,8 @@ def delete_xblock_index_doc(usage_key_str: str) -> None:
5959

6060
log.info("Updating content index document for XBlock with id: %s", usage_key)
6161

62-
api.delete_index_doc(usage_key)
62+
# Delete children index data for course blocks.
63+
api.delete_index_doc(usage_key, delete_children=True)
6364

6465

6566
@shared_task(base=LoggedTask, autoretry_for=(MeilisearchError, ConnectionError))
@@ -168,3 +169,17 @@ def delete_library_container_index_doc(container_key_str: str) -> None:
168169
log.info("Deleting content index document for library block with id: %s", container_key)
169170

170171
api.delete_index_doc(container_key)
172+
173+
174+
@shared_task(base=LoggedTask, autoretry_for=(MeilisearchError, ConnectionError))
175+
@set_code_owner_attribute
176+
def delete_course_index_docs(course_key_str: str) -> None:
177+
"""
178+
Celery task to delete the content index documents for a Course
179+
"""
180+
course_key = CourseKey.from_string(course_key_str)
181+
182+
log.info("Deleting all index documents related to course_key: %s", course_key)
183+
184+
# Delete children index data for course blocks.
185+
api.delete_docs_with_context_key(course_key)

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

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -618,14 +618,18 @@ def test_index_xblock_tags(self, mock_meilisearch) -> None:
618618
@override_settings(MEILISEARCH_ENABLED=True)
619619
def test_delete_index_xblock(self, mock_meilisearch) -> None:
620620
"""
621-
Test deleting an XBlock doc from the index.
621+
Test deleting an XBlock doc and its children docs from the index.
622622
"""
623-
api.delete_index_doc(self.sequential.usage_key)
623+
api.delete_index_doc(self.sequential.usage_key, delete_children=True)
624624

625625
mock_meilisearch.return_value.index.return_value.delete_document.assert_called_once_with(
626626
self.doc_sequential['id']
627627
)
628628

629+
mock_meilisearch.return_value.index.return_value.delete_documents.assert_called_once_with(
630+
filter=f'breadcrumbs.usage_key = "{self.sequential.usage_key}"'
631+
)
632+
629633
@override_settings(MEILISEARCH_ENABLED=True)
630634
def test_index_library_block_metadata(self, mock_meilisearch) -> None:
631635
"""
@@ -821,6 +825,17 @@ def test_delete_index_library_block(self, mock_meilisearch) -> None:
821825
self.doc_problem1['id']
822826
)
823827

828+
@override_settings(MEILISEARCH_ENABLED=True)
829+
def test_delete_docs_with_context_key(self, mock_meilisearch) -> None:
830+
"""
831+
Test deleting a all Block docs from the index using context_key.
832+
"""
833+
api.delete_docs_with_context_key(self.course.id)
834+
835+
mock_meilisearch.return_value.index.return_value.delete_documents.assert_called_once_with(
836+
filter=f'context_key = "{self.course.id}"'
837+
)
838+
824839
@override_settings(MEILISEARCH_ENABLED=True)
825840
def test_index_content_library_metadata(self, mock_meilisearch) -> None:
826841
"""

0 commit comments

Comments
 (0)