From c5f557832648e18c4f73309dedd3f1cb4e6885bd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=B4mulo=20Penido?= Date: Tue, 26 Mar 2024 11:10:20 -0300 Subject: [PATCH] test: test handlers --- openedx/core/djangoapps/content/search/api.py | 6 +- .../djangoapps/content/search/handlers.py | 46 +++++- .../core/djangoapps/content/search/tasks.py | 45 ++++- .../content/search/tests/test_api.py | 4 +- .../content/search/tests/test_handlers.py | 154 ++++++++++++++++++ 5 files changed, 242 insertions(+), 13 deletions(-) create mode 100644 openedx/core/djangoapps/content/search/tests/test_handlers.py diff --git a/openedx/core/djangoapps/content/search/api.py b/openedx/core/djangoapps/content/search/api.py index 401c5018a4e9..7962f00f95d6 100644 --- a/openedx/core/djangoapps/content/search/api.py +++ b/openedx/core/djangoapps/content/search/api.py @@ -168,7 +168,7 @@ def _using_temp_index(status_cb: Callable[[str], None] | None = None) -> Generat Create a new temporary Meilisearch index, populate it, then swap it to become the active index. """ - def nop(_): + def nop(_): # pragma: no cover pass if status_cb is None: @@ -243,8 +243,8 @@ def is_meilisearch_enabled() -> bool: Returns whether Meilisearch is enabled """ if hasattr(settings, "MEILISEARCH_INDEX_PREFIX"): - return settings.MEILISEARCH_ENABLED + return False @@ -377,7 +377,7 @@ def add_with_children(block): _wait_for_meili_tasks(tasks) -def delete_xblock_index_doc(usage_key: UsageKey) -> None: +def delete_index_doc(usage_key: UsageKey) -> None: """ Deletes the document for the given XBlock from the search index diff --git a/openedx/core/djangoapps/content/search/handlers.py b/openedx/core/djangoapps/content/search/handlers.py index 313906796542..db13a09bc6c0 100644 --- a/openedx/core/djangoapps/content/search/handlers.py +++ b/openedx/core/djangoapps/content/search/handlers.py @@ -5,15 +5,23 @@ import logging from django.dispatch import receiver -from openedx_events.content_authoring.data import XBlockData +from openedx_events.content_authoring.data import LibraryBlockData, XBlockData from openedx_events.content_authoring.signals import ( + LIBRARY_BLOCK_CREATED, + LIBRARY_BLOCK_DELETED, + LIBRARY_BLOCK_UPDATED, XBLOCK_CREATED, XBLOCK_DELETED, XBLOCK_UPDATED ) -from .tasks import delete_xblock_index_doc, upsert_xblock_index_doc from .api import only_if_meilisearch_enabled +from .tasks import ( + delete_library_block_index_doc, + delete_xblock_index_doc, + upsert_library_block_index_doc, + upsert_xblock_index_doc +) log = logging.getLogger(__name__) @@ -25,7 +33,7 @@ def xblock_created_handler(**kwargs) -> None: Create the index for the XBlock """ xblock_info = kwargs.get("xblock_info", None) - if not xblock_info or not isinstance(xblock_info, XBlockData): + if not xblock_info or not isinstance(xblock_info, XBlockData): # pragma: no cover log.error("Received null or incorrect data for event") return @@ -44,7 +52,7 @@ def xblock_updated_handler(**kwargs) -> None: Update the index for the XBlock and its children """ xblock_info = kwargs.get("xblock_info", None) - if not xblock_info or not isinstance(xblock_info, XBlockData): + if not xblock_info or not isinstance(xblock_info, XBlockData): # pragma: no cover log.error("Received null or incorrect data for event") return @@ -63,8 +71,36 @@ def xblock_deleted_handler(**kwargs) -> None: Delete the index for the XBlock """ xblock_info = kwargs.get("xblock_info", None) - if not xblock_info or not isinstance(xblock_info, XBlockData): + if not xblock_info or not isinstance(xblock_info, XBlockData): # pragma: no cover log.error("Received null or incorrect data for event") return delete_xblock_index_doc.delay(str(xblock_info.usage_key)) + + +@receiver(LIBRARY_BLOCK_CREATED) +@only_if_meilisearch_enabled +def content_library_updated_handler(**kwargs) -> None: + """ + Create or update the index for the content library block + """ + library_block_data = kwargs.get("library_block", None) + if not library_block_data or not isinstance(library_block_data, LibraryBlockData): # pragma: no cover + log.error("Received null or incorrect data for event") + return + + upsert_library_block_index_doc.delay(str(library_block_data.usage_key), update_metadata=True, update_tags=False) + + +@receiver(LIBRARY_BLOCK_DELETED) +@only_if_meilisearch_enabled +def content_library_deleted_handler(**kwargs) -> None: + """ + Delete the index for the content library block + """ + library_block_data = kwargs.get("library_block", None) + if not library_block_data or not isinstance(library_block_data, LibraryBlockData): # pragma: no cover + log.error("Received null or incorrect data for event") + return + + delete_library_block_index_doc.delay(str(library_block_data.usage_key)) diff --git a/openedx/core/djangoapps/content/search/tasks.py b/openedx/core/djangoapps/content/search/tasks.py index 877a32618aeb..52d3fd3ebfe5 100644 --- a/openedx/core/djangoapps/content/search/tasks.py +++ b/openedx/core/djangoapps/content/search/tasks.py @@ -10,6 +10,7 @@ from celery_utils.logged_task import LoggedTask from edx_django_utils.monitoring import set_code_owner_attribute from opaque_keys.edx.keys import UsageKey +from opaque_keys.edx.locator import LibraryUsageLocatorV2 from . import api @@ -30,7 +31,7 @@ def upsert_xblock_index_doc(usage_key_str: str, recursive: bool, update_metadata api.upsert_xblock_index_doc(usage_key, recursive, update_metadata, update_tags) return True - except Exception as e: # pylint: disable=broad-except + except Exception as e: # pylint: disable=broad-except pragma: no cover log.error("Error updating content index document for XBlock with id: %s. %s", usage_key_str, e) return False @@ -46,9 +47,47 @@ def delete_xblock_index_doc(usage_key_str: str) -> bool: log.info("Updating content index document for XBlock with id: %s", usage_key) - api.delete_xblock_index_doc(usage_key) + api.delete_index_doc(usage_key) return True - except Exception as e: # pylint: disable=broad-except + except Exception as e: # pylint: disable=broad-except pragma: no cover log.error("Error deleting content index document for XBlock with id: %s. %s", usage_key_str, e) return False + + +@shared_task(base=LoggedTask) +@set_code_owner_attribute +def upsert_library_block_index_doc(usage_key_str: str, update_metadata: bool, update_tags: bool) -> bool: + """ + Celery task to update the content index document for a library block + """ + try: + usage_key = LibraryUsageLocatorV2.from_string(usage_key_str) + + log.info("Updating content index document for library block with id: %s", usage_key) + + api.upsert_library_block_index_doc(usage_key, update_metadata, update_tags) + + return True + except Exception as e: # pylint: disable=broad-except pragma: no cover + log.error("Error updating content index document for libray block with id: %s. %s", usage_key_str, e) + return False + + +@shared_task(base=LoggedTask) +@set_code_owner_attribute +def delete_library_block_index_doc(usage_key_str: str) -> bool: + """ + Celery task to delete the content index document for a library block + """ + try: + usage_key = LibraryUsageLocatorV2.from_string(usage_key_str) + + log.info("Deleting content index document for library block with id: %s", usage_key) + + api.delete_index_doc(usage_key) + + return True + except Exception as e: # pylint: disable=broad-except pragma: no cover + log.error("Error deleting content index document for library block with id: %s. %s", usage_key_str, e) + return False diff --git a/openedx/core/djangoapps/content/search/tests/test_api.py b/openedx/core/djangoapps/content/search/tests/test_api.py index 0439f900d4c2..92ec12db6bca 100644 --- a/openedx/core/djangoapps/content/search/tests/test_api.py +++ b/openedx/core/djangoapps/content/search/tests/test_api.py @@ -153,7 +153,7 @@ def test_delete_index_xblock(self, mock_meilisearch): """ Test deleting an XBlock doc from the index. """ - api.delete_xblock_index_doc(self.sequential.usage_key) + api.delete_index_doc(self.sequential.usage_key) mock_meilisearch.return_value.index.return_value.delete_document.assert_called_once_with( self.doc_sequential['id'] @@ -177,7 +177,7 @@ def test_delete_index_library_block(self, mock_meilisearch): """ Test deleting a Library Block doc from the index. """ - api.delete_xblock_index_doc(self.problem.usage_key) + api.delete_index_doc(self.problem.usage_key) mock_meilisearch.return_value.index.return_value.delete_document.assert_called_once_with( self.doc_problem['id'] diff --git a/openedx/core/djangoapps/content/search/tests/test_handlers.py b/openedx/core/djangoapps/content/search/tests/test_handlers.py new file mode 100644 index 000000000000..1fa3f5316abc --- /dev/null +++ b/openedx/core/djangoapps/content/search/tests/test_handlers.py @@ -0,0 +1,154 @@ +from unittest.mock import MagicMock, patch + +from organizations.tests.factories import OrganizationFactory +from django.test import override_settings, LiveServerTestCase + +from common.djangoapps.student.tests.factories import UserFactory +from openedx.core.djangoapps.content_libraries import api as library_api +from xmodule.modulestore.tests.django_utils import TEST_DATA_SPLIT_MODULESTORE, ModuleStoreTestCase +from openedx.core.lib.blockstore_api.tests.base import BlockstoreAppTestMixin + +from .. import api + + +@patch("openedx.core.djangoapps.content.search.api._wait_for_meili_task", new=MagicMock(return_value=None)) +@patch("openedx.core.djangoapps.content.search.api.MeilisearchClient") +@override_settings(MEILISEARCH_ENABLED=True) +class TestUpdateIndexHandlers( + ModuleStoreTestCase, + BlockstoreAppTestMixin, + LiveServerTestCase, +): + """ + Test that the search index is updated when XBlocks and Library Blocks are modified + """ + + MODULESTORE = TEST_DATA_SPLIT_MODULESTORE + + def setUp(self): + super().setUp() + # Create user + self.user = UserFactory.create() + self.user_id = self.user.id + + self.orgA = OrganizationFactory.create(short_name="orgA") + + self.patcher = patch("openedx.core.djangoapps.content_tagging.tasks.modulestore", return_value=self.store) + self.addCleanup(self.patcher.stop) + self.patcher.start() + + api.clear_meilisearch_client() # Clear the Meilisearch client to avoid leaking state from other tests + + + def test_create_delete_xblock(self, meilisearch_client): + # Create course + course = self.store.create_course( + self.orgA.short_name, + "test_course", + "test_run", + self.user_id, + fields={"display_name": "Test Course"}, + ) + + # Create XBlocks + sequential = self.store.create_child(self.user_id, course.location, "sequential", "test_sequential") + meilisearch_client.return_value.index.return_value.update_documents.assert_called_with([ + { + 'id': 'block-v1orgatest_coursetest_runtypesequentialblocktest_sequential-0cdb9395', + 'type': 'course_block', + 'usage_key': 'block-v1:orgA+test_course+test_run+type@sequential+block@test_sequential', + 'block_id': 'test_sequential', + 'display_name': 'sequential', + 'block_type': 'sequential', + 'context_key': 'course-v1:orgA+test_course+test_run', + 'org': 'orgA', + 'breadcrumbs': [{'display_name': 'Test Course'}], 'content': {} + } + ]) + vertical = self.store.create_child(self.user_id, sequential.location, "vertical", "test_vertical") + meilisearch_client.return_value.index.return_value.update_documents.assert_called_with([ + { + 'id': 'block-v1orgatest_coursetest_runtypeverticalblocktest_vertical-011f143b', + 'type': 'course_block', + 'usage_key': 'block-v1:orgA+test_course+test_run+type@vertical+block@test_vertical', + 'block_id': 'test_vertical', + 'display_name': 'vertical', + 'block_type': 'vertical', + 'context_key': 'course-v1:orgA+test_course+test_run', + 'org': 'orgA', + 'breadcrumbs': [{'display_name': 'Test Course'}, {'display_name': 'sequential'}], + 'content': {} + } + ]) + + # Update the XBlock + sequential = self.store.get_item(sequential.location, self.user_id) # Refresh the XBlock + sequential.display_name = "Updated Sequential" + self.store.update_item(sequential, self.user_id) + + meilisearch_client.return_value.index.return_value.update_documents.assert_called_with([ + { + 'id': 'block-v1orgatest_coursetest_runtypesequentialblocktest_sequential-0cdb9395', + 'type': 'course_block', + 'usage_key': 'block-v1:orgA+test_course+test_run+type@sequential+block@test_sequential', + 'block_id': 'test_sequential', + 'display_name': 'Updated Sequential', + 'block_type': 'sequential', + 'context_key': 'course-v1:orgA+test_course+test_run', + 'org': 'orgA', + 'breadcrumbs': [{'display_name': 'Test Course'}], 'content': {} + }, + { + 'id': 'block-v1orgatest_coursetest_runtypeverticalblocktest_vertical-011f143b', + 'type': 'course_block', + 'usage_key': 'block-v1:orgA+test_course+test_run+type@vertical+block@test_vertical', + 'block_id': 'test_vertical', + 'display_name': 'vertical', + 'block_type': 'vertical', + 'context_key': 'course-v1:orgA+test_course+test_run', + 'org': 'orgA', + 'breadcrumbs': [{'display_name': 'Test Course'}, {'display_name': 'Updated Sequential'}], + 'content': {} + } + ]) + + # Delete the XBlock + self.store.delete_item(vertical.location, self.user_id) + + meilisearch_client.return_value.index.return_value.delete_document.assert_called_with( + 'block-v1orgatest_coursetest_runtypeverticalblocktest_vertical-011f143b' + ) + + + def test_create_delete_library_block(self, meilisearch_client): + # Create library + library = library_api.create_library( + org=self.orgA, + slug="lib_a", + title="Library Org A", + description="This is a library from Org A", + ) + + problem = library_api.create_library_block(library.key, "problem", "Problem1") + + meilisearch_client.return_value.index.return_value.update_documents.assert_called_with([ + { + 'id': 'lborgalib_aproblemproblem1-ca3186e9', + 'type': 'library_block', + 'usage_key': 'lb:orgA:lib_a:problem:Problem1', + 'block_id': 'Problem1', + 'display_name': 'Blank Problem', + 'block_type': 'problem', + 'context_key': 'lib:orgA:lib_a', + 'org': 'orgA', + 'breadcrumbs': [{'display_name': 'Library Org A'}], + 'content': {'problem_types': [], 'capa_content': ' '} + }, + ]) + + # Delete the Library Block + library_api.delete_library_block(problem.usage_key) + + meilisearch_client.return_value.index.return_value.delete_document.assert_called_with( + 'lborgalib_aproblemproblem1-ca3186e9' + )