Skip to content

Commit

Permalink
fix: skip block indexing if it raises an error while loading (#35139)
Browse files Browse the repository at this point in the history 
* fix: skip block indexing if it raises an error while loading
* test: add tests to the issue
  • Loading branch information
@rpenido
rpenido authored Jul 25, 2024
1 parent 88c3507 commit 238dca7
Show file tree
Hide file tree
Showing 2 changed files with 80 additions and 23 deletions.
21 changes: 12 additions & 9 deletions openedx/core/djangoapps/content/search/api.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -345,23 +345,26 @@ def rebuild_index(status_cb: Callable[[str], None] | None = None) -> None:
status_cb("Indexing libraries...")
for lib_key in lib_keys:
status_cb(f"{num_contexts_done + 1}/{num_contexts}. Now indexing library {lib_key}")
try:
docs = []
for component in lib_api.get_library_components(lib_key):
docs = []
for component in lib_api.get_library_components(lib_key):
try:
metadata = lib_api.LibraryXBlockMetadata.from_component(lib_key, component)
doc = {}
doc.update(searchable_doc_for_library_block(metadata))
doc.update(searchable_doc_tags(metadata.usage_key))
docs.append(doc)

except Exception as err: # pylint: disable=broad-except
status_cb(f"Error indexing library component {component}: {err}")
finally:
num_blocks_done += 1
if docs:
if docs:
try:
# Add all the docs in this library at once (usually faster than adding one at a time):
_wait_for_meili_task(client.index(temp_index_name).add_documents(docs))
except Exception as err: # pylint: disable=broad-except
status_cb(f"Error indexing library {lib_key}: {err}")
finally:
num_contexts_done += 1
except (TypeError, KeyError, MeilisearchError) as err:
status_cb(f"Error indexing library {lib_key}: {err}")

num_contexts_done += 1

############## Courses ##############
status_cb("Indexing courses...")
Expand Down
82 changes: 68 additions & 14 deletions openedx/core/djangoapps/content/search/tests/test_api.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -119,8 +119,8 @@ def setUp(self):
)
lib_access, _ = SearchAccess.objects.get_or_create(context_key=self.library.key)
# Populate it with a problem:
self.problem = library_api.create_library_block(self.library.key, "problem", "p1")
self.doc_problem = {
self.problem1 = library_api.create_library_block(self.library.key, "problem", "p1")
self.doc_problem1 = {
"id": "lborg1libproblemp1-a698218e",
"usage_key": "lb:org1:lib:problem:p1",
"block_id": "p1",
Expand All @@ -133,6 +133,20 @@ def setUp(self):
"type": "library_block",
"access_id": lib_access.id,
}
self.problem2 = library_api.create_library_block(self.library.key, "problem", "p2")
self.doc_problem2 = {
"id": "lborg1libproblemp2-b2f65e29",
"usage_key": "lb:org1:lib:problem:p2",
"block_id": "p2",
"display_name": "Blank Problem",
"block_type": "problem",
"context_key": "lib:org1:lib",
"org": "org1",
"breadcrumbs": [{"display_name": "Library"}],
"content": {"problem_types": [], "capa_content": " "},
"type": "library_block",
"access_id": lib_access.id,
}

# Create a couple of taxonomies with tags
self.taxonomyA = tagging_api.create_taxonomy(name="A", export_id="A")
Expand All @@ -159,14 +173,52 @@ def test_reindex_meilisearch(self, mock_meilisearch):
doc_sequential["tags"] = {}
doc_vertical = copy.deepcopy(self.doc_vertical)
doc_vertical["tags"] = {}
doc_problem = copy.deepcopy(self.doc_problem)
doc_problem["tags"] = {}
doc_problem1 = copy.deepcopy(self.doc_problem1)
doc_problem1["tags"] = {}
doc_problem2 = copy.deepcopy(self.doc_problem2)
doc_problem2["tags"] = {}

api.rebuild_index()
mock_meilisearch.return_value.index.return_value.add_documents.assert_has_calls(
[
call([doc_sequential, doc_vertical]),
call([doc_problem]),
call([doc_problem1, doc_problem2]),
],
any_order=True,
)

@override_settings(MEILISEARCH_ENABLED=True)
def test_reindex_meilisearch_library_block_error(self, mock_meilisearch):

# Add tags field to doc, since reindex calls includes tags
doc_sequential = copy.deepcopy(self.doc_sequential)
doc_sequential["tags"] = {}
doc_vertical = copy.deepcopy(self.doc_vertical)
doc_vertical["tags"] = {}
doc_problem2 = copy.deepcopy(self.doc_problem2)
doc_problem2["tags"] = {}

orig_from_component = library_api.LibraryXBlockMetadata.from_component

def mocked_from_component(lib_key, component):
# Simulate an error when processing problem 1
if component.key == 'xblock.v1:problem:p1':
raise Exception('Error')

return orig_from_component(lib_key, component)

with patch.object(
library_api.LibraryXBlockMetadata,
"from_component",
new=mocked_from_component,
):
api.rebuild_index()

mock_meilisearch.return_value.index.return_value.add_documents.assert_has_calls(
[
call([doc_sequential, doc_vertical]),
# Problem 1 should not be indexed
call([doc_problem2]),
],
any_order=True,
)
Expand Down Expand Up @@ -245,9 +297,9 @@ def test_index_library_block_metadata(self, mock_meilisearch):
"""
Test indexing a Library Block.
"""
api.upsert_library_block_index_doc(self.problem.usage_key)
api.upsert_library_block_index_doc(self.problem1.usage_key)

mock_meilisearch.return_value.index.return_value.update_documents.assert_called_once_with([self.doc_problem])
mock_meilisearch.return_value.index.return_value.update_documents.assert_called_once_with([self.doc_problem1])

@override_settings(MEILISEARCH_ENABLED=True)
def test_index_library_block_tags(self, mock_meilisearch):
Expand All @@ -256,19 +308,19 @@ def test_index_library_block_tags(self, mock_meilisearch):
"""

# Tag XBlock (these internally call `upsert_block_tags_index_docs`)
tagging_api.tag_object(str(self.problem.usage_key), self.taxonomyA, ["one", "two"])
tagging_api.tag_object(str(self.problem.usage_key), self.taxonomyB, ["three", "four"])
tagging_api.tag_object(str(self.problem1.usage_key), self.taxonomyA, ["one", "two"])
tagging_api.tag_object(str(self.problem1.usage_key), self.taxonomyB, ["three", "four"])

# Build expected docs with tags at each stage
doc_problem_with_tags1 = {
"id": self.doc_problem["id"],
"id": self.doc_problem1["id"],
"tags": {
'taxonomy': ['A'],
'level0': ['A > one', 'A > two']
}
}
doc_problem_with_tags2 = {
"id": self.doc_problem["id"],
"id": self.doc_problem1["id"],
"tags": {
'taxonomy': ['A', 'B'],
'level0': ['A > one', 'A > two', 'B > four', 'B > three']
Expand All @@ -288,10 +340,10 @@ def test_delete_index_library_block(self, mock_meilisearch):
"""
Test deleting a Library Block doc from the index.
"""
api.delete_index_doc(self.problem.usage_key)
api.delete_index_doc(self.problem1.usage_key)

mock_meilisearch.return_value.index.return_value.delete_document.assert_called_once_with(
self.doc_problem['id']
self.doc_problem1['id']
)

@override_settings(MEILISEARCH_ENABLED=True)
Expand All @@ -301,4 +353,6 @@ def test_index_content_library_metadata(self, mock_meilisearch):
"""
api.upsert_content_library_index_docs(self.library.key)

mock_meilisearch.return_value.index.return_value.update_documents.assert_called_once_with([self.doc_problem])
mock_meilisearch.return_value.index.return_value.update_documents.assert_called_once_with(
[self.doc_problem1, self.doc_problem2]
)

0 comments on commit 238dca7

Please sign in to comment.