diff --git a/cms/djangoapps/contentstore/tests/test_libraries.py b/cms/djangoapps/contentstore/tests/test_libraries.py index e7dcc3647886..376ba56d8dd9 100644 --- a/cms/djangoapps/contentstore/tests/test_libraries.py +++ b/cms/djangoapps/contentstore/tests/test_libraries.py @@ -440,10 +440,6 @@ def test_sync_if_source_library_changed(self): html_block_2 = modulestore().get_item(lc_block.children[0]) self.assertEqual(html_block_2.data, data2) - @patch( - 'openedx.core.djangoapps.content_libraries.tasks.SearchEngine.get_search_engine', - Mock(return_value=None, autospec=True), - ) def test_sync_if_capa_type_changed(self): """ Tests that children are automatically refreshed if capa type field changes """ name1, name2 = "Option Problem", "Multiple Choice Problem" diff --git a/cms/envs/common.py b/cms/envs/common.py index d8219ad11ca0..e4d23241bf4a 100644 --- a/cms/envs/common.py +++ b/cms/envs/common.py @@ -297,9 +297,6 @@ # Enable content libraries (modulestore) search functionality 'ENABLE_LIBRARY_INDEX': False, - # Enable content libraries (blockstore) indexing - 'ENABLE_CONTENT_LIBRARY_INDEX': False, - # .. toggle_name: FEATURES['ALLOW_COURSE_RERUNS'] # .. toggle_implementation: DjangoSetting # .. toggle_default: True diff --git a/cms/envs/devstack.py b/cms/envs/devstack.py index 275a1fd31e72..7deb16c7eb67 100644 --- a/cms/envs/devstack.py +++ b/cms/envs/devstack.py @@ -147,7 +147,6 @@ def should_show_debug_toolbar(request): # lint-amnesty, pylint: disable=missing ################################ SEARCH INDEX ################################ FEATURES['ENABLE_COURSEWARE_INDEX'] = True FEATURES['ENABLE_LIBRARY_INDEX'] = False -FEATURES['ENABLE_CONTENT_LIBRARY_INDEX'] = False SEARCH_ENGINE = "search.elastic.ElasticSearchEngine" ELASTIC_SEARCH_CONFIG = [ diff --git a/cms/envs/production.py b/cms/envs/production.py index d0a846d3da21..262d0d4c4629 100644 --- a/cms/envs/production.py +++ b/cms/envs/production.py @@ -510,7 +510,7 @@ def get_env_setting(setting): # Example: {'CN': 'http://api.xuetangx.com/edx/video?s3_url='} VIDEO_CDN_URL = ENV_TOKENS.get('VIDEO_CDN_URL', {}) -if FEATURES['ENABLE_COURSEWARE_INDEX'] or FEATURES['ENABLE_LIBRARY_INDEX'] or FEATURES['ENABLE_CONTENT_LIBRARY_INDEX']: +if FEATURES['ENABLE_COURSEWARE_INDEX'] or FEATURES['ENABLE_LIBRARY_INDEX']: # Use ElasticSearch for the search engine SEARCH_ENGINE = "search.elastic.ElasticSearchEngine" diff --git a/cms/envs/test.py b/cms/envs/test.py index f63b14c16775..118d7e27a79c 100644 --- a/cms/envs/test.py +++ b/cms/envs/test.py @@ -259,21 +259,10 @@ # Courseware Search Index FEATURES['ENABLE_COURSEWARE_INDEX'] = True FEATURES['ENABLE_LIBRARY_INDEX'] = True -FEATURES['ENABLE_CONTENT_LIBRARY_INDEX'] = False SEARCH_ENGINE = "search.tests.mock_search_engine.MockSearchEngine" FEATURES['ENABLE_ENROLLMENT_TRACK_USER_PARTITION'] = True -####################### ELASTICSEARCH TESTS ####################### -# Enable this when testing elasticsearch-based code which couldn't be tested using the mock engine -ENABLE_ELASTICSEARCH_FOR_TESTS = os.environ.get( - 'EDXAPP_ENABLE_ELASTICSEARCH_FOR_TESTS', 'no').lower() in ('true', 'yes', '1') - -TEST_ELASTICSEARCH_USE_SSL = os.environ.get( - 'EDXAPP_TEST_ELASTICSEARCH_USE_SSL', 'no').lower() in ('true', 'yes', '1') -TEST_ELASTICSEARCH_HOST = os.environ.get('EDXAPP_TEST_ELASTICSEARCH_HOST', 'edx.devstack.elasticsearch710') -TEST_ELASTICSEARCH_PORT = int(os.environ.get('EDXAPP_TEST_ELASTICSEARCH_PORT', '9200')) - ########################## AUTHOR PERMISSION ####################### FEATURES['ENABLE_CREATOR_GROUP'] = False diff --git a/lms/envs/test.py b/lms/envs/test.py index 37db856fb98f..39996e857ac5 100644 --- a/lms/envs/test.py +++ b/lms/envs/test.py @@ -418,16 +418,6 @@ FACEBOOK_APP_ID = "Test" FACEBOOK_API_VERSION = "v2.8" -####################### ELASTICSEARCH TESTS ####################### -# Enable this when testing elasticsearch-based code which couldn't be tested using the mock engine -ENABLE_ELASTICSEARCH_FOR_TESTS = os.environ.get( - 'EDXAPP_ENABLE_ELASTICSEARCH_FOR_TESTS', 'no').lower() in ('true', 'yes', '1') - -TEST_ELASTICSEARCH_USE_SSL = os.environ.get( - 'EDXAPP_TEST_ELASTICSEARCH_USE_SSL', 'no').lower() in ('true', 'yes', '1') -TEST_ELASTICSEARCH_HOST = os.environ.get('EDXAPP_TEST_ELASTICSEARCH_HOST', 'edx.devstack.elasticsearch710') -TEST_ELASTICSEARCH_PORT = int(os.environ.get('EDXAPP_TEST_ELASTICSEARCH_PORT', '9200')) - ######### custom courses ######### INSTALLED_APPS += ['lms.djangoapps.ccx', 'openedx.core.djangoapps.ccxcon.apps.CCXConnectorConfig'] FEATURES['CUSTOM_COURSES_EDX'] = True diff --git a/openedx/core/djangoapps/content_libraries/api.py b/openedx/core/djangoapps/content_libraries/api.py index 33d43cdc64f2..10383de336e5 100644 --- a/openedx/core/djangoapps/content_libraries/api.py +++ b/openedx/core/djangoapps/content_libraries/api.py @@ -67,7 +67,6 @@ from django.core.validators import validate_unicode_slug from django.db import IntegrityError, transaction from django.utils.translation import gettext as _ -from elasticsearch.exceptions import ConnectionError as ElasticConnectionError from lxml import etree from opaque_keys.edx.keys import LearningContextKey, UsageKey from opaque_keys.edx.locator import ( @@ -94,7 +93,6 @@ from openedx.core.djangoapps.content_libraries import permissions from openedx.core.djangoapps.content_libraries.constants import DRAFT_NAME, COMPLEX from openedx.core.djangoapps.content_libraries.library_bundle import LibraryBundle -from openedx.core.djangoapps.content_libraries.libraries_index import ContentLibraryIndexer, LibraryBlockIndexer from openedx.core.djangoapps.content_libraries.models import ( ContentLibrary, ContentLibraryPermission, @@ -290,56 +288,35 @@ def get_libraries_for_user(user, org=None, library_type=None): return permissions.perms[permissions.CAN_VIEW_THIS_CONTENT_LIBRARY].filter(user, qs) -def get_metadata_from_index(queryset, text_search=None): +def get_metadata(queryset, text_search=None): """ - Take a list of ContentLibrary objects and return metadata stored in - ContentLibraryIndex. + Take a list of ContentLibrary objects and return metadata from blockstore. """ - metadata = None - if ContentLibraryIndexer.indexing_is_enabled(): - try: - library_keys = [str(lib.library_key) for lib in queryset] - metadata = ContentLibraryIndexer.get_items(library_keys, text_search=text_search) - metadata_dict = { - item["id"]: item - for item in metadata - } - metadata = [ - metadata_dict[key] - if key in metadata_dict - else None - for key in library_keys - ] - except ElasticConnectionError as e: - log.exception(e) - - # If ContentLibraryIndex is not available, we query blockstore for a limited set of metadata - if metadata is None: - uuids = [lib.bundle_uuid for lib in queryset] - bundles = get_bundles(uuids=uuids, text_search=text_search) - - if text_search: - # Bundle APIs can't apply text_search on a bundle's org, so including those results here - queryset_org_search = queryset.filter(org__short_name__icontains=text_search) - if queryset_org_search.exists(): - uuids_org_search = [lib.bundle_uuid for lib in queryset_org_search] - bundles += get_bundles(uuids=uuids_org_search) - - bundle_dict = { - bundle.uuid: { - 'uuid': bundle.uuid, - 'title': bundle.title, - 'description': bundle.description, - 'version': bundle.latest_version, - } - for bundle in bundles + uuids = [lib.bundle_uuid for lib in queryset] + bundles = get_bundles(uuids=uuids, text_search=text_search) + + if text_search: + # Bundle APIs can't apply text_search on a bundle's org, so including those results here + queryset_org_search = queryset.filter(org__short_name__icontains=text_search) + if queryset_org_search.exists(): + uuids_org_search = [lib.bundle_uuid for lib in queryset_org_search] + bundles += get_bundles(uuids=uuids_org_search) + + bundle_dict = { + bundle.uuid: { + 'uuid': bundle.uuid, + 'title': bundle.title, + 'description': bundle.description, + 'version': bundle.latest_version, } - metadata = [ - bundle_dict[uuid] - if uuid in bundle_dict - else None - for uuid in uuids - ] + for bundle in bundles + } + metadata = [ + bundle_dict[uuid] + if uuid in bundle_dict + else None + for uuid in uuids + ] libraries = [ ContentLibraryMetadata( @@ -648,50 +625,28 @@ def get_library_blocks(library_key, text_search=None, block_types=None) -> list[ Returns a list of LibraryXBlockMetadata objects """ - metadata = None - if LibraryBlockIndexer.indexing_is_enabled(): - try: - filter_terms = { - 'library_key': [str(library_key)], - 'is_child': [False], - } - if block_types: - filter_terms['block_type'] = block_types - metadata = [ - { - **item, - "id": LibraryUsageLocatorV2.from_string(item['id']), - } - for item in LibraryBlockIndexer.get_items(filter_terms=filter_terms, text_search=text_search) - if item is not None - ] - except ElasticConnectionError as e: - log.exception(e) - - # If indexing is disabled, or connection to elastic failed - if metadata is None: - metadata = [] - ref = ContentLibrary.objects.get_by_key(library_key) # type: ignore[attr-defined] - lib_bundle = LibraryBundle(library_key, ref.bundle_uuid, draft_name=DRAFT_NAME) - usages = lib_bundle.get_top_level_usages() - - for usage_key in usages: - # For top-level definitions, we can go from definition key to usage key using the following, but this would - # not work for non-top-level blocks as they may have multiple usages. Top level blocks are guaranteed to - # have only a single usage in the library, which is part of the definition of top level block. - def_key = lib_bundle.definition_for_usage(usage_key) - display_name = get_block_display_name(def_key) - text_match = (text_search is None or - text_search.lower() in display_name.lower() or - text_search.lower() in str(usage_key).lower()) - type_match = (block_types is None or usage_key.block_type in block_types) - if text_match and type_match: - metadata.append({ - "id": usage_key, - "def_key": def_key, - "display_name": display_name, - "has_unpublished_changes": lib_bundle.does_definition_have_unpublished_changes(def_key), - }) + metadata = [] + ref = ContentLibrary.objects.get_by_key(library_key) # type: ignore[attr-defined] + lib_bundle = LibraryBundle(library_key, ref.bundle_uuid, draft_name=DRAFT_NAME) + usages = lib_bundle.get_top_level_usages() + + for usage_key in usages: + # For top-level definitions, we can go from definition key to usage key using the following, but this would + # not work for non-top-level blocks as they may have multiple usages. Top level blocks are guaranteed to + # have only a single usage in the library, which is part of the definition of top level block. + def_key = lib_bundle.definition_for_usage(usage_key) + display_name = get_block_display_name(def_key) + text_match = (text_search is None or + text_search.lower() in display_name.lower() or + text_search.lower() in str(usage_key).lower()) + type_match = (block_types is None or usage_key.block_type in block_types) + if text_match and type_match: + metadata.append({ + "id": usage_key, + "def_key": def_key, + "display_name": display_name, + "has_unpublished_changes": lib_bundle.does_definition_have_unpublished_changes(def_key), + }) return [ LibraryXBlockMetadata( diff --git a/openedx/core/djangoapps/content_libraries/docs/decisions/0001-index-libraries-in-elasticsearch.rst b/openedx/core/djangoapps/content_libraries/docs/decisions/0001-index-libraries-in-elasticsearch.rst index ed16be84a48f..eb003e1ca6df 100644 --- a/openedx/core/djangoapps/content_libraries/docs/decisions/0001-index-libraries-in-elasticsearch.rst +++ b/openedx/core/djangoapps/content_libraries/docs/decisions/0001-index-libraries-in-elasticsearch.rst @@ -4,7 +4,30 @@ Status ------ -Accepted +**Revoked** + +In Dec 2023, we decided to remove the code supporting this decision, because: + +* The index is disabled on edx.org, which will initially be the only user + of Content Libraries V2. +* As we migrate libraries from Modulestore to Blockstore and then from + Blockstore to Learning Core, the unused indexing code increases complexity + and decreases certainty. +* With the decision to migrate from Blockstore-the-service to an in-process + storage backend (that is: Blockstore-the-app or Learning Core), it seems + that we will be able to simply use Django ORM in order to filter/sort/search + Content Library V2 metadata for the library listing page. +* Searching Content Library V2 *block* content would still require indexing, + but we would rather implement that in Learning Core than use the current + implementation in the content_libraries app, which is untested, library- + specific, and doesn't take into account library versioning. It always uses + the latest draft, which is good for Library Authoring purposes, but not good for + Course Authoring purposes. + +It is possible that we will end up re-instating a modified version of this ADR +future. If that happens, we may re-use and adapt the original library index +code. + Context ------- diff --git a/openedx/core/djangoapps/content_libraries/libraries_index.py b/openedx/core/djangoapps/content_libraries/libraries_index.py deleted file mode 100644 index 82a79a652561..000000000000 --- a/openedx/core/djangoapps/content_libraries/libraries_index.py +++ /dev/null @@ -1,346 +0,0 @@ -""" Code to allow indexing content libraries """ - -import logging -from abc import ABC, abstractmethod - -from django.conf import settings -from django.dispatch import receiver -from elasticsearch.exceptions import ConnectionError as ElasticConnectionError -from search.elastic import _translate_hits, RESERVED_CHARACTERS -from search.search_engine_base import SearchEngine -from opaque_keys.edx.locator import LibraryUsageLocatorV2 -from openedx_events.content_authoring.data import ContentLibraryData, LibraryBlockData -from openedx_events.content_authoring.signals import ( - CONTENT_LIBRARY_CREATED, - CONTENT_LIBRARY_DELETED, - CONTENT_LIBRARY_UPDATED, - LIBRARY_BLOCK_CREATED, - LIBRARY_BLOCK_DELETED, - LIBRARY_BLOCK_UPDATED, -) - -from openedx.core.djangoapps.content_libraries.constants import DRAFT_NAME -from openedx.core.djangoapps.content_libraries.library_bundle import LibraryBundle -from openedx.core.djangoapps.content_libraries.models import ContentLibrary -from openedx.core.lib.blockstore_api import get_bundle - -log = logging.getLogger(__name__) - -MAX_SIZE = 10000 # 10000 is the maximum records elastic is able to return in a single result. Defaults to 10. - - -class SearchIndexerBase(ABC): - """ - Abstract Base Class for implementing library search indexers. - """ - INDEX_NAME = None - ENABLE_INDEXING_KEY = None - SCHEMA_VERSION = 0 - SEARCH_KWARGS = { - # Set this to True or 'wait_for' if immediate refresh is required after any update. - # See elastic docs for more information. - 'refresh': False - } - - @classmethod - @abstractmethod - def get_item_definition(cls, item): - """ - Returns a serializable dictionary which can be stored in elasticsearch. - """ - - @classmethod - def index_items(cls, items): - """ - Index the specified libraries. If they already exist, replace them with new ones. - """ - searcher = SearchEngine.get_search_engine(cls.INDEX_NAME) - items = [cls.get_item_definition(item) for item in items] - return searcher.index(items, **cls.SEARCH_KWARGS) - - @classmethod - def get_items(cls, ids=None, filter_terms=None, text_search=None): - """ - Retrieve a list of items from the index. - Arguments: - ids - List of ids to be searched for in the index - filter_terms - Dictionary of filters to be applied - text_search - String which is used to do a text search in the supported indexes. - """ - if filter_terms is None: - filter_terms = {} - if ids is not None: - filter_terms = { - "id": [str(item) for item in ids], - "schema_version": [cls.SCHEMA_VERSION], - **filter_terms, - } - if text_search: - response = cls._perform_elastic_search(filter_terms, text_search) - else: - searcher = SearchEngine.get_search_engine(cls.INDEX_NAME) - response = searcher.search(field_dictionary=filter_terms, size=MAX_SIZE) - - response = [result["data"] for result in response["results"]] - return sorted(response, key=lambda i: i["id"]) - - @classmethod - def remove_items(cls, ids): - """ - Remove the provided ids from the index - """ - searcher = SearchEngine.get_search_engine(cls.INDEX_NAME) - ids_str = [str(i) for i in ids] - searcher.remove(ids_str, **cls.SEARCH_KWARGS) - - @classmethod - def remove_all_items(cls): - """ - Remove all items from the index - """ - searcher = SearchEngine.get_search_engine(cls.INDEX_NAME) - response = searcher.search(filter_dictionary={}, size=MAX_SIZE) - ids = [result["data"]["id"] for result in response["results"]] - searcher.remove(ids, **cls.SEARCH_KWARGS) - - @classmethod - def indexing_is_enabled(cls): - """ - Checks to see if the indexing feature is enabled - """ - return settings.FEATURES.get(cls.ENABLE_INDEXING_KEY, False) - - @classmethod - def _perform_elastic_search(cls, filter_terms, text_search): - """ - Build a query and search directly on elasticsearch - """ - searcher = SearchEngine.get_search_engine(cls.INDEX_NAME) - return _translate_hits(searcher._es.search( # pylint: disable=protected-access - index=searcher.index_name, - body=cls.build_elastic_query(filter_terms, text_search), - size=MAX_SIZE - )) - - @staticmethod - def build_elastic_query(filter_terms, text_search): - """ - Build and return an elastic query for doing text search on a library - """ - # Remove reserved characters (and ") from the text to prevent unexpected errors. - text_search_normalised = text_search.translate(text_search.maketrans('', '', RESERVED_CHARACTERS + '"')) - text_search_normalised = text_search_normalised.replace('-', ' ') - # Wrap with asterix to enable partial matches - text_search_normalised = f"*{text_search_normalised}*" - terms = [ - { - 'terms': { - item: filter_terms[item] - } - } - for item in filter_terms - ] - return { - 'query': { - 'bool': { - 'must': [ - { - 'query_string': { - 'query': text_search_normalised, - "fields": ["content.*"], - 'minimum_should_match': '100%', - }, - }, - ], - 'filter': { - 'bool': { - 'must': terms, - } - } - }, - }, - } - - -class ContentLibraryIndexer(SearchIndexerBase): - """ - Class to perform indexing for blockstore-based content libraries - """ - - INDEX_NAME = "content_library_index" - ENABLE_INDEXING_KEY = "ENABLE_CONTENT_LIBRARY_INDEX" - SCHEMA_VERSION = 0 - - @classmethod - def get_item_definition(cls, item): - ref = ContentLibrary.objects.get_by_key(item) - lib_bundle = LibraryBundle(item, ref.bundle_uuid, draft_name=DRAFT_NAME) - num_blocks = len(lib_bundle.get_top_level_usages()) - last_published = lib_bundle.get_last_published_time() - last_published_str = None - if last_published: - last_published_str = last_published.strftime('%Y-%m-%dT%H:%M:%SZ') - (has_unpublished_changes, has_unpublished_deletes) = lib_bundle.has_changes() - - bundle_metadata = get_bundle(ref.bundle_uuid) - - # NOTE: Increment ContentLibraryIndexer.SCHEMA_VERSION if the following schema is updated to avoid dealing - # with outdated indexes which might cause errors due to missing/invalid attributes. - return { - "schema_version": ContentLibraryIndexer.SCHEMA_VERSION, - "id": str(item), - "uuid": str(bundle_metadata.uuid), - "title": bundle_metadata.title, - "description": bundle_metadata.description, - "num_blocks": num_blocks, - "version": bundle_metadata.latest_version, - "last_published": last_published_str, - "has_unpublished_changes": has_unpublished_changes, - "has_unpublished_deletes": has_unpublished_deletes, - # only 'content' field is analyzed by elasticsearch, and allows text-search - "content": { - "id": str(item), - "title": bundle_metadata.title, - "description": bundle_metadata.description, - }, - } - - -class LibraryBlockIndexer(SearchIndexerBase): - """ - Class to perform indexing on the XBlocks in content libraries. - """ - - INDEX_NAME = "content_library_block_index" - ENABLE_INDEXING_KEY = "ENABLE_CONTENT_LIBRARY_INDEX" - SCHEMA_VERSION = 0 - - @classmethod - def get_item_definition(cls, item): - from openedx.core.djangoapps.content_libraries.api import get_block_display_name, _lookup_usage_key - - def_key, lib_bundle = _lookup_usage_key(item) - is_child = item in lib_bundle.get_bundle_includes().keys() - - # NOTE: Increment LibraryBlockIndexer.SCHEMA_VERSION if the following schema is updated to avoid dealing - # with outdated indexes which might cause errors due to missing/invalid attributes. - return { - "schema_version": LibraryBlockIndexer.SCHEMA_VERSION, - "id": str(item), - "library_key": str(lib_bundle.library_key), - "is_child": is_child, - "def_key": str(def_key), - "display_name": get_block_display_name(def_key), - "block_type": def_key.block_type, - "has_unpublished_changes": lib_bundle.does_definition_have_unpublished_changes(def_key), - # only 'content' field is analyzed by elastisearch, and allows text-search - "content": { - "id": str(item), - "display_name": get_block_display_name(def_key), - }, - } - - -@receiver(CONTENT_LIBRARY_CREATED) -@receiver(CONTENT_LIBRARY_UPDATED) -def index_library(**kwargs): - """ - Index library when created or updated, or when its blocks are modified. - """ - content_library = kwargs.get('content_library', None) - if not content_library or not isinstance(content_library, ContentLibraryData): - log.error('Received null or incorrect data for event') - return - - library_key = content_library.library_key - update_blocks = content_library.update_blocks - if ContentLibraryIndexer.indexing_is_enabled(): - try: - ContentLibraryIndexer.index_items([library_key]) - if update_blocks: - blocks = LibraryBlockIndexer.get_items(filter_terms={ - 'library_key': str(library_key) - }) - usage_keys = [LibraryUsageLocatorV2.from_string(block['id']) for block in blocks] - LibraryBlockIndexer.index_items(usage_keys) - except ElasticConnectionError as e: - log.exception(e) - - -@receiver(LIBRARY_BLOCK_CREATED) -@receiver(LIBRARY_BLOCK_DELETED) -@receiver(LIBRARY_BLOCK_UPDATED) -def index_library_block(**kwargs): - """ - Index library when its blocks are created, modified, or deleted. - """ - library_block = kwargs.get('library_block', None) - if not library_block or not isinstance(library_block, LibraryBlockData): - log.error('Received null or incorrect data for event') - return - - library_key = library_block.library_key - if ContentLibraryIndexer.indexing_is_enabled(): - try: - ContentLibraryIndexer.index_items([library_key]) - except ElasticConnectionError as e: - log.exception(e) - - -@receiver(CONTENT_LIBRARY_DELETED) -def remove_library_index(**kwargs): - """ - Remove from index when library is deleted - """ - content_library = kwargs.get('content_library', None) - if not content_library or not isinstance(content_library, ContentLibraryData): - log.error('Received null or incorrect data for event') - return - - if ContentLibraryIndexer.indexing_is_enabled(): - library_key = content_library.library_key - try: - ContentLibraryIndexer.remove_items([library_key]) - blocks = LibraryBlockIndexer.get_items(filter_terms={ - 'library_key': str(library_key) - }) - LibraryBlockIndexer.remove_items([block['id'] for block in blocks]) - except ElasticConnectionError as e: - log.exception(e) - - -@receiver(LIBRARY_BLOCK_CREATED) -@receiver(LIBRARY_BLOCK_UPDATED) -def index_block(**kwargs): - """ - Index block metadata when created or updated - """ - library_block = kwargs.get('library_block', None) - if not library_block or not isinstance(library_block, LibraryBlockData): - log.error('Received null or incorrect data for event') - return - - usage_key = library_block.usage_key - if LibraryBlockIndexer.indexing_is_enabled(): - try: - LibraryBlockIndexer.index_items([usage_key]) - except ElasticConnectionError as e: - log.exception(e) - - -@receiver(LIBRARY_BLOCK_DELETED) -def remove_block_index(**kwargs): - """ - Remove the block from the index when deleted - """ - library_block = kwargs.get('library_block', None) - if not library_block or not isinstance(library_block, LibraryBlockData): - log.error('Received null or incorrect data for LIBRARY_BLOCK_DELETED') - return - - usage_key = library_block.usage_key - if LibraryBlockIndexer.indexing_is_enabled(): - try: - LibraryBlockIndexer.remove_items([usage_key]) - except ElasticConnectionError as e: - log.exception(e) diff --git a/openedx/core/djangoapps/content_libraries/management/commands/reindex_content_library.py b/openedx/core/djangoapps/content_libraries/management/commands/reindex_content_library.py deleted file mode 100644 index 6c3c8874e895..000000000000 --- a/openedx/core/djangoapps/content_libraries/management/commands/reindex_content_library.py +++ /dev/null @@ -1,81 +0,0 @@ -""" Management command to update content libraries' search index """ # lint-amnesty, pylint: disable=cyclic-import - - -import logging - -from textwrap import dedent - -from django.core.management import BaseCommand -from opaque_keys.edx.locator import LibraryLocatorV2 -from openedx.core.djangoapps.content_libraries.api import DRAFT_NAME -from openedx.core.djangoapps.content_libraries.libraries_index import ContentLibraryIndexer, LibraryBlockIndexer -from openedx.core.djangoapps.content_libraries.library_bundle import LibraryBundle -from openedx.core.djangoapps.content_libraries.models import ContentLibrary - -from cms.djangoapps.contentstore.management.commands.prompt import query_yes_no - - -class Command(BaseCommand): - """ - Command to reindex blockstore-based content libraries (single, multiple or all available). - - This isn't needed on a regular basis as signals in various library APIs update the index when creating, updating or - deleting libraries. - This is usually required when the schema of the index changes, or if indexes are out of sync due to indexing - being previously disabled or any other reason. - - Examples: - - ./manage.py reindex_content_library lib1 lib2 - reindexes libraries with keys lib1 and lib2 - ./manage.py reindex_content_library --all - reindexes all available libraries - ./manage.py reindex_content_library --clear-all - clear all libraries indexes - """ - help = dedent(__doc__) - CONFIRMATION_PROMPT_CLEAR = "This will clear all indexed libraries from elasticsearch. Do you want to continue?" - CONFIRMATION_PROMPT_ALL = "Reindexing all libraries might be a time consuming operation. Do you want to continue?" - - def add_arguments(self, parser): - parser.add_argument( - '--clear-all', - action='store_true', - dest='clear-all', - help='Clear all library indexes' - ) - parser.add_argument( - '--all', - action='store_true', - dest='all', - help='Reindex all libraries' - ) - parser.add_argument( - '--force', - action='store_true', - dest='force', - help='Run command without user prompt for confirmation' - ) - parser.add_argument('library_ids', nargs='*') - - def handle(self, *args, **options): - if options['clear-all']: - if options['force'] or query_yes_no(self.CONFIRMATION_PROMPT_CLEAR, default="no"): - logging.info("Removing all libraries from the index") - ContentLibraryIndexer.remove_all_items() - LibraryBlockIndexer.remove_all_items() - return - - if options['all']: - if options['force'] or query_yes_no(self.CONFIRMATION_PROMPT_ALL, default="no"): - logging.info("Indexing all libraries") - library_keys = [library.library_key for library in ContentLibrary.objects.all()] - else: - return - else: - logging.info("Indexing libraries: {}".format(options['library_ids'])) - library_keys = list(map(LibraryLocatorV2.from_string, options['library_ids'])) - - ContentLibraryIndexer.index_items(library_keys) - - for library_key in library_keys: - ref = ContentLibrary.objects.get_by_key(library_key) - lib_bundle = LibraryBundle(library_key, ref.bundle_uuid, draft_name=DRAFT_NAME) - LibraryBlockIndexer.index_items(lib_bundle.get_all_usages()) diff --git a/openedx/core/djangoapps/content_libraries/tasks.py b/openedx/core/djangoapps/content_libraries/tasks.py index c3c2e82ef556..3c259aaba956 100644 --- a/openedx/core/djangoapps/content_libraries/tasks.py +++ b/openedx/core/djangoapps/content_libraries/tasks.py @@ -28,10 +28,8 @@ from opaque_keys.edx.keys import UsageKey from opaque_keys.edx.locator import ( BlockUsageLocator, - LibraryUsageLocator, LibraryUsageLocatorV2 ) -from search.search_engine_base import SearchEngine from user_tasks.tasks import UserTask, UserTaskStatus from xblock.fields import Scope @@ -80,11 +78,6 @@ def on_progress(block_key, block_num, block_count, exception=None): ) -def _normalize_key_for_search(library_key): - """ Normalizes library key for use with search indexing """ - return library_key.replace(version_guid=None, branch=None) - - def _import_block(store, user_id, source_block, dest_parent_key): """ Recursively import a blockstore block and its children.` @@ -168,21 +161,7 @@ def _filter_child(store, usage_key, capa_type): def _problem_type_filter(store, library, capa_type): """ Filters library children by capa type.""" - try: - search_engine = SearchEngine.get_search_engine(index="library_index") - except: # pylint: disable=bare-except - search_engine = None - if search_engine: - filter_clause = { - "library": str(_normalize_key_for_search(library.location.library_key)), - "content_type": ProblemBlock.INDEX_CONTENT_TYPE, - "problem_types": capa_type - } - search_result = search_engine.search(field_dictionary=filter_clause) - results = search_result.get('results', []) - return [LibraryUsageLocator.from_string(item['data']['id']) for item in results] - else: - return [key for key in library.children if _filter_child(store, key, capa_type)] + return [key for key in library.children if _filter_child(store, key, capa_type)] def _import_from_blockstore(user_id, store, dest_block, blockstore_block_ids): diff --git a/openedx/core/djangoapps/content_libraries/tests/base.py b/openedx/core/djangoapps/content_libraries/tests/base.py index 65e4e43b959c..63b93509a3cb 100644 --- a/openedx/core/djangoapps/content_libraries/tests/base.py +++ b/openedx/core/djangoapps/content_libraries/tests/base.py @@ -4,17 +4,12 @@ from contextlib import contextmanager from io import BytesIO from urllib.parse import urlencode -from unittest import mock -from django.conf import settings from django.test import LiveServerTestCase -from django.test.utils import override_settings from organizations.models import Organization from rest_framework.test import APITestCase, APIClient -from search.search_engine_base import SearchEngine from common.djangoapps.student.tests.factories import UserFactory -from openedx.core.djangoapps.content_libraries.libraries_index import MAX_SIZE from openedx.core.djangoapps.content_libraries.constants import COMPLEX, ALL_RIGHTS_RESERVED from openedx.core.djangolib.testing.utils import skip_unless_cms from openedx.core.lib import blockstore_api @@ -51,45 +46,6 @@ URL_BLOCK_XBLOCK_HANDLER = '/api/xblock/v2/xblocks/{block_key}/handler/{user_id}-{secure_token}/{handler_name}/' -def elasticsearch_test(func): - """ - Decorator for tests which connect to elasticsearch when needed - """ - # This is disabled by default. Set to True if the elasticsearch engine is needed to test parts of code. - if settings.ENABLE_ELASTICSEARCH_FOR_TESTS: - func = override_settings(SEARCH_ENGINE="search.elastic.ElasticSearchEngine")(func) - func = override_settings(ELASTIC_SEARCH_CONFIG=[{ - 'use_ssl': settings.TEST_ELASTICSEARCH_USE_SSL, - 'host': settings.TEST_ELASTICSEARCH_HOST, - 'port': settings.TEST_ELASTICSEARCH_PORT, - }])(func) - func = mock.patch( - "openedx.core.djangoapps.content_libraries.libraries_index.SearchIndexerBase.SEARCH_KWARGS", - new={ - 'refresh': 'wait_for' - })(func) - return func - else: - @classmethod - def mock_perform(cls, filter_terms, text_search): - # pylint: disable=no-member - return SearchEngine.get_search_engine(cls.INDEX_NAME).search( - field_dictionary=filter_terms, - query_string=text_search, - size=MAX_SIZE - ) - - func = mock.patch( - "openedx.core.djangoapps.content_libraries.libraries_index.SearchIndexerBase.SEARCH_KWARGS", - new={} - )(func) - func = mock.patch( - "openedx.core.djangoapps.content_libraries.libraries_index.SearchIndexerBase._perform_elastic_search", - new=mock_perform - )(func) - return func - - @skip_unless_cms # Content Libraries REST API is only available in Studio class _ContentLibrariesRestApiTestMixin: """ diff --git a/openedx/core/djangoapps/content_libraries/tests/test_content_libraries.py b/openedx/core/djangoapps/content_libraries/tests/test_content_libraries.py index 7744df9c23f9..69ee1755188f 100644 --- a/openedx/core/djangoapps/content_libraries/tests/test_content_libraries.py +++ b/openedx/core/djangoapps/content_libraries/tests/test_content_libraries.py @@ -5,10 +5,8 @@ from unittest.mock import Mock, patch import ddt -from django.conf import settings from django.contrib.auth.models import Group from django.test.client import Client -from django.test.utils import override_settings from organizations.models import Organization from rest_framework.test import APITestCase @@ -22,10 +20,8 @@ LIBRARY_BLOCK_DELETED, LIBRARY_BLOCK_UPDATED, ) -from openedx.core.djangoapps.content_libraries.libraries_index import LibraryBlockIndexer, ContentLibraryIndexer from openedx.core.djangoapps.content_libraries.tests.base import ( ContentLibrariesRestApiTest, - elasticsearch_test, URL_BLOCK_METADATA_URL, URL_BLOCK_RENDER_VIEW, URL_BLOCK_GET_HANDLER_URL, @@ -61,13 +57,6 @@ class ContentLibrariesTestMixin: library slug and bundle UUID does not because it's assumed to be immutable and cached forever. """ - - def setUp(self): - super().setUp() - if settings.ENABLE_ELASTICSEARCH_FOR_TESTS: - ContentLibraryIndexer.remove_all_items() - LibraryBlockIndexer.remove_all_items() - def test_library_crud(self): """ Test Create, Read, Update, and Delete of a Content Library @@ -210,89 +199,83 @@ def test_library_validation(self): 'slug': ['Enter a valid “slug” consisting of Unicode letters, numbers, underscores, or hyphens.'], } - @ddt.data(True, False) @patch("openedx.core.djangoapps.content_libraries.views.LibraryApiPagination.page_size", new=2) - def test_list_library(self, is_indexing_enabled): + def test_list_library(self): """ Test the /libraries API and its pagination """ - with override_settings(FEATURES={**settings.FEATURES, 'ENABLE_CONTENT_LIBRARY_INDEX': is_indexing_enabled}): - lib1 = self._create_library(slug="some-slug-1", title="Existing Library") - lib2 = self._create_library(slug="some-slug-2", title="Existing Library") - if not is_indexing_enabled: - lib1['num_blocks'] = lib2['num_blocks'] = None - lib1['last_published'] = lib2['last_published'] = None - lib1['has_unpublished_changes'] = lib2['has_unpublished_changes'] = None - lib1['has_unpublished_deletes'] = lib2['has_unpublished_deletes'] = None - - result = self._list_libraries() - assert len(result) == 2 - assert lib1 in result - assert lib2 in result - result = self._list_libraries({'pagination': 'true'}) - assert len(result['results']) == 2 - assert result['next'] is None - - # Create another library which causes number of libraries to exceed the page size - self._create_library(slug="some-slug-3", title="Existing Library") - # Verify that if `pagination` param isn't sent, API still honors the max page size. - # This is for maintaining compatibility with older non pagination-aware clients. - result = self._list_libraries() - assert len(result) == 2 - - # Pagination enabled: - # Verify total elements and valid 'next' in page 1 - result = self._list_libraries({'pagination': 'true'}) - assert len(result['results']) == 2 - assert 'page=2' in result['next'] - assert 'pagination=true' in result['next'] - # Verify total elements and null 'next' in page 2 - result = self._list_libraries({'pagination': 'true', 'page': '2'}) - assert len(result['results']) == 1 - assert result['next'] is None - - @ddt.data(True, False) - def test_library_filters(self, is_indexing_enabled): + lib1 = self._create_library(slug="some-slug-1", title="Existing Library") + lib2 = self._create_library(slug="some-slug-2", title="Existing Library") + lib1['num_blocks'] = lib2['num_blocks'] = None + lib1['last_published'] = lib2['last_published'] = None + lib1['has_unpublished_changes'] = lib2['has_unpublished_changes'] = None + lib1['has_unpublished_deletes'] = lib2['has_unpublished_deletes'] = None + + result = self._list_libraries() + assert len(result) == 2 + assert lib1 in result + assert lib2 in result + result = self._list_libraries({'pagination': 'true'}) + assert len(result['results']) == 2 + assert result['next'] is None + + # Create another library which causes number of libraries to exceed the page size + self._create_library(slug="some-slug-3", title="Existing Library") + # Verify that if `pagination` param isn't sent, API still honors the max page size. + # This is for maintaining compatibility with older non pagination-aware clients. + result = self._list_libraries() + assert len(result) == 2 + + # Pagination enabled: + # Verify total elements and valid 'next' in page 1 + result = self._list_libraries({'pagination': 'true'}) + assert len(result['results']) == 2 + assert 'page=2' in result['next'] + assert 'pagination=true' in result['next'] + # Verify total elements and null 'next' in page 2 + result = self._list_libraries({'pagination': 'true', 'page': '2'}) + assert len(result['results']) == 1 + assert result['next'] is None + + def test_library_filters(self): """ Test the filters in the list libraries API """ - suffix = str(is_indexing_enabled) - with override_settings(FEATURES={**settings.FEATURES, 'ENABLE_CONTENT_LIBRARY_INDEX': is_indexing_enabled}): - self._create_library( - slug=f"test-lib-filter-{suffix}-1", title="Fob", description=f"Bar-{suffix}", library_type=VIDEO, - ) - self._create_library( - slug=f"test-lib-filter-{suffix}-2", title=f"Library-Title-{suffix}-2", description=f"Bar-{suffix}-2", - ) - self._create_library( - slug=f"l3{suffix}", title=f"Library-Title-{suffix}-3", description="Description", library_type=VIDEO, - ) + self._create_library( + slug="test-lib-filter-1", title="Fob", description="Bar", library_type=VIDEO, + ) + self._create_library( + slug="test-lib-filter-2", title="Library-Title-2", description="Bar-2", + ) + self._create_library( + slug="l3", title="Library-Title-3", description="Description", library_type=VIDEO, + ) - Organization.objects.get_or_create( - short_name=f"org-test-{suffix}", - defaults={"name": "Content Libraries Tachyon Exploration & Survey Team"}, - ) - self._create_library( - slug=f"l4-{suffix}", title=f"Library-Title-{suffix}-4", - description="Library-Description", org=f'org-test-{suffix}', - library_type=VIDEO, - ) - self._create_library( - slug="l5", title=f"Library-Title-{suffix}-5", description="Library-Description", - org=f'org-test-{suffix}', - ) + Organization.objects.get_or_create( + short_name="org-test", + defaults={"name": "Content Libraries Tachyon Exploration & Survey Team"}, + ) + self._create_library( + slug="l4", title="Library-Title-4", + description="Library-Description", org='org-test', + library_type=VIDEO, + ) + self._create_library( + slug="l5", title="Library-Title-5", description="Library-Description", + org='org-test', + ) - assert len(self._list_libraries()) == 5 - assert len(self._list_libraries({'org': f'org-test-{suffix}'})) == 2 - assert len(self._list_libraries({'text_search': f'test-lib-filter-{suffix}'})) == 2 - assert len(self._list_libraries({'text_search': f'test-lib-filter-{suffix}', 'type': VIDEO})) == 1 - assert len(self._list_libraries({'text_search': f'library-title-{suffix}'})) == 4 - assert len(self._list_libraries({'text_search': f'library-title-{suffix}', 'type': VIDEO})) == 2 - assert len(self._list_libraries({'text_search': f'bar-{suffix}'})) == 2 - assert len(self._list_libraries({'text_search': f'org-test-{suffix}'})) == 2 - assert len(self._list_libraries({'org': f'org-test-{suffix}', - 'text_search': f'library-title-{suffix}-4'})) == 1 - assert len(self._list_libraries({'type': VIDEO})) == 3 + assert len(self._list_libraries()) == 5 + assert len(self._list_libraries({'org': 'org-test'})) == 2 + assert len(self._list_libraries({'text_search': 'test-lib-filter'})) == 2 + assert len(self._list_libraries({'text_search': 'test-lib-filter', 'type': VIDEO})) == 1 + assert len(self._list_libraries({'text_search': 'library-title'})) == 4 + assert len(self._list_libraries({'text_search': 'library-title', 'type': VIDEO})) == 2 + assert len(self._list_libraries({'text_search': 'bar'})) == 2 + assert len(self._list_libraries({'text_search': 'org-test'})) == 2 + assert len(self._list_libraries({'org': 'org-test', + 'text_search': 'library-title-4'})) == 1 + assert len(self._list_libraries({'type': VIDEO})) == 3 # General Content Library XBlock tests: @@ -439,65 +422,61 @@ def test_library_blocks_studio_view(self): assert 'resources' in fragment assert 'Hello world!' in fragment['content'] - @ddt.data(True, False) @patch("openedx.core.djangoapps.content_libraries.views.LibraryApiPagination.page_size", new=2) - def test_list_library_blocks(self, is_indexing_enabled): + def test_list_library_blocks(self): """ Test the /libraries/{lib_key_str}/blocks API and its pagination """ - with override_settings(FEATURES={**settings.FEATURES, 'ENABLE_CONTENT_LIBRARY_INDEX': is_indexing_enabled}): - lib = self._create_library(slug="list_blocks-slug" + str(is_indexing_enabled), title="Library 1") - block1 = self._add_block_to_library(lib["id"], "problem", "problem1") - block2 = self._add_block_to_library(lib["id"], "unit", "unit1") - - self._add_block_to_library(lib["id"], "problem", "problem2", parent_block=block2["id"]) - - result = self._get_library_blocks(lib["id"]) - assert len(result) == 2 - assert block1 in result - - result = self._get_library_blocks(lib["id"], {'pagination': 'true'}) - assert len(result['results']) == 2 - assert result['next'] is None - - self._add_block_to_library(lib["id"], "problem", "problem3") - # Test pagination - result = self._get_library_blocks(lib["id"]) - assert len(result) == 3 - result = self._get_library_blocks(lib["id"], {'pagination': 'true'}) - assert len(result['results']) == 2 - assert 'page=2' in result['next'] - assert 'pagination=true' in result['next'] - result = self._get_library_blocks(lib["id"], {'pagination': 'true', 'page': '2'}) - assert len(result['results']) == 1 - assert result['next'] is None - - @ddt.data(True, False) - def test_library_blocks_filters(self, is_indexing_enabled): + lib = self._create_library(slug="list_blocks-slug", title="Library 1") + block1 = self._add_block_to_library(lib["id"], "problem", "problem1") + block2 = self._add_block_to_library(lib["id"], "unit", "unit1") + + self._add_block_to_library(lib["id"], "problem", "problem2", parent_block=block2["id"]) + + result = self._get_library_blocks(lib["id"]) + assert len(result) == 2 + assert block1 in result + + result = self._get_library_blocks(lib["id"], {'pagination': 'true'}) + assert len(result['results']) == 2 + assert result['next'] is None + + self._add_block_to_library(lib["id"], "problem", "problem3") + # Test pagination + result = self._get_library_blocks(lib["id"]) + assert len(result) == 3 + result = self._get_library_blocks(lib["id"], {'pagination': 'true'}) + assert len(result['results']) == 2 + assert 'page=2' in result['next'] + assert 'pagination=true' in result['next'] + result = self._get_library_blocks(lib["id"], {'pagination': 'true', 'page': '2'}) + assert len(result['results']) == 1 + assert result['next'] is None + + def test_library_blocks_filters(self): """ Test the filters in the list libraries API """ - with override_settings(FEATURES={**settings.FEATURES, 'ENABLE_CONTENT_LIBRARY_INDEX': is_indexing_enabled}): - lib = self._create_library(slug="test-lib-blocks" + str(is_indexing_enabled), title="Title") - block1 = self._add_block_to_library(lib["id"], "problem", "foo-bar") - self._add_block_to_library(lib["id"], "video", "vid-baz") - self._add_block_to_library(lib["id"], "html", "html-baz") - self._add_block_to_library(lib["id"], "problem", "foo-baz") - self._add_block_to_library(lib["id"], "problem", "bar-baz") - - self._set_library_block_olx(block1["id"], "") - - assert len(self._get_library_blocks(lib['id'])) == 5 - assert len(self._get_library_blocks(lib['id'], {'text_search': 'Foo'})) == 2 - assert len(self._get_library_blocks(lib['id'], {'text_search': 'Display'})) == 1 - assert len(self._get_library_blocks(lib['id'], {'text_search': 'Video'})) == 1 - assert len(self._get_library_blocks(lib['id'], {'text_search': 'Foo', 'block_type': 'video'})) == 0 - assert len(self._get_library_blocks(lib['id'], {'text_search': 'Baz', 'block_type': 'video'})) == 1 - assert len(self._get_library_blocks(lib['id'], {'text_search': 'Baz', 'block_type': ['video', 'html']})) ==\ - 2 - assert len(self._get_library_blocks(lib['id'], {'block_type': 'video'})) == 1 - assert len(self._get_library_blocks(lib['id'], {'block_type': 'problem'})) == 3 - assert len(self._get_library_blocks(lib['id'], {'block_type': 'squirrel'})) == 0 + lib = self._create_library(slug="test-lib-blocks", title="Title") + block1 = self._add_block_to_library(lib["id"], "problem", "foo-bar") + self._add_block_to_library(lib["id"], "video", "vid-baz") + self._add_block_to_library(lib["id"], "html", "html-baz") + self._add_block_to_library(lib["id"], "problem", "foo-baz") + self._add_block_to_library(lib["id"], "problem", "bar-baz") + + self._set_library_block_olx(block1["id"], "") + + assert len(self._get_library_blocks(lib['id'])) == 5 + assert len(self._get_library_blocks(lib['id'], {'text_search': 'Foo'})) == 2 + assert len(self._get_library_blocks(lib['id'], {'text_search': 'Display'})) == 1 + assert len(self._get_library_blocks(lib['id'], {'text_search': 'Video'})) == 1 + assert len(self._get_library_blocks(lib['id'], {'text_search': 'Foo', 'block_type': 'video'})) == 0 + assert len(self._get_library_blocks(lib['id'], {'text_search': 'Baz', 'block_type': 'video'})) == 1 + assert len(self._get_library_blocks(lib['id'], {'text_search': 'Baz', 'block_type': ['video', 'html']})) ==\ + 2 + assert len(self._get_library_blocks(lib['id'], {'block_type': 'video'})) == 1 + assert len(self._get_library_blocks(lib['id'], {'block_type': 'problem'})) == 3 + assert len(self._get_library_blocks(lib['id'], {'block_type': 'squirrel'})) == 0 @ddt.data( ('video-problem', VIDEO, 'problem', 400), @@ -1231,7 +1210,6 @@ def test_library_block_delete_event(self): ) -@elasticsearch_test class ContentLibrariesTest( ContentLibrariesTestMixin, ContentLibrariesRestApiTest, diff --git a/openedx/core/djangoapps/content_libraries/tests/test_libraries_index.py b/openedx/core/djangoapps/content_libraries/tests/test_libraries_index.py deleted file mode 100644 index 5dc333350519..000000000000 --- a/openedx/core/djangoapps/content_libraries/tests/test_libraries_index.py +++ /dev/null @@ -1,302 +0,0 @@ -""" -Testing indexing of blockstore based content libraries -""" -from unittest.mock import patch - -from django.conf import settings -from django.core.management import call_command -from django.test.utils import override_settings -from opaque_keys.edx.locator import LibraryLocatorV2, LibraryUsageLocatorV2 -from search.search_engine_base import SearchEngine - -from openedx.core.djangoapps.content_libraries.libraries_index import ContentLibraryIndexer, LibraryBlockIndexer -from openedx.core.djangoapps.content_libraries.tests.base import ( - ContentLibrariesRestApiTest, - elasticsearch_test, -) - - -class ContentLibraryIndexerTestMixin: - """ - Tests the operation of ContentLibraryIndexer - """ - - @elasticsearch_test - def setUp(self): - super().setUp() - ContentLibraryIndexer.remove_all_items() - LibraryBlockIndexer.remove_all_items() - self.searcher = SearchEngine.get_search_engine(ContentLibraryIndexer.INDEX_NAME) - - def test_index_libraries(self): - """ - Test if libraries are being indexed correctly - """ - result1 = self._create_library(slug="test-lib-index-1", title="Title 1", description="Description") - result2 = self._create_library(slug="test-lib-index-2", title="Title 2", description="Description") - - for result in [result1, result2]: - library_key = LibraryLocatorV2.from_string(result['id']) - response = ContentLibraryIndexer.get_items([library_key])[0] - - assert response['id'] == result['id'] - assert response['title'] == result['title'] - assert response['description'] == result['description'] - assert response['uuid'] == result['bundle_uuid'] - assert response['num_blocks'] == 0 - assert response['version'] == result['version'] - assert response['last_published'] is None - assert response['has_unpublished_changes'] is False - assert response['has_unpublished_deletes'] is False - - def test_schema_updates(self): - """ - Test that outdated indexes aren't retrieved - """ - with patch("openedx.core.djangoapps.content_libraries.libraries_index.ContentLibraryIndexer.SCHEMA_VERSION", - new=0): - result = self._create_library(slug="test-lib-schemaupdates-1", title="Title 1", description="Description") - library_key = LibraryLocatorV2.from_string(result['id']) - assert len(ContentLibraryIndexer.get_items([library_key])) == 1 - - with patch("openedx.core.djangoapps.content_libraries.libraries_index.ContentLibraryIndexer.SCHEMA_VERSION", - new=1): - assert len(ContentLibraryIndexer.get_items([library_key])) == 0 - - call_command("reindex_content_library", all=True, force=True) - - assert len(ContentLibraryIndexer.get_items([library_key])) == 1 - - def test_remove_all_libraries(self): - """ - Test if remove_all_items() deletes all libraries - """ - lib1 = self._create_library(slug="test-lib-rm-all-1", title="Title 1", description="Description") - lib2 = self._create_library(slug="test-lib-rm-all-2", title="Title 2", description="Description") - library_key1 = LibraryLocatorV2.from_string(lib1['id']) - library_key2 = LibraryLocatorV2.from_string(lib2['id']) - - assert len(ContentLibraryIndexer.get_items([library_key1, library_key2])) == 2 - - ContentLibraryIndexer.remove_all_items() - assert len(ContentLibraryIndexer.get_items()) == 0 - - def test_update_libraries(self): - """ - Test if indexes are updated when libraries are updated - """ - lib = self._create_library(slug="test-lib-update", title="Title", description="Description") - library_key = LibraryLocatorV2.from_string(lib['id']) - - self._update_library(lib['id'], title="New Title", description="New Title") - - response = ContentLibraryIndexer.get_items([library_key])[0] - - assert response['id'] == lib['id'] - assert response['title'] == 'New Title' - assert response['description'] == 'New Title' - assert response['uuid'] == lib['bundle_uuid'] - assert response['num_blocks'] == 0 - assert response['version'] == lib['version'] - assert response['last_published'] is None - assert response['has_unpublished_changes'] is False - assert response['has_unpublished_deletes'] is False - - self._delete_library(lib['id']) - assert ContentLibraryIndexer.get_items([library_key]) == [] - ContentLibraryIndexer.get_items([library_key]) - - def test_update_library_blocks(self): - """ - Test if indexes are updated when blocks in libraries are updated - """ - def commit_library_and_verify(library_key): - """ - Commit library changes, and verify that there are no uncommited changes anymore - """ - last_published = ContentLibraryIndexer.get_items([library_key])[0]['last_published'] - self._commit_library_changes(str(library_key)) - response = ContentLibraryIndexer.get_items([library_key])[0] - assert response['has_unpublished_changes'] is False - assert response['has_unpublished_deletes'] is False - assert response['last_published'] >= last_published - return response - - def verify_uncommitted_libraries(library_key, has_unpublished_changes, has_unpublished_deletes): - """ - Verify uncommitted changes and deletes in the index - """ - response = ContentLibraryIndexer.get_items([library_key])[0] - assert response['has_unpublished_changes'] == has_unpublished_changes - assert response['has_unpublished_deletes'] == has_unpublished_deletes - return response - - lib = self._create_library(slug="test-lib-update-block", title="Title", description="Description") - library_key = LibraryLocatorV2.from_string(lib['id']) - - # Verify uncommitted new blocks - block = self._add_block_to_library(lib['id'], "problem", "problem1") - response = verify_uncommitted_libraries(library_key, True, False) - assert response['last_published'] is None - assert response['num_blocks'] == 1 - # Verify committed new blocks - self._commit_library_changes(lib['id']) - response = verify_uncommitted_libraries(library_key, False, False) - assert response['num_blocks'] == 1 - # Verify uncommitted deleted blocks - self._delete_library_block(block['id']) - response = verify_uncommitted_libraries(library_key, True, True) - assert response['num_blocks'] == 0 - # Verify committed deleted blocks - self._commit_library_changes(lib['id']) - response = verify_uncommitted_libraries(library_key, False, False) - assert response['num_blocks'] == 0 - - block = self._add_block_to_library(lib['id'], "problem", "problem1") - self._commit_library_changes(lib['id']) - - # Verify changes to blocks - # Verify OLX updates on blocks - self._set_library_block_olx(block["id"], "") - verify_uncommitted_libraries(library_key, True, False) - commit_library_and_verify(library_key) - # Verify asset updates on blocks - self._set_library_block_asset(block["id"], "whatever.png", b"data") - verify_uncommitted_libraries(library_key, True, False) - commit_library_and_verify(library_key) - self._delete_library_block_asset(block["id"], "whatever.png") - verify_uncommitted_libraries(library_key, True, False) - commit_library_and_verify(library_key) - - lib2 = self._create_library(slug="test-lib-update-block-2", title="Title 2", description="Description") - self._add_block_to_library(lib2["id"], "problem", "problem1") - self._commit_library_changes(lib2["id"]) - - #Verify new links on libraries - self._link_to_library(lib["id"], "library_2", lib2["id"]) - verify_uncommitted_libraries(library_key, True, False) - #Verify reverting uncommitted changes - self._revert_library_changes(lib["id"]) - verify_uncommitted_libraries(library_key, False, False) - - -@override_settings(FEATURES={**settings.FEATURES, 'ENABLE_CONTENT_LIBRARY_INDEX': True}) -@elasticsearch_test -class ContentLibraryIndexerTest( - ContentLibraryIndexerTestMixin, - ContentLibrariesRestApiTest, -): - """ - Tests the operation of ContentLibraryIndexer using the installed Blockstore app. - """ - - -class LibraryBlockIndexerTestMixin: - """ - Tests the operation of LibraryBlockIndexer - """ - - @elasticsearch_test - def setUp(self): - super().setUp() - ContentLibraryIndexer.remove_all_items() - LibraryBlockIndexer.remove_all_items() - self.searcher = SearchEngine.get_search_engine(LibraryBlockIndexer.INDEX_NAME) - - def test_index_block(self): - """ - Test if libraries are being indexed correctly - """ - lib = self._create_library(slug="test-lib-index-1", title="Title 1", description="Description") - block1 = self._add_block_to_library(lib['id'], "problem", "problem1") - block2 = self._add_block_to_library(lib['id'], "problem", "problem2") - - assert len(LibraryBlockIndexer.get_items()) == 2 - - for block in [block1, block2]: - usage_key = LibraryUsageLocatorV2.from_string(block['id']) - response = LibraryBlockIndexer.get_items([usage_key])[0] - - assert response['id'] == block['id'] - assert response['def_key'] == block['def_key'] - assert response['block_type'] == block['block_type'] - assert response['display_name'] == block['display_name'] - assert response['has_unpublished_changes'] == block['has_unpublished_changes'] - - def test_schema_updates(self): - """ - Test that outdated indexes aren't retrieved - """ - lib = self._create_library(slug="test-lib--block-schemaupdates-1", title="Title 1", description="Description") - with patch("openedx.core.djangoapps.content_libraries.libraries_index.LibraryBlockIndexer.SCHEMA_VERSION", - new=0): - block = self._add_block_to_library(lib['id'], "problem", "problem1") - assert len(LibraryBlockIndexer.get_items([block['id']])) == 1 - - with patch("openedx.core.djangoapps.content_libraries.libraries_index.LibraryBlockIndexer.SCHEMA_VERSION", - new=1): - assert len(LibraryBlockIndexer.get_items([block['id']])) == 0 - - call_command("reindex_content_library", all=True, force=True) - - assert len(LibraryBlockIndexer.get_items([block['id']])) == 1 - - def test_remove_all_items(self): - """ - Test if remove_all_items() deletes all libraries - """ - lib1 = self._create_library(slug="test-lib-rm-all", title="Title 1", description="Description") - self._add_block_to_library(lib1['id'], "problem", "problem1") - self._add_block_to_library(lib1['id'], "problem", "problem2") - assert len(LibraryBlockIndexer.get_items()) == 2 - - LibraryBlockIndexer.remove_all_items() - assert len(LibraryBlockIndexer.get_items()) == 0 - - def test_crud_block(self): - """ - Test that CRUD operations on blocks are reflected in the index - """ - lib = self._create_library(slug="test-lib-crud-block", title="Title", description="Description") - block = self._add_block_to_library(lib['id'], "problem", "problem1") - - # Update OLX, verify updates in index - self._set_library_block_olx(block["id"], '') - response = LibraryBlockIndexer.get_items([block['id']])[0] - assert response['display_name'] == 'new_name' - assert response['has_unpublished_changes'] is True - - # Verify has_unpublished_changes after committing library - self._commit_library_changes(lib['id']) - response = LibraryBlockIndexer.get_items([block['id']])[0] - assert response['has_unpublished_changes'] is False - - # Verify has_unpublished_changes after reverting library - self._set_library_block_asset(block["id"], "whatever.png", b"data") - response = LibraryBlockIndexer.get_items([block['id']])[0] - assert response['has_unpublished_changes'] is True - - self._revert_library_changes(lib['id']) - response = LibraryBlockIndexer.get_items([block['id']])[0] - assert response['has_unpublished_changes'] is False - - # Verify that deleting block removes it from index - self._delete_library_block(block['id']) - assert LibraryBlockIndexer.get_items([block['id']]) == [] - - # Verify that deleting a library removes its blocks from index too - self._add_block_to_library(lib['id'], "problem", "problem1") - LibraryBlockIndexer.get_items([block['id']]) - self._delete_library(lib['id']) - assert LibraryBlockIndexer.get_items([block['id']]) == [] - - -@override_settings(FEATURES={**settings.FEATURES, 'ENABLE_CONTENT_LIBRARY_INDEX': True}) -@elasticsearch_test -class LibraryBlockIndexerTest( - LibraryBlockIndexerTestMixin, - ContentLibrariesRestApiTest, -): - """ - Tests the operation of LibraryBlockIndexer using the installed Blockstore app. - """ diff --git a/openedx/core/djangoapps/content_libraries/views.py b/openedx/core/djangoapps/content_libraries/views.py index c83a9c4fcb56..bd5f091b136c 100644 --- a/openedx/core/djangoapps/content_libraries/views.py +++ b/openedx/core/djangoapps/content_libraries/views.py @@ -172,12 +172,12 @@ def get(self, request): paginator = LibraryApiPagination() queryset = api.get_libraries_for_user(request.user, org=org, library_type=library_type) if text_search: - result = api.get_metadata_from_index(queryset, text_search=text_search) + result = api.get_metadata(queryset, text_search=text_search) result = paginator.paginate_queryset(result, request) else: # We can paginate queryset early and prevent fetching unneeded metadata paginated_qs = paginator.paginate_queryset(queryset, request) - result = api.get_metadata_from_index(paginated_qs) + result = api.get_metadata(paginated_qs) serializer = ContentLibraryMetadataSerializer(result, many=True) # Verify `pagination` param to maintain compatibility with older diff --git a/xmodule/library_tools.py b/xmodule/library_tools.py index 6d8106caeaf3..0e3c3af51063 100644 --- a/xmodule/library_tools.py +++ b/xmodule/library_tools.py @@ -185,7 +185,7 @@ def list_available_libraries(self): for lib in self.store.get_library_summaries() ] v2_query = library_api.get_libraries_for_user(user) - v2_libs_with_meta = library_api.get_metadata_from_index(v2_query) + v2_libs_with_meta = library_api.get_metadata(v2_query) v2_libs = [(lib.key, lib.title) for lib in v2_libs_with_meta] if settings.FEATURES.get('ENABLE_LIBRARY_AUTHORING_MICROFRONTEND'):