From ece4dc9741faa80200257f54a6abcda741c8ccad Mon Sep 17 00:00:00 2001 From: charles Date: Mon, 16 Feb 2026 10:25:05 +0100 Subject: [PATCH] =?UTF-8?q?=E2=99=BB=EF=B8=8F(backend)=20wording?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit SearchIndexer -> FindDocumentIndexer SEARCH_INDEXER_URL -> INDEXING_URL SEARCH_INDEXER_QUERY_URL -> SEARCH_URL Signed-off-by: charles --- src/backend/core/services/search_indexers.py | 13 +- src/backend/core/tests/commands/test_index.py | 6 +- src/backend/core/tests/conftest.py | 8 +- .../documents/test_api_documents_search.py | 435 +++++------------- .../tests/test_models_documents_indexer.py | 44 +- .../tests/test_services_search_indexers.py | 128 +++--- src/backend/impress/settings.py | 8 +- 7 files changed, 216 insertions(+), 426 deletions(-) diff --git a/src/backend/core/services/search_indexers.py b/src/backend/core/services/search_indexers.py index 4c5e24c0..493c8502 100644 --- a/src/backend/core/services/search_indexers.py +++ b/src/backend/core/services/search_indexers.py @@ -8,6 +8,7 @@ from functools import cache from django.conf import settings from django.contrib.auth.models import AnonymousUser from django.core.exceptions import ImproperlyConfigured +from django.db.models import Subquery from django.utils.module_loading import import_string import requests @@ -106,15 +107,13 @@ class BaseDocumentIndexer(ABC): Initialize the indexer. """ self.batch_size = settings.SEARCH_INDEXER_BATCH_SIZE - self.indexer_url = settings.SEARCH_INDEXER_URL + self.indexer_url = settings.INDEXING_URL self.indexer_secret = settings.SEARCH_INDEXER_SECRET - self.search_url = settings.SEARCH_INDEXER_QUERY_URL + self.search_url = settings.SEARCH_URL self.search_limit = settings.SEARCH_INDEXER_QUERY_LIMIT if not self.indexer_url: - raise ImproperlyConfigured( - "SEARCH_INDEXER_URL must be set in Django settings." - ) + raise ImproperlyConfigured("INDEXING_URL must be set in Django settings.") if not self.indexer_secret: raise ImproperlyConfigured( @@ -122,9 +121,7 @@ class BaseDocumentIndexer(ABC): ) if not self.search_url: - raise ImproperlyConfigured( - "SEARCH_INDEXER_QUERY_URL must be set in Django settings." - ) + raise ImproperlyConfigured("SEARCH_URL must be set in Django settings.") def index(self, queryset=None, batch_size=None): """ diff --git a/src/backend/core/tests/commands/test_index.py b/src/backend/core/tests/commands/test_index.py index ad7d39e6..78d30249 100644 --- a/src/backend/core/tests/commands/test_index.py +++ b/src/backend/core/tests/commands/test_index.py @@ -11,7 +11,7 @@ from django.db import transaction import pytest from core import factories -from core.services.search_indexers import SearchIndexer +from core.services.search_indexers import FindDocumentIndexer @pytest.mark.django_db @@ -19,7 +19,7 @@ from core.services.search_indexers import SearchIndexer def test_index(): """Test the command `index` that run the Find app indexer for all the available documents.""" user = factories.UserFactory() - indexer = SearchIndexer() + indexer = FindDocumentIndexer() with transaction.atomic(): doc = factories.DocumentFactory() @@ -36,7 +36,7 @@ def test_index(): str(no_title_doc.path): {"users": [user.sub]}, } - with mock.patch.object(SearchIndexer, "push") as mock_push: + with mock.patch.object(FindDocumentIndexer, "push") as mock_push: call_command("index") push_call_args = [call.args[0] for call in mock_push.call_args_list] diff --git a/src/backend/core/tests/conftest.py b/src/backend/core/tests/conftest.py index 65e39269..a342499d 100644 --- a/src/backend/core/tests/conftest.py +++ b/src/backend/core/tests/conftest.py @@ -39,12 +39,10 @@ def indexer_settings_fixture(settings): get_document_indexer.cache_clear() - settings.SEARCH_INDEXER_CLASS = "core.services.search_indexers.SearchIndexer" + settings.SEARCH_INDEXER_CLASS = "core.services.search_indexers.FindDocumentIndexer" settings.SEARCH_INDEXER_SECRET = "ThisIsAKeyForTest" - settings.SEARCH_INDEXER_URL = "http://localhost:8081/api/v1.0/documents/index/" - settings.SEARCH_INDEXER_QUERY_URL = ( - "http://localhost:8081/api/v1.0/documents/search/" - ) + settings.INDEXING_URL = "http://localhost:8081/api/v1.0/documents/index/" + settings.SEARCH_URL = "http://localhost:8081/api/v1.0/documents/search/" settings.SEARCH_INDEXER_COUNTDOWN = 1 yield settings diff --git a/src/backend/core/tests/documents/test_api_documents_search.py b/src/backend/core/tests/documents/test_api_documents_search.py index a9ac2e47..389247d1 100644 --- a/src/backend/core/tests/documents/test_api_documents_search.py +++ b/src/backend/core/tests/documents/test_api_documents_search.py @@ -1,46 +1,31 @@ """ -Tests for Documents API endpoint in impress's core app: list +Tests for Documents API endpoint in impress's core app: search """ -import random -from json import loads as json_loads - -from django.test import RequestFactory +from unittest import mock import pytest import responses from faker import Faker +from rest_framework import response as drf_response from rest_framework.test import APIClient -from core import factories, models +from core import factories from core.services.search_indexers import get_document_indexer fake = Faker() pytestmark = pytest.mark.django_db -def build_search_url(**kwargs): - """Build absolute uri for search endpoint with ORDERED query arguments""" - return ( - RequestFactory() - .get("/api/v1.0/documents/search/", dict(sorted(kwargs.items()))) - .build_absolute_uri() - ) - - -@pytest.mark.parametrize("role", models.LinkRoleChoices.values) -@pytest.mark.parametrize("reach", models.LinkReachChoices.values) +@mock.patch("core.services.search_indexers.FindDocumentIndexer.search_query") @responses.activate -def test_api_documents_search_anonymous(reach, role, indexer_settings): +def test_api_documents_search_anonymous(search_query, indexer_settings): """ - Anonymous users should not be allowed to search documents whatever the - link reach and link role + Anonymous users should be allowed to search documents with Find. """ - indexer_settings.SEARCH_INDEXER_QUERY_URL = "http://find/api/v1.0/search" + indexer_settings.SEARCH_URL = "http://find/api/v1.0/search" - factories.DocumentFactory(link_reach=reach, link_role=role) - - # Find response + # mock Find response responses.add( responses.POST, "http://find/api/v1.0/search", @@ -48,7 +33,22 @@ def test_api_documents_search_anonymous(reach, role, indexer_settings): status=200, ) - response = APIClient().get("/api/v1.0/documents/search/", data={"q": "alpha"}) + q = "alpha" + response = APIClient().get("/api/v1.0/documents/search/", data={"q": q}) + + assert search_query.call_count == 1 + assert search_query.call_args[1] == { + "data": { + "q": q, + "visited": [], + "services": ["docs"], + "nb_results": 50, + "order_by": "updated_at", + "order_direction": "desc", + "path": None, + }, + "token": None, + } assert response.status_code == 200 assert response.json() == { @@ -59,31 +59,96 @@ def test_api_documents_search_anonymous(reach, role, indexer_settings): } -def test_api_documents_search_endpoint_is_none(indexer_settings): +@mock.patch("core.api.viewsets.DocumentViewSet.list") +def test_api_documents_search_fall_back_on_search_list(mock_list, indexer_settings): """ - Missing SEARCH_INDEXER_QUERY_URL, so the indexer is not properly configured. - Should fallback on title filter + When indexer is not configured and no path is provided, + should fall back on list method """ - indexer_settings.SEARCH_INDEXER_QUERY_URL = None - + indexer_settings.SEARCH_URL = None assert get_document_indexer() is None user = factories.UserFactory() - document = factories.DocumentFactory(title="alpha") - access = factories.UserDocumentAccessFactory(document=document, user=user) - client = APIClient() client.force_login(user) - response = client.get("/api/v1.0/documents/search/", data={"q": "alpha"}) - - assert response.status_code == 200 - content = response.json() - results = content.pop("results") - assert content == { - "count": 1, + mocked_response = { + "count": 0, "next": None, "previous": None, + "results": [{"title": "mocked list result"}], + } + mock_list.return_value = drf_response.Response(mocked_response) + + q = "alpha" + response = client.get("/api/v1.0/documents/search/", data={"q": q}) + + assert mock_list.call_count == 1 + assert mock_list.call_args[0][0].GET.get("title") == q + assert response.json() == mocked_response + + +@mock.patch("core.api.viewsets.DocumentViewSet._list_descendants") +def test_api_documents_search_fallback_on_search_list_sub_docs( + mock_list_descendants, indexer_settings +): + """ + When indexer is not configured and path parameter is provided, + should call _list_descendants() method + """ + indexer_settings.SEARCH_URL = "http://find/api/v1.0/search" + assert get_document_indexer() is not None + + user = factories.UserFactory() + client = APIClient() + client.force_login(user) + + parent = factories.DocumentFactory(title="parent", users=[user]) + + mocked_response = { + "count": 0, + "next": None, + "previous": None, + "results": [{"title": "mocked _list_descendants result"}], + } + mock_list_descendants.return_value = drf_response.Response(mocked_response) + + q = "alpha" + response = client.get( + "/api/v1.0/documents/search/", data={"q": q, "path": parent.path} + ) + + assert mock_list_descendants.call_count == 1 + assert mock_list_descendants.call_args[0][0].GET.get("title") == q + assert mock_list_descendants.call_args[0][0].GET.get("path") == parent.path + assert response.json() == mocked_response + + +@mock.patch("core.api.viewsets.DocumentViewSet._title_search") +def test_api_documents_search_indexer_crashes(mock_title_search, indexer_settings): + """ + When indexer is configured but crashes -> falls back on title_search + """ + # indexer is properly configured + indexer_settings.SEARCH_URL = None + assert get_document_indexer() is None + # but returns an error when the query is sent + responses.add( + responses.POST, + "http://find/api/v1.0/search", + json=[{"error": "Some indexer error"}], + status=404, + ) + + user = factories.UserFactory() + client = APIClient() + client.force_login(user) + + mocked_response = { + "count": 0, + "next": None, + "previous": None, + "results": [{"title": "mocked title_search result"}], } mock_list.return_value = drf_response.Response(mocked_response) @@ -177,10 +242,10 @@ def test_api_documents_search_indexer_crashes(mock_title_search, indexer_setting @responses.activate def test_api_documents_search_invalid_params(indexer_settings): """Validate the format of documents as returned by the search view.""" - indexer_settings.SEARCH_INDEXER_QUERY_URL = "http://find/api/v1.0/search" + indexer_settings.SEARCH_URL = "http://find/api/v1.0/search" + assert get_document_indexer() is not None user = factories.UserFactory() - client = APIClient() client.force_login(user) @@ -189,49 +254,28 @@ def test_api_documents_search_invalid_params(indexer_settings): assert response.status_code == 400 assert response.json() == {"q": ["This field is required."]} - response = client.get("/api/v1.0/documents/search/", data={"q": " "}) - - assert response.status_code == 400 - assert response.json() == {"q": ["This field may not be blank."]} - - response = client.get( - "/api/v1.0/documents/search/", data={"q": "any", "page": "NaN"} - ) - - assert response.status_code == 400 - assert response.json() == {"page": ["A valid integer is required."]} - @responses.activate -def test_api_documents_search_format(indexer_settings): +def test_api_documents_search_success(indexer_settings): """Validate the format of documents as returned by the search view.""" - indexer_settings.SEARCH_INDEXER_QUERY_URL = "http://find/api/v1.0/search" - + indexer_settings.SEARCH_URL = "http://find/api/v1.0/search" assert get_document_indexer() is not None - user = factories.UserFactory() - - client = APIClient() - client.force_login(user) - - user_a, user_b, user_c = factories.UserFactory.create_batch(3) - document = factories.DocumentFactory( - title="alpha", - users=(user_a, user_c), - link_traces=(user, user_b), - ) - access = factories.UserDocumentAccessFactory(document=document, user=user) + document = {"id": "doc-123", "title": "alpha", "path": "path/to/alpha.pdf"} # Find response responses.add( responses.POST, "http://find/api/v1.0/search", json=[ - {"_id": str(document.pk)}, + { + "_id": str(document["id"]), + "_source": {"title": document["title"], "path": document["path"]}, + }, ], status=200, ) - response = client.get("/api/v1.0/documents/search/", data={"q": "alpha"}) + response = APIClient().get("/api/v1.0/documents/search/", data={"q": "alpha"}) assert response.status_code == 200 content = response.json() @@ -241,249 +285,6 @@ def test_api_documents_search_format(indexer_settings): "next": None, "previous": None, } - assert len(results) == 1 - assert results[0] == { - "id": str(document.id), - "abilities": document.get_abilities(user), - "ancestors_link_reach": None, - "ancestors_link_role": None, - "computed_link_reach": document.computed_link_reach, - "computed_link_role": document.computed_link_role, - "created_at": document.created_at.isoformat().replace("+00:00", "Z"), - "creator": str(document.creator.id), - "depth": 1, - "excerpt": document.excerpt, - "link_reach": document.link_reach, - "link_role": document.link_role, - "nb_accesses_ancestors": 3, - "nb_accesses_direct": 3, - "numchild": 0, - "path": document.path, - "title": document.title, - "updated_at": document.updated_at.isoformat().replace("+00:00", "Z"), - "deleted_at": None, - "user_role": access.role, - } - - -@responses.activate -@pytest.mark.parametrize( - "pagination, status, expected", - ( - ( - {"page": 1, "page_size": 10}, - 200, - { - "count": 10, - "previous": None, - "next": None, - "range": (0, None), - }, - ), - ( - {}, - 200, - { - "count": 10, - "previous": None, - "next": None, - "range": (0, None), - "api_page_size": 21, # default page_size is 20 - }, - ), - ( - {"page": 2, "page_size": 10}, - 404, - {}, - ), - ( - {"page": 1, "page_size": 5}, - 200, - { - "count": 10, - "previous": None, - "next": {"page": 2, "page_size": 5}, - "range": (0, 5), - }, - ), - ( - {"page": 2, "page_size": 5}, - 200, - { - "count": 10, - "previous": {"page_size": 5}, - "next": None, - "range": (5, None), - }, - ), - ({"page": 3, "page_size": 5}, 404, {}), - ), -) -def test_api_documents_search_pagination( - indexer_settings, pagination, status, expected -): - """Documents should be ordered by descending "score" by default""" - indexer_settings.SEARCH_INDEXER_QUERY_URL = "http://find/api/v1.0/search" - - assert get_document_indexer() is not None - - user = factories.UserFactory() - - client = APIClient() - client.force_login(user) - - docs = factories.DocumentFactory.create_batch(10, title="alpha", users=[user]) - - docs_by_uuid = {str(doc.pk): doc for doc in docs} - api_results = [{"_id": id} for id in docs_by_uuid.keys()] - - # reorder randomly to simulate score ordering - random.shuffle(api_results) - - # Find response - # pylint: disable-next=assignment-from-none - api_search = responses.add( - responses.POST, - "http://find/api/v1.0/search", - json=api_results, - status=200, - ) - - response = client.get( - "/api/v1.0/documents/search/", - data={ - "q": "alpha", - **pagination, - }, - ) - - assert response.status_code == status - - if response.status_code < 300: - previous_url = ( - build_search_url(q="alpha", **expected["previous"]) - if expected["previous"] - else None - ) - next_url = ( - build_search_url(q="alpha", **expected["next"]) - if expected["next"] - else None - ) - start, end = expected["range"] - - content = response.json() - - assert content["count"] == expected["count"] - assert content["previous"] == previous_url - assert content["next"] == next_url - - results = content.pop("results") - - # The find api results ordering by score is kept - assert [r["id"] for r in results] == [r["_id"] for r in api_results[start:end]] - - # Check the query parameters. - assert api_search.call_count == 1 - assert api_search.calls[0].response.status_code == 200 - assert json_loads(api_search.calls[0].request.body) == { - "q": "alpha", - "visited": [], - "services": ["docs"], - "nb_results": 50, - "order_by": "updated_at", - "order_direction": "desc", - } - - -@responses.activate -@pytest.mark.parametrize( - "pagination, status, expected", - ( - ( - {"page": 1, "page_size": 10}, - 200, - {"count": 10, "previous": None, "next": None, "range": (0, None)}, - ), - ( - {}, - 200, - {"count": 10, "previous": None, "next": None, "range": (0, None)}, - ), - ( - {"page": 2, "page_size": 10}, - 404, - {}, - ), - ( - {"page": 1, "page_size": 5}, - 200, - { - "count": 10, - "previous": None, - "next": {"page": 2, "page_size": 5}, - "range": (0, 5), - }, - ), - ( - {"page": 2, "page_size": 5}, - 200, - { - "count": 10, - "previous": {"page_size": 5}, - "next": None, - "range": (5, None), - }, - ), - ({"page": 3, "page_size": 5}, 404, {}), - ), -) -def test_api_documents_search_pagination_endpoint_is_none( - indexer_settings, pagination, status, expected -): - """Documents should be ordered by descending "-updated_at" by default""" - indexer_settings.SEARCH_INDEXER_QUERY_URL = None - - assert get_document_indexer() is None - - user = factories.UserFactory() - - client = APIClient() - client.force_login(user) - - factories.DocumentFactory.create_batch(10, title="alpha", users=[user]) - - response = client.get( - "/api/v1.0/documents/search/", - data={ - "q": "alpha", - **pagination, - }, - ) - - assert response.status_code == status - - if response.status_code < 300: - previous_url = ( - build_search_url(q="alpha", **expected["previous"]) - if expected["previous"] - else None - ) - next_url = ( - build_search_url(q="alpha", **expected["next"]) - if expected["next"] - else None - ) - queryset = models.Document.objects.order_by("-updated_at") - start, end = expected["range"] - expected_results = [str(d.pk) for d in queryset[start:end]] - - content = response.json() - - assert content["count"] == expected["count"] - assert content["previous"] == previous_url - assert content["next"] == next_url - - results = content.pop("results") - - assert [r["id"] for r in results] == expected_results + assert results == [ + {"id": document["id"], "title": document["title"], "path": document["path"]} + ] diff --git a/src/backend/core/tests/test_models_documents_indexer.py b/src/backend/core/tests/test_models_documents_indexer.py index 9e171f72..ccbc3a22 100644 --- a/src/backend/core/tests/test_models_documents_indexer.py +++ b/src/backend/core/tests/test_models_documents_indexer.py @@ -12,7 +12,7 @@ from django.db import transaction import pytest from core import factories, models -from core.services.search_indexers import SearchIndexer +from core.services.search_indexers import FindDocumentIndexer pytestmark = pytest.mark.django_db @@ -30,7 +30,7 @@ def reset_throttle(): reset_batch_indexer_throttle() -@mock.patch.object(SearchIndexer, "push") +@mock.patch.object(FindDocumentIndexer, "push") @pytest.mark.usefixtures("indexer_settings") @pytest.mark.django_db(transaction=True) def test_models_documents_post_save_indexer(mock_push): @@ -41,7 +41,7 @@ def test_models_documents_post_save_indexer(mock_push): accesses = {} data = [call.args[0] for call in mock_push.call_args_list] - indexer = SearchIndexer() + indexer = FindDocumentIndexer() assert len(data) == 1 @@ -64,14 +64,14 @@ def test_models_documents_post_save_indexer_no_batches(indexer_settings): """Test indexation task on doculment creation, no throttle""" indexer_settings.SEARCH_INDEXER_COUNTDOWN = 0 - with mock.patch.object(SearchIndexer, "push") as mock_push: + with mock.patch.object(FindDocumentIndexer, "push") as mock_push: with transaction.atomic(): doc1, doc2, doc3 = factories.DocumentFactory.create_batch(3) accesses = {} data = [call.args[0] for call in mock_push.call_args_list] - indexer = SearchIndexer() + indexer = FindDocumentIndexer() # 3 calls assert len(data) == 3 @@ -91,7 +91,7 @@ def test_models_documents_post_save_indexer_no_batches(indexer_settings): assert cache.get("file-batch-indexer-throttle") is None -@mock.patch.object(SearchIndexer, "push") +@mock.patch.object(FindDocumentIndexer, "push") @pytest.mark.django_db(transaction=True) def test_models_documents_post_save_indexer_not_configured(mock_push, indexer_settings): """Task should not start an indexation when disabled""" @@ -106,13 +106,13 @@ def test_models_documents_post_save_indexer_not_configured(mock_push, indexer_se assert mock_push.assert_not_called -@mock.patch.object(SearchIndexer, "push") +@mock.patch.object(FindDocumentIndexer, "push") @pytest.mark.django_db(transaction=True) def test_models_documents_post_save_indexer_wrongly_configured( mock_push, indexer_settings ): """Task should not start an indexation when disabled""" - indexer_settings.SEARCH_INDEXER_URL = None + indexer_settings.INDEXING_URL = None user = factories.UserFactory() @@ -123,7 +123,7 @@ def test_models_documents_post_save_indexer_wrongly_configured( assert mock_push.assert_not_called -@mock.patch.object(SearchIndexer, "push") +@mock.patch.object(FindDocumentIndexer, "push") @pytest.mark.usefixtures("indexer_settings") @pytest.mark.django_db(transaction=True) def test_models_documents_post_save_indexer_with_accesses(mock_push): @@ -145,7 +145,7 @@ def test_models_documents_post_save_indexer_with_accesses(mock_push): data = [call.args[0] for call in mock_push.call_args_list] - indexer = SearchIndexer() + indexer = FindDocumentIndexer() assert len(data) == 1 assert sorted(data[0], key=itemgetter("id")) == sorted( @@ -158,7 +158,7 @@ def test_models_documents_post_save_indexer_with_accesses(mock_push): ) -@mock.patch.object(SearchIndexer, "push") +@mock.patch.object(FindDocumentIndexer, "push") @pytest.mark.usefixtures("indexer_settings") @pytest.mark.django_db(transaction=True) def test_models_documents_post_save_indexer_deleted(mock_push): @@ -207,7 +207,7 @@ def test_models_documents_post_save_indexer_deleted(mock_push): data = [call.args[0] for call in mock_push.call_args_list] - indexer = SearchIndexer() + indexer = FindDocumentIndexer() assert len(data) == 2 @@ -244,14 +244,14 @@ def test_models_documents_indexer_hard_deleted(): factories.UserDocumentAccessFactory(document=doc, user=user) # Call task on deleted document. - with mock.patch.object(SearchIndexer, "push") as mock_push: + with mock.patch.object(FindDocumentIndexer, "push") as mock_push: doc.delete() # Hard delete document are not re-indexed. assert mock_push.assert_not_called -@mock.patch.object(SearchIndexer, "push") +@mock.patch.object(FindDocumentIndexer, "push") @pytest.mark.usefixtures("indexer_settings") @pytest.mark.django_db(transaction=True) def test_models_documents_post_save_indexer_restored(mock_push): @@ -308,7 +308,7 @@ def test_models_documents_post_save_indexer_restored(mock_push): data = [call.args[0] for call in mock_push.call_args_list] - indexer = SearchIndexer() + indexer = FindDocumentIndexer() # All docs are re-indexed assert len(data) == 2 @@ -337,16 +337,16 @@ def test_models_documents_post_save_indexer_restored(mock_push): @pytest.mark.usefixtures("indexer_settings") def test_models_documents_post_save_indexer_throttle(): """Test indexation task skipping on document update""" - indexer = SearchIndexer() + indexer = FindDocumentIndexer() user = factories.UserFactory() - with mock.patch.object(SearchIndexer, "push"): + with mock.patch.object(FindDocumentIndexer, "push"): with transaction.atomic(): docs = factories.DocumentFactory.create_batch(5, users=(user,)) accesses = {str(item.path): {"users": [user.sub]} for item in docs} - with mock.patch.object(SearchIndexer, "push") as mock_push: + with mock.patch.object(FindDocumentIndexer, "push") as mock_push: # Simulate 1 running task cache.set("document-batch-indexer-throttle", 1) @@ -359,7 +359,7 @@ def test_models_documents_post_save_indexer_throttle(): assert [call.args[0] for call in mock_push.call_args_list] == [] - with mock.patch.object(SearchIndexer, "push") as mock_push: + with mock.patch.object(FindDocumentIndexer, "push") as mock_push: # No waiting task cache.delete("document-batch-indexer-throttle") @@ -389,7 +389,7 @@ def test_models_documents_access_post_save_indexer(): """Test indexation task on DocumentAccess update""" users = factories.UserFactory.create_batch(3) - with mock.patch.object(SearchIndexer, "push"): + with mock.patch.object(FindDocumentIndexer, "push"): with transaction.atomic(): doc = factories.DocumentFactory(users=users) doc_accesses = models.DocumentAccess.objects.filter(document=doc).order_by( @@ -398,7 +398,7 @@ def test_models_documents_access_post_save_indexer(): reset_batch_indexer_throttle() - with mock.patch.object(SearchIndexer, "push") as mock_push: + with mock.patch.object(FindDocumentIndexer, "push") as mock_push: with transaction.atomic(): for doc_access in doc_accesses: doc_access.save() @@ -426,7 +426,7 @@ def test_models_items_access_post_save_indexer_no_throttle(indexer_settings): reset_batch_indexer_throttle() - with mock.patch.object(SearchIndexer, "push") as mock_push: + with mock.patch.object(FindDocumentIndexer, "push") as mock_push: with transaction.atomic(): for doc_access in doc_accesses: doc_access.save() diff --git a/src/backend/core/tests/test_services_search_indexers.py b/src/backend/core/tests/test_services_search_indexers.py index 43fc6fda..18c90979 100644 --- a/src/backend/core/tests/test_services_search_indexers.py +++ b/src/backend/core/tests/test_services_search_indexers.py @@ -15,7 +15,7 @@ from requests import HTTPError from core import factories, models, utils from core.services.search_indexers import ( BaseDocumentIndexer, - SearchIndexer, + FindDocumentIndexer, get_document_indexer, get_visited_document_ids_of, ) @@ -78,41 +78,41 @@ def test_services_search_indexer_is_configured(indexer_settings): # Valid class indexer_settings.SEARCH_INDEXER_CLASS = ( - "core.services.search_indexers.SearchIndexer" + "core.services.search_indexers.FindDocumentIndexer" ) get_document_indexer.cache_clear() assert get_document_indexer() is not None - indexer_settings.SEARCH_INDEXER_URL = "" + indexer_settings.INDEXING_URL = "" # Invalid url get_document_indexer.cache_clear() assert not get_document_indexer() -def test_services_search_indexer_url_is_none(indexer_settings): +def test_services_indexing_url_is_none(indexer_settings): """ - Indexer should raise RuntimeError if SEARCH_INDEXER_URL is None or empty. + Indexer should raise RuntimeError if INDEXING_URL is None or empty. """ - indexer_settings.SEARCH_INDEXER_URL = None + indexer_settings.INDEXING_URL = None with pytest.raises(ImproperlyConfigured) as exc_info: - SearchIndexer() + FindDocumentIndexer() - assert "SEARCH_INDEXER_URL must be set in Django settings." in str(exc_info.value) + assert "INDEXING_URL must be set in Django settings." in str(exc_info.value) -def test_services_search_indexer_url_is_empty(indexer_settings): +def test_services_indexing_url_is_empty(indexer_settings): """ - Indexer should raise RuntimeError if SEARCH_INDEXER_URL is empty string. + Indexer should raise RuntimeError if INDEXING_URL is empty string. """ - indexer_settings.SEARCH_INDEXER_URL = "" + indexer_settings.INDEXING_URL = "" with pytest.raises(ImproperlyConfigured) as exc_info: - SearchIndexer() + FindDocumentIndexer() - assert "SEARCH_INDEXER_URL must be set in Django settings." in str(exc_info.value) + assert "INDEXING_URL must be set in Django settings." in str(exc_info.value) def test_services_search_indexer_secret_is_none(indexer_settings): @@ -122,7 +122,7 @@ def test_services_search_indexer_secret_is_none(indexer_settings): indexer_settings.SEARCH_INDEXER_SECRET = None with pytest.raises(ImproperlyConfigured) as exc_info: - SearchIndexer() + FindDocumentIndexer() assert "SEARCH_INDEXER_SECRET must be set in Django settings." in str( exc_info.value @@ -136,39 +136,35 @@ def test_services_search_indexer_secret_is_empty(indexer_settings): indexer_settings.SEARCH_INDEXER_SECRET = "" with pytest.raises(ImproperlyConfigured) as exc_info: - SearchIndexer() + FindDocumentIndexer() assert "SEARCH_INDEXER_SECRET must be set in Django settings." in str( exc_info.value ) -def test_services_search_endpoint_is_none(indexer_settings): +def test_services_search_url_is_none(indexer_settings): """ - Indexer should raise RuntimeError if SEARCH_INDEXER_QUERY_URL is None. + Indexer should raise RuntimeError if SEARCH_URL is None. """ - indexer_settings.SEARCH_INDEXER_QUERY_URL = None + indexer_settings.SEARCH_URL = None with pytest.raises(ImproperlyConfigured) as exc_info: - SearchIndexer() + FindDocumentIndexer() - assert "SEARCH_INDEXER_QUERY_URL must be set in Django settings." in str( - exc_info.value - ) + assert "SEARCH_URL must be set in Django settings." in str(exc_info.value) -def test_services_search_endpoint_is_empty(indexer_settings): +def test_services_search_url_is_empty(indexer_settings): """ - Indexer should raise RuntimeError if SEARCH_INDEXER_QUERY_URL is empty. + Indexer should raise RuntimeError if SEARCH_URL is empty. """ - indexer_settings.SEARCH_INDEXER_QUERY_URL = "" + indexer_settings.SEARCH_URL = "" with pytest.raises(ImproperlyConfigured) as exc_info: - SearchIndexer() + FindDocumentIndexer() - assert "SEARCH_INDEXER_QUERY_URL must be set in Django settings." in str( - exc_info.value - ) + assert "SEARCH_URL must be set in Django settings." in str(exc_info.value) @pytest.mark.usefixtures("indexer_settings") @@ -192,7 +188,7 @@ def test_services_search_indexers_serialize_document_returns_expected_json(): } } - indexer = SearchIndexer() + indexer = FindDocumentIndexer() result = indexer.serialize_document(document, accesses) assert set(result.pop("users")) == {str(user_a.sub), str(user_b.sub)} @@ -221,7 +217,7 @@ def test_services_search_indexers_serialize_document_deleted(): parent.soft_delete() document.refresh_from_db() - indexer = SearchIndexer() + indexer = FindDocumentIndexer() result = indexer.serialize_document(document, {}) assert result["is_active"] is False @@ -232,7 +228,7 @@ def test_services_search_indexers_serialize_document_empty(): """Empty documents returns empty content in the serialized json.""" document = factories.DocumentFactory(content="", title=None) - indexer = SearchIndexer() + indexer = FindDocumentIndexer() result = indexer.serialize_document(document, {}) assert result["content"] == "" @@ -246,7 +242,7 @@ def test_services_search_indexers_index_errors(indexer_settings): """ factories.DocumentFactory() - indexer_settings.SEARCH_INDEXER_URL = "http://app-find/api/v1.0/documents/index/" + indexer_settings.INDEXING_URL = "http://app-find/api/v1.0/documents/index/" responses.add( responses.POST, @@ -256,10 +252,10 @@ def test_services_search_indexers_index_errors(indexer_settings): ) with pytest.raises(HTTPError): - SearchIndexer().index() + FindDocumentIndexer().index() -@patch.object(SearchIndexer, "push") +@patch.object(FindDocumentIndexer, "push") def test_services_search_indexers_batches_pass_only_batch_accesses( mock_push, indexer_settings ): @@ -276,7 +272,7 @@ def test_services_search_indexers_batches_pass_only_batch_accesses( access = factories.UserDocumentAccessFactory(document=document) expected_user_subs[str(document.id)] = str(access.user.sub) - assert SearchIndexer().index() == 5 + assert FindDocumentIndexer().index() == 5 # Should be 3 batches: 2 + 2 + 1 assert mock_push.call_count == 3 @@ -299,7 +295,7 @@ def test_services_search_indexers_batches_pass_only_batch_accesses( assert seen_doc_ids == {str(d.id) for d in documents} -@patch.object(SearchIndexer, "push") +@patch.object(FindDocumentIndexer, "push") @pytest.mark.usefixtures("indexer_settings") def test_services_search_indexers_batch_size_argument(mock_push): """ @@ -314,7 +310,7 @@ def test_services_search_indexers_batch_size_argument(mock_push): access = factories.UserDocumentAccessFactory(document=document) expected_user_subs[str(document.id)] = str(access.user.sub) - assert SearchIndexer().index(batch_size=2) == 5 + assert FindDocumentIndexer().index(batch_size=2) == 5 # Should be 3 batches: 2 + 2 + 1 assert mock_push.call_count == 3 @@ -337,7 +333,7 @@ def test_services_search_indexers_batch_size_argument(mock_push): assert seen_doc_ids == {str(d.id) for d in documents} -@patch.object(SearchIndexer, "push") +@patch.object(FindDocumentIndexer, "push") @pytest.mark.usefixtures("indexer_settings") def test_services_search_indexers_ignore_empty_documents(mock_push): """ @@ -349,7 +345,7 @@ def test_services_search_indexers_ignore_empty_documents(mock_push): empty_title = factories.DocumentFactory(title="") empty_content = factories.DocumentFactory(content="") - assert SearchIndexer().index() == 3 + assert FindDocumentIndexer().index() == 3 assert mock_push.call_count == 1 @@ -365,7 +361,7 @@ def test_services_search_indexers_ignore_empty_documents(mock_push): } -@patch.object(SearchIndexer, "push") +@patch.object(FindDocumentIndexer, "push") def test_services_search_indexers_skip_empty_batches(mock_push, indexer_settings): """ Documents indexing batch can be empty if all the docs are empty. @@ -377,14 +373,14 @@ def test_services_search_indexers_skip_empty_batches(mock_push, indexer_settings # Only empty docs factories.DocumentFactory.create_batch(5, content="", title="") - assert SearchIndexer().index() == 1 + assert FindDocumentIndexer().index() == 1 assert mock_push.call_count == 1 results = [doc["id"] for doc in mock_push.call_args[0][0]] assert results == [str(document.id)] -@patch.object(SearchIndexer, "push") +@patch.object(FindDocumentIndexer, "push") @pytest.mark.usefixtures("indexer_settings") def test_services_search_indexers_ancestors_link_reach(mock_push): """Document accesses and reach should take into account ancestors link reaches.""" @@ -395,7 +391,7 @@ def test_services_search_indexers_ancestors_link_reach(mock_push): parent = factories.DocumentFactory(parent=grand_parent, link_reach="public") document = factories.DocumentFactory(parent=parent, link_reach="restricted") - assert SearchIndexer().index() == 4 + assert FindDocumentIndexer().index() == 4 results = {doc["id"]: doc for doc in mock_push.call_args[0][0]} assert len(results) == 4 @@ -405,7 +401,7 @@ def test_services_search_indexers_ancestors_link_reach(mock_push): assert results[str(document.id)]["reach"] == "public" -@patch.object(SearchIndexer, "push") +@patch.object(FindDocumentIndexer, "push") @pytest.mark.usefixtures("indexer_settings") def test_services_search_indexers_ancestors_users(mock_push): """Document accesses and reach should include users from ancestors.""" @@ -415,7 +411,7 @@ def test_services_search_indexers_ancestors_users(mock_push): parent = factories.DocumentFactory(parent=grand_parent, users=[user_p]) document = factories.DocumentFactory(parent=parent, users=[user_d]) - assert SearchIndexer().index() == 3 + assert FindDocumentIndexer().index() == 3 results = {doc["id"]: doc for doc in mock_push.call_args[0][0]} assert len(results) == 3 @@ -428,7 +424,7 @@ def test_services_search_indexers_ancestors_users(mock_push): } -@patch.object(SearchIndexer, "push") +@patch.object(FindDocumentIndexer, "push") @pytest.mark.usefixtures("indexer_settings") def test_services_search_indexers_ancestors_teams(mock_push): """Document accesses and reach should include teams from ancestors.""" @@ -436,7 +432,7 @@ def test_services_search_indexers_ancestors_teams(mock_push): parent = factories.DocumentFactory(parent=grand_parent, teams=["team_p"]) document = factories.DocumentFactory(parent=parent, teams=["team_d"]) - assert SearchIndexer().index() == 3 + assert FindDocumentIndexer().index() == 3 results = {doc["id"]: doc for doc in mock_push.call_args[0][0]} assert len(results) == 3 @@ -451,9 +447,9 @@ def test_push_uses_correct_url_and_data(mock_post, indexer_settings): push() should call requests.post with the correct URL from settings the timeout set to 10 seconds and the data as JSON. """ - indexer_settings.SEARCH_INDEXER_URL = "http://example.com/index" + indexer_settings.INDEXING_URL = "http://example.com/index" - indexer = SearchIndexer() + indexer = FindDocumentIndexer() sample_data = [{"id": "123", "title": "Test"}] mock_response = mock_post.return_value @@ -464,7 +460,7 @@ def test_push_uses_correct_url_and_data(mock_post, indexer_settings): mock_post.assert_called_once() args, kwargs = mock_post.call_args - assert args[0] == indexer_settings.SEARCH_INDEXER_URL + assert args[0] == indexer_settings.INDEXING_URL assert kwargs.get("json") == sample_data assert kwargs.get("timeout") == 10 @@ -542,9 +538,7 @@ def test_services_search_indexers_search_errors(indexer_settings): """ factories.DocumentFactory() - indexer_settings.SEARCH_INDEXER_QUERY_URL = ( - "http://app-find/api/v1.0/documents/search/" - ) + indexer_settings.SEARCH_URL = "http://app-find/api/v1.0/documents/search/" responses.add( responses.POST, @@ -554,17 +548,17 @@ def test_services_search_indexers_search_errors(indexer_settings): ) with pytest.raises(HTTPError): - SearchIndexer().search("alpha", token="mytoken") + FindDocumentIndexer().search("alpha", token="mytoken") @patch("requests.post") def test_services_search_indexers_search(mock_post, indexer_settings): """ - search() should call requests.post to SEARCH_INDEXER_QUERY_URL with the + search() should call requests.post to SEARCH_URL with the document ids from linktraces. """ user = factories.UserFactory() - indexer = SearchIndexer() + indexer = FindDocumentIndexer() mock_response = mock_post.return_value mock_response.raise_for_status.return_value = None # No error @@ -582,7 +576,7 @@ def test_services_search_indexers_search(mock_post, indexer_settings): args, kwargs = mock_post.call_args - assert args[0] == indexer_settings.SEARCH_INDEXER_QUERY_URL + assert args[0] == indexer_settings.SEARCH_URL query_data = kwargs.get("json") assert query_data["q"] == "alpha" @@ -605,7 +599,7 @@ def test_services_search_indexers_search_nb_results(mock_post, indexer_settings) indexer_settings.SEARCH_INDEXER_QUERY_LIMIT = 25 user = factories.UserFactory() - indexer = SearchIndexer() + indexer = FindDocumentIndexer() mock_response = mock_post.return_value mock_response.raise_for_status.return_value = None # No error @@ -623,7 +617,7 @@ def test_services_search_indexers_search_nb_results(mock_post, indexer_settings) args, kwargs = mock_post.call_args - assert args[0] == indexer_settings.SEARCH_INDEXER_QUERY_URL + assert args[0] == indexer_settings.SEARCH_URL assert kwargs.get("json")["nb_results"] == 25 # The argument overrides the setting value @@ -631,14 +625,14 @@ def test_services_search_indexers_search_nb_results(mock_post, indexer_settings) args, kwargs = mock_post.call_args - assert args[0] == indexer_settings.SEARCH_INDEXER_QUERY_URL + assert args[0] == indexer_settings.SEARCH_URL assert kwargs.get("json")["nb_results"] == 109 def test_search_indexer_get_title_with_localized_field(): """Test extracting title from localized title field.""" source = {"title.extension": "Bonjour", "id": 1, "content": "test"} - result = SearchIndexer.get_title(source) + result = FindDocumentIndexer.get_title(source) assert result == "Bonjour" @@ -646,7 +640,7 @@ def test_search_indexer_get_title_with_localized_field(): def test_search_indexer_get_title_with_multiple_localized_fields(): """Test that first matching localized title is returned.""" source = {"title.extension": "Bonjour", "title.en": "Hello", "id": 1} - result = SearchIndexer.get_title(source) + result = FindDocumentIndexer.get_title(source) assert result in ["Bonjour", "Hello"] @@ -654,7 +648,7 @@ def test_search_indexer_get_title_with_multiple_localized_fields(): def test_search_indexer_get_title_fallback_to_plain_title(): """Test fallback to plain 'title' field when no localized field exists.""" source = {"title": "Hello World", "id": 1} - result = SearchIndexer.get_title(source) + result = FindDocumentIndexer.get_title(source) assert result == "Hello World" @@ -662,7 +656,7 @@ def test_search_indexer_get_title_fallback_to_plain_title(): def test_search_indexer_get_title_no_title_field(): """Test that empty string is returned when no title field exists.""" source = {"id": 1, "content": "test"} - result = SearchIndexer.get_title(source) + result = FindDocumentIndexer.get_title(source) assert result == "" @@ -670,7 +664,7 @@ def test_search_indexer_get_title_no_title_field(): def test_search_indexer_get_title_with_empty_localized_title(): """Test that fallback works when localized title is empty.""" source = {"title.extension": "", "title": "Fallback Title", "id": 1} - result = SearchIndexer.get_title(source) + result = FindDocumentIndexer.get_title(source) assert result == "Fallback Title" @@ -678,6 +672,6 @@ def test_search_indexer_get_title_with_empty_localized_title(): def test_search_indexer_get_title_with_multiple_extension(): """Test extracting title from title field with multiple extensions.""" source = {"title.extension_1.extension_2": "Bonjour", "id": 1, "content": "test"} - result = SearchIndexer.get_title(source) + result = FindDocumentIndexer.get_title(source) assert result == "Bonjour" diff --git a/src/backend/impress/settings.py b/src/backend/impress/settings.py index b8ef63c9..e8c66e87 100755 --- a/src/backend/impress/settings.py +++ b/src/backend/impress/settings.py @@ -113,8 +113,8 @@ class Base(Configuration): SEARCH_INDEXER_BATCH_SIZE = values.IntegerValue( default=100_000, environ_name="SEARCH_INDEXER_BATCH_SIZE", environ_prefix=None ) - SEARCH_INDEXER_URL = values.Value( - default=None, environ_name="SEARCH_INDEXER_URL", environ_prefix=None + INDEXING_URL = values.Value( + default=None, environ_name="INDEXING_URL", environ_prefix=None ) SEARCH_INDEXER_COUNTDOWN = values.IntegerValue( default=1, environ_name="SEARCH_INDEXER_COUNTDOWN", environ_prefix=None @@ -122,8 +122,8 @@ class Base(Configuration): SEARCH_INDEXER_SECRET = values.Value( default=None, environ_name="SEARCH_INDEXER_SECRET", environ_prefix=None ) - SEARCH_INDEXER_QUERY_URL = values.Value( - default=None, environ_name="SEARCH_INDEXER_QUERY_URL", environ_prefix=None + SEARCH_URL = values.Value( + default=None, environ_name="SEARCH_URL", environ_prefix=None ) SEARCH_INDEXER_QUERY_LIMIT = values.PositiveIntegerValue( default=50, environ_name="SEARCH_INDEXER_QUERY_LIMIT", environ_prefix=None