✨(backend) some refactor of indexer classes & modules

Rename FindDocumentIndexer as SearchIndexer
Rename FindDocumentSerializer as SearchDocumentSerializer
Rename package core.tasks.find as core.task.search
Remove logs on http errors in SearchIndexer
Factorise some code in search API view.

Signed-off-by: Fabre Florian <[email protected]>

Author: joehybird

env.d/development/common

@@ -79,7 +79,7 @@ Y_PROVIDER_API_KEY=yprovider-api-key
 THEME_CUSTOMIZATION_CACHE_TIMEOUT=15
 
 # Indexer
-SEARCH_INDEXER_CLASS="core.services.search_indexers.FindDocumentIndexer"
+SEARCH_INDEXER_CLASS="core.services.search_indexers.SearchIndexer"
 SEARCH_INDEXER_SECRET=find-api-key-for-docs-with-exactly-50-chars-length   # Key generated by create_demo in Find app.
 SEARCH_INDEXER_URL="http://find:8000/api/v1.0/documents/index/"
 SEARCH_INDEXER_QUERY_URL="http://find:8000/api/v1.0/documents/search/"

src/backend/core/api/serializers.py

@@ -890,8 +890,8 @@ class MoveDocumentSerializer(serializers.Serializer):
     )
 
 
-class FindDocumentSerializer(serializers.Serializer):
-    """Serializer for Find search requests"""
+class SearchDocumentSerializer(serializers.Serializer):
+    """Serializer for fulltext search requests through Find application"""
 
     q = serializers.CharField(required=True, allow_blank=False, trim_whitespace=True)
     page_size = serializers.IntegerField(

src/backend/core/api/viewsets.py

@@ -1051,63 +1051,72 @@ def duplicate(self, request, *args, **kwargs):
             {"id": str(duplicated_document.id)}, status=status.HTTP_201_CREATED
         )
 
+    def _simple_search_queryset(self, params):
+        """
+        Returns a queryset filtered by the content of the document title
+        """
+        text = params.validated_data["q"]
+
+        # As the 'list' view we get a prefiltered queryset (deleted docs are excluded)
+        queryset = self.get_queryset()
+        filterset = DocumentFilter({"title": text}, queryset=queryset)
+
+        if not filterset.is_valid():
+            raise drf.exceptions.ValidationError(filterset.errors)
+
+        return filterset.filter_queryset(queryset)
+
+    def _fulltext_search_queryset(self, indexer, token, user, params):
+        """
+        Returns a queryset from the results the fulltext search of Find
+        """
+        text = params.validated_data["q"]
+        queryset = models.Document.objects.all()
+
+        # Retrieve the documents ids from Find.
+        results = indexer.search(
+            text=text,
+            token=token,
+            visited=get_visited_document_ids_of(queryset, user),
+            page=params.validated_data.get("page", 1),
+            page_size=params.validated_data.get("page_size", 20),
+        )
+
+        return queryset.filter(pk__in=results)
+
     @drf.decorators.action(detail=False, methods=["get"], url_path="search")
     @method_decorator(refresh_oidc_access_token)
     def search(self, request, *args, **kwargs):
         """
         Returns a DRF response containing the filtered, annotated and ordered document list.
 
-        Applies filtering based on request parameter 'q' from `FindDocumentSerializer`.
+        Applies filtering based on request parameter 'q' from `SearchDocumentSerializer`.
         Depending of the configuration it can be:
          - A fulltext search through the opensearch indexation app "find" if the backend is
-           enabled (see SEARCH_BACKEND_CLASS)
+           enabled (see SEARCH_INDEXER_CLASS)
          - A filtering by the model field 'title'.
 
         The ordering is always by the most recent first.
         """
         access_token = request.session.get("oidc_access_token")
         user = request.user
 
-        serializer = serializers.FindDocumentSerializer(data=request.query_params)
-        serializer.is_valid(raise_exception=True)
+        params = serializers.SearchDocumentSerializer(data=request.query_params)
+        params.is_valid(raise_exception=True)
 
         indexer = get_document_indexer()
-        text = serializer.validated_data["q"]
-
-        # The indexer is not configured, so we fallback on a simple filter on the
-        # model field 'title'.
-        if not indexer:
-            # As the 'list' view we get a prefiltered queryset (deleted docs are excluded)
-            queryset = self.get_queryset()
-            filterset = DocumentFilter({"title": text}, queryset=queryset)
-
-            if not filterset.is_valid():
-                raise drf.exceptions.ValidationError(filterset.errors)
 
-            queryset = filterset.filter_queryset(queryset).order_by("-updated_at")
-
-            return self.get_response_for_queryset(
-                queryset,
-                context={
-                    "request": request,
-                },
+        if indexer:
+            queryset = self._fulltext_search_queryset(
+                indexer, token=access_token, user=user, params=params
             )
-
-        queryset = models.Document.objects.all()
-
-        # Retrieve the documents ids from Find.
-        results = indexer.search(
-            text=text,
-            token=access_token,
-            visited=get_visited_document_ids_of(queryset, user),
-            page=serializer.validated_data.get("page", 1),
-            page_size=serializer.validated_data.get("page_size", 20),
-        )
-
-        queryset = queryset.filter(pk__in=results).order_by("-updated_at")
+        else:
+            # The indexer is not configured, we fallback on a simple icontains filter by the
+            # model field 'title'.
+            queryset = self._simple_search_queryset(params)
 
         return self.get_response_for_queryset(
-            queryset,
+            queryset.order_by("-updated_at"),
             context={
                 "request": request,
             },

src/backend/core/services/search_indexers.py

@@ -223,7 +223,7 @@ def search_query(self, data, token) -> dict:
         """
 
 
-class FindDocumentIndexer(BaseDocumentIndexer):
+class SearchIndexer(BaseDocumentIndexer):
     """
     Document indexer that pushes documents to La Suite Find app.
     """
@@ -270,18 +270,14 @@ def search_query(self, data, token) -> requests.Response:
         Returns:
             dict: A JSON-serializable dictionary.
         """
-        try:
-            response = requests.post(
-                self.search_url,
-                json=data,
-                headers={"Authorization": f"Bearer {token}"},
-                timeout=10,
-            )
-            response.raise_for_status()
-            return response.json()
-        except requests.exceptions.HTTPError as e:
-            logger.error("HTTPError: %s", e)
-            raise
+        response = requests.post(
+            self.search_url,
+            json=data,
+            headers={"Authorization": f"Bearer {token}"},
+            timeout=10,
+        )
+        response.raise_for_status()
+        return response.json()
 
     def push(self, data):
         """
@@ -290,14 +286,10 @@ def push(self, data):
         Args:
             data (list): List of document dictionaries.
         """
-        try:
-            response = requests.post(
-                self.indexer_url,
-                json=data,
-                headers={"Authorization": f"Bearer {self.indexer_secret}"},
-                timeout=10,
-            )
-            response.raise_for_status()
-        except requests.exceptions.HTTPError as e:
-            logger.error("HTTPError: %s", e)
-            raise
+        response = requests.post(
+            self.indexer_url,
+            json=data,
+            headers={"Authorization": f"Bearer {self.indexer_secret}"},
+            timeout=10,
+        )
+        response.raise_for_status()

src/backend/core/signals.py

@@ -9,7 +9,7 @@
 from django.dispatch import receiver
 
 from . import models
-from .tasks.find import trigger_document_indexer
+from .tasks.search import trigger_document_indexer
 
 
 @receiver(signals.post_save, sender=models.Document)

src/backend/core/tasks/find.py renamed to src/backend/core/tasks/search.py

@@ -11,15 +11,15 @@
 import pytest
 
 from core import factories
-from core.services.search_indexers import FindDocumentIndexer
+from core.services.search_indexers import SearchIndexer
 
 
 @pytest.mark.django_db
 @pytest.mark.usefixtures("indexer_settings")
 def test_index():
     """Test the command `index` that run the Find app indexer for all the available documents."""
     user = factories.UserFactory()
-    indexer = FindDocumentIndexer()
+    indexer = SearchIndexer()
 
     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(FindDocumentIndexer, "push") as mock_push:
+    with mock.patch.object(SearchIndexer, "push") as mock_push:
         call_command("index")
 
         push_call_args = [call.args[0] for call in mock_push.call_args_list]

src/backend/core/tests/commands/test_index.py

@@ -39,7 +39,7 @@ def indexer_settings_fixture(settings):
 
     get_document_indexer.cache_clear()
 
-    settings.SEARCH_INDEXER_CLASS = "core.services.search_indexers.FindDocumentIndexer"
+    settings.SEARCH_INDEXER_CLASS = "core.services.search_indexers.SearchIndexer"
     settings.SEARCH_INDEXER_SECRET = "ThisIsAKeyForTest"
     settings.SEARCH_INDEXER_URL = "http://localhost:8081/api/v1.0/documents/index/"
     settings.SEARCH_INDEXER_QUERY_URL = (

src/backend/core/tests/conftest.py

@@ -21,7 +21,7 @@
 import pytest
 
 from core import factories, models
-from core.services.search_indexers import FindDocumentIndexer
+from core.services.search_indexers import SearchIndexer
 
 pytestmark = pytest.mark.django_db
 
@@ -1450,7 +1450,7 @@ def test_models_documents_compute_ancestors_links_paths_mapping_structure(
         }
 
 
-@mock.patch.object(FindDocumentIndexer, "push")
+@mock.patch.object(SearchIndexer, "push")
 @pytest.mark.django_db(transaction=True)
 def test_models_documents_post_save_indexer(mock_push, indexer_settings):
     """Test indexation task on document creation"""
@@ -1462,7 +1462,7 @@ def test_models_documents_post_save_indexer(mock_push, indexer_settings):
     accesses = {}
     data = [call.args[0] for call in mock_push.call_args_list]
 
-    indexer = FindDocumentIndexer()
+    indexer = SearchIndexer()
 
     assert sorted(data, key=itemgetter("id")) == sorted(
         [
@@ -1479,7 +1479,7 @@ def test_models_documents_post_save_indexer(mock_push, indexer_settings):
     assert cache.get(f"doc-indexer-debounce-{doc3.pk}") == 0
 
 
-@mock.patch.object(FindDocumentIndexer, "push")
+@mock.patch.object(SearchIndexer, "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"""
@@ -1492,7 +1492,7 @@ def test_models_documents_post_save_indexer_not_configured(mock_push, indexer_se
     assert mock_push.call_args_list == []
 
 
-@mock.patch.object(FindDocumentIndexer, "push")
+@mock.patch.object(SearchIndexer, "push")
 @pytest.mark.django_db(transaction=True)
 def test_models_documents_post_save_indexer_with_accesses(mock_push, indexer_settings):
     """Test indexation task on document creation"""
@@ -1515,7 +1515,7 @@ def test_models_documents_post_save_indexer_with_accesses(mock_push, indexer_set
 
     data = [call.args[0] for call in mock_push.call_args_list]
 
-    indexer = FindDocumentIndexer()
+    indexer = SearchIndexer()
 
     assert sorted(data, key=itemgetter("id")) == sorted(
         [
@@ -1532,7 +1532,7 @@ def test_models_documents_post_save_indexer_with_accesses(mock_push, indexer_set
     assert cache.get(f"doc-indexer-debounce-{doc3.pk}") == 0
 
 
-@mock.patch.object(FindDocumentIndexer, "push")
+@mock.patch.object(SearchIndexer, "push")
 @pytest.mark.django_db(transaction=True)
 def test_models_documents_post_save_indexer_deleted(mock_push, indexer_settings):
     """Indexation task on deleted or ancestor_deleted documents"""
@@ -1575,7 +1575,7 @@ def test_models_documents_post_save_indexer_deleted(mock_push, indexer_settings)
 
     data = [call.args[0] for call in mock_push.call_args_list]
 
-    indexer = FindDocumentIndexer()
+    indexer = SearchIndexer()
 
     # Even deleted document are re-indexed : only update their status in the future ?
     assert sorted(data, key=itemgetter("id")) == sorted(
@@ -1594,7 +1594,7 @@ def test_models_documents_post_save_indexer_deleted(mock_push, indexer_settings)
     assert cache.get(f"doc-indexer-debounce-{doc_ancestor_deleted.pk}") == 0
 
 
-@mock.patch.object(FindDocumentIndexer, "push")
+@mock.patch.object(SearchIndexer, "push")
 @pytest.mark.django_db(transaction=True)
 def test_models_documents_post_save_indexer_restored(mock_push, indexer_settings):
     """Restart indexation task on restored documents"""
@@ -1648,7 +1648,7 @@ def test_models_documents_post_save_indexer_restored(mock_push, indexer_settings
 
     data = [call.args[0] for call in mock_push.call_args_list]
 
-    indexer = FindDocumentIndexer()
+    indexer = SearchIndexer()
 
     # All docs are re-indexed
     assert sorted(data, key=itemgetter("id")) == sorted(
@@ -1668,10 +1668,10 @@ def test_models_documents_post_save_indexer_debounce(indexer_settings):
     """Test indexation task skipping on document update"""
     indexer_settings.SEARCH_INDEXER_COUNTDOWN = 0
 
-    indexer = FindDocumentIndexer()
+    indexer = SearchIndexer()
     user = factories.UserFactory()
 
-    with mock.patch.object(FindDocumentIndexer, "push"):
+    with mock.patch.object(SearchIndexer, "push"):
         with transaction.atomic():
             doc = factories.DocumentFactory()
             factories.UserDocumentAccessFactory(document=doc, user=user)
@@ -1680,7 +1680,7 @@ def test_models_documents_post_save_indexer_debounce(indexer_settings):
         str(doc.path): {"users": [user.sub]},
     }
 
-    with mock.patch.object(FindDocumentIndexer, "push") as mock_push:
+    with mock.patch.object(SearchIndexer, "push") as mock_push:
         # Simulate 1 waiting task
         cache.set(f"doc-indexer-debounce-{doc.pk}", 1)
 
@@ -1691,7 +1691,7 @@ def test_models_documents_post_save_indexer_debounce(indexer_settings):
 
         assert [call.args[0] for call in mock_push.call_args_list] == []
 
-    with mock.patch.object(FindDocumentIndexer, "push") as mock_push:
+    with mock.patch.object(SearchIndexer, "push") as mock_push:
         # No waiting task
         cache.set(f"doc-indexer-debounce-{doc.pk}", 0)
 
@@ -1709,10 +1709,10 @@ def test_models_documents_access_post_save_indexer(indexer_settings):
     """Test indexation task on DocumentAccess update"""
     indexer_settings.SEARCH_INDEXER_COUNTDOWN = 0
 
-    indexer = FindDocumentIndexer()
+    indexer = SearchIndexer()
     user = factories.UserFactory()
 
-    with mock.patch.object(FindDocumentIndexer, "push"):
+    with mock.patch.object(SearchIndexer, "push"):
         with transaction.atomic():
             doc = factories.DocumentFactory()
             doc_access = factories.UserDocumentAccessFactory(document=doc, user=user)
@@ -1721,9 +1721,9 @@ def test_models_documents_access_post_save_indexer(indexer_settings):
         str(doc.path): {"users": [user.sub]},
     }
 
-    indexer = FindDocumentIndexer()
+    indexer = SearchIndexer()
 
-    with mock.patch.object(FindDocumentIndexer, "push") as mock_push:
+    with mock.patch.object(SearchIndexer, "push") as mock_push:
         with transaction.atomic():
             doc_access.save()