diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 2ebd5ff1..c138fb65 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -17,6 +17,9 @@ jobs: python-version: ['3.11', '3.12'] toxenv: [django42, django52, quality] + env: + MEILISEARCH_URL: http://localhost:7700 + steps: - uses: actions/checkout@v2 - name: setup python @@ -24,8 +27,11 @@ jobs: with: python-version: ${{ matrix.python-version }} + - name: Start Meilisearch + run: make meili-up + - name: Start Elasticsearch - run: make test.start_elasticsearch + run: make elastic-up - name: Install pip run: pip install -r requirements/pip.txt diff --git a/Makefile b/Makefile index adff3746..4e746d8d 100644 --- a/Makefile +++ b/Makefile @@ -59,3 +59,34 @@ test: test_with_es ## run tests and generate coverage report install-local: ## installs your local edx-search into the LMS and CMS python virtualenvs docker exec -t edx.devstack.lms bash -c '. /edx/app/edxapp/venvs/edxapp/bin/activate && cd /edx/app/edxapp/edx-platform && pip uninstall -y edx-search && pip install -e /edx/src/edx-search && pip freeze | grep edx-search' docker exec -t edx.devstack.cms bash -c '. /edx/app/edxapp/venvs/edxapp/bin/activate && cd /edx/app/edxapp/edx-platform && pip uninstall -y edx-search && pip install -e /edx/src/edx-search && pip freeze | grep edx-search' + +test-all: create-test-network meili-up elastic-up + @MEILISEARCH_MASTER_KEY=test_master_key python manage.py test || true + @$(MAKE) meili-down + @$(MAKE) elastic-down + + +meili-up: create-test-network + @echo "Starting Meilisearch..." + @docker compose up -d test_meilisearch + @echo "Waiting for Meilisearch to be healthy..." + @timeout 15 bash -c \ + 'until curl -sf http://localhost:7700/health > /dev/null; do echo "Waiting..."; sleep 1; done' + +meili-down: + @echo "Shutting down Meilisearch..." + @docker compose down test_meilisearch + + +elastic-up: create-test-network + @echo "Starting Elasticsearch..." + @docker compose up -d test_elasticsearch + @echo "Waiting for Elasticsearch to be healthy..." + @timeout 30 bash -c 'until curl -s http://localhost:9200/_cluster/health | grep -q "status"; do echo "Waiting..."; sleep 2; done' + +elastic-down: + @echo "Shutting down Elasticsearch..." + docker compose down test_elasticsearch + +create-test-network: + docker network inspect test_network >/dev/null 2>&1 || docker network create --driver bridge test_network diff --git a/docker-compose.yml b/docker-compose.yml index 2be4003b..15276afc 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -1,3 +1,5 @@ +version: '3.8' + services: test_elasticsearch: @@ -18,7 +20,31 @@ services: - data01:/usr/share/elasticsearch/data ports: - "9200:9200" + networks: + - test_network + + test_meilisearch: + # Keep in sync with DOCKER_IMAGE_MEILISEARCH + # in https://github.com/overhangio/tutor/blob/main/tutor/templates/config/defaults.yml + image: getmeili/meilisearch:v1.8.4 + ports: + - "7700:7700" + networks: + - test_network + environment: + MEILISEARCH_MASTER_KEY: test_master_key + healthcheck: + test: ["CMD", "curl", "-f", "http://localhost:7700/health"] + interval: 2s + timeout: 1s + retries: 10 volumes: data01: driver: local + +networks: + test_network: + name: test_network + driver: bridge + external: true diff --git a/edxsearch/settings.py b/edxsearch/settings.py index e200eaff..614c89b7 100644 --- a/edxsearch/settings.py +++ b/edxsearch/settings.py @@ -26,7 +26,7 @@ # This is just a container for running tests DEBUG = True -ALLOWED_HOSTS = [] +ALLOWED_HOSTS = ['testserver', 'localhost', '127.0.0.1'] TEMPLATES = [ { @@ -128,3 +128,14 @@ } } } + +MEILISEARCH_API_KEY = os.environ.get("MEILISEARCH_MASTER_KEY", "test_master_key") +MEILISEARCH_URL = os.environ.get("MEILISEARCH_URL", "http://localhost:7700") + +ELASTIC_SEARCH_CONFIG = [ + { + 'use_ssl': False, + 'host': 'localhost', + 'port': 9200 + } +] diff --git a/search/api.py b/search/api.py index 81c77994..87586cd1 100644 --- a/search/api.py +++ b/search/api.py @@ -129,6 +129,7 @@ def course_discovery_search( from_=0, field_dictionary=None, enable_course_sorting_by_start_date=False, + is_multivalue=False, # Determines whether to use single-faceted or multi-faceted search ): """ Course Discovery activities against the search engine index of course details @@ -166,6 +167,7 @@ def course_discovery_search( exclude_dictionary=exclude_dictionary, aggregation_terms=course_discovery_aggregations(), sort_by=sort_by, + is_multivalue=is_multivalue, ) return results diff --git a/search/elastic.py b/search/elastic.py index d2144840..4607abd7 100644 --- a/search/elastic.py +++ b/search/elastic.py @@ -23,7 +23,7 @@ RESERVED_CHARACTERS = "+=> dict: + """ + Compute the aggregation requests that we'll send with the query when searching with multi-value faceting. + Typical shape of the args: + aggregation_terms={'language': {}, 'modes': {}, 'org': {}} + field_dictionary={'enrollment_start': , + 'language': ['en', 'fr']} + """ + aggs = {} + for facet_field, options in aggregation_terms.items(): + filters_excluding_facet = { + field: value for field, value in field_dictionary.items() + if field != facet_field + } + filter_clauses = [ + _get_filter_field(field, value) + for field, value in filters_excluding_facet.items() + if value + ] + facet_filter = { + "bool": { + "must": filter_clauses + } + } if filter_clauses else {"match_all": {}} + + aggs[facet_field] = { + "filter": facet_filter, + "aggs": { + "values": { + "terms": { + "field": facet_field, + **options + } + } + } + } + + return { + "global_aggs": { + "global": {}, + "aggs": aggs + } + } + + class ElasticSearchEngine(SearchEngine): """ ElasticSearch implementation of SearchEngine abstraction @@ -654,8 +717,13 @@ def search(self, } body = {"query": query} + + is_multivalue = kwargs.pop("is_multivalue", False) if aggregation_terms: - body["aggs"] = _process_aggregation_terms(aggregation_terms) + if is_multivalue: + body["aggs"] = _process_multivalue_aggregations(aggregation_terms, field_dictionary) + else: + body["aggs"] = _process_aggregation_terms(aggregation_terms) if sort_by: body["sort"] = self._transform_sort_by(sort_by) @@ -668,7 +736,7 @@ def search(self, log.exception("error while searching index - %r", ex) raise - return _translate_hits(es_response) + return _translate_hits(es_response, aggregation_terms, is_multivalue) def _transform_sort_by(self, fields: list[SortField]): """ diff --git a/search/meilisearch.py b/search/meilisearch.py index 6e25972f..5264d74c 100644 --- a/search/meilisearch.py +++ b/search/meilisearch.py @@ -180,19 +180,55 @@ def search( """ See meilisearch docs: https://www.meilisearch.com/docs/reference/api/search """ + is_multivalue = kwargs.pop("is_multivalue", False) opt_params = get_search_params( field_dictionary=field_dictionary, filter_dictionary=filter_dictionary, exclude_dictionary=exclude_dictionary, aggregation_terms=aggregation_terms, sort_by=self._transform_sort_by(sort_by) if sort_by else None, + is_multivalue=is_multivalue, **kwargs, ) if log_search_params: logger.info("Search query: opt_params=%s", opt_params) meilisearch_results = self.meilisearch_index.search(query_string, opt_params) - processed_results = process_results(meilisearch_results, self.index_name) - return processed_results + + if is_multivalue: + self._expand_facet_distibutions(field_dictionary, query_string, opt_params, meilisearch_results) + + return process_results(meilisearch_results, self.index_name) + + def _expand_facet_distibutions( + self, + field_dictionary: dict, + query_string: str, + opt_params: dict, + meilisearch_results: dict + ) -> dict: + """ + For each selected facet, get all its available options within the selected filters. + """ + for facet in field_dictionary.keys(): + expanded_facet_distribution = self._get_expanded_distribution( + query_string, + facet, + opt_params.get("filter", []), + ) + meilisearch_results.setdefault("facetDistribution", {})[facet] = expanded_facet_distribution + + def _get_expanded_distribution(self, query: str, facet_to_exclude: str, filter_rules: list) -> dict: + """ + Run a secondary query excluding one facet to get its full distribution. + Only return distribution data, without any actual results. + """ + secondary_opt_params = { + 'facets': [facet_to_exclude], + 'filter': [rule for rule in filter_rules if not rule.startswith(f"{facet_to_exclude} = ")], + 'limit': 0, + } + result = self.meilisearch_index.search(query, secondary_opt_params) + return result.get("facetDistribution", {}).get(facet_to_exclude, {}) def remove(self, doc_ids, **kwargs): """ @@ -408,11 +444,11 @@ def get_search_params( # Exclusion and inclusion filters filters = [] if field_dictionary: - filters += get_filter_rules(field_dictionary) + filters += get_filter_rules(field_dictionary, or_fields=params.get("facets")) if filter_dictionary: - filters += get_filter_rules(filter_dictionary, optional=True) + filters += get_filter_rules(filter_dictionary, optional=True, or_fields=params.get("facets")) if exclude_dictionary: - filters += get_filter_rules(exclude_dictionary, exclude=True) + filters += get_filter_rules(exclude_dictionary, exclude=True, or_fields=params.get("facets")) if filters: params["filter"] = filters @@ -426,26 +462,35 @@ def get_search_params( def get_filter_rules( - rule_dict: dict[str, t.Any], exclude: bool = False, optional: bool = False + rule_dict: dict[str, t.Any], exclude: bool = False, optional: bool = False, or_fields: list[str] | None = None, ) -> list[str | list[str]]: """ Convert inclusion/exclusion rules. """ + or_fields = or_fields or [] rules = [] - for key, value in rule_dict.items(): - if isinstance(value, list): - key_rules = [ - get_filter_rule(key, v, exclude=exclude, optional=optional) - for v in value - ] + for field_name, field_value in rule_dict.items(): + if isinstance(field_value, list): if exclude: - rules.extend(key_rules) + # Always flat list of NOT rules + for nested_value in field_value: + rules.append(get_filter_rule(field_name, nested_value, exclude=True, optional=optional)) else: - rules.append(key_rules) + if field_name in or_fields: + # Multi-value facet → OR logic as a single string + assert not optional, "optional=True not supported in OR filter branch" + or_expr = " OR ".join(f'{field_name} = "{nested_value}"' for nested_value in field_value) + rules.append(or_expr) + else: + # Non-facet field → multiple AND rules + rules += [ + get_filter_rule( + field_name, nested_value, exclude=exclude, optional=optional + ) for nested_value in field_value + ] else: - rules.append( - get_filter_rule(key, value, exclude=exclude, optional=optional) - ) + rules.append(get_filter_rule(field_name, field_value, exclude=exclude, optional=optional)) + return rules diff --git a/search/tests/test_course_discovery.py b/search/tests/test_course_discovery.py index 409292ea..22705086 100644 --- a/search/tests/test_course_discovery.py +++ b/search/tests/test_course_discovery.py @@ -4,6 +4,7 @@ """ Tests for search functionalty """ import copy +import time from datetime import datetime import ddt @@ -15,6 +16,7 @@ from search.api import course_discovery_search, NoSearchEngineError from search.elastic import ElasticSearchEngine from search.tests.utils import SearcherMixin, TEST_INDEX_NAME +from search.meilisearch import get_meilisearch_client, create_indexes from .mock_search_engine import MockSearchEngine @@ -398,6 +400,80 @@ def test_course_matching(self, term, result_count): results = course_discovery_search(term) self.assertEqual(results["total"], result_count) + def test_aggregating_with_single_values_in_two_facets(self): + DemoCourse.get_and_index(self.searcher, { + "language": "en", + "org": "EDX", + "modes": "audit", + }) + + DemoCourse.get_and_index(self.searcher, { + "language": "en", + "org": "ORG2", + "modes": "honor", + }) + + results = course_discovery_search( + search_term="", + field_dictionary={"language": "en", "org": "EDX"} + ) + + self.assertIn("audit", results["aggs"]["modes"]["terms"]) + self.assertNotIn("honor", results["aggs"]["modes"]["terms"]) + self.assertEqual(results["aggs"]["language"]["terms"], {"en": 1}) + self.assertEqual(results["aggs"]["org"]["terms"], {"EDX": 1}) + + def test_aggregating_with_multi_value_facet(self): + DemoCourse.get_and_index(self.searcher, { + "org": "EDX", + "language": "en", + "modes": "audit", + }) + + DemoCourse.get_and_index(self.searcher, { + "org": "EDX", + "language": "fr", + "modes": "honor", + }) + + DemoCourse.get_and_index(self.searcher, { + "org": "ORG2", + "language": "uk", + "modes": "verified", + }) + + results = course_discovery_search( + search_term="", + field_dictionary={"language": ["en", "fr"]}, + is_multivalue=True + ) + + aggregations = results["aggs"] + self.assertIn("en", aggregations["language"]["terms"]) + self.assertIn("fr", aggregations["language"]["terms"]) + self.assertIn("uk", aggregations["language"]["terms"]) + self.assertDictEqual(aggregations["language"]["terms"], {"en": 1, "fr": 1, "uk": 1}) + + self.assertNotIn("verified", aggregations["modes"]["terms"]) + self.assertDictEqual(aggregations["modes"]["terms"], {"audit": 1, "honor": 1}) + + self.assertNotIn("ORG2", aggregations["org"]["terms"]) + self.assertDictEqual(aggregations["org"]["terms"], {"EDX": 2}) + + def test_aggregating_facet_narrowed_if_single_value_search(self): + DemoCourse.get_and_index(self.searcher, {"language": "en", "modes": "audit"}) + + DemoCourse.get_and_index(self.searcher, {"language": "en", "modes": "honor"}) + + results = course_discovery_search( + search_term="", + field_dictionary={"modes": ["honor"]}, + is_multivalue=False + ) + + self.assertNotIn("audit", results["aggs"]["modes"]["terms"]) + self.assertDictEqual(results["aggs"]["modes"]["terms"], {'honor': 1}) + @override_settings(SEARCH_ENGINE=None) class TestNone(TestCase): @@ -407,3 +483,283 @@ def test_perform_search(self): """ search opertaion should yeild an exception with no search engine """ with self.assertRaises(NoSearchEngineError): course_discovery_search("abc test") + + +@override_settings(SEARCH_ENGINE="search.meilisearch.MeilisearchEngine") +@override_settings(COURSEWARE_INFO_INDEX_NAME=TEST_INDEX_NAME) +class TestMeilisearchCourseDiscoverySearch(TestCase, SearcherMixin): + """ + Integration tests using real Meilisearch engine. + """ + + def setUp(self): + super().setUp() + create_indexes({TEST_INDEX_NAME: [ + "language", + "modes", + "org", + "catalog_visibility", + "enrollment_start", + "enrollment_end", + ]}) + self.wait_for_meilisearch_indexing() + + def tearDown(self): + client = get_meilisearch_client() + client.index(TEST_INDEX_NAME).delete() + super().tearDown() + + @staticmethod + def wait_for_meilisearch_indexing(): + """Helper method adding a tiny delay for Meilisearch to finish updating the index.""" + client = get_meilisearch_client() + task = client.index(TEST_INDEX_NAME).get_tasks().results[-1] + client.wait_for_task(task.uid) + time.sleep(0.2) + + def test_course_matching_empty_index(self): + """ Check for empty result count before indexing """ + results = course_discovery_search("defensive") + self.assertEqual(results["total"], 0) + + def test_course_matching(self): + """ Make sure that matches within content can be located and processed """ + DemoCourse.get_and_index(self.searcher, { + "content": { + "short_description": "This is a defensive move", + "overview": "Defensive teams often win" + } + }) + self.wait_for_meilisearch_indexing() + + DemoCourse.get_and_index(self.searcher, { + "content": { + "short_description": "This is an offensive move", + "overview": "Offensive teams often win" + } + }) + self.wait_for_meilisearch_indexing() + + DemoCourse.get_and_index(self.searcher, { + "content": { + "short_description": "This is a hyphenated move", + "overview": "Highly-offensive teams often win" + } + }) + self.wait_for_meilisearch_indexing() + + results = course_discovery_search() + self.assertEqual(results["total"], 3) + + @override_settings(COURSE_DISCOVERY_AGGREGATIONS={"subject": {}, "lang": {}}) + def test_aggregating_override(self): + """ + Test that aggregation under consideration can be specified + with custom setting + """ + create_indexes({TEST_INDEX_NAME: [ + "lang", + "subject", + ]}) + + DemoCourse.get_and_index(self.searcher, {"subject": "Mathematics", "lang": ["en", "fr"]}) + DemoCourse.get_and_index(self.searcher, {"subject": "Mathematics", "lang": ["en"]}) + DemoCourse.get_and_index(self.searcher, {"subject": "History", "lang": ["en"]}) + DemoCourse.get_and_index(self.searcher, {"subject": "History", "lang": ["fr"]}) + DemoCourse.get_and_index(self.searcher, {"lang": ["de"]}) + self.wait_for_meilisearch_indexing() + + results = course_discovery_search() + self.assertEqual(results["total"], 5) + self.assertIn("aggs", results) + + self.assertNotIn("org", results["aggs"]) + self.assertNotIn("modes", results["aggs"]) + + self.assertIn("subject", results["aggs"]) + self.assertEqual(results["aggs"]["subject"]["total"], 4) + self.assertEqual(results["aggs"]["subject"]["terms"]["Mathematics"], 2) + self.assertEqual(results["aggs"]["subject"]["terms"]["History"], 2) + + self.assertIn("lang", results["aggs"]) + self.assertEqual(results["aggs"]["lang"]["total"], 6) + self.assertEqual(results["aggs"]["lang"]["terms"]["en"], 3) + self.assertEqual(results["aggs"]["lang"]["terms"]["fr"], 2) + self.assertEqual(results["aggs"]["lang"]["terms"]["de"], 1) + + def test_course_list(self): + """ No arguments to course_discovery_search should show all available courses""" + results = course_discovery_search() + self.assertEqual(results["total"], 0) + + DemoCourse.get_and_index(self.searcher) + self.wait_for_meilisearch_indexing() + results = course_discovery_search() + self.assertEqual(results["total"], 1) + + def test_discovery_field_matching(self): + """ Test that field specifications only show those results with the desired field values """ + DemoCourse.get_and_index(self.searcher, {"org": "OrgA"}) + DemoCourse.get_and_index(self.searcher, {"org": "OrgB"}) + self.wait_for_meilisearch_indexing() + + results = course_discovery_search() + self.assertEqual(results["total"], 2) + + results = course_discovery_search(field_dictionary={"org": "OrgA"}) + self.assertEqual(results["total"], 1) + self.assertEqual(results["results"][0]["data"]["org"], "OrgA") + + results = course_discovery_search(field_dictionary={"org": "OrgB"}) + self.assertEqual(results["total"], 1) + self.assertEqual(results["results"][0]["data"]["org"], "OrgB") + + def test_multivalue_field_matching(self): + """ + Test that field specifications only show those results with the desired + field values - even when there is an array of possible values + """ + DemoCourse.get_and_index(self.searcher, {"modes": ["honor", "verified"]}) + DemoCourse.get_and_index(self.searcher, {"modes": ["honor"]}) + self.wait_for_meilisearch_indexing() + + results = course_discovery_search() + self.assertEqual(results["total"], 2) + + results = course_discovery_search(field_dictionary={"modes": "honor"}) + self.assertEqual(results["total"], 2) + + results = course_discovery_search(field_dictionary={"modes": "verified"}) + self.assertEqual(results["total"], 1) + + def test_enroll_date(self): + """ + Test that we don't show any courses that have no published enrollment date, or an enrollment date in the future + """ + # demo_course_1 should be found cos it has a date that is valid + DemoCourse.get_and_index(self.searcher, {"enrollment_start": datetime(2014, 1, 1)}) + + # demo_course_2 should not be found because it has enrollment_start date set explicitly to None + DemoCourse.get_and_index(self.searcher, {"enrollment_start": None}) + + # demo_course_3 should not be found because it has enrollment_start date in the future + DemoCourse.get_and_index(self.searcher, {"enrollment_start": datetime(2114, 1, 1)}) + + # demo_course_4 should not be found because it has no enrollment_start specification + DemoCourse.get_and_index(self.searcher, {}, ["enrollment_start"]) + + self.wait_for_meilisearch_indexing() + + results = course_discovery_search() + self.assertEqual(results["total"], 1) + + additional_course = DemoCourse.get() + DemoCourse.index(self.searcher, [additional_course]) + self.wait_for_meilisearch_indexing() + + results = course_discovery_search() + self.assertEqual(results["total"], 2) + + # Mark the course as having ended enrollment + additional_course["enrollment_end"] = datetime(2015, 1, 1) + DemoCourse.index(self.searcher, [additional_course]) + self.wait_for_meilisearch_indexing() + + results = course_discovery_search() + self.assertEqual(results["total"], 1) + + def test_aggregating(self): + DemoCourse.get_and_index(self.searcher, {"language": "en", "org": "EDX"}) + self.wait_for_meilisearch_indexing() + + DemoCourse.get_and_index(self.searcher, {"language": "fr", "org": "ORG2"}) + self.wait_for_meilisearch_indexing() + + results = course_discovery_search( + search_term="", + field_dictionary={"language": ["en"]}, + is_multivalue=True + ) + self.assertDictEqual(results["aggs"]["language"]["terms"], {"en": 1, "fr": 1}) + self.assertDictEqual(results["aggs"]["org"]["terms"], {"EDX": 1}) + + def test_aggregating_with_single_values_in_two_facets(self): + DemoCourse.get_and_index(self.searcher, { + "language": "en", + "org": "EDX", + "modes": "audit", + }) + self.wait_for_meilisearch_indexing() + + DemoCourse.get_and_index(self.searcher, { + "language": "en", + "org": "ORG2", + "modes": "honor", + }) + self.wait_for_meilisearch_indexing() + + results = course_discovery_search( + search_term="", + field_dictionary={"language": "en", "org": "EDX"} + ) + + self.assertIn("audit", results["aggs"]["modes"]["terms"]) + self.assertNotIn("honor", results["aggs"]["modes"]["terms"]) + self.assertEqual(results["aggs"]["language"]["terms"], {"en": 1}) + self.assertEqual(results["aggs"]["org"]["terms"], {"EDX": 1}) + + def test_aggregating_with_multi_value_facet(self): + DemoCourse.get_and_index(self.searcher, { + "org": "EDX", + "language": "en", + "modes": "audit", + }) + self.wait_for_meilisearch_indexing() + + DemoCourse.get_and_index(self.searcher, { + "org": "EDX", + "language": "fr", + "modes": "honor", + }) + self.wait_for_meilisearch_indexing() + + DemoCourse.get_and_index(self.searcher, { + "org": "ORG2", + "language": "uk", + "modes": "verified", + }) + self.wait_for_meilisearch_indexing() + + results = course_discovery_search( + search_term="", + field_dictionary={"language": ["en", "fr"]}, + is_multivalue=True + ) + + aggregations = results["aggs"] + self.assertIn("en", aggregations["language"]["terms"]) + self.assertIn("fr", aggregations["language"]["terms"]) + self.assertIn("uk", aggregations["language"]["terms"]) + self.assertDictEqual(aggregations["language"]["terms"], {"en": 1, "fr": 1, "uk": 1}) + + self.assertNotIn("verified", aggregations["modes"]["terms"]) + self.assertDictEqual(aggregations["modes"]["terms"], {"audit": 1, "honor": 1}) + + self.assertNotIn("ORG2", aggregations["org"]["terms"]) + self.assertDictEqual(aggregations["org"]["terms"], {"EDX": 2}) + + def test_aggregating_facet_narrowed_if_single_value_search(self): + DemoCourse.get_and_index(self.searcher, {"language": "en", "modes": "audit"}) + self.wait_for_meilisearch_indexing() + + DemoCourse.get_and_index(self.searcher, {"language": "en", "modes": "honor"}) + self.wait_for_meilisearch_indexing() + + results = course_discovery_search( + search_term="", + field_dictionary={"modes": ["honor"]}, + is_multivalue=False + ) + + self.assertNotIn("audit", results["aggs"]["modes"]["terms"]) + self.assertDictEqual(results["aggs"]["modes"]["terms"], {'honor': 1}) diff --git a/search/tests/test_elasticsearch.py b/search/tests/test_elasticsearch.py new file mode 100644 index 00000000..d4ff1d30 --- /dev/null +++ b/search/tests/test_elasticsearch.py @@ -0,0 +1,341 @@ +#!/usr/bin/env python +# Some of the subclasses that get used as settings-overrides will yield this pylint +# error, but they do get used when included as part of the override_settings +""" Tests for search functionality """ + +from unittest.mock import patch, MagicMock + +from django.test import TestCase +from django.test.utils import override_settings +from elasticsearch import exceptions +from elasticsearch.helpers import BulkIndexError +from search.elastic import RESERVED_CHARACTERS, ElasticSearchEngine +from search.tests.tests import MockSearchTests +from search.tests.utils import TEST_INDEX_NAME, ErroringElasticImpl, SearcherMixin + + +@override_settings(ELASTIC_SEARCH_INDEX_PREFIX='prefixed_') +@override_settings(SEARCH_ENGINE="search.tests.utils.ForceRefreshElasticSearchEngine") +class ElasticSearchPrefixTests(MockSearchTests): + """ + Override that runs the same tests for ElasticSearchTests, + but with a prefixed index name. + """ + + @property + def index_name(self): + """ + The search index name to be used for this test. + """ + return f"prefixed_{TEST_INDEX_NAME}" + + +@override_settings(SEARCH_ENGINE="search.tests.utils.ForceRefreshElasticSearchEngine") +class ElasticSearchTests(MockSearchTests): + """ Override that runs the same tests for ElasticSearchEngine instead of MockSearchEngine """ + + def test_reserved_characters(self): + """ Make sure that we handle when reserved characters were passed into query_string """ + test_string = "What the ! is this?" + self.searcher.index([{"content": {"name": test_string}}]) + + response = self.searcher.search_string(test_string) + self.assertEqual(response["total"], 1) + + response = self.searcher.search_string("something else !") + self.assertEqual(response["total"], 0) + + response = self.searcher.search_string("something ! else") + self.assertEqual(response["total"], 0) + + for char in RESERVED_CHARACTERS: + # previously these would throw exceptions + response = self.searcher.search_string(char) + self.assertEqual(response["total"], 0) + + def test_aggregation_options(self): + """ + Test that aggregate options work alongside aggregations - notice + unsupported in mock for now size - is the only option for now + """ + self._index_for_aggs() + + response = self.searcher.search() + self.assertEqual(response["total"], 7) + self.assertNotIn("aggs", response) + + aggregation_terms = { + "subject": {"size": 2}, + "org": {"size": 2} + } + response = self.searcher.search(aggregation_terms=aggregation_terms) + self.assertEqual(response["total"], 7) + self.assertIn("aggs", response) + aggregation_results = response["aggs"] + self.assertEqual(aggregation_results["subject"]["total"], 6) + subject_term_counts = aggregation_results["subject"]["terms"] + self.assertEqual(subject_term_counts["mathematics"], 3) + self.assertEqual(subject_term_counts["physics"], 2) + self.assertNotIn("history", subject_term_counts) + self.assertEqual(aggregation_results["subject"]["other"], 1) + + self.assertEqual(aggregation_results["org"]["total"], 7) + org_term_counts = aggregation_results["org"]["terms"] + self.assertEqual(org_term_counts["Harvard"], 4) + self.assertEqual(org_term_counts["MIT"], 2) + self.assertNotIn("edX", org_term_counts) + self.assertEqual(aggregation_results["org"]["other"], 1) + + +@override_settings(SEARCH_ENGINE="search.tests.utils.ForceRefreshElasticSearchEngine") +@override_settings(ELASTIC_SEARCH_IMPL=ErroringElasticImpl) +class ErroringElasticTests(TestCase, SearcherMixin): + """ testing handling of elastic exceptions when they happen """ + + def test_index_failure_bulk(self): + """ the index operation should fail """ + with patch('search.elastic.bulk', return_value=[0, [exceptions.ElasticsearchException()]]): + with self.assertRaises(exceptions.ElasticsearchException): + self.searcher.index([{"name": "abc test"}]) + + def test_index_failure_general(self): + """ the index operation should fail """ + with patch('search.elastic.bulk', side_effect=Exception()): + with self.assertRaises(Exception): + self.searcher.index([{"name": "abc test"}]) + + def test_search_failure(self): + """ the search operation should fail """ + with self.assertRaises(exceptions.ElasticsearchException): + self.searcher.search("abc test") + + def test_remove_failure_bulk(self): + """ the remove operation should fail """ + doc_id = 'test_id' + error = {'delete': { + 'status': 500, '_index': 'test_index', '_version': 1, 'found': True, '_id': doc_id + }} + with patch('search.elastic.bulk', side_effect=BulkIndexError('Simulated error', [error])): + with self.assertRaises(BulkIndexError): + self.searcher.remove(["test_id"]) + + def test_remove_failure_general(self): + """ the remove operation should fail """ + with patch('search.elastic.bulk', side_effect=Exception()): + with self.assertRaises(Exception): + self.searcher.remove(["test_id"]) + + +@override_settings(SEARCH_ENGINE="search.elastic.ElasticSearchEngine") +@override_settings(ELASTIC_SEARCH_CONFIG=[{'host': '127.0.0.1'}, {'host': 'localhost'}]) +class ElasticConfigTest(TestCase, SearcherMixin): + """ Tests correct configuration of the elasticsearch instance. """ + + def test_config(self): + """ should be configured with the correct hosts """ + elasticsearch = self.searcher._es # pylint: disable=protected-access + hosts = elasticsearch.transport.hosts + self.assertEqual(hosts, [{'host': '127.0.0.1'}, {'host': 'localhost'}]) + + +class ElasticSearchUnitTests(TestCase): + """ + ElasticSearch tests. + """ + + @patch("search.elastic.Elasticsearch") + def test_multivalue_aggregations_translated_correctly(self, mock_elasticsearch_class): + """Tests that multivalue facet aggregations return full facet buckets despite filtering.""" + mock_es = MagicMock() + mock_elasticsearch_class.return_value = mock_es + + mock_es.search.return_value = { + "hits": { + "total": {"value": 2}, + "max_score": 1.0, + "hits": [ + { + "_source": {"org": "OrgA", "language": "en"}, + "_score": 1.0 + }, + { + "_source": {"org": "OrgC", "language": "en"}, + "_score": 0.8 + } + ] + }, + "aggregations": { + "global_aggs": { + "language": { + "doc_count": 3, + "values": { + "buckets": [ + {"key": "en", "doc_count": 2}, + {"key": "fr", "doc_count": 1} + ] + } + }, + "org": { + "doc_count": 2, + "values": { + "buckets": [ + {"key": "OrgA", "doc_count": 1}, + {"key": "OrgC", "doc_count": 1} + ] + } + } + } + }, + "took": 2, + } + + engine = ElasticSearchEngine(index=TEST_INDEX_NAME) + + result = engine.search( + field_dictionary={"language": ["en"]}, + aggregation_terms={ + "language": {}, + "org": {}, + }, + is_multivalue=True + ) + + self.assertEqual(result["total"], 2) + self.assertEqual(result["aggs"]["language"]["terms"]["en"], 2) + self.assertEqual(result["aggs"]["language"]["terms"]["fr"], 1) + self.assertEqual(set(result["aggs"]["org"]["terms"].keys()), {"OrgA", "OrgC"}) + + mock_es.search.assert_called_once() + + @patch("search.elastic.Elasticsearch") + def test_multivalue_with_empty_filters_uses_match_all(self, mock_elasticsearch_class): + """Tests that multivalue aggregation works when no filters are applied.""" + mock_es = MagicMock() + mock_elasticsearch_class.return_value = mock_es + + mock_es.search.return_value = { + "hits": { + "total": {"value": 3}, + "max_score": 0.0, + "hits": [] + }, + "aggregations": { + "global_aggs": { + "language": { + "doc_count": 3, + "values": { + "buckets": [ + {"key": "en", "doc_count": 2}, + {"key": "fr", "doc_count": 1} + ] + } + } + } + }, + "took": 2, + } + + engine = ElasticSearchEngine(index=TEST_INDEX_NAME) + + result = engine.search( + aggregation_terms={"language": {}}, + field_dictionary={}, + is_multivalue=True + ) + + self.assertEqual(result["total"], 3) + self.assertEqual(result["aggs"]["language"]["terms"]["en"], 2) + self.assertEqual(result["aggs"]["language"]["terms"]["fr"], 1) + + @patch("search.elastic.Elasticsearch") + def test_regular_aggregations_do_not_use_global_aggs(self, mock_elasticsearch_class): + """Tests that single-value aggregation does not include global_aggs wrapper.""" + mock_es = MagicMock() + mock_elasticsearch_class.return_value = mock_es + mock_es.search.return_value = { + "hits": { + "total": {"value": 1}, + "max_score": 1.0, + "hits": [{ + "_source": {"org": "OrgX", "language": "en"}, + "_score": 1.0 + }] + }, + "aggregations": { + "language": { + "buckets": [ + {"key": "en", "doc_count": 1} + ], + "doc_count_error_upper_bound": 0, + "sum_other_doc_count": 0 + }, + "total_language_docs": {"value": 1.0}, + "total_modes_docs": {"value": 1.0}, + "total_org_docs": {"value": 1.0} + }, + "took": 2, + } + + engine = ElasticSearchEngine(index=TEST_INDEX_NAME) + + result = engine.search( + field_dictionary={"language": ["en"]}, + aggregation_terms={"language": {}}, + is_multivalue=False + ) + + self.assertEqual(result["total"], 1) + self.assertEqual(result["aggs"]["language"]["terms"]["en"], 1) + + call_args = mock_es.search.call_args[1] + search_body = call_args["body"] + self.assertIn("aggs", search_body) + self.assertIn("language", search_body["aggs"]) + self.assertNotIn("global_aggs", search_body["aggs"]) + + @patch("search.elastic.Elasticsearch") + def test_multivalue_aggregations_use_global_aggs(self, mock_elasticsearch_class): + """Tests that multi-value aggregation includes global_aggs wrapper.""" + mock_es = MagicMock() + mock_elasticsearch_class.return_value = mock_es + mock_es.search.return_value = { + "hits": { + "total": {"value": 1}, + "max_score": 1.0, + "hits": [{ + "_source": {"org": "OrgX", "language": "en"}, + "_score": 1.0 + }] + }, + "aggregations": { + "global_aggs": { + "language": { + "doc_count": 1, + "values": { + "buckets": [ + {"key": "en", "doc_count": 1} + ] + } + } + } + }, + "took": 2, + } + + engine = ElasticSearchEngine(index=TEST_INDEX_NAME) + + result = engine.search( + field_dictionary={"language": ["en"]}, + aggregation_terms={"language": {}}, + is_multivalue=True + ) + + self.assertEqual(result["total"], 1) + self.assertEqual(result["aggs"]["language"]["terms"]["en"], 1) + + call_args = mock_es.search.call_args[1] + search_body = call_args["body"] + + self.assertIn("aggs", search_body) + self.assertIn("global_aggs", search_body["aggs"]) + self.assertIn("language", search_body["aggs"]["global_aggs"]["aggs"]) diff --git a/search/tests/test_engines.py b/search/tests/test_engines.py index 7888041c..ed078a2e 100644 --- a/search/tests/test_engines.py +++ b/search/tests/test_engines.py @@ -6,92 +6,12 @@ import json import os from datetime import datetime -from unittest.mock import patch from django.test import TestCase from django.test.utils import override_settings -from elasticsearch import exceptions -from elasticsearch.helpers import BulkIndexError from search.api import NoSearchEngineError, perform_search -from search.elastic import RESERVED_CHARACTERS -from search.tests.mock_search_engine import (MockSearchEngine, - json_date_to_datetime) +from search.tests.mock_search_engine import MockSearchEngine, json_date_to_datetime from search.tests.tests import MockSearchTests -from search.tests.utils import (TEST_INDEX_NAME, ErroringElasticImpl, - SearcherMixin) - - -@override_settings(ELASTIC_SEARCH_INDEX_PREFIX='prefixed_') -@override_settings(SEARCH_ENGINE="search.tests.utils.ForceRefreshElasticSearchEngine") -class ElasticSearchPrefixTests(MockSearchTests): - """ - Override that runs the same tests for ElasticSearchTests, - but with a prefixed index name. - """ - - @property - def index_name(self): - """ - The search index name to be used for this test. - """ - return f"prefixed_{TEST_INDEX_NAME}" - - -@override_settings(SEARCH_ENGINE="search.tests.utils.ForceRefreshElasticSearchEngine") -class ElasticSearchTests(MockSearchTests): - """ Override that runs the same tests for ElasticSearchEngine instead of MockSearchEngine """ - - def test_reserved_characters(self): - """ Make sure that we handle when reserved characters were passed into query_string """ - test_string = "What the ! is this?" - self.searcher.index([{"content": {"name": test_string}}]) - - response = self.searcher.search_string(test_string) - self.assertEqual(response["total"], 1) - - response = self.searcher.search_string("something else !") - self.assertEqual(response["total"], 0) - - response = self.searcher.search_string("something ! else") - self.assertEqual(response["total"], 0) - - for char in RESERVED_CHARACTERS: - # previously these would throw exceptions - response = self.searcher.search_string(char) - self.assertEqual(response["total"], 0) - - def test_aggregation_options(self): - """ - Test that aggregate options work alongside aggregations - notice - unsupported in mock for now size - is the only option for now - """ - self._index_for_aggs() - - response = self.searcher.search() - self.assertEqual(response["total"], 7) - self.assertNotIn("aggs", response) - - aggregation_terms = { - "subject": {"size": 2}, - "org": {"size": 2} - } - response = self.searcher.search(aggregation_terms=aggregation_terms) - self.assertEqual(response["total"], 7) - self.assertIn("aggs", response) - aggregation_results = response["aggs"] - self.assertEqual(aggregation_results["subject"]["total"], 6) - subject_term_counts = aggregation_results["subject"]["terms"] - self.assertEqual(subject_term_counts["mathematics"], 3) - self.assertEqual(subject_term_counts["physics"], 2) - self.assertNotIn("history", subject_term_counts) - self.assertEqual(aggregation_results["subject"]["other"], 1) - - self.assertEqual(aggregation_results["org"]["total"], 7) - org_term_counts = aggregation_results["org"]["terms"] - self.assertEqual(org_term_counts["Harvard"], 4) - self.assertEqual(org_term_counts["MIT"], 2) - self.assertNotIn("edX", org_term_counts) - self.assertEqual(aggregation_results["org"]["other"], 1) @override_settings(MOCK_SEARCH_BACKING_FILE="./testfile.pkl") @@ -191,45 +111,6 @@ def test_disabled_index(self): self.assertEqual(response["total"], 0) -@override_settings(SEARCH_ENGINE="search.tests.utils.ForceRefreshElasticSearchEngine") -@override_settings(ELASTIC_SEARCH_IMPL=ErroringElasticImpl) -class ErroringElasticTests(TestCase, SearcherMixin): - """ testing handling of elastic exceptions when they happen """ - - def test_index_failure_bulk(self): - """ the index operation should fail """ - with patch('search.elastic.bulk', return_value=[0, [exceptions.ElasticsearchException()]]): - with self.assertRaises(exceptions.ElasticsearchException): - self.searcher.index([{"name": "abc test"}]) - - def test_index_failure_general(self): - """ the index operation should fail """ - with patch('search.elastic.bulk', side_effect=Exception()): - with self.assertRaises(Exception): - self.searcher.index([{"name": "abc test"}]) - - def test_search_failure(self): - """ the search operation should fail """ - with self.assertRaises(exceptions.ElasticsearchException): - self.searcher.search("abc test") - - def test_remove_failure_bulk(self): - """ the remove operation should fail """ - doc_id = 'test_id' - error = {'delete': { - 'status': 500, '_index': 'test_index', '_version': 1, 'found': True, '_id': doc_id - }} - with patch('search.elastic.bulk', side_effect=BulkIndexError('Simulated error', [error])): - with self.assertRaises(BulkIndexError): - self.searcher.remove(["test_id"]) - - def test_remove_failure_general(self): - """ the remove operation should fail """ - with patch('search.elastic.bulk', side_effect=Exception()): - with self.assertRaises(Exception): - self.searcher.remove(["test_id"]) - - @override_settings(SEARCH_ENGINE=None) class TestNone(TestCase): """ Tests correct skipping of operation when no search engine is defined """ @@ -238,15 +119,3 @@ def test_perform_search(self): """ search opertaion should yeild an exception with no search engine """ with self.assertRaises(NoSearchEngineError): perform_search("abc test") - - -@override_settings(SEARCH_ENGINE="search.elastic.ElasticSearchEngine") -@override_settings(ELASTIC_SEARCH_CONFIG=[{'host': '127.0.0.1'}, {'host': 'localhost'}]) -class TestElasticConfig(TestCase, SearcherMixin): - """ Tests correct configuration of the elasticsearch instance. """ - - def test_config(self): - """ should be configured with the correct hosts """ - elasticsearch = self.searcher._es # pylint: disable=protected-access - hosts = elasticsearch.transport.hosts - self.assertEqual(hosts, [{'host': '127.0.0.1'}, {'host': 'localhost'}]) diff --git a/search/tests/test_meilisearch.py b/search/tests/test_meilisearch.py index d482c688..fc2a45dd 100644 --- a/search/tests/test_meilisearch.py +++ b/search/tests/test_meilisearch.py @@ -3,7 +3,7 @@ """ from datetime import datetime -from unittest.mock import Mock, patch, PropertyMock +from unittest.mock import Mock, patch, MagicMock, PropertyMock import django.test from django.utils import timezone @@ -11,6 +11,7 @@ import pytest from requests import Response +from search.api import course_discovery_aggregations from search.utils import DateRange, ValueRange import search.meilisearch @@ -52,6 +53,8 @@ class EngineTests(django.test.TestCase): MeilisearchEngine tests. """ + aggregation_terms = course_discovery_aggregations() + def test_index_empty_document(self): assert not search.meilisearch.process_nested_document({}) @@ -198,34 +201,43 @@ def test_search_with_facets(self): } == aggs["modes"] def test_search_params(self): - params = search.meilisearch.get_search_params() + params = search.meilisearch.get_search_params(aggregation_terms=self.aggregation_terms) self.assertTrue(params["showRankingScore"]) - params = search.meilisearch.get_search_params(from_=0) + params = search.meilisearch.get_search_params(from_=0, aggregation_terms=self.aggregation_terms) assert 0 == params["offset"] def test_search_params_exclude_dictionary(self): # Simple value params = search.meilisearch.get_search_params( - exclude_dictionary={"course_visibility": "none"} + exclude_dictionary={"course_visibility": "none"}, + aggregation_terms=self.aggregation_terms ) assert ['NOT course_visibility = "none"'] == params["filter"] # Multiple IDs params = search.meilisearch.get_search_params( - exclude_dictionary={"id": ["1", "2"]} + exclude_dictionary={"id": ["1", "2"]}, + aggregation_terms=self.aggregation_terms ) assert [ f'NOT {search.meilisearch.PRIMARY_KEY_FIELD_NAME} = "{search.meilisearch.id2pk("1")}"', f'NOT {search.meilisearch.PRIMARY_KEY_FIELD_NAME} = "{search.meilisearch.id2pk("2")}"', ] == params["filter"] + params = search.meilisearch.get_search_params( + exclude_dictionary={"language": ["en", "fr"]}, + aggregation_terms=self.aggregation_terms + ) + assert ['NOT language = "en"', 'NOT language = "fr"'] == params["filter"] + def test_search_params_field_dictionary(self): params = search.meilisearch.get_search_params( field_dictionary={ "course": "course-v1:testorg+test1+alpha", "org": "testorg", - } + }, + aggregation_terms=self.aggregation_terms, ) assert [ 'course = "course-v1:testorg+test1+alpha"', @@ -237,28 +249,32 @@ def test_engine_search_orgs_list(self): field_dictionary={ 'mode': 'honor', "org": ["testorg", "testorg2"], - } + }, + aggregation_terms=self.aggregation_terms, ) assert [ 'mode = "honor"', - ['org = "testorg"', 'org = "testorg2"'], + 'org = "testorg" OR org = "testorg2"', ] == params["filter"] def test_search_params_filter_dictionary(self): params = search.meilisearch.get_search_params( - filter_dictionary={"key": "value"} + filter_dictionary={"key": "value"}, + aggregation_terms=self.aggregation_terms, ) assert ['key = "value" OR key NOT EXISTS'] == params["filter"] def test_search_params_value_range(self): params = search.meilisearch.get_search_params( - filter_dictionary={"value": ValueRange(lower=1, upper=2)} + filter_dictionary={"value": ValueRange(lower=1, upper=2)}, + aggregation_terms=self.aggregation_terms, ) assert ["(value >= 1 AND value <= 2) OR value NOT EXISTS"] == params["filter"] params = search.meilisearch.get_search_params( - filter_dictionary={"value": ValueRange(lower=1)} + filter_dictionary={"value": ValueRange(lower=1)}, + aggregation_terms=self.aggregation_terms, ) assert ["value >= 1 OR value NOT EXISTS"] == params["filter"] @@ -268,14 +284,16 @@ def test_search_params_date_range(self): "enrollment_end": DateRange( lower=datetime(2024, 1, 1), upper=datetime(2024, 1, 2) ) - } + }, + aggregation_terms=self.aggregation_terms, ) assert [ "(enrollment_end >= 1704067200.0 AND enrollment_end <= 1704153600.0) OR enrollment_end NOT EXISTS" ] == params["filter"] params = search.meilisearch.get_search_params( - filter_dictionary={"enrollment_end": DateRange(lower=datetime(2024, 1, 1))} + filter_dictionary={"enrollment_end": DateRange(lower=datetime(2024, 1, 1))}, + aggregation_terms=self.aggregation_terms, ) assert [ "enrollment_end >= 1704067200.0 OR enrollment_end NOT EXISTS" @@ -283,6 +301,7 @@ def test_search_params_date_range(self): def test_search_params_sort_by(self): params = search.meilisearch.get_search_params( + aggregation_terms=self.aggregation_terms, sort_by=[ search.dataclasses.SortField(name="start", order="asc"), search.dataclasses.SortField(name="title", order="desc"), @@ -294,7 +313,7 @@ def test_search_params_sort_by(self): ] == params["sort"] # No sort by - params = search.meilisearch.get_search_params(sort_by=[]) + params = search.meilisearch.get_search_params(aggregation_terms=self.aggregation_terms, sort_by=[]) assert params.get("sort") is None @patch('search.meilisearch.MeilisearchEngine.meilisearch_index', new_callable=PropertyMock) @@ -350,7 +369,7 @@ def test_engine_search(self, mock_meilisearch_index): "estimatedTotalHits": 1, } - results = engine.search( + result = engine.search( query_string="abc", field_dictionary={ "course": "course-v1:testorg+test1+alpha", @@ -362,7 +381,7 @@ def test_engine_search(self, mock_meilisearch_index): log_search_params=True, ) - engine.meilisearch_index.search.assert_called_with( + engine.meilisearch_index.search.assert_called_once_with( "abc", { "showRankingScore": True, @@ -375,23 +394,25 @@ def test_engine_search(self, mock_meilisearch_index): ], }, ) - assert results == { - "aggs": {}, - "max_score": 0.865, - "results": [ - { - "_id": "course-v1:OpenedX+DemoX+DemoCourse", - "_index": "my_index", - "_type": "_doc", - "data": { - "id": "course-v1:OpenedX+DemoX+DemoCourse", - "pk": "f381d4f1914235c9532576c0861d09b484ade634", + + self.assertGreaterEqual( + result.items(), + { + "max_score": 0.865, + "took": 0, + "results": [ + { + "_id": "course-v1:OpenedX+DemoX+DemoCourse", + "_index": "my_index", + "_type": "_doc", + "data": { + "id": "course-v1:OpenedX+DemoX+DemoCourse", + "pk": "f381d4f1914235c9532576c0861d09b484ade634", + }, }, - }, - ], - "took": 0, - "total": 1, - } + ] + }.items() + ) @patch('search.meilisearch.MeilisearchEngine.meilisearch_index', new_callable=PropertyMock) def test_engine_remove(self, mock_meilisearch_index): @@ -406,6 +427,167 @@ def test_engine_remove(self, mock_meilisearch_index): engine.remove(doc_ids=[doc_id]) engine.meilisearch_index.delete_documents.assert_called_with([doc_pk]) + def test_multivalue_search_uses_or_to_join_rules_within_facet(self): + filter_dict = { + "language": ["en", "fr"] + } + rules = search.meilisearch.get_filter_rules(filter_dict, or_fields=["org", "modes", "language"]) + + self.assertListEqual(rules, ['language = "en" OR language = "fr"']) + + def test_multivalue_search_expands_selected_facet_without_filtering(self): + multivalue_distribution = {'en': 1, 'fr': 2} + + engine = search.meilisearch.MeilisearchEngine(index="test_index") + engine.meilisearch_index.search = Mock( + return_value={ + 'hits': [], + 'query': '', + 'processingTimeMs': 0, + 'limit': 0, + 'offset': 0, + 'estimatedTotalHits': 4, + 'facetDistribution': + {'language': multivalue_distribution}, + 'facetStats': {} + } + ) + + original_filter = [ + 'language = "en" OR language = "fr"', + 'modes = "audit" OR modes = "honor"', + 'org = "EDX"', + ] + selected_facet = 'language' + actual_distribution = engine._get_expanded_distribution( # pylint: disable=protected-access + '', selected_facet, original_filter + ) + self.assertDictEqual(actual_distribution, multivalue_distribution) + (query, opt_params), _ = engine.meilisearch_index.search.call_args # pylint: disable=unused-variable + self.assertIn(selected_facet, opt_params['facets']) + self.assertFalse(any(rule.startswith(f'{selected_facet} = ') for rule in opt_params['filter'])) + + def test_multivalue_search_merges_expanded_facet_distributions(self): + engine = search.meilisearch.MeilisearchEngine(index='test_index') + engine.meilisearch_index.search = Mock(side_effect=[ + { + "hits": [], + "query": "", + "processingTimeMs": 5, + "limit": 20, + "offset": 0, + "estimatedTotalHits": 0, + "facetDistribution": { + "language": {"en": 2}, # Narrowed distribution after selecting a facet value + "org": {"EDX": 2} + }, + }, + { + "hits": [], + "facetDistribution": { + "language": {"en": 2, "fr": 1} # Expanded distribution for multivalue search + } + } + ]) + + results = engine.search( + query_string='', + field_dictionary={'language': ['en']}, + aggregation_terms=self.aggregation_terms, + is_multivalue=True, + ) + aggregations = results["aggs"] + self.assertIn("language", aggregations) + self.assertIn("org", aggregations) + self.assertDictEqual( + aggregations["language"]["terms"], + {"en": 2, "fr": 1} + ) + self.assertDictEqual(aggregations["org"]["terms"], {"EDX": 2}) + + def test_single_value_search_narrows_selected_facet(self): + engine = search.meilisearch.MeilisearchEngine(index='test_index') + engine.meilisearch_index.search = Mock(side_effect=[ + { + "hits": [], + "query": "", + "processingTimeMs": 5, + "limit": 20, + "offset": 0, + "estimatedTotalHits": 0, + "facetDistribution": { + "language": {"en": 2}, + "org": {"EDX": 2} + }, + }, + { + "hits": [], + "facetDistribution": { + "language": {"en": 2, "fr": 1} + } + } + ]) + + results = engine.search( + query_string='', + field_dictionary={'language': ['en']}, + aggregation_terms=self.aggregation_terms, + is_multivalue=False, + ) + aggregations = results["aggs"] + self.assertIn("language", aggregations) + self.assertIn("org", aggregations) + self.assertDictEqual( + aggregations["language"]["terms"], + {"en": 2} + ) + self.assertDictEqual(aggregations["org"]["terms"], {"EDX": 2}) + + def test_facet_expansion_not_triggered_if_not_multivalue(self): + engine = search.meilisearch.MeilisearchEngine(index="test_index") + engine._expand_facet_distibutions = MagicMock() # pylint: disable=protected-access + engine.meilisearch_index.search = Mock( + return_value={ + "hits": [], + "facetDistribution": {}, + "estimatedTotalHits": 0, + "processingTimeMs": 1, + } + ) + engine.search( + field_dictionary={"language": "en"}, + aggregation_terms=self.aggregation_terms, + is_multivalue=False + ) + engine._expand_facet_distibutions.assert_not_called() # pylint: disable=protected-access + + def test_facet_expansion_is_triggered_if_multivalue(self): + engine = search.meilisearch.MeilisearchEngine(index="test_index") + engine._expand_facet_distibutions = MagicMock() # pylint: disable=protected-access + engine.meilisearch_index.search = Mock( + return_value={ + "hits": [], + "facetDistribution": {}, + "estimatedTotalHits": 0, + "processingTimeMs": 1, + } + ) + engine.search( + query_string="demo", + field_dictionary={"language": ["en"]}, + aggregation_terms=self.aggregation_terms, + is_multivalue=True + ) + engine._expand_facet_distibutions.assert_called_once() # pylint: disable=protected-access + + def test_multivalue_nonfacet_field_expands_to_multiple_rules(self): + # "title" is not a facet field + rules = search.meilisearch.get_filter_rules({"title": ["Intro", "Advanced"]}) + self.assertIn('title = "Intro"', rules) + self.assertIn('title = "Advanced"', rules) + # Both values should appear separately + self.assertEqual(len(rules), 2) + class UtilitiesTests(django.test.TestCase): """ diff --git a/search/tests/test_views.py b/search/tests/test_views.py index 7c3aff46..9427e465 100644 --- a/search/tests/test_views.py +++ b/search/tests/test_views.py @@ -226,7 +226,7 @@ def test_empty_search_string(self): self.assertGreater(code, 499) self.assertEqual(results["error"], "No search term provided for search") - # pylint: disable=too-many-statements,wrong-assert-type + # pylint: disable=too-many-statements def test_pagination(self): """ test searching using the course url """ self.searcher.index([ @@ -350,7 +350,7 @@ def test_page_size_too_large(self): code, results = post_request({"search_string": "Little Darling", "page_size": 101}) self.assertEqual(code, 500) - self.assertTrue("error" in results) + self.assertIn("error", results) @override_settings(SEARCH_ENGINE="search.tests.utils.ErroringSearchEngine")