From 6a0d65b4f409d487afbc50b82321a1cf1d250ed8 Mon Sep 17 00:00:00 2001 From: charles Date: Thu, 19 Feb 2026 11:13:01 +0100 Subject: [PATCH] =?UTF-8?q?=E2=9C=A8(backend)=20implement=20Find=20feature?= =?UTF-8?q?=20flags?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit For beta testing purposes we need to be able to activate Find hybrid search to some users, Find full-text search to some others and leave remaining users on basic DRF title search. Signed-off-by: charles --- src/backend/core/api/viewsets.py | 29 +++++++++--- src/backend/core/enums.py | 14 +++++- src/backend/core/services/search_indexers.py | 45 ++++++++++++++++--- .../documents/test_api_documents_search.py | 10 +++++ .../test_services_find_document_indexer.py | 10 ++++- .../tests/test_services_search_indexers.py | 12 ++--- 6 files changed, 100 insertions(+), 20 deletions(-) diff --git a/src/backend/core/api/viewsets.py b/src/backend/core/api/viewsets.py index 8f38bde8..8c266f41 100644 --- a/src/backend/core/api/viewsets.py +++ b/src/backend/core/api/viewsets.py @@ -32,6 +32,7 @@ from django.utils.translation import gettext_lazy as _ import requests import rest_framework as drf +import waffle from botocore.exceptions import ClientError from csp.constants import NONE from csp.decorators import csp_update @@ -63,6 +64,7 @@ from core.services.search_indexers import ( from core.tasks.mail import send_ask_for_access_mail from core.utils import extract_attachments, filter_descendants +from ..enums import SearchType from . import permissions, serializers, utils from .filters import ( DocumentFilter, @@ -1181,6 +1183,10 @@ class DocumentViewSet( """ params = serializers.SearchDocumentSerializer(data=request.query_params) params.is_valid(raise_exception=True) + search_type = self._get_search_type() + + if search_type == SearchType.TITLE: + return self._title_search(request, params.validated_data, *args, **kwargs) indexer = get_document_indexer() if indexer is None: @@ -1188,17 +1194,29 @@ class DocumentViewSet( return self._title_search(request, params.validated_data, *args, **kwargs) try: - return self._search_with_indexer(indexer, request, params=params) + return self._search_with_indexer( + indexer, request, params=params, search_type=search_type + ) except requests.exceptions.RequestException as e: logger.error("Error while searching documents with indexer: %s", e) # fallback on title search if the indexer is not reached - return self._title_search( - request, params.validated_data, *args, **kwargs - ) + return self._title_search(request, params.validated_data, *args, **kwargs) + def _get_search_type(self) -> SearchType: + """ + Returns the search type to use for the search endpoint based on feature flags. + If a user has both flags activated the most advanced search is used + (HYBRID > FULL_TEXT > TITLE). + A user with no flag will default to the basic title search. + """ + if waffle.flag_is_active(self.request, "flag_find_hybrid_search"): + return SearchType.HYBRID + if waffle.flag_is_active(self.request, "flag_find_full_text_search"): + return SearchType.FULL_TEXT + return SearchType.TITLE @staticmethod - def _search_with_indexer(indexer, request, params): + def _search_with_indexer(indexer, request, params, search_type): """ Returns a list of documents matching the query (q) according to the configured indexer. """ @@ -1206,6 +1224,7 @@ class DocumentViewSet( results = indexer.search( q=params.validated_data["q"], + search_type=search_type, token=request.session.get("oidc_access_token"), path=( params.validated_data["path"] diff --git a/src/backend/core/enums.py b/src/backend/core/enums.py index 46e62b2c..3c761822 100644 --- a/src/backend/core/enums.py +++ b/src/backend/core/enums.py @@ -3,7 +3,7 @@ Core application enums declaration """ import re -from enum import StrEnum +from enum import Enum, StrEnum from django.conf import global_settings, settings from django.db import models @@ -46,3 +46,15 @@ class DocumentAttachmentStatus(StrEnum): PROCESSING = "processing" READY = "ready" + + +class SearchType(str, Enum): + """ + Defines the possible search types for a document search query. + - TITLE: DRF based search in the title of the documents only. + - HYBRID and FULL_TEXT: more advanced search based on Find indexer. + """ + + TITLE = "title" + HYBRID = "hybrid" + FULL_TEXT = "full-text" diff --git a/src/backend/core/services/search_indexers.py b/src/backend/core/services/search_indexers.py index 1a364706..7f4294d9 100644 --- a/src/backend/core/services/search_indexers.py +++ b/src/backend/core/services/search_indexers.py @@ -13,6 +13,7 @@ from django.utils.module_loading import import_string import requests from core import models, utils +from core.enums import SearchType logger = logging.getLogger(__name__) @@ -68,7 +69,7 @@ def get_batch_accesses_by_users_and_teams(paths): return dict(access_by_document_path) -def get_visited_document_ids_of(queryset, user): +def get_visited_document_ids_of(queryset, user) -> tuple[str, ...]: """ Returns the ids of the documents that have a linktrace to the user and NOT owned. It will be use to limit the opensearch responses to the public documents already @@ -92,7 +93,7 @@ def get_visited_document_ids_of(queryset, user): .distinct("pk") ) - return [str(id) for id in docs.values_list("pk", flat=True)] + return tuple(str(id) for id in docs.values_list("pk", flat=True)) class BaseDocumentIndexer(ABC): @@ -181,8 +182,16 @@ class BaseDocumentIndexer(ABC): Must be implemented by subclasses. """ - # pylint: disable-next=too-many-arguments,too-many-positional-arguments - def search(self, q, token, visited=(), nb_results=None, path=None): + # pylint: disable=too-many-arguments, too-many-positional-arguments + def search( # noqa : PLR0913 + self, + q: str, + token: str, + visited: tuple[str, ...] = (), + nb_results: int = None, + path: str = None, + search_type: SearchType = None, + ): """ Search for documents in Find app. Ensure the same default ordering as "Docs" list : -updated_at @@ -200,6 +209,8 @@ class BaseDocumentIndexer(ABC): Defaults to 50 if not specified. path (str, optional): The parent path to search descendants of. + search_type: + hybrid or full-text search """ nb_results = nb_results or self.search_limit results = self.search_query( @@ -211,6 +222,7 @@ class BaseDocumentIndexer(ABC): "order_by": "updated_at", "order_direction": "desc", "path": path, + "search_type": search_type, }, token=token, ) @@ -231,10 +243,29 @@ class FindDocumentIndexer(BaseDocumentIndexer): Document indexer that indexes and searches documents with La Suite Find app. """ - # pylint: disable=too-many-arguments,too-many-positional-arguments - def search(self, q, token, visited=(), nb_results=None, path=None): + # pylint: disable=too-many-arguments, too-many-positional-arguments + def search( # noqa : PLR0913 + self, + q: str, + token: str, + visited: tuple[()] = (), + nb_results: int = None, + path: str = None, + search_type: SearchType = None, + ): """format Find search results""" - search_results = super().search(q, token, visited, nb_results, path) + search_results = ( + super() + .search( + q=q, + token=token, + visited=visited, + nb_results=nb_results, + path=path, + search_type=search_type, + ) + .get("results", []) + ) return [ { **hit["_source"], 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 969ac4b5..4aefb237 100644 --- a/src/backend/core/tests/documents/test_api_documents_search.py +++ b/src/backend/core/tests/documents/test_api_documents_search.py @@ -9,14 +9,23 @@ import responses from faker import Faker from rest_framework import response as drf_response from rest_framework.test import APIClient +from waffle.testutils import override_flag from core import factories +from core.enums import SearchType from core.services.search_indexers import get_document_indexer fake = Faker() pytestmark = pytest.mark.django_db +@pytest.fixture(autouse=True) +def enable_flag_find_hybrid_search(): + """Enable flag_find_hybrid_search for all tests in this module.""" + with override_flag("flag_find_hybrid_search", active=True): + yield + + @mock.patch("core.services.search_indexers.FindDocumentIndexer.search_query") @responses.activate def test_api_documents_search_anonymous(search_query, indexer_settings): @@ -46,6 +55,7 @@ def test_api_documents_search_anonymous(search_query, indexer_settings): "order_by": "updated_at", "order_direction": "desc", "path": None, + "search_type": SearchType.HYBRID, }, "token": None, } diff --git a/src/backend/core/tests/test_services_find_document_indexer.py b/src/backend/core/tests/test_services_find_document_indexer.py index 074d0114..9a080cd7 100644 --- a/src/backend/core/tests/test_services_find_document_indexer.py +++ b/src/backend/core/tests/test_services_find_document_indexer.py @@ -12,6 +12,7 @@ from django.db import transaction import pytest from core import factories, models +from core.enums import SearchType from core.services.search_indexers import FindDocumentIndexer pytestmark = pytest.mark.django_db @@ -473,8 +474,14 @@ def test_find_document_indexer_search(mock_search_query): nb_results = 10 path = "/some/path/" visited = ["doc-123"] + search_type = SearchType.HYBRID results = FindDocumentIndexer().search( - q=q, token=token, nb_results=nb_results, path=path, visited=visited + q=q, + token=token, + nb_results=nb_results, + path=path, + visited=visited, + search_type=search_type, ) mock_search_query.assert_called_once() @@ -487,6 +494,7 @@ def test_find_document_indexer_search(mock_search_query): "order_by": "updated_at", "order_direction": "desc", "path": path, + "search_type": search_type, } assert len(results) == 2 diff --git a/src/backend/core/tests/test_services_search_indexers.py b/src/backend/core/tests/test_services_search_indexers.py index 18c90979..01131e39 100644 --- a/src/backend/core/tests/test_services_search_indexers.py +++ b/src/backend/core/tests/test_services_search_indexers.py @@ -494,7 +494,7 @@ def test_get_visited_document_ids_of(): factories.UserDocumentAccessFactory(user=user, document=doc2) # The second document have an access for the user - assert get_visited_document_ids_of(queryset, user) == [str(doc1.pk)] + assert get_visited_document_ids_of(queryset, user) == (str(doc1.pk),) @pytest.mark.usefixtures("indexer_settings") @@ -528,7 +528,7 @@ def test_get_visited_document_ids_of_deleted(): doc_deleted.soft_delete() # Only the first document is not deleted - assert get_visited_document_ids_of(queryset, user) == [str(doc.pk)] + assert get_visited_document_ids_of(queryset, user) == (str(doc.pk),) @responses.activate @@ -548,7 +548,7 @@ def test_services_search_indexers_search_errors(indexer_settings): ) with pytest.raises(HTTPError): - FindDocumentIndexer().search("alpha", token="mytoken") + FindDocumentIndexer().search(q="alpha", token="mytoken") @patch("requests.post") @@ -572,7 +572,7 @@ def test_services_search_indexers_search(mock_post, indexer_settings): visited = get_visited_document_ids_of(models.Document.objects.all(), user) - indexer.search("alpha", visited=visited, token="mytoken") + indexer.search(q="alpha", visited=visited, token="mytoken") args, kwargs = mock_post.call_args @@ -613,7 +613,7 @@ def test_services_search_indexers_search_nb_results(mock_post, indexer_settings) visited = get_visited_document_ids_of(models.Document.objects.all(), user) - indexer.search("alpha", visited=visited, token="mytoken") + indexer.search(q="alpha", visited=visited, token="mytoken") args, kwargs = mock_post.call_args @@ -621,7 +621,7 @@ def test_services_search_indexers_search_nb_results(mock_post, indexer_settings) assert kwargs.get("json")["nb_results"] == 25 # The argument overrides the setting value - indexer.search("alpha", visited=visited, token="mytoken", nb_results=109) + indexer.search(q="alpha", visited=visited, token="mytoken", nb_results=109) args, kwargs = mock_post.call_args